pkgconfig: Apply PKG_CONFIG_SYSROOTDIR when generating paths #7461
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…vironment Previously PkgConfigParser.parse() would read the sysroot directory directly from the `PKG_CONFIG_SYSROOT` environment variable. Passing it in as a parameter is more convenient for testing.
Both `pkgconf` and `pkg-config` reject some of our test .pc files because they don't define the 'Name:', 'Version:' and 'Description:' fields. Fixing this makes it possible to check that SwiftPM's handling of these files matches the upstream utilities.
SwiftPM's pkg-config implementation sets the 'pc_sysrootdir' variable,
but most .pc files do not use this variable directly. Instead, the
pkg-config tool rewrites the generated paths to include the sysroot
prefix when necessary.
There are two major pkg-config implementations which handle sysroot
differently:
* `pkg-config` (the original freedesktop.org implementation) prepends
sysroot after variable expansion, when in creates the compiler flag
lists
* `pkgconf` (a newer implementation) prepends sysroot to variables
when they are defined, so sysroot is included when they are
expanded
pkg-config's method skips single character compiler flags, such as '-I'
and '-L', and has special cases for longer options. It does not handle
spaces between the flags and their values properly, and prepends sysroot
multiple times in some cases, such as when the .pc file uses the
sysroot_dir variable directly or has been rewritten to hard-code the
sysroot prefix.
pkgconf's method handles spaces correctly, although it also makes
extra checks to ensure that sysroot is not applied more than once.
In 2024 pkg-config is the more popular option according to Homebrew
installation statistics, but the major Linux distributions
have generally switched to pkgconf.
We will use pkgconf's method here as it seems more robust than
pkg-config's, and pkgconf's greater popularity on Linux means libraries
developed there may depend on the specific way it handles .pc files.
|
@swift-ci test |
|
|
MaxDesiatov
reviewed
Apr 16, 2024
| } | ||
| } | ||
|
|
||
| // Trim duplicate sysroot prefixes, matching the approach of pkgconf |
There was a problem hiding this comment.
doc comment nit
Suggested change
| // Trim duplicate sysroot prefixes, matching the approach of pkgconf | |
| /// Trim duplicate sysroot prefixes, matching the approach of pkgconf |
MaxDesiatov
reviewed
Apr 16, 2024
| } | ||
| } | ||
|
|
||
| // Apply sysroot to generated paths, matching the approach of pkgconf |
There was a problem hiding this comment.
doc comment nit
Suggested change
| // Apply sysroot to generated paths, matching the approach of pkgconf | |
| /// Apply sysroot to generated paths, matching the approach of pkgconf |
|
@euanh feel free to cherry-pick this to Do you think it's worth mentioning this fix in the changelog? If so, would you mind adding an entry to |
|
Ah, merged somewhat early, sorry. If it is changelog-worthy after all, that can be addressed in a follow-up PR. |
MaxDesiatov
pushed a commit
that referenced
this pull request
Apr 19, 2024
…7472) Cherry pick of #7461 SwiftPM's pkg-config implementation sets the `pc_sysrootdir` variable, but most `.pc` files do not use this variable directly. Instead, they rely on the pkg-config tool rewriting the generated paths to include the sysroot prefix when necessary. SwiftPM does not do this, so it does not generate the correct compiler flags to use libraries from a sysroot. This problem was reported in issue #7409 There are two major pkg-config implementations which handle sysroot differently: * `pkg-config` (the original https://pkg-config.freedesktop.org implementation) prepends sysroot after variable expansion, when it creates the compiler flag lists * `pkgconf` (the newer http://pkgconf.org implementation) prepends sysroot to variables when they are defined, so sysroot is included when they are expanded `pkg-config`'s method skips single character compiler flags, such as `-I` and `-L`, and has special cases for longer options. It does not handle spaces between the flags and their values properly, and prepends sysroot multiple times in some cases, such as when the .pc file uses the `sysroot_dir` variable directly or has been rewritten to hard-code the sysroot prefix. `pkgconf`'s method handles spaces correctly, although it also makes extra checks to ensure that sysroot is not applied more than once. In 2024 `pkg-config` is the more popular option according to Homebrew installation statistics, but the major Linux distributions have generally switched to `pkgconf`. We will use `pkgconf`'s method here as it seems more robust than `pkg-config`'s, and `pkgconf`'s greater popularity on Linux means libraries developed there may depend on the specific way it handles `.pc` files. SwiftPM will now apply the sysroot prefix to compiler flags, such as include (`-I`) and library (`-L`) search paths. This is a partial fix for #7409. The sysroot prefix is only applied when the `PKG_CONFIG_SYSROOT_DIR` environment variable is set. A future commit could apply an appropriate sysroot automatically when the `--experimental-swift-sdk` flag is used. **Scope**: Limited to packages relying on system libraries that utilize pkg-config. **Risk**: Low, changes are isolated and the scope is limited to a small fraction of packages. **Testing**: Automated with new test cases. **Issues**: #7409 **Reviewer**: @MaxDesiatov.
furby-tm
pushed a commit
to wabiverse/swift-package-manager
that referenced
this pull request
May 15, 2024
…ng#7461) pkgconfig: Apply PKG_CONFIG_SYSROOTDIR when generating paths ### Motivation: SwiftPM's pkg-config implementation sets the `pc_sysrootdir` variable, but most `.pc` files do not use this variable directly. Instead, they rely on the pkg-config tool rewriting the generated paths to include the sysroot prefix when necessary. SwiftPM does not do this, so it does not generate the correct compiler flags to use libraries from a sysroot. This problem was reported in issue swiftlang#7409 ### Modifications: There are two major pkg-config implementations which handle sysroot differently: * `pkg-config` (the original https://pkg-config.freedesktop.org implementation) prepends sysroot after variable expansion, when it creates the compiler flag lists * `pkgconf` (the newer http://pkgconf.org implementation) prepends sysroot to variables when they are defined, so sysroot is included when they are expanded `pkg-config`'s method skips single character compiler flags, such as `-I` and `-L`, and has special cases for longer options. It does not handle spaces between the flags and their values properly, and prepends sysroot multiple times in some cases, such as when the .pc file uses the `sysroot_dir` variable directly or has been rewritten to hard-code the sysroot prefix. `pkgconf`'s method handles spaces correctly, although it also makes extra checks to ensure that sysroot is not applied more than once. In 2024 `pkg-config` is the more popular option according to Homebrew installation statistics, but the major Linux distributions have generally switched to `pkgconf`. We will use `pkgconf`'s method here as it seems more robust than `pkg-config`'s, and `pkgconf`'s greater popularity on Linux means libraries developed there may depend on the specific way it handles `.pc` files. ### Result: SwiftPM will now apply the sysroot prefix to compiler flags, such as include (`-I`) and library (`-L`) search paths. This is a partial fix for swiftlang#7409. The sysroot prefix is only applied when the `PKG_CONFIG_SYSROOT_DIR` environment variable is set. A future commit could apply an appropriate sysroot automatically when the `--experimental-swift-sdk` flag is used.
furby-tm
pushed a commit
to wabiverse/swift-package-manager
that referenced
this pull request
May 15, 2024
…ng#7461) pkgconfig: Apply PKG_CONFIG_SYSROOTDIR when generating paths ### Motivation: SwiftPM's pkg-config implementation sets the `pc_sysrootdir` variable, but most `.pc` files do not use this variable directly. Instead, they rely on the pkg-config tool rewriting the generated paths to include the sysroot prefix when necessary. SwiftPM does not do this, so it does not generate the correct compiler flags to use libraries from a sysroot. This problem was reported in issue swiftlang#7409 ### Modifications: There are two major pkg-config implementations which handle sysroot differently: * `pkg-config` (the original https://pkg-config.freedesktop.org implementation) prepends sysroot after variable expansion, when it creates the compiler flag lists * `pkgconf` (the newer http://pkgconf.org implementation) prepends sysroot to variables when they are defined, so sysroot is included when they are expanded `pkg-config`'s method skips single character compiler flags, such as `-I` and `-L`, and has special cases for longer options. It does not handle spaces between the flags and their values properly, and prepends sysroot multiple times in some cases, such as when the .pc file uses the `sysroot_dir` variable directly or has been rewritten to hard-code the sysroot prefix. `pkgconf`'s method handles spaces correctly, although it also makes extra checks to ensure that sysroot is not applied more than once. In 2024 `pkg-config` is the more popular option according to Homebrew installation statistics, but the major Linux distributions have generally switched to `pkgconf`. We will use `pkgconf`'s method here as it seems more robust than `pkg-config`'s, and `pkgconf`'s greater popularity on Linux means libraries developed there may depend on the specific way it handles `.pc` files. ### Result: SwiftPM will now apply the sysroot prefix to compiler flags, such as include (`-I`) and library (`-L`) search paths. This is a partial fix for swiftlang#7409. The sysroot prefix is only applied when the `PKG_CONFIG_SYSROOT_DIR` environment variable is set. A future commit could apply an appropriate sysroot automatically when the `--experimental-swift-sdk` flag is used.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
pkgconfig: Apply PKG_CONFIG_SYSROOTDIR when generating paths
Motivation:
SwiftPM's pkg-config implementation sets the
pc_sysrootdirvariable,but most
.pcfiles do not use this variable directly. Instead, they rely onthe pkg-config tool rewriting the generated paths to include the sysroot
prefix when necessary. SwiftPM does not do this, so it does not generate
the correct compiler flags to use libraries from a sysroot.
This problem was reported in issue #7409
Modifications:
There are two major pkg-config implementations which handle sysroot
differently:
pkg-config(the original https://pkg-config.freedesktop.orgimplementation) prepends sysroot after variable expansion,
when it creates the compiler flag lists
pkgconf(the newer http://pkgconf.org implementation) prependssysroot to variables when they are defined, so sysroot is included
when they are expanded
pkg-config's method skips single character compiler flags, such as-Iand
-L, and has special cases for longer options. It does not handlespaces between the flags and their values properly, and prepends sysroot
multiple times in some cases, such as when the .pc file uses the
sysroot_dirvariable directly or has been rewritten to hard-code thesysroot prefix.
pkgconf's method handles spaces correctly, although it also makesextra checks to ensure that sysroot is not applied more than once.
In 2024
pkg-configis the more popular option according to Homebrewinstallation statistics, but the major Linux distributions have generally
switched to
pkgconf.We will use
pkgconf's method here as it seems more robust thanpkg-config's, andpkgconf's greater popularity on Linux means librariesdeveloped there may depend on the specific way it handles
.pcfiles.Result:
SwiftPM will now apply the sysroot prefix to compiler flags, such as
include (
-I) and library (-L) search paths.This is a partial fix for #7409.
The sysroot prefix is only applied when the
PKG_CONFIG_SYSROOT_DIRenvironment variable is set. A future commit could apply an appropriate
sysroot automatically when the
--experimental-swift-sdkflag is used.