Skip to content

Commit

Permalink
initialization warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
tomerd committed Jan 26, 2022
1 parent e60a6d2 commit 615db52
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 35 deletions.
10 changes: 3 additions & 7 deletions Sources/Basics/FileSystem+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ extension FileSystem {
}

extension FileSystem {
public func getOrCreateSwiftPMConfigurationDirectory() throws -> AbsolutePath {
public func getOrCreateSwiftPMConfigurationDirectory(warningHandler: (String) -> Void) throws -> AbsolutePath {
let idiomaticConfigurationDirectory = self.swiftPMConfigurationDirectory

// temporary 5.6, remove on next version: transition from previous configuration location
Expand All @@ -111,9 +111,7 @@ extension FileSystem {
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()
warningHandler("Usage of \(file) has been deprecated. Please delete it and use the new \(destination) instead.")
}
}
// in the case where ~/.swiftpm/configuration is the idiomatic location (eg on Linux)
Expand All @@ -130,9 +128,7 @@ extension FileSystem {
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()
warningHandler("Usage of \(file) has been deprecated. Please delete it and use the new \(destination) instead.")
}
}
}
Expand Down
1 change: 1 addition & 0 deletions Sources/Commands/SwiftTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,7 @@ public class SwiftTool {
sharedRepositoriesCacheEnabled: self.options.useRepositoriesCache,
fingerprintCheckingMode: self.options.resolverFingerprintCheckingMode
),
initializationWarningHandler: { self.observabilityScope.emit(warning: $0) },
customManifestLoader: self.getManifestLoader(), // FIXME: ideally we would not customize the manifest loader
customRepositoryProvider: repositoryProvider, // FIXME: ideally we would not customize the repository provider. its currently done for shutdown handling which can be better abstracted
delegate: delegate
Expand Down
10 changes: 6 additions & 4 deletions Sources/SourceControl/RepositoryManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,22 @@ public class RepositoryManager {
/// Create a new empty manager.
///
/// - Parameters:
/// - fileSystem: The filesystem to operate on.
/// - path: The path under which to store repositories. This should be a
/// directory in which the content can be completely managed by this
/// instance.
/// - provider: The repository provider.
/// - cachePath: The repository cache location.
/// - cacheLocalPackages: Should cache local packages as well. For testing purposes.
/// - initializationWarningHandler: Initialization warnings handler.
/// - delegate: The repository manager delegate.
/// - fileSystem: The filesystem to operate on.
public init(
fileSystem: FileSystem,
path: AbsolutePath,
provider: RepositoryProvider,
cachePath: AbsolutePath? = .none,
cacheLocalPackages: Bool = false,
initializationWarningHandler: (String) -> Void,
delegate: RepositoryManagerDelegate? = .none
) {
self.fileSystem = fileSystem
Expand All @@ -107,9 +111,7 @@ public class RepositoryManager {
} catch {
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)\n")
TSCBasic.stderrStream.flush()
initializationWarningHandler("unable to restore checkouts state: \(error)")
}
}

