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 ExtUtils::PkgConfig to discover the CFLAGS and LDFLAGS for external libraries #210

Merged
merged 4 commits into from
Nov 15, 2024

Conversation

marc-vanderwal
Copy link
Contributor

Purpose

This PR aims to make detection of CFLAGS and LDFLAGS more reliable and portable across *nix systems by calling out to pkg-config. By doing so, libraries that have been installed by hand (in /usr/local or a private prefix such as $HOME/.local) or through alternative package managers are more likely to be successfully detected.

Context

See #129.

Changes

  • Use ExtUtils::PkgConfig to detect CFLAGS and LDFLAGS for libraries we link against.
  • Install ExtUtils::PkgConfig in the build environment when generating the Docker image.

How to test this PR

Run perl Makefile.PL; it should still work.

Building the Dockerfile should also still work.

@marc-vanderwal marc-vanderwal force-pushed the bugfix/#129 branch 2 times, most recently from 1992abc to 3998044 Compare November 12, 2024 10:10
@matsduf matsduf added T-Feature Type: New feature in software or test case description V-Patch Versioning: The change gives an update of patch in version. labels Nov 12, 2024
@matsduf
Copy link
Contributor

matsduf commented Nov 12, 2024

@marc-vanderwal, milestone?

@marc-vanderwal
Copy link
Contributor Author

I wasn’t sure about the milestone. It might be nice to have it in 2024.2, though. What do you think?

@matsduf
Copy link
Contributor

matsduf commented Nov 12, 2024

On FreeBSD:

Can't locate ExtUtils/PkgConfig.pm in @INC (you may need to install the ExtUtils::PkgConfig module) (@INC contains: inc /usr/local/lib/perl5/site_perl/mach/5.36 /usr/local/lib/perl5/site_perl /usr/local/lib/perl5/5.36/mach /usr/local/lib/perl5/5.36) at Makefile.PL line 3.

Installation instructions for build environment must be updated too. https://github.com/zonemaster/zonemaster/tree/develop/docs/internal/distrib-testing

@matsduf
Copy link
Contributor

matsduf commented Nov 12, 2024

I wasn’t sure about the milestone. It might be nice to have it in 2024.2, though. What do you think?

If other documents are updated too it should be fine.

@marc-vanderwal
Copy link
Contributor Author

marc-vanderwal commented Nov 12, 2024

Yes, I’m preparing a PR for the installation instructions in zonemaster/zonemaster because we need one more package to be installed. I’ll set the milestone to 2024.2 then.

@marc-vanderwal marc-vanderwal added this to the v2024.2 milestone Nov 12, 2024
marc-vanderwal added a commit to marc-vanderwal/zonemaster that referenced this pull request Nov 12, 2024
A pull request on zonemaster-ldns [1] now requires ExtUtils::PkgConfig.
Update the documentation to that effect.

[1]: zonemaster/zonemaster-ldns#210
marc-vanderwal added a commit to marc-vanderwal/zonemaster that referenced this pull request Nov 12, 2024
A pull request on zonemaster-ldns [1] now requires ExtUtils::PkgConfig.
Update the documentation to that effect.

[1]: zonemaster/zonemaster-ldns#210
On platforms or systems where libraries might be installed somewhere
else than the usual /usr/local/lib or /usr/lib, running perl Makefile.PL
might fail to detect the location of the libraries.

Fortunately, pkg-config can help us determine which appropriate CFLAGS
and LDFLAGS to append to the compiler and linker command lines
respectively, and ExtUtils::PkgConfig provides a simple wrapper around
that command line utility.

This unfortunately requires one more dependency, but that library is
widely packaged across Linux distributions and FreeBSD. It’s packaged as
follows:

 * on Alpine Linux: perl-extutils-pkgconfig;
 * on Debian-based OSes: libextutils-pkgconfig-perl;
 * on FreeBSD: p5-ExtUtils-PkgConfig;
 * on Red Hat-based OSes: perl-ExtUtils-PkgConfig.
Install ExtUtils::PkgConfig in the build container.
As a drive-by change, update the NSEC test in rr.t because the .se has
started using ZONEMD.
Copy link
Member

@mattias-p mattias-p left a comment

Choose a reason for hiding this comment

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

I tested this on Arch Linux and it seems to work.

@marc-vanderwal marc-vanderwal merged commit ae904a7 into zonemaster:develop Nov 15, 2024
3 checks passed
marc-vanderwal added a commit to marc-vanderwal/zonemaster-engine that referenced this pull request Nov 21, 2024
Pull request zonemaster/zonemaster-ldns#210
added an additional build-time dependency on ExtUtils::PkgConfig that
wasn’t mirrored in the CI configuration for Zonemaster::Engine,
therefore breaking CI. Adding the missing dependency should restore CI
to normal working state.
marc-vanderwal added a commit to marc-vanderwal/zonemaster-engine that referenced this pull request Nov 21, 2024
Pull request zonemaster/zonemaster-ldns#210
added an additional build-time dependency on ExtUtils::PkgConfig that
wasn’t mirrored in the CI configuration for Zonemaster::Engine,
therefore breaking CI. Adding the missing dependency should restore CI
to normal working state.
marc-vanderwal added a commit to marc-vanderwal/zonemaster-engine that referenced this pull request Nov 21, 2024
Pull request zonemaster/zonemaster-ldns#210
added an additional build-time dependency on ExtUtils::PkgConfig that
wasn’t mirrored in the CI configuration for Zonemaster::Engine,
therefore breaking CI. Adding the missing dependency should restore CI
to normal working state.
marc-vanderwal added a commit to marc-vanderwal/zonemaster-engine that referenced this pull request Nov 21, 2024
Pull request zonemaster/zonemaster-ldns#210
added an additional build-time dependency on ExtUtils::PkgConfig that
wasn’t mirrored in the CI configuration for Zonemaster::Engine,
therefore breaking CI. Adding the missing dependency should restore CI
to normal working state.
marc-vanderwal added a commit to marc-vanderwal/zonemaster-ldns that referenced this pull request Nov 21, 2024
Pull request zonemaster#210 forgot to document the change in README.md. Let’s fix
that before quickly before the next release.
@matsduf matsduf linked an issue Nov 24, 2024 that may be closed by this pull request
@MichaelTimbert MichaelTimbert added the S-ReleaseTested Status: The PR has been successfully tested in release testing label Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-ReleaseTested Status: The PR has been successfully tested in release testing T-Feature Type: New feature in software or test case description V-Patch Versioning: The change gives an update of patch in version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

libidn not found with Homebrew on Mac
4 participants