pkgconfig: Apply PKG_CONFIG_SYSROOTDIR when generating paths

Sources/PackageLoading/PkgConfig.swift

@@ -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
@@ -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)")

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)
     }

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

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 

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

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

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 

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

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}

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

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