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

Use pkg-config to detect PAM when possible #479

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

floppym
Copy link
Contributor

@floppym floppym commented Mar 5, 2024

This fixes a link error on Gentoo Linux by not putting -L/usr/lib in the
link command on 64-bit systems. The correct path is -L/usr/lib64, and
this is the default path used by GCC and clang.

Users may override pkg-config by setting PAM_CFLAGS and PAM_LDFLAGS in
the environment before calling configure. This is standard behavior for
the PKG_CHECK_MODULES macro.

The legacy detection logic is maintained when a path is given as an
argument to --with-pam. Note that this logic is broken when libdir is
not "lib".

@Neustradamus Neustradamus added the Author Author answer is needed label Apr 17, 2024
@Neustradamus
Copy link
Member

@paulusmack, @enaess: What do you think?

@paulusmack
Copy link
Collaborator

I would like the commit message to have at least a sentence saying why this is a good thing and what will be the effect from the point of view of someone running ./configure. For example, are there now any new possibilities for options to ./configure, or do any existing options work any differently now?

As for the substance of the change, I want to hear from @enaess .

@floppym
Copy link
Contributor Author

floppym commented Apr 19, 2024

@paulusmack I updated the commit message with an explanation.

@Neustradamus
Copy link
Member

@paulusmack: Have you seen @floppym changes?

@enaess
Copy link
Contributor

enaess commented Apr 24, 2024

I'll have a look tomorrow, but also worth noting is that there are other ax_***.m4 files that use the same templated pattern.

Copy link
Contributor

@enaess enaess left a comment

Choose a reason for hiding this comment

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

Generally I am good with the idea of using pkg-config w/fallback of the hard-coded values in case pkg-config doesn't exist.

I'd additionally like to see similar change in the other .m4 files for pcap, atm, srp, and others to avoid the pam m4 file from deviating from the new "norm" if you like.

m4/ax_check_pam.m4 Outdated Show resolved Hide resolved
m4/ax_check_pam.m4 Show resolved Hide resolved
This fixes a link error on Gentoo Linux by not putting -L/usr/lib in the
link command on 64-bit systems. The correct path is -L/usr/lib64, and
this is the default path used by GCC and clang.

Users may override pkg-config by setting PAM_CFLAGS and PAM_LDFLAGS in
the environment before calling configure. This is standard behavior for
the PKG_CHECK_MODULES macro.

The legacy detection logic is maintained when a path is given as an
argument to --with-pam. Note that this logic is broken when libdir is
not "lib".

Signed-off-by: Mike Gilbert <[email protected]>
@floppym
Copy link
Contributor Author

floppym commented Apr 24, 2024

I'd additionally like to see similar change in the other .m4 files for pcap, atm, srp, and others to avoid the pam m4 file from deviating from the new "norm" if you like.

I can sync things up in separate commits once the maintainers approve of what I have done with PAM. No sense in having me do a bunch of extra work if it will be rejected anyway.

@enaess
Copy link
Contributor

enaess commented Apr 24, 2024

Could open an issue against the other .m4 files, but okay from me.

@paulusmack paulusmack merged commit 9c5701c into ppp-project:master Apr 26, 2024
31 checks passed
@Neustradamus Neustradamus removed the request for review from paulusmack May 3, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author Author answer is needed
Development

Successfully merging this pull request may close these issues.

None yet

4 participants