Expand Down
55 changes: 35 additions & 20 deletions Sources/Workspace/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ public class Workspace {
/// - location: Workspace location configuration.
/// - authorizationProvider: Provider of authentication information for outbound network requests.
/// - configuration: Configuration to fine tune the dependency resolution behavior.
/// - initializationWarningHandler: Initialization warnings handler
/// - 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.
Expand All @@ -263,6 +264,7 @@ public class Workspace {
location: Location,
authorizationProvider: AuthorizationProvider? = .none,
configuration: WorkspaceConfiguration? = .none,
initializationWarningHandler: ((String) -> Void)? = .none,
// optional customization used for advanced integration situations
customManifestLoader: ManifestLoaderProtocol? = .none,
customPackageContainerProvider: PackageContainerProvider? = .none,
Expand All @@ -275,6 +277,7 @@ public class Workspace {
location: location,
authorizationProvider: authorizationProvider,
configuration: configuration,
initializationWarningHandler: initializationWarningHandler,
customRegistriesConfiguration: .none,
customFingerprints: .none,
customMirrors: .none,
Expand Down Expand Up @@ -303,6 +306,7 @@ public class Workspace {
/// - 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.
/// - initializationWarningHandler: Initialization warnings handler
/// - 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.
Expand All @@ -312,6 +316,7 @@ public class Workspace {
forRootPackage packagePath: AbsolutePath,
authorizationProvider: AuthorizationProvider? = .none,
configuration: WorkspaceConfiguration? = .none,
initializationWarningHandler: ((String) -> Void)? = .none,
// optional customization used for advanced integration situations
customManifestLoader: ManifestLoaderProtocol? = .none,
customPackageContainerProvider: PackageContainerProvider? = .none,
Expand All @@ -324,6 +329,7 @@ public class Workspace {
try self.init(
fileSystem: fileSystem,
location: location,
initializationWarningHandler: initializationWarningHandler,
customManifestLoader: customManifestLoader,
customPackageContainerProvider: customPackageContainerProvider,
customRepositoryProvider: customRepositoryProvider,
Expand All @@ -342,6 +348,7 @@ public class Workspace {
/// - 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.
/// - initializationWarningHandler: Initialization warnings handler
/// - 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.
Expand All @@ -351,6 +358,7 @@ public class Workspace {
forRootPackage packagePath: AbsolutePath,
authorizationProvider: AuthorizationProvider? = .none,
configuration: WorkspaceConfiguration? = .none,
initializationWarningHandler: ((String) -> Void)? = .none,
// optional customization used for advanced integration situations
customToolchain: UserToolchain,
customPackageContainerProvider: PackageContainerProvider? = .none,
Expand All @@ -369,15 +377,14 @@ public class Workspace {
forRootPackage: packagePath,
authorizationProvider: authorizationProvider,
configuration: configuration,
initializationWarningHandler: initializationWarningHandler,
customManifestLoader: manifestLoader,
customPackageContainerProvider: customPackageContainerProvider,
customRepositoryProvider: customRepositoryProvider,
delegate: delegate
)
}



// deprecate 12/21
@_disfavoredOverload
@available(*, deprecated, message: "use alternative initializer")
Expand Down Expand Up @@ -418,6 +425,7 @@ public class Workspace {
location: location,
authorizationProvider: authorizationProvider,
configuration: configuration,
initializationWarningHandler: .none,
customRegistriesConfiguration: registries,
customFingerprints: customFingerprintStorage,
customMirrors: mirrors,
Expand Down Expand Up @@ -533,6 +541,7 @@ public class Workspace {
location: Location,
authorizationProvider: AuthorizationProvider? = .none,
configuration: WorkspaceConfiguration? = .none,
initializationWarningHandler: ((String) -> Void)? = .none,
// optional customization, primarily designed for testing but also used in some cases by libSwiftPM consumers
customRegistriesConfiguration: RegistryConfiguration? = .none,
customFingerprints: PackageFingerprintStorage? = .none,
Expand All @@ -555,6 +564,7 @@ public class Workspace {
location: location,
authorizationProvider: authorizationProvider,
configuration: configuration,
initializationWarningHandler: initializationWarningHandler,
customRegistriesConfiguration: customRegistriesConfiguration,
customFingerprints: customFingerprints,
customMirrors: customMirrors,
Expand All @@ -578,6 +588,7 @@ public class Workspace {
location: Location,
authorizationProvider: AuthorizationProvider?,
configuration: WorkspaceConfiguration?,
initializationWarningHandler: ((String) -> Void)?,
// optional customization, primarily designed for testing but also used in some cases by libSwiftPM consumers
customRegistriesConfiguration: RegistryConfiguration?,
customFingerprints: PackageFingerprintStorage?,
Expand All @@ -595,11 +606,11 @@ public class Workspace {
// delegate
delegate: WorkspaceDelegate?
) throws {
// we do not store the observabilityScope in the workspace initializer as the workspace is designed to be long lived.
// we do not store 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.

let initializationWarningHandler = initializationWarningHandler ?? warnToStderr
// validate locations, returning a potentially modified one to deal with non-accessible or non-writable shared locations
let location = try location.validatingSharedLocations(fileSystem: fileSystem)
let location = try location.validatingSharedLocations(fileSystem: fileSystem, warningHandler: initializationWarningHandler)

let currentToolsVersion = customToolsVersion ?? ToolsVersion.currentToolsVersion
let toolsVersionLoader = ToolsVersionLoader(currentToolsVersion: currentToolsVersion)
Expand All @@ -623,6 +634,7 @@ public class Workspace {
path: location.repositoriesDirectory,
provider: repositoryProvider,
cachePath: configuration.sharedRepositoriesCacheEnabled ? location.sharedRepositoriesCacheDirectory : .none,
initializationWarningHandler: initializationWarningHandler,
delegate: delegate.map(WorkspaceRepositoryManagerDelegate.init(workspaceDelegate:))
)

Expand Down Expand Up @@ -680,7 +692,11 @@ public class Workspace {

self.configuration = configuration

self.state = WorkspaceState(fileSystem: fileSystem, storageDirectory: self.location.workingDirectory)
self.state = WorkspaceState(
fileSystem: fileSystem,
storageDirectory: self.location.workingDirectory,
initializationWarningHandler: initializationWarningHandler
)
}
}

Expand Down Expand Up @@ -4012,16 +4028,16 @@ extension FileSystem {
}

extension Workspace.Location {
func validatingSharedLocations(fileSystem: FileSystem) throws -> Self {
func validatingSharedLocations(
fileSystem: FileSystem,
warningHandler: (String) -> Void
) throws -> Self {
var location = self

// local configuration directory must be accessible, throw if we cannot access it
//try fileSystem.withLock(on: location.localConfigurationDirectory, type: .exclusive, {})

// 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()
let defaultDirectory = try? fileSystem.getOrCreateSwiftPMConfigurationDirectory(warningHandler: warningHandler)
if sharedConfigurationDirectory != defaultDirectory {
// custom location must be writable, throw if we cannot access it
try withTemporaryFile(dir: sharedConfigurationDirectory) { _ in }
Expand All @@ -4031,9 +4047,7 @@ extension Workspace.Location {
try withTemporaryFile(dir: sharedConfigurationDirectory) { _ in }
} catch {
location.sharedConfigurationDirectory = .none
// FIXME: We should emit a warning here using the diagnostic engine.
TSCBasic.stderrStream.write("warning: \(sharedConfigurationDirectory) is not accessible or not writable, disabling user-level configuration features. \(error)\n")
TSCBasic.stderrStream.flush()
warningHandler("\(sharedConfigurationDirectory) is not accessible or not writable, disabling user-level configuration features. \(error)")
}
}
}
Expand All @@ -4051,9 +4065,7 @@ extension Workspace.Location {
try withTemporaryFile(dir: sharedSecurityDirectory) { _ in }
} catch {
location.sharedSecurityDirectory = .none
// FIXME: We should emit a warning here using the diagnostic engine.
TSCBasic.stderrStream.write("warning: \(sharedSecurityDirectory) is not accessible or not writable, disabling user-level security features. \(error)\n")
TSCBasic.stderrStream.flush()
warningHandler("\(sharedSecurityDirectory) is not accessible or not writable, disabling user-level security features. \(error)")
}
}
}
Expand All @@ -4071,12 +4083,15 @@ extension Workspace.Location {
try withTemporaryFile(dir: sharedCacheDirectory) { _ in }
} catch {
location.sharedCacheDirectory = .none
// FIXME: We should emit a warning here using the diagnostic engine.
TSCBasic.stderrStream.write("warning: \(sharedCacheDirectory) is not accessible or not writable, disabling user-level cache features. \(error)\n")
TSCBasic.stderrStream.flush()
warningHandler("\(sharedCacheDirectory) is not accessible or not writable, disabling user-level cache features. \(error)")
}
}
}
return location
}
}

fileprivate func warnToStderr(_ message: String) {
TSCBasic.stderrStream.write("warning: \(message)\n")
TSCBasic.stderrStream.flush()
}
10 changes: 6 additions & 4 deletions Sources/Workspace/WorkspaceState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ public final class WorkspaceState {
/// storage
private let storage: WorkspaceStateStorage

init(fileSystem: FileSystem, storageDirectory: AbsolutePath) {
init(
fileSystem: FileSystem,
storageDirectory: AbsolutePath,
initializationWarningHandler: (String) -> Void
) {
self.storagePath = storageDirectory.appending(component: "workspace-state.json")
self.storage = WorkspaceStateStorage(path: self.storagePath, fileSystem: fileSystem)

Expand All @@ -49,9 +53,7 @@ public final class WorkspaceState {
self.dependencies = Workspace.ManagedDependencies()
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)\n")
TSCBasic.stderrStream.flush()
initializationWarningHandler("unable to restore workspace state: \(error)")
}
}

Expand Down
19 changes: 19 additions & 0 deletions Tests/SourceControlTests/RepositoryManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,25 @@ class RepositoryManagerTests: XCTestCase {
}

extension RepositoryManager {
public convenience init(
fileSystem: FileSystem,
path: AbsolutePath,
provider: RepositoryProvider,
cachePath: AbsolutePath? = .none,
cacheLocalPackages: Bool = false,
delegate: RepositoryManagerDelegate? = .none
) {
self.init(
fileSystem: fileSystem,
path: path,
provider: provider,
cachePath: cachePath,
cacheLocalPackages: cacheLocalPackages,
initializationWarningHandler: { _ in },
delegate: delegate
)
}

fileprivate func lookup(repository: RepositorySpecifier, skipUpdate: Bool = false) throws -> RepositoryHandle {
return try tsc_await { self.lookup(repository: repository, skipUpdate: skipUpdate, on: .global(), completion: $0) }
}
Expand Down
21 changes: 21 additions & 0 deletions Tests/WorkspaceTests/SourceControlPackageContainerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -673,3 +673,24 @@ extension PackageContainerProvider {
try tsc_await { self.getContainer(for: package, skipUpdate: skipUpdate, observabilityScope: ObservabilitySystem.NOOP, on: .global(), completion: $0) }
}
}

extension RepositoryManager {
fileprivate convenience init(
fileSystem: FileSystem,
path: AbsolutePath,
provider: RepositoryProvider,
cachePath: AbsolutePath? = .none,
cacheLocalPackages: Bool = false,
delegate: RepositoryManagerDelegate? = .none
) {
self.init(
fileSystem: fileSystem,
path: path,
provider: provider,
cachePath: cachePath,
cacheLocalPackages: cacheLocalPackages,
initializationWarningHandler: { _ in },
delegate: delegate
)
}
}
6 changes: 6 additions & 0 deletions Tests/WorkspaceTests/WorkspaceStateTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -490,3 +490,9 @@ final class WorkspaceStateTests: XCTestCase {
XCTAssertTrue(state.artifacts.isEmpty)
}
}

extension WorkspaceState {
fileprivate convenience init(fileSystem: FileSystem, storageDirectory: AbsolutePath) {
self.init(fileSystem: fileSystem, storageDirectory: storageDirectory, initializationWarningHandler: { _ in })
}
}

0 comments on commit 615db52

Please sign in to comment.