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

pkgconfig: Disable pkgconf and pkg-config's path stripping #10644

Closed
wants to merge 1 commit into from

Conversation

michaelkuhn
Copy link
Member

By default, pkgconf and pkg-config strip out system paths. This can cause problems because Spack sets CPATH, which is taken into account when determining the system paths.

The difference of this change can be tested with: spack build-env gettext -- pkg-config --cflags libxml-2.0

Before this change, the output should be empty because the libxml2 package explicitly sets CPATH, which causes pkgconf and pkg-config to strip out its include directory. After this change, the output should contain the include directory.

@scheibelp
Copy link
Member

Enabling system includes could work well with Spack since it prepends all dependency include directories. Although there could be cases where

  • Spack misses nested include directories
  • Those directories would be picked up by pkgconfig
  • But there are competing headers in /usr/include, which may appear before the Spack-installed header directories

That seems like a small risk considering it at first. Are you aware of issues with enabling system includes?

Side note: Ideally (although the logic is being reworked now e.g. at #10623), another option would be to add an override for .headers in the libxml2 package. Although if libxml2 defines a .pc file which mentions <libxml2-prefix>/include/libxml2/, then dependencies which use pkgconfig should be able to locate it without libxml2 defining a headers method (and also without updating CPATH).

@michaelkuhn
Copy link
Member Author

That seems like a small risk considering it at first. Are you aware of issues with enabling system includes?

As far as I know, this should not change anything. I have also never seen a pkg-config file that includes "real" system paths such as /usr/include.

I have actually needed to set these variables in the past to make builds work for software (outside of Spack) that used Spack packages as dependencies: pkg-config was called while the modules were loaded (and thus CPATH was set), which caused the packages' paths to be stripped. Afterwards the build failed whenever the modules were not loaded.

TBH, this might be a niche use case but I also find it counter-intuitive to strip out paths that were set explicitly in the pkg-config file.

@scheibelp
Copy link
Member

Apologies but #10675 created a minor conflict here.

For Spack-based builds, I'm leaning toward #10623 being a better long-term solution: generally when this CPATH warning is printed, I think it means that the Spack package should override .headers; otherwise, for packages which don't use pkgconfig, Spack will not set up the appropriate -I options.

I have actually needed to set these variables in the past to make builds work for software (outside of Spack) that used Spack packages as dependencies

I think a workable approach would be to make use of SPACK_INCLUDE_DIRS after #10623. If I understand correctly, you mean that you use the CPATH settings made by some Spack packages. SPACK_INCLUDE_DIRS should contain all those directories as a subset.

I suppose a middle ground would be to keep some sort of debug message encouraging the user to update .headers but to allow system includes.

I have also never seen a pkg-config file that includes "real" system paths such as /usr/include.

That is useful info, but on the other hand I don't think this is guaranteed. For example at https://linux.die.net/man/1/pkg-config:

PKG_CONFIG_ALLOW_SYSTEM_CFLAGS
Don't strip -I/usr/include out of cflags.

By default, pkgconf and pkg-config strip out system paths. This can
cause problems because Spack sets `CPATH`, which is taken into
account when determining the system paths.

The difference of this change can be tested with:
`spack build-env gettext -- pkg-config --cflags libxml-2.0`

Before this change, the output should be empty because the libxml2
package explicitly sets `CPATH`, which causes pkgconf and pkg-config to
strip out its include directory. After this change, the output should
contain the include directory.
@michaelkuhn
Copy link
Member Author

Apologies but #10675 created a minor conflict here.

No problem. :-) I have pushed an update that resolves the conflict.

For Spack-based builds, I'm leaning toward #10623 being a better long-term solution: generally when this CPATH warning is printed, I think it means that the Spack package should override .headers; otherwise, for packages which don't use pkgconfig, Spack will not set up the appropriate -I options.

Good point. I have updated the PR to leave the warning alone. #10623 should then probably update it to steer packagers towards .headers.

I think a workable approach would be to make use of SPACK_INCLUDE_DIRS after #10623. If I understand correctly, you mean that you use the CPATH settings made by some Spack packages. SPACK_INCLUDE_DIRS should contain all those directories as a subset.

The problem I ran into was actually just pkg-config stripping out paths that it believed to be system paths (because Spack's modules set CPATH).

That is useful info, but on the other hand I don't think this is guaranteed. For example at https://linux.die.net/man/1/pkg-config:

PKG_CONFIG_ALLOW_SYSTEM_CFLAGS
Don't strip -I/usr/include out of cflags.

I also wondered about this. One way would be to set PKG_CONFIG_SYSTEM_INCLUDE_PATH and PKG_CONFIG_SYSTEM_LIBRARY_PATH to /usr/include and /usr/lib but the latter is not guaranteed to be /usr/lib on all distributions and this apparently is a pkgconf-specific extension (that is, does not work with the legacy pkg-config).

@scheibelp
Copy link
Member

The problem I ran into was actually just pkg-config stripping out paths that it believed to be system paths (because Spack's modules set CPATH).

I think I understand: you are loading Spack-generated modules which set CPATH, and that is interfering with non-Spack builds?

If so, one option is to use the environment_blacklist configuration option for Spack module generation (see: https://spack.readthedocs.io/en/latest/tutorial_modules.html#filter-unwanted-modifications-to-the-environment).

I'm primarily digging into this because the uncertainty on the behavior of PKG_CONFIG_ALLOW_SYSTEM_CFLAGS.

@michaelkuhn
Copy link
Member Author

If so, one option is to use the environment_blacklist configuration option for Spack module generation (see: https://spack.readthedocs.io/en/latest/tutorial_modules.html#filter-unwanted-modifications-to-the-environment).

I do not really want to change the module files themselves since other people might expect the unmodified ones.

I'm primarily digging into this because the uncertainty on the behavior of PKG_CONFIG_ALLOW_SYSTEM_CFLAGS.

I typically prefer pkg-config to return what the authors of the .pc file intended. Even though I have not encountered such a case yet, the deduplication might screw up the ordering of include directories etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants