[6.0] pkgconfig: Apply PKG_CONFIG_SYSROOTDIR when generating paths (swiftlang#7472)

Cherry pick of swiftlang#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
swiftlang#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
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.

**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**: swiftlang#7409
**Reviewer**: @MaxDesiatov.

Author: euanh

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