diff --git a/Sources/PackageLoading/PkgConfig.swift b/Sources/PackageLoading/PkgConfig.swift index 2fdb1d60788..8838be90135 100644 --- a/Sources/PackageLoading/PkgConfig.swift +++ b/Sources/PackageLoading/PkgConfig.swift @@ -47,6 +47,7 @@ public struct PkgConfig { name: String, additionalSearchPaths: [AbsolutePath]? = .none, brewPrefix: AbsolutePath? = .none, + sysrootDir: AbsolutePath? = .none, fileSystem: FileSystem, observabilityScope: ObservabilityScope ) throws { @@ -54,6 +55,7 @@ public struct PkgConfig { name: name, additionalSearchPaths: additionalSearchPaths ?? [], brewPrefix: brewPrefix, + sysrootDir: sysrootDir, loadingContext: LoadingContext(), fileSystem: fileSystem, observabilityScope: observabilityScope @@ -64,6 +66,7 @@ public struct PkgConfig { name: String, additionalSearchPaths: [AbsolutePath], brewPrefix: AbsolutePath?, + sysrootDir: AbsolutePath?, loadingContext: LoadingContext, fileSystem: FileSystem, observabilityScope: ObservabilityScope @@ -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]) { @@ -103,6 +106,7 @@ public struct PkgConfig { name: dep, additionalSearchPaths: additionalSearchPaths, brewPrefix: brewPrefix, + sysrootDir: sysrootDir, loadingContext: loadingContext, fileSystem: fileSystem, observabilityScope: observabilityScope @@ -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 { @@ -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") { @@ -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)") diff --git a/Tests/PackageLoadingTests/PkgConfigParserTests.swift b/Tests/PackageLoadingTests/PkgConfigParserTests.swift index 009f1328ff4..110bf941ff7 100644 --- a/Tests/PackageLoadingTests/PkgConfigParserTests.swift +++ b/Tests/PackageLoadingTests/PkgConfigParserTests.swift @@ -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) } diff --git a/Tests/PackageLoadingTests/pkgconfigInputs/case_insensitive.pc b/Tests/PackageLoadingTests/pkgconfigInputs/case_insensitive.pc index b65a14c7d16..83017682ea9 100644 --- a/Tests/PackageLoadingTests/pkgconfigInputs/case_insensitive.pc +++ b/Tests/PackageLoadingTests/pkgconfigInputs/case_insensitive.pc @@ -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 diff --git a/Tests/PackageLoadingTests/pkgconfigInputs/deps_variable.pc b/Tests/PackageLoadingTests/pkgconfigInputs/deps_variable.pc index 225d86d1369..de06c50db85 100644 --- a/Tests/PackageLoadingTests/pkgconfigInputs/deps_variable.pc +++ b/Tests/PackageLoadingTests/pkgconfigInputs/deps_variable.pc @@ -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 diff --git a/Tests/PackageLoadingTests/pkgconfigInputs/double_sysroot.pc b/Tests/PackageLoadingTests/pkgconfigInputs/double_sysroot.pc new file mode 100644 index 00000000000..be7abee01ce --- /dev/null +++ b/Tests/PackageLoadingTests/pkgconfigInputs/double_sysroot.pc @@ -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 diff --git a/Tests/PackageLoadingTests/pkgconfigInputs/dummy_dependency.pc b/Tests/PackageLoadingTests/pkgconfigInputs/dummy_dependency.pc index c4ae253c81a..e3a797923da 100644 --- a/Tests/PackageLoadingTests/pkgconfigInputs/dummy_dependency.pc +++ b/Tests/PackageLoadingTests/pkgconfigInputs/dummy_dependency.pc @@ -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 diff --git a/Tests/PackageLoadingTests/pkgconfigInputs/empty_cflags.pc b/Tests/PackageLoadingTests/pkgconfigInputs/empty_cflags.pc index 0f7f8e6c1a2..a7d2ffa6bae 100644 --- a/Tests/PackageLoadingTests/pkgconfigInputs/empty_cflags.pc +++ b/Tests/PackageLoadingTests/pkgconfigInputs/empty_cflags.pc @@ -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 diff --git a/Tests/PackageLoadingTests/pkgconfigInputs/escaped_spaces.pc b/Tests/PackageLoadingTests/pkgconfigInputs/escaped_spaces.pc index f00283e070f..e0e77e73774 100644 --- a/Tests/PackageLoadingTests/pkgconfigInputs/escaped_spaces.pc +++ b/Tests/PackageLoadingTests/pkgconfigInputs/escaped_spaces.pc @@ -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 diff --git a/Tests/PackageLoadingTests/pkgconfigInputs/failure_case.pc b/Tests/PackageLoadingTests/pkgconfigInputs/failure_case.pc index e81dc924502..f1981c1b3d4 100644 --- a/Tests/PackageLoadingTests/pkgconfigInputs/failure_case.pc +++ b/Tests/PackageLoadingTests/pkgconfigInputs/failure_case.pc @@ -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} diff --git a/Tests/PackageLoadingTests/pkgconfigInputs/not_double_sysroot.pc b/Tests/PackageLoadingTests/pkgconfigInputs/not_double_sysroot.pc new file mode 100644 index 00000000000..c537069beb6 --- /dev/null +++ b/Tests/PackageLoadingTests/pkgconfigInputs/not_double_sysroot.pc @@ -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 \ No newline at end of file diff --git a/Tests/PackageLoadingTests/pkgconfigInputs/quotes_failure.pc b/Tests/PackageLoadingTests/pkgconfigInputs/quotes_failure.pc index 3006c2fbd85..07ec4503a76 100644 --- a/Tests/PackageLoadingTests/pkgconfigInputs/quotes_failure.pc +++ b/Tests/PackageLoadingTests/pkgconfigInputs/quotes_failure.pc @@ -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