From c5bcf54dd2095a94672263a6d5a46ea04ee9ccf7 Mon Sep 17 00:00:00 2001 From: tom doron Date: Mon, 20 Dec 2021 19:19:24 -0800 Subject: [PATCH] cleanup --- Sources/Basics/FileSystem+Extensions.swift | 10 +- Sources/Commands/APIDigester.swift | 6 +- Sources/Commands/SwiftTool.swift | 79 +-- Sources/SourceControl/RepositoryManager.swift | 10 +- Sources/Workspace/Workspace.swift | 182 ++++--- Sources/Workspace/WorkspaceState.swift | 6 +- Tests/FunctionalTests/PluginTests.swift | 2 +- .../PluginInvocationTests.swift | 2 +- .../RepositoryManagerTests.swift | 72 ++- .../SourceControlPackageContainerTests.swift | 18 +- .../WorkspaceTests/WorkspaceStateTests.swift | 457 +++++++++--------- Tests/WorkspaceTests/WorkspaceTests.swift | 19 +- 12 files changed, 440 insertions(+), 423 deletions(-) diff --git a/Sources/Basics/FileSystem+Extensions.swift b/Sources/Basics/FileSystem+Extensions.swift index 575cad505cd..0b5e2e9fefd 100644 --- a/Sources/Basics/FileSystem+Extensions.swift +++ b/Sources/Basics/FileSystem+Extensions.swift @@ -89,7 +89,7 @@ extension FileSystem { } extension FileSystem { - public func getOrCreateSwiftPMConfigurationDirectory(observabilityScope: ObservabilityScope?) throws -> AbsolutePath { + public func getOrCreateSwiftPMConfigurationDirectory() throws -> AbsolutePath { let idiomaticConfigurationDirectory = self.swiftPMConfigurationDirectory // temporary 5.6, remove on next version: transition from previous configuration location @@ -108,10 +108,12 @@ extension FileSystem { .filter{ self.isFile($0) && !self.isSymlink($0) && $0.extension != "lock"} for file in configurationFiles { let destination = idiomaticConfigurationDirectory.appending(component: file.basename) - observabilityScope?.emit(warning: "Usage of \(file) has been deprecated. Please delete it and use the new \(destination) instead.") if !self.exists(destination) { try self.copy(from: file, to: destination) } + // FIXME: We should emit a warning here using the diagnostic engine. + TSCBasic.stderrStream.write("warning: Usage of \(file) has been deprecated. Please delete it and use the new \(destination) instead.\n") + TSCBasic.stderrStream.flush() } } // in the case where ~/.swiftpm/configuration is the idiomatic location (eg on Linux) @@ -125,10 +127,12 @@ extension FileSystem { .filter{ self.isFile($0) && !self.isSymlink($0) && $0.extension != "lock"} for file in configurationFiles { let destination = idiomaticConfigurationDirectory.appending(component: file.basename) - observabilityScope?.emit(warning: "Usage of \(file) has been deprecated. Please delete it and use the new \(destination) instead.") if !self.exists(destination) { try self.copy(from: file, to: destination) } + // FIXME: We should emit a warning here using the diagnostic engine. + TSCBasic.stderrStream.write("warning: Usage of \(file) has been deprecated. Please delete it and use the new \(destination) instead.\n") + TSCBasic.stderrStream.flush() } } } diff --git a/Sources/Commands/APIDigester.swift b/Sources/Commands/APIDigester.swift index 921570a0574..c74f0192be7 100644 --- a/Sources/Commands/APIDigester.swift +++ b/Sources/Commands/APIDigester.swift @@ -99,13 +99,11 @@ struct APIDigesterBaselineDumper { try workingCopy.checkout(revision: baselineRevision) // Create the workspace for this package. - let workspace = try Workspace( - forRootPackage: baselinePackageRoot - ) + let workspace = try Workspace(forRootPackage: baselinePackageRoot) let graph = try workspace.loadPackageGraph( rootPath: baselinePackageRoot, - observabilityScope: observabilityScope + observabilityScope: self.observabilityScope ) // Don't emit a baseline for a module that didn't exist yet in this revision. diff --git a/Sources/Commands/SwiftTool.swift b/Sources/Commands/SwiftTool.swift index e103adfea3f..0a066f9e969 100644 --- a/Sources/Commands/SwiftTool.swift +++ b/Sources/Commands/SwiftTool.swift @@ -494,11 +494,6 @@ public class SwiftTool { return workspace } - // we do not take an observabilityScope in the workspace initializer as the workspace is designed to be long lived - // instead observabilityScope is passed into the individual workspace methods which are short lived - // however, the workspace initializer may need to emit diagnostics, for which the CLI sets this thread local - Thread.current.threadDictionary["observabilityScope"] = observabilityScope - let isXcodeBuildSystemEnabled = self.options.buildSystem == .xcode let repositoryProvider = GitRepositoryProvider(processSet: self.processSet) let delegate = ToolWorkspaceDelegate(self.outputStream, logLevel: self.logLevel, observabilityScope: self.observabilityScope) @@ -546,72 +541,7 @@ public class SwiftTool { return try Workspace.DefaultLocations.resolvedVersionsFile(forRootPackage: self.getPackageRoot()) } - /* - func getMirrorsConfig(sharedConfigurationDirectory: AbsolutePath? = nil) throws -> Workspace.Configuration.Mirrors { - let sharedConfigurationDirectory = sharedConfigurationDirectory ?? self.sharedConfigurationDirectory - let sharedMirrorFile = sharedConfigurationDirectory.map { Workspace.DefaultLocations.mirrorsConfigurationFile(at: $0) } - return try .init( - localMirrorFile: self.mirrorsConfigFile(), - sharedMirrorFile: sharedMirrorFile, - fileSystem: localFileSystem - ) - } - - private func mirrorsConfigFile() throws -> AbsolutePath { - // TODO: does this make sense now that we a global configuration as well? or should we at least rename it? - // Look for the override in the environment. - if let envPath = ProcessEnv.vars["SWIFTPM_MIRROR_CONFIG"] { - return try AbsolutePath(validating: envPath) - } - - // Otherwise, use the default path. - // TODO: replace multiroot-data-file with explicit overrides - if let multiRootPackageDataFile = options.multirootPackageDataFile { - // migrate from legacy location - let legacyPath = multiRootPackageDataFile.appending(components: "xcshareddata", "swiftpm", "config") - let newPath = multiRootPackageDataFile.appending(components: "xcshareddata", "swiftpm", "configuration", "mirrors.json") - if localFileSystem.exists(legacyPath) { - try localFileSystem.createDirectory(newPath.parentDirectory, recursive: true) - try localFileSystem.move(from: legacyPath, to: newPath) - } - return newPath - } - - // migrate from legacy location - let legacyPath = try self.getPackageRoot().appending(components: ".swiftpm", "config") - let newPath = try Workspace.DefaultLocations.mirrorsConfigurationFile(forRootPackage: self.getPackageRoot()) - if localFileSystem.exists(legacyPath) { - observabilityScope.emit(warning: "Usage of \(legacyPath) has been deprecated. Please delete it and use the new \(newPath) instead.") - if !localFileSystem.exists(newPath) { - try localFileSystem.createDirectory(newPath.parentDirectory, recursive: true) - try localFileSystem.copy(from: legacyPath, to: newPath) - } - } - return newPath - } - - func getRegistriesConfig(sharedConfigurationDirectory: AbsolutePath? = nil) throws -> Workspace.Configuration.Registries { - let localRegistriesFile = try Workspace.DefaultLocations.registriesConfigurationFile(forRootPackage: self.getPackageRoot()) - - let sharedConfigurationDirectory = sharedConfigurationDirectory ?? self.sharedConfigurationDirectory - let sharedRegistriesFile = sharedConfigurationDirectory.map { - Workspace.DefaultLocations.registriesConfigurationFile(at: $0) - } - - return try .init( - localRegistriesFile: localRegistriesFile, - sharedRegistriesFile: sharedRegistriesFile, - fileSystem: localFileSystem - ) - }*/ - internal func getLocalConfigurationDirectory() throws -> AbsolutePath { - // TODO: does this make sense now that we a global configuration as well? or should we at least rename it? - // Look for the override in the environment. - //if let envPath = ProcessEnv.vars["SWIFTPM_MIRROR_CONFIG"] { - // return try AbsolutePath(validating: envPath) - //} - // Otherwise, use the default path. // TODO: replace multiroot-data-file with explicit overrides if let multiRootPackageDataFile = options.multirootPackageDataFile { @@ -1075,11 +1005,8 @@ private func getSharedSecurityDirectory(options: SwiftToolOptions, observability return explicitSecurityPath } - do { - let sharedSecurityDirectory = try localFileSystem.getOrCreateSwiftPMSecurityDirectory() - // And make sure we can write files (locking the directory writes a lock file) - try localFileSystem.withLock(on: sharedSecurityDirectory, type: .exclusive) { } - return sharedSecurityDirectory + do { + return try localFileSystem.getOrCreateSwiftPMSecurityDirectory() } catch { observabilityScope.emit(warning: "Failed creating default security location, \(error)") return .none @@ -1096,7 +1023,7 @@ private func getSharedConfigurationDirectory(options: SwiftToolOptions, observab } do { - return try localFileSystem.getOrCreateSwiftPMConfigurationDirectory(observabilityScope: observabilityScope) + return try localFileSystem.getOrCreateSwiftPMConfigurationDirectory() } catch { observabilityScope.emit(warning: "Failed creating default configuration location, \(error)") return .none diff --git a/Sources/SourceControl/RepositoryManager.swift b/Sources/SourceControl/RepositoryManager.swift index f4bfd070d45..5766c592bdd 100644 --- a/Sources/SourceControl/RepositoryManager.swift +++ b/Sources/SourceControl/RepositoryManager.swift @@ -82,14 +82,14 @@ public class RepositoryManager { fileSystem: FileSystem, path: AbsolutePath, provider: RepositoryProvider, - delegate: RepositoryManagerDelegate? = nil, - cachePath: AbsolutePath? = nil, - cacheLocalPackages: Bool? = nil + cachePath: AbsolutePath? = .none, + cacheLocalPackages: Bool = false, + delegate: RepositoryManagerDelegate? = .none ) { self.fileSystem = fileSystem self.path = path self.cachePath = cachePath - self.cacheLocalPackages = cacheLocalPackages ?? false + self.cacheLocalPackages = cacheLocalPackages self.provider = provider self.delegate = delegate @@ -108,7 +108,7 @@ public class RepositoryManager { self.repositories = [:] try? self.storage.reset() // FIXME: We should emit a warning here using the diagnostic engine. - TSCBasic.stderrStream.write("warning: unable to restore checkouts state: \(error)") + TSCBasic.stderrStream.write("warning: unable to restore checkouts state: \(error)\n") TSCBasic.stderrStream.flush() } } diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index 3f6345f6b71..5009e6ff394 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -247,7 +247,6 @@ public class Workspace { /// - Parameters: /// - fileSystem: The file system to use. /// - location: Workspace location configuration. - /// - registries: Configuration for registries /// - authorizationProvider: Provider of authentication information for outbound network requests. /// - configuration: Configuration to fine tune the dependency resolution behavior. /// - customManifestLoader: Custom manifest loader. Used to customize how manifest are loaded. @@ -288,6 +287,92 @@ public class Workspace { ) } + /// A convenience method for creating a workspace for the given root + /// package path. + /// + /// The root package path is used to compute the build directory and other + /// default paths. + /// + /// - Parameters: + /// - fileSystem: The file system to use, defaults to local file system. + /// - forRootPackage: The path for the root package. + /// - authorizationProvider: Provider of authentication information for outbound network requests. + /// - configuration: Configuration to fine tune the dependency resolution behavior. + /// - customManifestLoader: Custom manifest loader. Used to customize how manifest are loaded. + /// - customPackageContainerProvider: Custom package container provider. Used to provide specialized package providers. + /// - customRepositoryProvider: Custom repository provider. Used to customize source control access. + /// - delegate: Delegate for workspace events + public convenience init( + fileSystem: FileSystem? = .none, + forRootPackage packagePath: AbsolutePath, + authorizationProvider: AuthorizationProvider? = .none, + configuration: WorkspaceConfiguration? = .none, + // optional customization used for advanced integration situations + customManifestLoader: ManifestLoaderProtocol? = .none, + customPackageContainerProvider: PackageContainerProvider? = .none, + customRepositoryProvider: RepositoryProvider? = .none, + // delegate + delegate: WorkspaceDelegate? = .none + ) throws { + let fileSystem = fileSystem ?? localFileSystem + let location = Location(forRootPackage: packagePath, fileSystem: fileSystem) + try self.init( + fileSystem: fileSystem, + location: location, + customManifestLoader: customManifestLoader, + customPackageContainerProvider: customPackageContainerProvider, + customRepositoryProvider: customRepositoryProvider, + delegate: delegate + ) + } + + /// A convenience method for creating a workspace for the given root + /// package path. + /// + /// The root package path is used to compute the build directory and other + /// default paths. + /// + /// - Parameters: + /// - fileSystem: The file system to use, defaults to local file system. + /// - forRootPackage: The path for the root package. + /// - authorizationProvider: Provider of authentication information for outbound network requests. + /// - configuration: Configuration to fine tune the dependency resolution behavior. + /// - customToolchain: Custom toolchain. Used to create a customized ManifestLoader, customizing how manifest are loaded. + /// - customPackageContainerProvider: Custom package container provider. Used to provide specialized package providers. + /// - customRepositoryProvider: Custom repository provider. Used to customize source control access. + /// - delegate: Delegate for workspace events + public convenience init( + fileSystem: FileSystem? = .none, + forRootPackage packagePath: AbsolutePath, + authorizationProvider: AuthorizationProvider? = .none, + configuration: WorkspaceConfiguration? = .none, + // optional customization used for advanced integration situations + customToolchain: UserToolchain, + customPackageContainerProvider: PackageContainerProvider? = .none, + customRepositoryProvider: RepositoryProvider? = .none, + // delegate + delegate: WorkspaceDelegate? = .none + ) throws { + let fileSystem = fileSystem ?? localFileSystem + let location = Location(forRootPackage: packagePath, fileSystem: fileSystem) + let manifestLoader = ManifestLoader( + toolchain: customToolchain.configuration, + cacheDir: location.sharedManifestsCacheDirectory + ) + try self.init( + fileSystem: fileSystem, + forRootPackage: packagePath, + authorizationProvider: authorizationProvider, + configuration: configuration, + customManifestLoader: manifestLoader, + customPackageContainerProvider: customPackageContainerProvider, + customRepositoryProvider: customRepositoryProvider, + delegate: delegate + ) + } + + + // deprecate 12/21 @_disfavoredOverload @available(*, deprecated, message: "use alternative initializer") @@ -406,64 +491,6 @@ public class Workspace { } } - /// A convenience method for creating a workspace for the given root - /// package path. - /// - /// The root package path is used to compute the build directory and other - /// default paths. - /// - /// - Parameters: - /// - fileSystem: The file system to use, defaults to local file system. - /// - forRootPackage: The path for the root package. - /// - customToolchain: A custom toolchain. - /// - delegate: Delegate for workspace events - public convenience init( - fileSystem: FileSystem? = .none, - forRootPackage packagePath: AbsolutePath, - customToolchain: UserToolchain, - delegate: WorkspaceDelegate? = .none - ) throws { - let fileSystem = fileSystem ?? localFileSystem - let location = Location(forRootPackage: packagePath, fileSystem: fileSystem) - let manifestLoader = ManifestLoader( - toolchain: customToolchain.configuration, - cacheDir: location.sharedManifestsCacheDirectory - ) - try self.init( - fileSystem: fileSystem, - forRootPackage: packagePath, - customManifestLoader: manifestLoader, - delegate: delegate - ) - } - - /// A convenience method for creating a workspace for the given root - /// package path. - /// - /// The root package path is used to compute the build directory and other - /// default paths. - /// - /// - Parameters: - /// - fileSystem: The file system to use, defaults to local file system. - /// - forRootPackage: The path for the root package. - /// - customManifestLoader: A custom manifest loader. - /// - delegate: Delegate for workspace events - public convenience init( - fileSystem: FileSystem? = .none, - forRootPackage packagePath: AbsolutePath, - customManifestLoader: ManifestLoaderProtocol? = .none, - delegate: WorkspaceDelegate? = .none - ) throws { - let fileSystem = fileSystem ?? localFileSystem - let location = Location(forRootPackage: packagePath, fileSystem: fileSystem) - try self .init( - fileSystem: fileSystem, - location: location, - customManifestLoader: customManifestLoader, - delegate: delegate - ) - } - /// A convenience method for creating a workspace for the given root /// package path. /// @@ -478,9 +505,10 @@ public class Workspace { delegate: WorkspaceDelegate? = nil, identityResolver: IdentityResolver? = nil ) -> Workspace { - let workspace = try! Workspace(forRootPackage: packagePath, - customManifestLoader: manifestLoader, - delegate: delegate + let workspace = try! Workspace( + forRootPackage: packagePath, + customManifestLoader: manifestLoader, + delegate: delegate ) if let repositoryManager = repositoryManager { workspace.repositoryManager = repositoryManager @@ -562,12 +590,11 @@ public class Workspace { // delegate delegate: WorkspaceDelegate? ) throws { - // we do not take an observabilityScope in the workspace initializer as the workspace is designed to be long lived. + // we do not store the observabilityScope in the workspace initializer as the workspace is designed to be long lived. // instead, observabilityScope is passed into the individual workspace methods which are short lived. - // however, the workspace initializer may need to emit diagnostics, for which a consumer like the CLI can set as a thread local - let observabilityScope = Thread.current.threadDictionary["observabilityScope"] as? ObservabilityScope + // validate locations, returning a potentially modified one to deal with non-accessible or non-writable shared locations - let location = try location.validatingSharedLocations(fileSystem: fileSystem, observabilityScope: observabilityScope) + let location = try location.validatingSharedLocations(fileSystem: fileSystem) let currentToolsVersion = customToolsVersion ?? ToolsVersion.currentToolsVersion let toolsVersionLoader = ToolsVersionLoader(currentToolsVersion: currentToolsVersion) @@ -590,8 +617,8 @@ public class Workspace { fileSystem: fileSystem, path: location.repositoriesDirectory, provider: repositoryProvider, - delegate: delegate.map(WorkspaceRepositoryManagerDelegate.init(workspaceDelegate:)), - cachePath: configuration.sharedRepositoriesCacheEnabled ? location.sharedRepositoriesCacheDirectory : .none + cachePath: configuration.sharedRepositoriesCacheEnabled ? location.sharedRepositoriesCacheDirectory : .none, + delegate: delegate.map(WorkspaceRepositoryManagerDelegate.init(workspaceDelegate:)) ) let fingerprints = customFingerprints ?? location.sharedFingerprintsDirectory.map { @@ -654,7 +681,7 @@ public class Workspace { self.configuration = configuration - self.state = WorkspaceState(dataPath: self.location.workingDirectory, fileSystem: fileSystem) + self.state = WorkspaceState(fileSystem: fileSystem, storageDirectory: self.location.workingDirectory) } } @@ -3847,7 +3874,8 @@ extension Workspace.Location { } extension Workspace.Location { - func validatingSharedLocations(fileSystem: FileSystem, observabilityScope: ObservabilityScope?) throws -> Self { + // FIXME: is there a better validation than a lock file? + func validatingSharedLocations(fileSystem: FileSystem) throws -> Self { var location = self // local configuration directory must be accessible, throw if we cannot access it @@ -3856,7 +3884,7 @@ extension Workspace.Location { // check that shared configuration directory is accessible, or warn + reset if not if let sharedConfigurationDirectory = self.sharedConfigurationDirectory { // it may not always be possible to create default location (for example de to restricted sandbox) - let defaultDirectory = try? fileSystem.getOrCreateSwiftPMConfigurationDirectory(observabilityScope: observabilityScope) + let defaultDirectory = try? fileSystem.getOrCreateSwiftPMConfigurationDirectory() if sharedConfigurationDirectory != defaultDirectory { // custom location must be writable, throw if we cannot access it try fileSystem.withLock(on: sharedConfigurationDirectory.appending(component: "test"), type: .exclusive, {}) @@ -3865,8 +3893,10 @@ extension Workspace.Location { do { try fileSystem.withLock(on: sharedConfigurationDirectory.appending(component: "test"), type: .exclusive, {}) } catch { - observabilityScope?.emit(warning: "\(sharedConfigurationDirectory) is not writable, disabling user-level configuration features. \(error)") location.sharedConfigurationDirectory = .none + // FIXME: We should emit a warning here using the diagnostic engine. + TSCBasic.stderrStream.write("warning: \(sharedConfigurationDirectory) is not writable, disabling user-level configuration features. \(error)\n") + TSCBasic.stderrStream.flush() } } } @@ -3883,8 +3913,10 @@ extension Workspace.Location { do { try fileSystem.withLock(on: sharedSecurityDirectory.appending(component: "test"), type: .exclusive, {}) } catch { - observabilityScope?.emit(warning: "\(sharedSecurityDirectory) is not writable, disabling user-level security features. \(error)") location.sharedSecurityDirectory = .none + // FIXME: We should emit a warning here using the diagnostic engine. + TSCBasic.stderrStream.write("warning: \(sharedSecurityDirectory) is not writable, disabling user-level security features. \(error)\n") + TSCBasic.stderrStream.flush() } } } @@ -3901,8 +3933,10 @@ extension Workspace.Location { do { try fileSystem.withLock(on: sharedCacheDirectory.appending(component: "test"), type: .exclusive, {}) } catch { - observabilityScope?.emit(warning: "\(sharedCacheDirectory) is not writable, disabling user-level cache features. \(error)") location.sharedCacheDirectory = .none + // FIXME: We should emit a warning here using the diagnostic engine. + TSCBasic.stderrStream.write("warning: \(sharedCacheDirectory) is not writable, disabling user-level cache features. \(error)\n") + TSCBasic.stderrStream.flush() } } } diff --git a/Sources/Workspace/WorkspaceState.swift b/Sources/Workspace/WorkspaceState.swift index 44102aa8b1a..1404b31a6bd 100644 --- a/Sources/Workspace/WorkspaceState.swift +++ b/Sources/Workspace/WorkspaceState.swift @@ -29,8 +29,8 @@ public final class WorkspaceState { /// storage private let storage: WorkspaceStateStorage - init(dataPath: AbsolutePath, fileSystem: FileSystem) { - self.storagePath = dataPath.appending(component: "workspace-state.json") + init(fileSystem: FileSystem, storageDirectory: AbsolutePath) { + self.storagePath = storageDirectory.appending(component: "workspace-state.json") self.storage = WorkspaceStateStorage(path: self.storagePath, fileSystem: fileSystem) // Load the state from disk, if possible. @@ -50,7 +50,7 @@ public final class WorkspaceState { self.artifacts = Workspace.ManagedArtifacts() try? self.storage.reset() // FIXME: We should emit a warning here using the diagnostic engine. - TSCBasic.stderrStream.write("warning: unable to restore workspace state: \(error)") + TSCBasic.stderrStream.write("warning: unable to restore workspace state: \(error)\n") TSCBasic.stderrStream.flush() } } diff --git a/Tests/FunctionalTests/PluginTests.swift b/Tests/FunctionalTests/PluginTests.swift index cef7cf3b87c..42cc4fd3fa2 100644 --- a/Tests/FunctionalTests/PluginTests.swift +++ b/Tests/FunctionalTests/PluginTests.swift @@ -331,7 +331,7 @@ class PluginTests: XCTestCase { let observability = ObservabilitySystem.makeForTesting() let workspace = try Workspace( fileSystem: localFileSystem, - location: .init(forRootPackage: packageDir, fileSystem: localFileSystem), + forRootPackage: packageDir, customManifestLoader: ManifestLoader(toolchain: ToolchainConfiguration.default), delegate: MockWorkspaceDelegate() ) diff --git a/Tests/SPMBuildCoreTests/PluginInvocationTests.swift b/Tests/SPMBuildCoreTests/PluginInvocationTests.swift index 5cce0b4b5b1..9b9f7279555 100644 --- a/Tests/SPMBuildCoreTests/PluginInvocationTests.swift +++ b/Tests/SPMBuildCoreTests/PluginInvocationTests.swift @@ -240,7 +240,7 @@ class PluginInvocationTests: XCTestCase { let observability = ObservabilitySystem.makeForTesting() let workspace = try Workspace( fileSystem: localFileSystem, - location: .init(forRootPackage: packageDir, fileSystem: localFileSystem), + forRootPackage: packageDir, customManifestLoader: ManifestLoader(toolchain: ToolchainConfiguration.default), delegate: MockWorkspaceDelegate() ) diff --git a/Tests/SourceControlTests/RepositoryManagerTests.swift b/Tests/SourceControlTests/RepositoryManagerTests.swift index b419486aff4..8c7bf27e355 100644 --- a/Tests/SourceControlTests/RepositoryManagerTests.swift +++ b/Tests/SourceControlTests/RepositoryManagerTests.swift @@ -8,12 +8,11 @@ See http://swift.org/CONTRIBUTORS.txt for Swift project authors */ -import XCTest - +import Basics +import SPMTestSupport @testable import SourceControl import TSCBasic - -import SPMTestSupport +import XCTest private enum DummyError: Swift.Error { case invalidRepository @@ -266,11 +265,17 @@ class RepositoryManagerTests: XCTestCase { try testWithTemporaryDirectory { path in let provider = DummyRepositoryProvider() let delegate = DummyRepositoryManagerDelegate() + delegate.willFetchGroup = DispatchGroup() delegate.didFetchGroup = DispatchGroup() delegate.willUpdateGroup = DispatchGroup() delegate.didUpdateGroup = DispatchGroup() - let manager = RepositoryManager(fileSystem: localFileSystem, path: path, provider: provider, delegate: delegate) + let manager = RepositoryManager( + fileSystem: localFileSystem, + path: path, + provider: provider, + delegate: delegate + ) // Check that we can "fetch" a repository. let dummyRepo = RepositorySpecifier(path: .init("/dummy")) @@ -394,7 +399,6 @@ class RepositoryManagerTests: XCTestCase { fileSystem: localFileSystem, path: repositoriesPath, provider: provider, - delegate: delegate, cachePath: cachePath, cacheLocalPackages: true ) @@ -456,7 +460,12 @@ class RepositoryManagerTests: XCTestCase { delegate.didFetchGroup = DispatchGroup() try localFileSystem.createDirectory(repos, recursive: true) - let manager = RepositoryManager(fileSystem: localFileSystem, path: repos, provider: provider, delegate: delegate) + let manager = RepositoryManager( + fileSystem: localFileSystem, + path: repos, + provider: provider, + delegate: delegate + ) let dummyRepo = RepositorySpecifier(path: .init("/dummy")) delegate.willFetchGroup?.enter() @@ -489,7 +498,12 @@ class RepositoryManagerTests: XCTestCase { delegate.willFetchGroup = DispatchGroup() delegate.didFetchGroup = DispatchGroup() - let manager = RepositoryManager(fileSystem: localFileSystem, path: path, provider: provider, delegate: delegate) + let manager = RepositoryManager( + fileSystem: localFileSystem, + path: path, + provider: provider, + delegate: delegate + ) let dummyRepo = RepositorySpecifier(path: .init("/dummy")) delegate.willFetchGroup?.enter() @@ -509,7 +523,12 @@ class RepositoryManagerTests: XCTestCase { delegate.willFetchGroup = DispatchGroup() delegate.didFetchGroup = DispatchGroup() - let manager = RepositoryManager(fileSystem: localFileSystem, path: path, provider: provider, delegate: delegate) + let manager = RepositoryManager( + fileSystem: localFileSystem, + path: path, + provider: provider, + delegate: delegate + ) let dummyRepo = RepositorySpecifier(path: .init("/dummy")) _ = try manager.lookup(repository: dummyRepo) @@ -526,9 +545,19 @@ class RepositoryManagerTests: XCTestCase { delegate.willFetchGroup = DispatchGroup() delegate.didFetchGroup = DispatchGroup() - var manager = RepositoryManager(fileSystem: localFileSystem, path: path, provider: provider, delegate: delegate) + var manager = RepositoryManager( + fileSystem: localFileSystem, + path: path, + provider: provider, + delegate: delegate + ) try! localFileSystem.removeFileTree(path.appending(component: "checkouts-state.json")) - manager = RepositoryManager(fileSystem: localFileSystem, path: path, provider: provider, delegate: delegate) + manager = RepositoryManager( + fileSystem: localFileSystem, + path: path, + provider: provider, + delegate: delegate + ) let dummyRepo = RepositorySpecifier(path: .init("/dummy")) delegate.willFetchGroup?.enter() @@ -548,7 +577,12 @@ class RepositoryManagerTests: XCTestCase { try testWithTemporaryDirectory { path in let provider = DummyRepositoryProvider() let delegate = DummyRepositoryManagerDelegate() - let manager = RepositoryManager(fileSystem: localFileSystem, path: path, provider: provider, delegate: delegate) + let manager = RepositoryManager( + fileSystem: localFileSystem, + path: path, + provider: provider, + delegate: delegate + ) let dummyRepo = RepositorySpecifier(path: .init("/dummy")) // Condition to check if we have finished all lookups. let doneCondition = Condition() @@ -589,7 +623,12 @@ class RepositoryManagerTests: XCTestCase { try localFileSystem.createDirectory(repos, recursive: true) - let manager = RepositoryManager(fileSystem: localFileSystem, path: repos, provider: provider, delegate: delegate) + let manager = RepositoryManager( + fileSystem: localFileSystem, + path: repos, + provider: provider, + delegate: delegate + ) let dummyRepo = RepositorySpecifier(path: .init("/dummy")) delegate.willFetchGroup?.enter() @@ -629,7 +668,12 @@ class RepositoryManagerTests: XCTestCase { delegate.didFetchGroup = DispatchGroup() try localFileSystem.createDirectory(repos, recursive: true) - let manager = RepositoryManager(fileSystem: localFileSystem, path: repos, provider: provider, delegate: delegate) + let manager = RepositoryManager( + fileSystem: localFileSystem, + path: repos, + provider: provider, + delegate: delegate + ) let dummyRepo = RepositorySpecifier(path: .init("/dummy")) // Perform a lookup. diff --git a/Tests/WorkspaceTests/SourceControlPackageContainerTests.swift b/Tests/WorkspaceTests/SourceControlPackageContainerTests.swift index e5cd30823f1..053f6fbbe46 100644 --- a/Tests/WorkspaceTests/SourceControlPackageContainerTests.swift +++ b/Tests/WorkspaceTests/SourceControlPackageContainerTests.swift @@ -554,7 +554,12 @@ class SourceControlPackageContainerTests: XCTestCase { // Create a repository manager for it. let repoProvider = GitRepositoryProvider() - let repositoryManager = RepositoryManager(fileSystem: localFileSystem, path: packageDir, provider: repoProvider, delegate: nil) + let repositoryManager = RepositoryManager( + fileSystem: localFileSystem, + path: packageDir, + provider: repoProvider, + delegate: .none + ) // Create a container provider, configured with a mock manifest loader that will return the package manifest. let manifest = Manifest.createRootManifest( @@ -595,12 +600,11 @@ class SourceControlPackageContainerTests: XCTestCase { } } + // From rdar://problem/65284674 + // RepositoryPackageContainer used to erroneously cache dependencies based only on version, + // storing the result of the first product filter and then continually returning it for other filters too. + // This lead to corrupt graph states. func testRepositoryPackageContainerCache() throws { - // From rdar://problem/65284674 - // RepositoryPackageContainer used to erroneously cache dependencies based only on version, - // storing the result of the first product filter and then continually returning it for other filters too. - // This lead to corrupt graph states. - try testWithTemporaryDirectory { temporaryDirectory in let packageDirectory = temporaryDirectory.appending(component: "Package") try localFileSystem.createDirectory(packageDirectory) @@ -619,7 +623,7 @@ class SourceControlPackageContainerTests: XCTestCase { fileSystem: localFileSystem, path: packageDirectory, provider: repositoryProvider, - delegate: nil + delegate: .none ) let version = Version(1, 0, 0) diff --git a/Tests/WorkspaceTests/WorkspaceStateTests.swift b/Tests/WorkspaceTests/WorkspaceStateTests.swift index cd99e3bf0e9..57dd8c16a55 100644 --- a/Tests/WorkspaceTests/WorkspaceStateTests.swift +++ b/Tests/WorkspaceTests/WorkspaceStateTests.swift @@ -8,8 +8,9 @@ See http://swift.org/CONTRIBUTORS.txt for Swift project authors */ -import TSCBasic +import Basics @testable import Workspace +import TSCBasic import XCTest final class WorkspaceStateTests: XCTestCase { @@ -17,71 +18,71 @@ final class WorkspaceStateTests: XCTestCase { let fs = InMemoryFileSystem() let buildDir = AbsolutePath("/.build") - let statePath = buildDir.appending(component: "workspace-state.json") + try fs.createDirectory(buildDir, recursive: true) - try fs.writeFileContents(statePath) { - $0 <<< - """ - { - "version": 4, - "object": { - "artifacts": [], - "dependencies": [ - { - "basedOn": null, - "packageRef": { - "identity": "yams", - "kind": "remote", - "location": "https://github.com/jpsim/Yams.git", - "name": "Yams" - }, - "state": { - "checkoutState": { - "revision": "9ff1cc9327586db4e0c8f46f064b6a82ec1566fa", - "version": "4.0.6" - }, - "name": "checkout" - }, - "subpath": "Yams" + let statePath = buildDir.appending(component: "workspace-state.json") + try fs.writeFileContents(statePath, string: + """ + { + "version": 4, + "object": { + "artifacts": [], + "dependencies": [ + { + "basedOn": null, + "packageRef": { + "identity": "yams", + "kind": "remote", + "location": "https://github.com/jpsim/Yams.git", + "name": "Yams" + }, + "state": { + "checkoutState": { + "revision": "9ff1cc9327586db4e0c8f46f064b6a82ec1566fa", + "version": "4.0.6" + }, + "name": "checkout" + }, + "subpath": "Yams" + }, + { + "basedOn": null, + "packageRef": { + "identity": "swift-tools-support-core", + "kind": "remote", + "location": "https://github.com/apple/swift-tools-support-core.git", + "name": "swift-tools-support-core" + }, + "state": { + "checkoutState": { + "branch": "main", + "revision": "f9bbd6b80d67408021576adf6247e17c2e957d92", + "version": null + }, + "name": "checkout" }, - { - "basedOn": null, - "packageRef": { - "identity": "swift-tools-support-core", - "kind": "remote", - "location": "https://github.com/apple/swift-tools-support-core.git", - "name": "swift-tools-support-core" - }, - "state": { - "checkoutState": { - "branch": "main", - "revision": "f9bbd6b80d67408021576adf6247e17c2e957d92", - "version": null - }, - "name": "checkout" - }, - "subpath": "swift-tools-support-core" + "subpath": "swift-tools-support-core" + }, + { + "basedOn": null, + "packageRef": { + "identity": "swift-argument-parser", + "kind": "local", + "location": "/Users/tomerd/code/swift/swift-argument-parser", + "name": "swift-argument-parser" }, - { - "basedOn": null, - "packageRef": { - "identity": "swift-argument-parser", - "kind": "local", - "location": "/Users/tomerd/code/swift/swift-argument-parser", - "name": "swift-argument-parser" - }, - "state": { - "name": "local" - }, - "subpath": "swift-argument-parser" - } - ] - } + "state": { + "name": "local" + }, + "subpath": "swift-argument-parser" + } + ] } - """ - } + } + """ + ) - let state = WorkspaceState(dataPath: buildDir, fileSystem: fs) + let state = WorkspaceState(fileSystem: fs, storageDirectory: buildDir) XCTAssertTrue(state.dependencies.contains(where: { $0.packageRef.identity == .plain("yams") })) XCTAssertTrue(state.dependencies.contains(where: { $0.packageRef.identity == .plain("swift-tools-support-core") })) XCTAssertTrue(state.dependencies.contains(where: { $0.packageRef.identity == .plain("swift-argument-parser") })) @@ -91,71 +92,71 @@ final class WorkspaceStateTests: XCTestCase { let fs = InMemoryFileSystem() let buildDir = AbsolutePath("/.build") - let statePath = buildDir.appending(component: "workspace-state.json") + try fs.createDirectory(buildDir, recursive: true) - try fs.writeFileContents(statePath) { - $0 <<< - """ - { - "version": 4, - "object": { - "artifacts": [], - "dependencies": [ - { - "basedOn": null, - "packageRef": { - "identity": "yams", - "kind": "remote", - "path": "https://github.com/jpsim/Yams.git", - "name": "Yams" - }, - "state": { - "checkoutState": { - "revision": "9ff1cc9327586db4e0c8f46f064b6a82ec1566fa", - "version": "4.0.6" - }, - "name": "checkout" - }, - "subpath": "Yams" + let statePath = buildDir.appending(component: "workspace-state.json") + try fs.writeFileContents(statePath, string: + """ + { + "version": 4, + "object": { + "artifacts": [], + "dependencies": [ + { + "basedOn": null, + "packageRef": { + "identity": "yams", + "kind": "remote", + "path": "https://github.com/jpsim/Yams.git", + "name": "Yams" + }, + "state": { + "checkoutState": { + "revision": "9ff1cc9327586db4e0c8f46f064b6a82ec1566fa", + "version": "4.0.6" + }, + "name": "checkout" + }, + "subpath": "Yams" + }, + { + "basedOn": null, + "packageRef": { + "identity": "swift-tools-support-core", + "kind": "remote", + "path": "https://github.com/apple/swift-tools-support-core.git", + "name": "swift-tools-support-core" + }, + "state": { + "checkoutState": { + "branch": "main", + "revision": "f9bbd6b80d67408021576adf6247e17c2e957d92", + "version": null + }, + "name": "checkout" }, - { - "basedOn": null, - "packageRef": { - "identity": "swift-tools-support-core", - "kind": "remote", - "path": "https://github.com/apple/swift-tools-support-core.git", - "name": "swift-tools-support-core" - }, - "state": { - "checkoutState": { - "branch": "main", - "revision": "f9bbd6b80d67408021576adf6247e17c2e957d92", - "version": null - }, - "name": "checkout" - }, - "subpath": "swift-tools-support-core" + "subpath": "swift-tools-support-core" + }, + { + "basedOn": null, + "packageRef": { + "identity": "swift-argument-parser", + "kind": "local", + "path": "/Users/tomerd/code/swift/swift-argument-parser", + "name": "swift-argument-parser" }, - { - "basedOn": null, - "packageRef": { - "identity": "swift-argument-parser", - "kind": "local", - "path": "/Users/tomerd/code/swift/swift-argument-parser", - "name": "swift-argument-parser" - }, - "state": { - "name": "local" - }, - "subpath": "swift-argument-parser" - } - ] - } + "state": { + "name": "local" + }, + "subpath": "swift-argument-parser" + } + ] } - """ - } + } + """ + ) - let state = WorkspaceState(dataPath: buildDir, fileSystem: fs) + let state = WorkspaceState(fileSystem: fs, storageDirectory: buildDir) XCTAssertTrue(state.dependencies.contains(where: { $0.packageRef.identity == .plain("yams") })) XCTAssertTrue(state.dependencies.contains(where: { $0.packageRef.identity == .plain("swift-tools-support-core") })) XCTAssertTrue(state.dependencies.contains(where: { $0.packageRef.identity == .plain("swift-argument-parser") })) @@ -165,71 +166,71 @@ final class WorkspaceStateTests: XCTestCase { let fs = InMemoryFileSystem() let buildDir = AbsolutePath("/.build") - let statePath = buildDir.appending(component: "workspace-state.json") + try fs.createDirectory(buildDir, recursive: true) - try fs.writeFileContents(statePath) { - $0 <<< - """ - { - "version": 5, - "object": { - "artifacts": [], - "dependencies": [ - { - "basedOn": null, - "packageRef": { - "identity": "yams", - "kind": "remoteSourceControl", - "location": "https://github.com/jpsim/Yams.git", - "name": "Yams" - }, - "state": { - "checkoutState": { - "revision": "9ff1cc9327586db4e0c8f46f064b6a82ec1566fa", - "version": "4.0.6" - }, - "name": "checkout" - }, - "subpath": "Yams" + let statePath = buildDir.appending(component: "workspace-state.json") + try fs.writeFileContents(statePath, string: + """ + { + "version": 5, + "object": { + "artifacts": [], + "dependencies": [ + { + "basedOn": null, + "packageRef": { + "identity": "yams", + "kind": "remoteSourceControl", + "location": "https://github.com/jpsim/Yams.git", + "name": "Yams" + }, + "state": { + "checkoutState": { + "revision": "9ff1cc9327586db4e0c8f46f064b6a82ec1566fa", + "version": "4.0.6" + }, + "name": "checkout" + }, + "subpath": "Yams" + }, + { + "basedOn": null, + "packageRef": { + "identity": "swift-tools-support-core", + "kind": "remoteSourceControl", + "location": "https://github.com/apple/swift-tools-support-core.git", + "name": "swift-tools-support-core" + }, + "state": { + "checkoutState": { + "branch": "main", + "revision": "f9bbd6b80d67408021576adf6247e17c2e957d92" + }, + "name": "checkout" }, - { - "basedOn": null, - "packageRef": { - "identity": "swift-tools-support-core", - "kind": "remoteSourceControl", - "location": "https://github.com/apple/swift-tools-support-core.git", - "name": "swift-tools-support-core" - }, - "state": { - "checkoutState": { - "branch": "main", - "revision": "f9bbd6b80d67408021576adf6247e17c2e957d92" - }, - "name": "checkout" - }, - "subpath": "swift-tools-support-core" + "subpath": "swift-tools-support-core" + }, + { + "basedOn": null, + "packageRef": { + "identity": "swift-argument-parser", + "kind": "fileSystem", + "location": "/Users/tomerd/code/swift/swift-argument-parser", + "name": "swift-argument-parser" }, - { - "basedOn": null, - "packageRef": { - "identity": "swift-argument-parser", - "kind": "fileSystem", - "location": "/Users/tomerd/code/swift/swift-argument-parser", - "name": "swift-argument-parser" - }, - "state": { - "name": "local", - "path": "/Users/tomerd/code/swift/swift-argument-parser" - }, - "subpath": "swift-argument-parser" - } - ] - } + "state": { + "name": "local", + "path": "/Users/tomerd/code/swift/swift-argument-parser" + }, + "subpath": "swift-argument-parser" + } + ] } - """ - } + } + """ + ) - let state = WorkspaceState(dataPath: buildDir, fileSystem: fs) + let state = WorkspaceState(fileSystem: fs, storageDirectory: buildDir) XCTAssertTrue(state.dependencies.contains(where: { $0.packageRef.identity == .plain("yams") })) XCTAssertTrue(state.dependencies.contains(where: { $0.packageRef.identity == .plain("swift-tools-support-core") })) XCTAssertTrue(state.dependencies.contains(where: { $0.packageRef.identity == .plain("swift-argument-parser") })) @@ -239,57 +240,57 @@ final class WorkspaceStateTests: XCTestCase { let fs = InMemoryFileSystem() let buildDir = AbsolutePath("/.build") - let statePath = buildDir.appending(component: "workspace-state.json") + try fs.createDirectory(buildDir, recursive: true) - try fs.writeFileContents(statePath) { - $0 <<< - """ - { - "version": 5, - "object": { - "artifacts": [], - "dependencies": [ - { - "basedOn": null, - "packageRef": { - "identity": "yams", - "kind": "remoteSourceControl", - "location": "https://github.com/jpsim/Yams.git", - "name": "Yams" - }, - "state": { - "checkoutState": { - "revision": "9ff1cc9327586db4e0c8f46f064b6a82ec1566fa", - "version": "4.0.6" - }, - "name": "checkout" - }, - "subpath": "Yams" + let statePath = buildDir.appending(component: "workspace-state.json") + try fs.writeFileContents(statePath, string: + """ + { + "version": 5, + "object": { + "artifacts": [], + "dependencies": [ + { + "basedOn": null, + "packageRef": { + "identity": "yams", + "kind": "remoteSourceControl", + "location": "https://github.com/jpsim/Yams.git", + "name": "Yams" + }, + "state": { + "checkoutState": { + "revision": "9ff1cc9327586db4e0c8f46f064b6a82ec1566fa", + "version": "4.0.6" + }, + "name": "checkout" + }, + "subpath": "Yams" + }, + { + "basedOn": null, + "packageRef": { + "identity": "swift-argument-parser", + "kind": "remoteSourceControl", + "location": "https://github.com/apple/swift-argument-parser.git", + "name": "swift-argument-parser" + }, + "state": { + "checkoutState": { + "revision": "83b23d940471b313427da226196661856f6ba3e0", + "version": "0.4.4" + }, + "name": "checkout" }, - { - "basedOn": null, - "packageRef": { - "identity": "swift-argument-parser", - "kind": "remoteSourceControl", - "location": "https://github.com/apple/swift-argument-parser.git", - "name": "swift-argument-parser" - }, - "state": { - "checkoutState": { - "revision": "83b23d940471b313427da226196661856f6ba3e0", - "version": "0.4.4" - }, - "name": "checkout" - }, - "subpath": "swift-argument-parser" - } - ] - } + "subpath": "swift-argument-parser" + } + ] } - """ - } + } + """ + ) - let state = WorkspaceState(dataPath: buildDir, fileSystem: fs) + let state = WorkspaceState(fileSystem: fs, storageDirectory: buildDir) try state.save() let serialized = try fs.readFileContents(statePath).description diff --git a/Tests/WorkspaceTests/WorkspaceTests.swift b/Tests/WorkspaceTests/WorkspaceTests.swift index 583fcc2dc55..dff5375daf4 100644 --- a/Tests/WorkspaceTests/WorkspaceTests.swift +++ b/Tests/WorkspaceTests/WorkspaceTests.swift @@ -125,6 +125,7 @@ final class WorkspaceTests: XCTestCase { func testInterpreterFlags() throws { let fs = localFileSystem + try testWithTemporaryDirectory { path in let foo = path.appending(component: "foo") @@ -138,7 +139,7 @@ final class WorkspaceTests: XCTestCase { let sandbox = path.appending(component: "ws") return try Workspace( fileSystem: fs, - location: .init(forRootPackage: sandbox, fileSystem: fs), + forRootPackage: sandbox, customManifestLoader: manifestLoader, delegate: MockWorkspaceDelegate() ) @@ -178,6 +179,7 @@ final class WorkspaceTests: XCTestCase { func testManifestParseError() throws { let observability = ObservabilitySystem.makeForTesting() + try testWithTemporaryDirectory { path in let pkgDir = path.appending(component: "MyPkg") try localFileSystem.writeFileContents(pkgDir.appending(component: "Package.swift")) { @@ -193,7 +195,7 @@ final class WorkspaceTests: XCTestCase { } let workspace = try Workspace( fileSystem: localFileSystem, - location: .init(forRootPackage: pkgDir, fileSystem: localFileSystem), + forRootPackage: pkgDir, customManifestLoader: ManifestLoader(toolchain: ToolchainConfiguration.default), delegate: MockWorkspaceDelegate() ) @@ -3960,7 +3962,10 @@ final class WorkspaceTests: XCTestCase { // Load the workspace. let observability = ObservabilitySystem.makeForTesting() - let workspace = try Workspace(forRootPackage: packagePath, customToolchain: UserToolchain.default) + let workspace = try Workspace( + forRootPackage: packagePath, + customToolchain: UserToolchain.default + ) // From here the API should be simple and straightforward: let manifest = try tsc_await { @@ -8295,7 +8300,7 @@ final class WorkspaceTests: XCTestCase { let sandbox = path.appending(component: "ws") let workspace = try Workspace( fileSystem: fs, - location: .init(forRootPackage: sandbox, fileSystem: fs), + forRootPackage: sandbox, customManifestLoader: manifestLoader, delegate: MockWorkspaceDelegate() ) @@ -8360,7 +8365,7 @@ final class WorkspaceTests: XCTestCase { let delegate = MockWorkspaceDelegate() let workspace = try Workspace( fileSystem: fs, - location: .init(forRootPackage: .root, fileSystem: fs), + forRootPackage: .root, customManifestLoader: TestLoader(error: .none), delegate: delegate ) @@ -8375,7 +8380,7 @@ final class WorkspaceTests: XCTestCase { let delegate = MockWorkspaceDelegate() let workspace = try Workspace( fileSystem: fs, - location: .init(forRootPackage: .root, fileSystem: fs), + forRootPackage: .root, customManifestLoader: TestLoader(error: Diagnostics.fatalError), delegate: delegate ) @@ -8390,7 +8395,7 @@ final class WorkspaceTests: XCTestCase { let delegate = MockWorkspaceDelegate() let workspace = try Workspace( fileSystem: fs, - location: .init(forRootPackage: .root, fileSystem: fs), + forRootPackage: .root, customManifestLoader: TestLoader(error: StringError("boom")), delegate: delegate )