Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

automake and autoconf improvements #23

Merged
merged 6 commits into from
Jun 5, 2024
Merged

automake and autoconf improvements #23

merged 6 commits into from
Jun 5, 2024

Conversation

kloczek
Copy link
Contributor

@kloczek kloczek commented Feb 10, 2024

  • remove not used include $(GLIB_MAKEFILE) lines
  • use automake to build intel_lpmd_control
  • use autoconf AC_CONFIG_FILES() to generate data/intel_lpmd.service and data/org.freedesktop.intel_lpmd.service instead custom sed rules
  • more [] quotes in configure ac
  • use $rundir and bumped minimum autoconf version required to 2.70
  • remove use AM_CFLAGS in main Makefile.am
  • remove . from SUBDIRS in main Makefile.am
  • use PKG_CHECK_MODULES() to detect dbus-glib-1
  • remove redundant AC_SUBST(FOO_CFLAGS) and AC_SUBST(FOO_LIBS) (PKG_CHECK_MODULES() adds that automatically)
  • simplify man pages installation (use automake man_MANS)

- remove not used `include $(GLIB_MAKEFILE)` lines
- use automake to build intel_lpmd_control
- use autoconf AC_CONFIG_FILES() to generate data/intel_lpmd.service and
  data/org.freedesktop.intel_lpmd.service instead custom sed rules
- more [] quotes in configure ac
- use $rundir and bumped minimum autoconf version required to 2.70
- remove use AM_CFLAGS in main Makefile.am
- remove . from SUBDIRS in main Makefile.am
- use PKG_CHECK_MODULES() to detect dbus-glib-1
- remove redundant AC_SUBST(FOO_CFLAGS) and AC_SUBST(FOO_LIBS)
  (PKG_CHECK_MODULES() adds that automatically)

Signed-off-by: Tomasz Kłoczko <kloczek@github.com>
This file is no longer needed.
This script is pure POSIX sh so it do not need to be as bash script.
@kloczek kloczek changed the title automake and auto conf improvements automake and autoconf improvements Feb 10, 2024
@kloczek
Copy link
Contributor Author

kloczek commented Feb 10, 2024

With those changes check target is not failing and whole tree is now ready to move tests/ to automake test units.

@kloczek
Copy link
Contributor Author

kloczek commented Feb 10, 2024

Also distcheck target should be working now as well.

@@ -1,4 +1,4 @@
#!/bin/bash
#!/usr/bin/sh

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This script is pure POSIX sh so it do not need to be as bash script.

This is the wrong solution to marking a script as POSIX sh. Please use /bin/sh exclusively for this, as that is the portable filename for the "sh" command.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please notice that currently NONE ot the OS distros (Linux/Solaris/*BSD) uses /usr separation and all those OSess distros uses /usr/bin/sh as standard POSIX sh location (Solaris and *BSD uses that more than decade).
Just look on Fedora or Debian packaging standards.
Fedora rpm currently automatically changed /bin/ to /usr/bin/ in all shells shebangs https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/rawhide/f/brp-mangle-shebangs

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please notice that currently NONE ot the OS distros (Linux/Solaris/*BSD) uses /usr separation and all those OSess distros uses /usr/bin/sh as standard POSIX sh location (Solaris and *BSD uses that more than decade).

Clearly you have no idea what you're talking about in the slightest.

$ uname -a
FreeBSD build 14.0-RELEASE-p5 FreeBSD 14.0-RELEASE-p5 #0: Tue Feb 13 23:37:36 UTC 2024     root@amd64-builder.daemonology.net:/usr/obj/usr/src/amd64.amd64/sys/GENERIC amd64

$ file /usr/bin/sh /bin/sh
/usr/bin/sh: cannot open `/usr/bin/sh' (No such file or directory)
/bin/sh:     ELF 64-bit LSB pie executable, x86-64, version 1 (FreeBSD), dynamically linked, interpreter /libexec/ld-elf.so.1, for FreeBSD 14.0 (1400097), FreeBSD-style, stripped

Not only is /usr/bin/sh not the "standard POSIX sh location", it isn't a valid location at all. It doesn't exist. It never did and never has and most likely never will.

It's generally unlikely to find /usr/bin/sh except as a side effect of Linux-specific systems that happen to have implemented /bin as a symlink to /usr/bin, which isn't even universal on Linux either. You won't get very far on Alpine Linux, for example, if you assume that /usr/bin/sh exists.

This is why it is NOT the "standard POSIX sh location".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, guys
May I know if we are on the same page now, for the discussion above?

@zhang-rui
Copy link
Contributor

hi, do we reach a conclusion on this? :)

Signed-off-by: Tomasz Kłoczko <kloczek@github.com>
@zhang-rui zhang-rui merged commit 91567df into intel:main Jun 5, 2024
@kloczek
Copy link
Contributor Author

kloczek commented Jun 5, 2024

Thx 👍

@zhang-rui
Copy link
Contributor

hi @kloczek
Just noticed that only the first patch has your SOB.
Can you please update the PR with SOB in each commit?

@kloczek
Copy link
Contributor Author

kloczek commented Jun 6, 2024

SOB?

@zhang-rui
Copy link
Contributor

"Signed-off-by" tag :)

@kloczek
Copy link
Contributor Author

kloczek commented Jun 6, 2024

It is "Signed-off-by: Tomasz Kłoczko kloczek@github.com"

@zhang-rui
Copy link
Contributor

Check these two commits,
d66428a
and
9280364

The first commit has "Signed-off-by" but the second one does not.

zhang-rui added a commit to zhang-rui/intel-lpmd that referenced this pull request Jul 4, 2024
This reverts commit 91567df, reversing
changes made to 0e91a42.

Need to address
intel#35
intel#36
first.
zhang-rui added a commit that referenced this pull request Jul 4, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Revert "Merge pull request #23 from kloczek/main"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants