use idiomatic location for security directory and update location of configuration directory

tomerd · tomerd · commit 72c774c3d966 · 2021-12-14T11:42:40.000-08:00
motivation: use idiomatic location for security directory on macOS

changes:
* allow users to customize security directory location with new --security-path CLI option
* on macOS use &lt;user&gt;/Library/org.swiftpm/security for security files and symlink from ~/.swiftpm/security
* move configuration directory from &lt;user&gt;/Library/org.swiftpm to &lt;user&gt;/Library/org.swiftpm/configuration
* add migration code from old configuraiton location to new one
* add and adjust tests
* update docker setup for new locations
diff --git a/Sources/Basics/FileSystem+Extensions.swift b/Sources/Basics/FileSystem+Extensions.swift
@@ -21,10 +21,9 @@ extension FileSystem {
     public var dotSwiftPM: AbsolutePath {
         self.homeDirectory.appending(component: ".swiftpm")
     }
-    
-    /// SwiftPM security directory
-    public var swiftPMSecurityDirectory: AbsolutePath {
-        self.dotSwiftPM.appending(component: "security")
+
+    fileprivate var idiomaticSwiftPMDirectory: AbsolutePath? {
+        return FileManager.default.urls(for: .libraryDirectory, in: .userDomainMask).first.flatMap { AbsolutePath($0.path) }?.appending(component: "org.swift.swiftpm")
     }
 }
 
@@ -69,52 +68,117 @@ extension FileSystem {
     }
 }
 
-// MARK: - config
+// MARK: - configuration
 
 extension FileSystem {
-    private var idiomaticUserConfigDirectory: AbsolutePath? {
+    /*private var idiomaticUserConfigDirectory: AbsolutePath? {
         return FileManager.default.urls(for: .libraryDirectory, in: .userDomainMask).first.flatMap { AbsolutePath($0.path) }
-    }
+    }*/
 
     /// SwiftPM config directory under user's config directory (if exists)
-    public var swiftPMConfigDirectory: AbsolutePath {
-        if let path = self.idiomaticUserConfigDirectory {
-            return path.appending(component: "org.swift.swiftpm")
+    public var swiftPMConfigurationDirectory: AbsolutePath {
+        if let path = self.idiomaticSwiftPMDirectory {
+            return path.appending(component: "configuration")
         } else {
-            return self.dotSwiftPMConfigDirectory
+            return self.dotSwiftPMConfigurationDirectory
         }
     }
 
-    fileprivate var dotSwiftPMConfigDirectory: AbsolutePath {
-        return self.dotSwiftPM.appending(component: "config")
+    fileprivate var dotSwiftPMConfigurationDirectory: AbsolutePath {
+        return self.dotSwiftPM.appending(component: "configuration")
+    }
+}
+
+extension FileSystem {
+    public func getOrCreateSwiftPMConfigurationDirectory() throws -> AbsolutePath {
+        let idiomaticConfigurationDirectory = self.swiftPMConfigurationDirectory
+
+        // temporary 5.6, remove on next version: transition from previous configuration location
+        if idiomaticConfigurationDirectory != self.dotSwiftPMConfigurationDirectory {
+            // move content from old directory to new one but create symlink for the files for backwards compatibility (eg older xcode)
+            if !self.exists(idiomaticConfigurationDirectory) {
+                try self.createDirectory(idiomaticConfigurationDirectory, recursive: true)
+            }
+            let oldConfigDirectory = idiomaticConfigurationDirectory.parentDirectory
+            let content = try self.getDirectoryContents(oldConfigDirectory)
+            for item in content {
+                if self.isFile(oldConfigDirectory.appending(component: item)) && !self.isSymlink(oldConfigDirectory.appending(component: item)) {
+                    try self.move(from: oldConfigDirectory.appending(component: item), to: idiomaticConfigurationDirectory.appending(component: item))
+                    try self.createSymbolicLink(oldConfigDirectory.appending(component: item), pointingAt: idiomaticConfigurationDirectory.appending(component: item), relative: false)
+                }
+            }
+            // remove old symlink
+            let oldConfigSymlink = self.dotSwiftPM.appending(component: "config")
+            if self.exists(oldConfigSymlink, followSymlink: false) && self.isSymlink(oldConfigSymlink) {
+                try self.removeFileTree(oldConfigSymlink)
+            }
+        } else {
+            // rename the old directory to the new one but create symlink for backwards compatibility (just in case someone relies on it)
+            let oldConfigDirectory = self.dotSwiftPM.appending(component: "config")
+            if self.exists(oldConfigDirectory, followSymlink: false) && self.isDirectory(oldConfigDirectory) {
+                try self.move(from: oldConfigDirectory, to: idiomaticConfigurationDirectory)
+                try self.createSymbolicLink(oldConfigDirectory, pointingAt: idiomaticConfigurationDirectory, relative: false)
+            }
+        }
+
+        // Create idiomatic if necessary
+        if !self.exists(idiomaticConfigurationDirectory) {
+            try self.createDirectory(idiomaticConfigurationDirectory, recursive: true)
+        }
+        // Create ~/.swiftpm if necessary
+        if !self.exists(self.dotSwiftPM) {
+            try self.createDirectory(self.dotSwiftPM, recursive: true)
+        }
+        // Create ~/.swiftpm/configuration symlink if necessary
+        if !self.exists(self.dotSwiftPMConfigurationDirectory, followSymlink: false) {
+            try self.createSymbolicLink(dotSwiftPMConfigurationDirectory, pointingAt: idiomaticConfigurationDirectory, relative: false)
+        }
+
+        return idiomaticConfigurationDirectory
     }
 }
 
+// MARK: - security
+
 extension FileSystem {
-    public func getOrCreateSwiftPMConfigDirectory() throws -> AbsolutePath {
-        let idiomaticConfigDirectory = self.swiftPMConfigDirectory
+    /// SwiftPM security directory under user's security directory (if exists)
+    public var swiftPMSecurityDirectory: AbsolutePath {
+        if let path = self.idiomaticSwiftPMDirectory {
+            return path.appending(component: "security")
+        } else {
+            return self.dotSwiftPMSecurityDirectory
+        }
+    }
 
-        // temporary 5.5, remove on next version: transition from ~/.swiftpm/config to idiomatic location + symbolic link
-        if idiomaticConfigDirectory != self.dotSwiftPMConfigDirectory &&
-            self.exists(self.dotSwiftPMConfigDirectory) && self.isDirectory(self.dotSwiftPMConfigDirectory) &&
-            !self.exists(idiomaticConfigDirectory) {
-            print("transitioning \(self.dotSwiftPMConfigDirectory) to \(idiomaticConfigDirectory)")
-            try self.move(from: self.dotSwiftPMConfigDirectory, to: idiomaticConfigDirectory)
+    fileprivate var dotSwiftPMSecurityDirectory: AbsolutePath {
+        return self.dotSwiftPM.appending(component: "security")
+    }
+}
+
+extension FileSystem {
+    public func getOrCreateSwiftPMSecurityDirectory() throws -> AbsolutePath {
+        let idiomaticSecurityDirectory = self.swiftPMSecurityDirectory
+
+        // temporary 5.6, remove on next version: transition from ~/.swiftpm/security to idiomatic location + symbolic link
+        if idiomaticSecurityDirectory != self.dotSwiftPMSecurityDirectory &&
+            self.exists(self.dotSwiftPMSecurityDirectory) &&
+            self.isDirectory(self.dotSwiftPMSecurityDirectory) {
+            try self.removeFileTree(self.dotSwiftPMSecurityDirectory)
         }
 
         // Create idiomatic if necessary
-        if !self.exists(idiomaticConfigDirectory) {
-            try self.createDirectory(idiomaticConfigDirectory, recursive: true)
+        if !self.exists(idiomaticSecurityDirectory) {
+            try self.createDirectory(idiomaticSecurityDirectory, recursive: true)
         }
         // Create ~/.swiftpm if necessary
         if !self.exists(self.dotSwiftPM) {
             try self.createDirectory(self.dotSwiftPM, recursive: true)
         }
-        // Create ~/.swiftpm/config symlink if necessary
-        if !self.exists(self.dotSwiftPMConfigDirectory, followSymlink: false) {
-            try self.createSymbolicLink(dotSwiftPMConfigDirectory, pointingAt: idiomaticConfigDirectory, relative: false)
+        // Create ~/.swiftpm/security symlink if necessary
+        if !self.exists(self.dotSwiftPMSecurityDirectory, followSymlink: false) {
+            try self.createSymbolicLink(dotSwiftPMSecurityDirectory, pointingAt: idiomaticSecurityDirectory, relative: false)
         }
-        return idiomaticConfigDirectory
+        return idiomaticSecurityDirectory
     }
 }
 
diff --git a/Sources/Commands/Options.swift b/Sources/Commands/Options.swift
@@ -149,6 +149,9 @@ public struct SwiftToolOptions: ParsableArguments {
     @Option(help: "Specify the shared configuration directory")
     var configPath: AbsolutePath?
 
+    @Option(help: "Specify the shared security directory")
+    var securityPath: AbsolutePath?
+
     /// Disables repository caching.
     @Flag(name: .customLong("repository-cache"), inversion: .prefixedEnableDisable, help: "Use a shared cache when fetching repositories")
     var useRepositoriesCache: Bool = true
diff --git a/Sources/Commands/SwiftTool.swift b/Sources/Commands/SwiftTool.swift
@@ -246,6 +246,8 @@ protocol SwiftCommand: ParsableCommand {
 extension SwiftCommand {
     public func run() throws {
         let swiftTool = try SwiftTool(options: swiftOptions)
+        // make sure common directories are created
+        try swiftTool.createSharedDirectories()
         try self.run(swiftTool)
         if swiftTool.observabilityScope.errorsReported || swiftTool.executionStatus == .failure {
             throw ExitCode.failure
@@ -634,29 +636,39 @@ public class SwiftTool {
         }
 
         do {
-            return try localFileSystem.getOrCreateSwiftPMConfigDirectory()
+            return try localFileSystem.getOrCreateSwiftPMConfigurationDirectory()
         } catch {
             self.observabilityScope.emit(warning: "Failed creating default configuration location, \(error)")
             return .none
         }
     }
     
     private func getSharedSecurityDirectory() throws -> AbsolutePath? {
-        do {
-            let fileSystem = localFileSystem
-            let sharedSecurityDirectory = fileSystem.swiftPMSecurityDirectory
-            if !fileSystem.exists(sharedSecurityDirectory) {
-                try fileSystem.createDirectory(sharedSecurityDirectory, recursive: true)
+        if let explicitSecurityPath = options.securityPath {
+            // Create the explicit security path if necessary
+            if !localFileSystem.exists(explicitSecurityPath) {
+                try localFileSystem.createDirectory(explicitSecurityPath, recursive: true)
             }
+            return explicitSecurityPath
+        }
+
+        do {
+            let sharedSecurityDirectory = try localFileSystem.getOrCreateSwiftPMSecurityDirectory()
             // And make sure we can write files (locking the directory writes a lock file)
-            try fileSystem.withLock(on: sharedSecurityDirectory, type: .exclusive) { }
+            try localFileSystem.withLock(on: sharedSecurityDirectory, type: .exclusive) { }
             return sharedSecurityDirectory
         } catch {
-            self.observabilityScope.emit(warning: "Failed creating shared security directory: \(error)")
+            self.observabilityScope.emit(warning: "Failed creating default security location, \(error)")
             return .none
         }
     }
 
+    fileprivate func createSharedDirectories() throws {
+        _ = try getSharedCacheDirectory()
+        _ = try getSharedConfigurationDirectory()
+        _ = try getSharedSecurityDirectory()
+    }
+
     /// Returns the currently active workspace.
     func getActiveWorkspace() throws -> Workspace {
         if let workspace = _workspace {
diff --git a/Sources/PackageCollections/Providers/JSONPackageCollectionProvider.swift b/Sources/PackageCollections/Providers/JSONPackageCollectionProvider.swift
@@ -55,7 +55,7 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
         self.decoder = JSONDecoder.makeWithDefaults()
         self.validator = JSONModel.Validator(configuration: configuration.validator)
         self.signatureValidator = signatureValidator ?? PackageCollectionSigning(
-            trustedRootCertsDir: configuration.trustedRootCertsDir ?? fileSystem.swiftPMConfigDirectory.appending(component: "trust-root-certs").asURL,
+            trustedRootCertsDir: configuration.trustedRootCertsDir ?? fileSystem.swiftPMConfigurationDirectory.appending(component: "trust-root-certs").asURL,
             additionalTrustedRootCerts: sourceCertPolicy.allRootCerts.map { Array($0) },
             observabilityScope: observabilityScope,
             callbackQueue: .sharedConcurrent
diff --git a/Sources/PackageCollections/Storage/FilePackageCollectionsSourcesStorage.swift b/Sources/PackageCollections/Storage/FilePackageCollectionsSourcesStorage.swift
@@ -26,7 +26,7 @@ struct FilePackageCollectionsSourcesStorage: PackageCollectionsSourcesStorage {
     init(fileSystem: FileSystem = localFileSystem, path: AbsolutePath? = nil) {
         self.fileSystem = fileSystem
 
-        self.path = path ?? fileSystem.swiftPMConfigDirectory.appending(component: "collections.json")
+        self.path = path ?? fileSystem.swiftPMConfigurationDirectory.appending(component: "collections.json")
         self.encoder = JSONEncoder.makeWithDefaults()
         self.decoder = JSONDecoder.makeWithDefaults()
     }
diff --git a/Sources/SPMTestSupport/MockWorkspace.swift b/Sources/SPMTestSupport/MockWorkspace.swift
@@ -241,7 +241,7 @@ public final class MockWorkspace {
                 resolvedVersionsFile: self.sandbox.appending(component: "Package.resolved"),
                 sharedSecurityDirectory: self.fileSystem.swiftPMSecurityDirectory,  
                 sharedCacheDirectory: self.fileSystem.swiftPMCacheDirectory,
-                sharedConfigurationDirectory: self.fileSystem.swiftPMConfigDirectory
+                sharedConfigurationDirectory: self.fileSystem.swiftPMConfigurationDirectory
             ),
             mirrors: self.mirrors,
             customToolsVersion: self.toolsVersion,
diff --git a/Sources/Workspace/WorkspaceConfiguration.swift b/Sources/Workspace/WorkspaceConfiguration.swift
@@ -126,7 +126,7 @@ extension Workspace {
                 resolvedVersionsFile: DefaultLocations.resolvedVersionsFile(forRootPackage: rootPath),
                 sharedSecurityDirectory: fileSystem.swiftPMSecurityDirectory,
                 sharedCacheDirectory: fileSystem.swiftPMCacheDirectory,
-                sharedConfigurationDirectory: fileSystem.swiftPMConfigDirectory
+                sharedConfigurationDirectory: fileSystem.swiftPMConfigurationDirectory
             )
         }
     }
diff --git a/Tests/FunctionalTests/MiscellaneousTests.swift b/Tests/FunctionalTests/MiscellaneousTests.swift
@@ -415,7 +415,7 @@ class MiscellaneousTestCase: XCTestCase {
             XCTAssert(output.contains("does not exist"), "Error from git was not propagated to process output: \(output)")
         }
     }
-    
+
     func testLocalPackageUsedAsURL() throws {
         fixture(name: "Miscellaneous/LocalPackageAsURL", createGitRepo: false) { prefix in
             // This fixture has a setup that is trying to use a local package
@@ -431,7 +431,7 @@ class MiscellaneousTestCase: XCTestCase {
             XCTAssert(output.contains("Cannot clone from local directory"), "Didn't find expected output: \(output)")
         }
     }
-    
+
     func testUnicode() {
         #if !os(Linux) && !os(Android) // TODO: - Linux has trouble with this and needs investigation.
         fixture(name: "Miscellaneous/Unicode") { prefix in
@@ -505,7 +505,7 @@ class MiscellaneousTestCase: XCTestCase {
             XCTAssertMatch(stderr, .contains("warning: '--generate-linuxmain' option is deprecated"))
         }
     }
-    
+
     func testGenerateLinuxMain() {
         #if os(macOS)
         fixture(name: "Miscellaneous/TestDiscovery/Simple") { path in
@@ -552,13 +552,13 @@ class MiscellaneousTestCase: XCTestCase {
         }
         #endif
     }
-    
+
     func testTestsCanLinkAgainstExecutable() throws {
         // Check if the host compiler supports the '-entry-point-function-name' flag.
         #if swift(<5.5)
         try XCTSkipIf(true, "skipping because host compiler doesn't support '-entry-point-function-name'")
         #endif
-        
+
         fixture(name: "Miscellaneous/TestableExe") { prefix in
             do {
                 let (stdout, _) = try executeSwiftTest(prefix)
@@ -648,6 +648,17 @@ class MiscellaneousTestCase: XCTestCase {
             try SwiftPMProduct.SwiftBuild.execute(["--cache-path", customCachePath.pathString], packagePath: path)
             XCTAssertDirectoryExists(customCachePath)
         }
+
+        fixture(name: "Miscellaneous/Simple") { path in
+            try localFileSystem.chmod(.userUnWritable, path: path)
+            let customCachePath = path.appending(components: "custom", "cache")
+            XCTAssertNoSuchPath(customCachePath)
+            let result = try SwiftPMProduct.SwiftBuild.executeProcess(["--cache-path", customCachePath.pathString], packagePath: path)
+            XCTAssert(result.exitStatus != .terminated(code: 0))
+            let output = try result.utf8stderrOutput()
+            XCTAssert(output.contains("error: You don’t have permission"), "expected permissions error")
+            XCTAssertNoSuchPath(customCachePath)
+        }
     }
 
     func testCustomConfigPath() {
@@ -657,5 +668,36 @@ class MiscellaneousTestCase: XCTestCase {
             try SwiftPMProduct.SwiftBuild.execute(["--config-path", customConfigPath.pathString], packagePath: path)
             XCTAssertDirectoryExists(customConfigPath)
         }
+
+        fixture(name: "Miscellaneous/Simple") { path in
+            try localFileSystem.chmod(.userUnWritable, path: path)
+            let customConfigPath = path.appending(components: "custom", "config")
+            XCTAssertNoSuchPath(customConfigPath)
+            let result = try SwiftPMProduct.SwiftBuild.executeProcess(["--config-path", customConfigPath.pathString], packagePath: path)
+            XCTAssert(result.exitStatus != .terminated(code: 0))
+            let output = try result.utf8stderrOutput()
+            XCTAssert(output.contains("error: You don’t have permission"), "expected permissions error")
+            XCTAssertNoSuchPath(customConfigPath)
+        }
+    }
+
+    func testCustomSecurityPath() {
+        fixture(name: "Miscellaneous/Simple") { path in
+            let customSecurityPath = path.appending(components: "custom", "security")
+            XCTAssertNoSuchPath(customSecurityPath)
+            try SwiftPMProduct.SwiftBuild.execute(["--security-path", customSecurityPath.pathString], packagePath: path)
+            XCTAssertDirectoryExists(customSecurityPath)
+        }
+
+        fixture(name: "Miscellaneous/Simple") { path in
+            try localFileSystem.chmod(.userUnWritable, path: path)
+            let customSecurityPath = path.appending(components: "custom", "security")
+            XCTAssertNoSuchPath(customSecurityPath)
+            let result = try SwiftPMProduct.SwiftBuild.executeProcess(["--security-path", customSecurityPath.pathString], packagePath: path)
+            XCTAssert(result.exitStatus != .terminated(code: 0))
+            let output = try result.utf8stderrOutput()
+            XCTAssert(output.contains("error: You don’t have permission"), "expected permissions error")
+            XCTAssertNoSuchPath(customSecurityPath)
+        }
     }
 }
diff --git a/Utilities/Docker/docker-compose.2004.55.yaml b/Utilities/Docker/docker-compose.2004.55.yaml
@@ -16,7 +16,6 @@ services:
       args:
         ubuntu_version: "focal"
         swift_version: "5.5"
-        base_image: "swiftlang/swift:nightly-5.5-focal"
 
   build:
     image: swift-package-manager:20.04-5.5
diff --git a/Utilities/Docker/docker-compose.yaml b/Utilities/Docker/docker-compose.yaml
@@ -27,7 +27,8 @@ services:
       - ~/.ssh:/root/.ssh
       - ~/.cache:/root/.cache
       - ~/.swiftpm/cache:/root/.swiftpm/cache
-      - ~/.swiftpm/config:/root/.swiftpm/config
+      - ~/.swiftpm/configuration:/root/.swiftpm/configuration
+      - ~/.swiftpm/security:/root/.swiftpm/security
       # swift-package-manager code
       - ../..:/code/swift-package-manager:z
       # bootstrap script requires dependencies to be pre-fetched and in a specific place

Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ struct FilePackageCollectionsSourcesStorage: PackageCollectionsSourcesStorage {`
`26`	`26`	`init(fileSystem: FileSystem = localFileSystem, path: AbsolutePath? = nil) {`
`27`	`27`	`self.fileSystem = fileSystem`
`28`	`28`
`29`		`- self.path = path ?? fileSystem.swiftPMConfigDirectory.appending(component: "collections.json")`
	`29`	`+ self.path = path ?? fileSystem.swiftPMConfigurationDirectory.appending(component: "collections.json")`
`30`	`30`	`self.encoder = JSONEncoder.makeWithDefaults()`
`31`	`31`	`self.decoder = JSONDecoder.makeWithDefaults()`
`32`	`32`	`}`
Original file line number	Diff line number	Diff line change
`@@ -126,7 +126,7 @@ extension Workspace {`
`126`	`126`	`resolvedVersionsFile: DefaultLocations.resolvedVersionsFile(forRootPackage: rootPath),`
`127`	`127`	`sharedSecurityDirectory: fileSystem.swiftPMSecurityDirectory,`
`128`	`128`	`sharedCacheDirectory: fileSystem.swiftPMCacheDirectory,`
`129`		`- sharedConfigurationDirectory: fileSystem.swiftPMConfigDirectory`
	`129`	`+ sharedConfigurationDirectory: fileSystem.swiftPMConfigurationDirectory`
`130`	`130`	`)`
`131`	`131`	`}`
`132`	`132`	`}`