Skip to content

Commit

Permalink
pkgconfig: Apply PKG_CONFIG_SYSROOTDIR when generating paths
Browse files Browse the repository at this point in the history
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
apple#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
apple#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.
  • Loading branch information
euanh committed Apr 19, 2024
1 parent 9ede8a8 commit 7868d7e
Show file tree
Hide file tree
Showing 11 changed files with 206 additions and 6 deletions.
94 changes: 90 additions & 4 deletions Sources/PackageLoading/PkgConfig.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,15 @@ public struct PkgConfig {
name: String,
additionalSearchPaths: [AbsolutePath]? = .none,
brewPrefix: AbsolutePath? = .none,
sysrootDir: AbsolutePath? = .none,
fileSystem: FileSystem,
observabilityScope: ObservabilityScope
) throws {
try self.init(
name: name,
additionalSearchPaths: additionalSearchPaths ?? [],
brewPrefix: brewPrefix,
sysrootDir: sysrootDir,
loadingContext: LoadingContext(),
fileSystem: fileSystem,
observabilityScope: observabilityScope
Expand All @@ -64,6 +66,7 @@ public struct PkgConfig {
name: String,
additionalSearchPaths: [AbsolutePath],
brewPrefix: AbsolutePath?,
sysrootDir: AbsolutePath?,
loadingContext: LoadingContext,
fileSystem: FileSystem,
observabilityScope: ObservabilityScope
Expand All @@ -85,7 +88,7 @@ public struct PkgConfig {
)
}

var parser = try PkgConfigParser(pcFile: pcFile, fileSystem: fileSystem)
var parser = try PkgConfigParser(pcFile: pcFile, fileSystem: fileSystem, sysrootDir: ProcessEnv.block["PKG_CONFIG_SYSROOT_DIR"])
try parser.parse()

func getFlags(from dependencies: [String]) throws -> (cFlags: [String], libs: [String]) {
Expand All @@ -103,6 +106,7 @@ public struct PkgConfig {
name: dep,
additionalSearchPaths: additionalSearchPaths,
brewPrefix: brewPrefix,
sysrootDir: sysrootDir,
loadingContext: loadingContext,
fileSystem: fileSystem,
observabilityScope: observabilityScope
Expand Down Expand Up @@ -162,13 +166,93 @@ internal struct PkgConfigParser {
public private(set) var privateDependencies = [String]()
public private(set) var cFlags = [String]()
public private(set) var libs = [String]()
public private(set) var sysrootDir: String?

public init(pcFile: AbsolutePath, fileSystem: FileSystem) throws {
public init(pcFile: AbsolutePath, fileSystem: FileSystem, sysrootDir: String?) throws {
guard fileSystem.isFile(pcFile) else {
throw StringError("invalid pcfile \(pcFile)")
}
self.pcFile = pcFile
self.fileSystem = fileSystem
self.sysrootDir = sysrootDir
}

// Compress repeated path separators to one.
private func compressPathSeparators(_ value: String) -> String {
let components = value.components(separatedBy: "/").filter { !$0.isEmpty }.joined(separator: "/")
if value.hasPrefix("/") {
return "/" + components
} else {
return components
}
}

// Trim duplicate sysroot prefixes, matching the approach of pkgconf
private func trimDuplicateSysroot(_ value: String) -> String {
// If sysroot has been applied more than once, remove the first instance.
// pkgconf makes this check after variable expansion to handle rare .pc
// files which expand ${pc_sysrootdir} directly:
// https://github.com/pkgconf/pkgconf/issues/123
//
// For example:
// /sysroot/sysroot/remainder -> /sysroot/remainder
//
// However, pkgconf's algorithm searches for an additional sysrootdir anywhere in
// the string after the initial prefix, rather than looking for two sysrootdir prefixes
// directly next to each other:
//
// /sysroot/filler/sysroot/remainder -> /filler/sysroot/remainder
//
// It might seem more logical not to strip sysroot in this case, as it is not a double
// prefix, but for compatibility trimDuplicateSysroot is faithful to pkgconf's approach
// in the functions `pkgconf_tuple_parse` and `should_rewrite_sysroot`.

// Only trim if sysroot is defined with a meaningful value
guard let sysrootDir, sysrootDir != "/" else {
return value
}

// Only trim absolute paths starting with sysroot
guard value.hasPrefix("/"), value.hasPrefix(sysrootDir) else {
return value
}

// If sysroot appears multiple times, trim the prefix
// N.B. sysroot can appear anywhere in the remainder
// of the value, mirroring pkgconf's logic
let pathSuffix = value.dropFirst(sysrootDir.count)
if pathSuffix.contains(sysrootDir) {
return String(pathSuffix)
} else {
return value
}
}

// Apply sysroot to generated paths, matching the approach of pkgconf
private func applySysroot(_ value: String) -> String {
// The two main pkg-config implementations handle sysroot differently:
//
// `pkg-config` (freedesktop.org) prepends sysroot after variable expansion, when in creates the compiler flag lists
// `pkgconf` 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 requires 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.

if value.hasPrefix("/"), let sysrootDir, !value.hasPrefix(sysrootDir) {
return compressPathSeparators(trimDuplicateSysroot(sysrootDir + value))
} else {
return compressPathSeparators(trimDuplicateSysroot(value))
}
}

public mutating func parse() throws {
Expand All @@ -183,7 +267,9 @@ internal struct PkgConfigParser {
variables["pcfiledir"] = pcFile.parentDirectory.pathString

// Add pc_sysrootdir variable. This is the path of the sysroot directory for pc files.
variables["pc_sysrootdir"] = ProcessEnv.block["PKG_CONFIG_SYSROOT_DIR"] ?? AbsolutePath.root.pathString
// pkgconf does not define pc_sysrootdir if the path of the .pc file is outside sysrootdir.
// SwiftPM does not currently make that check.
variables["pc_sysrootdir"] = sysrootDir ?? AbsolutePath.root.pathString

let fileContents: String = try fileSystem.readFileContents(pcFile)
for line in fileContents.components(separatedBy: "\n") {
Expand All @@ -199,7 +285,7 @@ internal struct PkgConfigParser {
// Found a variable.
let (name, maybeValue) = line.spm_split(around: "=")
let value = maybeValue?.spm_chuzzle() ?? ""
variables[name.spm_chuzzle() ?? ""] = try resolveVariables(value)
variables[name.spm_chuzzle() ?? ""] = try applySysroot(resolveVariables(value))
} else {
// Unexpected thing in the pc file, abort.
throw PkgConfigError.parsingError("Unexpected line: \(line) in \(pcFile)")
Expand Down
84 changes: 82 additions & 2 deletions Tests/PackageLoadingTests/PkgConfigParserTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -244,12 +244,92 @@ final class PkgConfigParserTests: XCTestCase {
}
}

func testSysrootDir() throws {
// sysroot should be prepended to all path variables, and should therefore appear in cflags and libs.
try loadPCFile("gtk+-3.0.pc", sysrootDir: "/opt/sysroot/somewhere") { parser in
XCTAssertEqual(parser.variables, [
"libdir": "/opt/sysroot/somewhere/usr/local/Cellar/gtk+3/3.18.9/lib",
"gtk_host": "x86_64-apple-darwin15.3.0",
"includedir": "/opt/sysroot/somewhere/usr/local/Cellar/gtk+3/3.18.9/include",
"prefix": "/opt/sysroot/somewhere/usr/local/Cellar/gtk+3/3.18.9",
"gtk_binary_version": "3.0.0",
"exec_prefix": "/opt/sysroot/somewhere/usr/local/Cellar/gtk+3/3.18.9",
"targets": "quartz",
"pcfiledir": parser.pcFile.parentDirectory.pathString,
"pc_sysrootdir": "/opt/sysroot/somewhere"
])
XCTAssertEqual(parser.dependencies, ["gdk-3.0", "atk", "cairo", "cairo-gobject", "gdk-pixbuf-2.0", "gio-2.0"])
XCTAssertEqual(parser.privateDependencies, ["atk", "epoxy", "gio-unix-2.0"])
XCTAssertEqual(parser.cFlags, ["-I/opt/sysroot/somewhere/usr/local/Cellar/gtk+3/3.18.9/include/gtk-3.0"])
XCTAssertEqual(parser.libs, ["-L/opt/sysroot/somewhere/usr/local/Cellar/gtk+3/3.18.9/lib", "-lgtk-3"])
}

// sysroot should be not be prepended if it is already a prefix
// - pkgconf makes this check, but pkg-config does not
// - If the .pc file lies outside sysrootDir, pkgconf sets pc_sysrootdir to the empty string
// https://github.com/pkgconf/pkgconf/issues/213
// SwiftPM does not currently implement this special case.
try loadPCFile("gtk+-3.0.pc", sysrootDir: "/usr/local/Cellar") { parser in
XCTAssertEqual(parser.variables, [
"libdir": "/usr/local/Cellar/gtk+3/3.18.9/lib",
"gtk_host": "x86_64-apple-darwin15.3.0",
"includedir": "/usr/local/Cellar/gtk+3/3.18.9/include",
"prefix": "/usr/local/Cellar/gtk+3/3.18.9",
"gtk_binary_version": "3.0.0",
"exec_prefix": "/usr/local/Cellar/gtk+3/3.18.9",
"targets": "quartz",
"pcfiledir": parser.pcFile.parentDirectory.pathString,
"pc_sysrootdir": "/usr/local/Cellar"
])
XCTAssertEqual(parser.dependencies, ["gdk-3.0", "atk", "cairo", "cairo-gobject", "gdk-pixbuf-2.0", "gio-2.0"])
XCTAssertEqual(parser.privateDependencies, ["atk", "epoxy", "gio-unix-2.0"])
XCTAssertEqual(parser.cFlags, ["-I/usr/local/Cellar/gtk+3/3.18.9/include/gtk-3.0"])
XCTAssertEqual(parser.libs, ["-L/usr/local/Cellar/gtk+3/3.18.9/lib", "-lgtk-3"])
}

// sysroot should be not be double-prepended if it is used explicitly by the .pc file
// - pkgconf makes this check, but pkg-config does not
try loadPCFile("double_sysroot.pc", sysrootDir: "/sysroot") { parser in
XCTAssertEqual(parser.variables, [
"prefix": "/sysroot/usr",
"datarootdir": "/sysroot/usr/share",
"pkgdatadir": "/sysroot/usr/share/pkgdata",
"pcfiledir": parser.pcFile.parentDirectory.pathString,
"pc_sysrootdir": "/sysroot"
])
}

// pkgconfig strips a leading sysroot prefix if sysroot appears anywhere else in the
// expanded variable. SwiftPM's implementation is faithful to pkgconfig, even
// thought it might seem more logical not to strip the prefix in this case.
try loadPCFile("not_double_sysroot.pc", sysrootDir: "/sysroot") { parser in
XCTAssertEqual(parser.variables, [
"prefix": "/sysroot/usr",
"datarootdir": "/sysroot/usr/share",
"pkgdatadir": "/filler/sysroot/usr/share/pkgdata",
"pcfiledir": parser.pcFile.parentDirectory.pathString,
"pc_sysrootdir": "/sysroot"
])
}

// pkgconfig does not strip sysroot if it is a relative path
try loadPCFile("double_sysroot.pc", sysrootDir: "sysroot") { parser in
XCTAssertEqual(parser.variables, [
"prefix": "sysroot/usr",
"datarootdir": "sysroot/usr/share",
"pkgdatadir": "sysroot/sysroot/usr/share/pkgdata",
"pcfiledir": parser.pcFile.parentDirectory.pathString,
"pc_sysrootdir": "sysroot"
])
}
}

private func pcFilePath(_ inputName: String) -> AbsolutePath {
return AbsolutePath(#file).parentDirectory.appending(components: "pkgconfigInputs", inputName)
}

private func loadPCFile(_ inputName: String, body: ((PkgConfigParser) -> Void)? = nil) throws {
var parser = try PkgConfigParser(pcFile: pcFilePath(inputName), fileSystem: localFileSystem)
private func loadPCFile(_ inputName: String, sysrootDir: String? = nil, body: ((PkgConfigParser) -> Void)? = nil) throws {
var parser = try PkgConfigParser(pcFile: pcFilePath(inputName), fileSystem: localFileSystem, sysrootDir: sysrootDir)
try parser.parse()
body?(parser)
}
Expand Down
3 changes: 3 additions & 0 deletions Tests/PackageLoadingTests/pkgconfigInputs/case_insensitive.pc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ prefix=/usr/local/bin
exec_prefix=${prefix}

#some comment
Name: case_insensitive
Version: 1
Description: Demonstrate that pkgconfig keys are case-insensitive

# upstream pkg-config parser allows Cflags & CFlags as key
CFlags: -I/usr/local/include
3 changes: 3 additions & 0 deletions Tests/PackageLoadingTests/pkgconfigInputs/deps_variable.pc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ prefix=/usr/local/bin
exec_prefix=${prefix}
my_dep=atk
#some comment
Name: deps_variable
Version: 1
Description: Demonstrate use of a locally-defined variable

Requires: gdk-3.0 >= 1.0.0 ${my_dep}
Libs: -L${prefix} -lgtk-3
Expand Down
7 changes: 7 additions & 0 deletions Tests/PackageLoadingTests/pkgconfigInputs/double_sysroot.pc
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
prefix=/usr
datarootdir=${prefix}/share
pkgdatadir=${pc_sysrootdir}/${datarootdir}/pkgdata

Name: double_sysroot
Description: Demonstrate double-prefixing of pc_sysrootdir (https://github.com/pkgconf/pkgconf/issues/123)
Version: 1
3 changes: 3 additions & 0 deletions Tests/PackageLoadingTests/pkgconfigInputs/dummy_dependency.pc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ prefix=/usr/local/bin
exec_prefix=${prefix}

#some comment
Name: dummy_dependency
Version: 1
Description: Demonstrate a blank dependency entry

Requires: pango, , fontconfig >= 2.13.0
Libs:-L${prefix} -lpangoft2-1.0
3 changes: 3 additions & 0 deletions Tests/PackageLoadingTests/pkgconfigInputs/empty_cflags.pc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ prefix=/usr/local/bin
exec_prefix=${prefix}

#some comment
Name: empty_cflags
Version: 1
Description: Demonstrate an empty cflags list

Requires: gdk-3.0 atk
Libs:-L${prefix} -lgtk-3
Expand Down
3 changes: 3 additions & 0 deletions Tests/PackageLoadingTests/pkgconfigInputs/escaped_spaces.pc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ prefix=/usr/local/bin
exec_prefix=${prefix}
my_dep=atk
#some comment
Name: escaped_spaces
Version: 1
Description: Demonstrate use of escape characters in flag values

Requires: gdk-3.0 >= 1.0.0 ${my_dep}
Libs: -L"${prefix}" -l"gtk 3" -wantareal\\here -one\\ -two
Expand Down
3 changes: 3 additions & 0 deletions Tests/PackageLoadingTests/pkgconfigInputs/failure_case.pc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ prefix=/usr/local/bin
exec_prefix=${prefix}

#some comment
Name: failure_case
Version: 1
Description: Demonstrate failure caused by use of an undefined variable

Requires: gdk-3.0 >= 1.0.0
Libs: -L${prefix} -lgtk-3 ${my_dep}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
prefix=/usr
datarootdir=${prefix}/share
pkgdatadir=${pc_sysrootdir}/filler/${datarootdir}/pkgdata

Name: double_sysroot
Description: Demonstrate pc_sysrootdir appearing elsewhere in a path - this is not a double prefix and should not be removed
3 changes: 3 additions & 0 deletions Tests/PackageLoadingTests/pkgconfigInputs/quotes_failure.pc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ prefix=/usr/local/bin
exec_prefix=${prefix}
my_dep=atk
#some comment
Name: quotes_failure
Version: 1
Description: Demonstrate failure due to unbalanced quotes

Requires: gdk-3.0 >= 1.0.0 ${my_dep}
Libs: -L"${prefix}" -l"gt"k3" -wantareal\\here -one\\ -two
Expand Down

0 comments on commit 7868d7e

Please sign in to comment.