Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enhance configuraiton for dependencies mirrors #3670

Merged
merged 2 commits into from
Aug 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Fixtures/Miscellaneous/Simple/Foo.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
public func foo() {
{}()
}
12 changes: 12 additions & 0 deletions Fixtures/Miscellaneous/Simple/Package.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// swift-tools-version:5.2
import PackageDescription

let package = Package(
name: "Foo",
products: [
.library(name: "Foo", targets: ["Foo"]),
],
targets: [
.target(name: "Foo", path: "./"),
]
)
15 changes: 14 additions & 1 deletion Sources/Basics/FileSystem+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
*/

import TSCBasic
import class Foundation.FileManager
import struct Foundation.Data
import TSCBasic

// MARK: - user level

Expand Down Expand Up @@ -110,3 +111,15 @@ extension FileSystem {
return idiomaticConfigDirectory
}
}

// MARK: - Utilities

extension FileSystem {
public func readFileContents(_ path: AbsolutePath) throws -> Data {
return try Data(self.readFileContents(path).contents)
}

public func writeFileContents(_ path: AbsolutePath, data: Data) throws {
return try self.writeFileContents(path, bytes: ByteString(data))
}
}
3 changes: 1 addition & 2 deletions Sources/Commands/Options.swift
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,7 @@ public struct SwiftToolOptions: ParsableArguments {
@Option(help: "Specify the shared cache directory")
var cachePath: AbsolutePath?

// TODO: add actual help when ready to be used
@Option(help: .hidden)
@Option(help: "Specify the shared configuration directory")
var configPath: AbsolutePath?

/// Disables repository caching.
Expand Down
12 changes: 7 additions & 5 deletions Sources/Commands/SwiftPackageTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ extension SwiftPackageTool {

case .set(let value):
guard let toolsVersion = ToolsVersion(string: value) else {
// FIXME: Probably lift this error defination to ToolsVersion.
// FIXME: Probably lift this error definition to ToolsVersion.
throw ToolsVersionLoader.Error.malformedToolsVersionSpecification(.versionSpecifier(.isMisspelt(value)))
}
try rewriteToolsVersionSpecification(toDefaultManifestIn: pkg, specifying: toolsVersion, fileSystem: localFileSystem)
Expand Down Expand Up @@ -895,8 +895,9 @@ extension SwiftPackageTool.Config {
throw ExitCode.failure
}

config.mirrors.set(mirrorURL: mirrorURL, forURL: originalURL)
try config.saveState()
try config.applyLocal { mirrors in
mirrors.set(mirrorURL: mirrorURL, forURL: originalURL)
}
}
}

Expand Down Expand Up @@ -929,8 +930,9 @@ extension SwiftPackageTool.Config {
throw ExitCode.failure
}

try config.mirrors.unset(originalOrMirrorURL: originalOrMirrorURL)
try config.saveState()
try config.applyLocal { mirrors in
try mirrors.unset(originalOrMirrorURL: originalOrMirrorURL)
}
}
}

Expand Down
103 changes: 63 additions & 40 deletions Sources/Commands/SwiftTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -452,42 +452,63 @@ public class SwiftTool {
}
}

func editablesPath() throws -> AbsolutePath {
private func editsDirectory() throws -> AbsolutePath {
// TODO: replace multiroot-data-file with explicit overrides
if let multiRootPackageDataFile = options.multirootPackageDataFile {
return multiRootPackageDataFile.appending(component: "Packages")
}
return try getPackageRoot().appending(component: "Packages")
return try Workspace.DefaultLocations.editsDirectory(forRootPackage: self.getPackageRoot())
}

func resolvedVersionsFilePath() throws -> AbsolutePath {
private func resolvedVersionsFile() throws -> AbsolutePath {
// TODO: replace multiroot-data-file with explicit overrides
if let multiRootPackageDataFile = options.multirootPackageDataFile {
return multiRootPackageDataFile.appending(components: "xcshareddata", "swiftpm", "Package.resolved")
}
return try getPackageRoot().appending(component: "Package.resolved")
return try Workspace.DefaultLocations.resolvedVersionsFile(forRootPackage: self.getPackageRoot())
}

func mirrorsConfigFilePath() throws -> AbsolutePath {
func getMirrorsConfig(sharedConfigurationDirectory: AbsolutePath? = nil) throws -> Workspace.Configuration.Mirrors {
let sharedConfigurationDirectory = try sharedConfigurationDirectory ?? self.getSharedConfigurationDirectory()
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 {
return multiRootPackageDataFile.appending(components: "xcshareddata", "swiftpm", "config")
// 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
}
return try getPackageRoot().appending(components: ".swiftpm", "config")
}

func getMirrorsConfig() throws -> Workspace.Configuration {
return try _mirrorsConfig.get()
// 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) {
try localFileSystem.createDirectory(newPath.parentDirectory, recursive: true)
try localFileSystem.move(from: legacyPath, to: newPath)
}
return newPath
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattt @neonichu @abertelrud note the code above does the migration from old path to new one. we should carefully integrate that into the clients such as Xcode

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Among the other development-oriented documents in SwiftPM, maybe we should have one specifically focused on clients like IDEs that calls out these things. Or maybe a separate section in the change log. Just a thought.

}

private lazy var _mirrorsConfig: Result<Workspace.Configuration, Swift.Error> = {
return Result(catching: { try Workspace.Configuration(path: try mirrorsConfigFilePath(), fileSystem: localFileSystem) })
}()

func netrcFilePath() throws -> AbsolutePath? {
private func netrcFilePath() throws -> AbsolutePath? {
guard options.netrc ||
options.netrcFilePath != nil ||
options.netrcOptional else { return nil }
Expand All @@ -499,43 +520,43 @@ public class SwiftTool {
throw ExitCode.failure
} else {
diagnostics.emit(warning: "Did not find optional .netrc file at \(resolvedPath.pathString).")
return nil
return .none
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed this change a couple of diffs. Is this the recommended style for conditionals?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to use .none when the optional marks a clear functional decision, e.g. let cache = .none is more semantic in my mind then let cache = nil

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I'll start doing the same, and if/when we do a swift-format pass on the whole codebase we can do the rest then.

}
}
return resolvedPath
}

private func getCachePath(fileSystem: FileSystem = localFileSystem) throws -> AbsolutePath? {
private func getSharedCacheDirectory() throws -> AbsolutePath? {
if let explicitCachePath = options.cachePath {
// Create the explicit cache path if necessary
if !fileSystem.exists(explicitCachePath) {
try fileSystem.createDirectory(explicitCachePath, recursive: true)
if !localFileSystem.exists(explicitCachePath) {
try localFileSystem.createDirectory(explicitCachePath, recursive: true)
}
return explicitCachePath
}

do {
return try fileSystem.getOrCreateSwiftPMCacheDirectory()
return try localFileSystem.getOrCreateSwiftPMCacheDirectory()
} catch {
self.diagnostics.emit(warning: "Failed creating default cache locations, \(error)")
return nil
self.diagnostics.emit(warning: "Failed creating default cache location, \(error)")
return .none
}
}

private func getConfigPath(fileSystem: FileSystem = localFileSystem) throws -> AbsolutePath? {
private func getSharedConfigurationDirectory() throws -> AbsolutePath? {
if let explicitConfigPath = options.configPath {
// Create the explicit config path if necessary
if !fileSystem.exists(explicitConfigPath) {
try fileSystem.createDirectory(explicitConfigPath, recursive: true)
if !localFileSystem.exists(explicitConfigPath) {
try localFileSystem.createDirectory(explicitConfigPath, recursive: true)
}
return explicitConfigPath
}

do {
return try fileSystem.getOrCreateSwiftPMConfigDirectory()
return try localFileSystem.getOrCreateSwiftPMConfigDirectory()
} catch {
self.diagnostics.emit(warning: "Failed creating default config locations, \(error)")
return nil
self.diagnostics.emit(warning: "Failed creating default configuration location, \(error)")
return .none
}
}

Expand All @@ -548,25 +569,27 @@ public class SwiftTool {
let isVerbose = options.verbosity != 0
let delegate = ToolWorkspaceDelegate(self.stdoutStream, isVerbose: isVerbose, diagnostics: diagnostics)
let provider = GitRepositoryProvider(processSet: processSet)
let cachePath = self.options.useRepositoriesCache ? try self.getCachePath() : .none
_ = try self.getConfigPath() // TODO: actually use this in the workspace
let sharedCacheDirectory = try self.getSharedCacheDirectory()
let sharedConfigurationDirectory = try self.getSharedConfigurationDirectory()
let isXcodeBuildSystemEnabled = self.options.buildSystem == .xcode
let workspace = try Workspace(
fileSystem: localFileSystem,
location: .init(
workingDirectory: buildPath,
editsDirectory: try editablesPath(),
resolvedVersionsFilePath: try resolvedVersionsFilePath(),
sharedCacheDirectory: cachePath
editsDirectory: self.editsDirectory(),
resolvedVersionsFile: self.resolvedVersionsFile(),
sharedCacheDirectory: sharedCacheDirectory,
sharedConfigurationDirectory: sharedConfigurationDirectory
),
netrcFilePath: try netrcFilePath(),
mirrors: self.getMirrorsConfig().mirrors,
customManifestLoader: try getManifestLoader(), // FIXME: doe we really need to customize it?
mirrors: self.getMirrorsConfig(sharedConfigurationDirectory: sharedConfigurationDirectory).mirrors,
netrcFilePath: self.netrcFilePath(),
customManifestLoader: self.getManifestLoader(), // FIXME: doe we really need to customize it?
customRepositoryProvider: provider, // FIXME: doe we really need to customize it?
additionalFileRules: isXcodeBuildSystemEnabled ? FileRuleDescription.xcbuildFileTypes : FileRuleDescription.swiftpmFileTypes,
resolverUpdateEnabled: !options.skipDependencyUpdate,
resolverPrefetchingEnabled: options.shouldEnableResolverPrefetching,
resolverTracingEnabled: options.enableResolverTrace,
sharedRepositoriesCacheEnabled: self.options.useRepositoriesCache,
delegate: delegate
)
_workspace = workspace
Expand Down Expand Up @@ -858,16 +881,16 @@ public class SwiftTool {
switch (self.options.shouldDisableManifestCaching, self.options.manifestCachingMode) {
case (true, _):
// backwards compatibility
cachePath = nil
cachePath = .none
case (false, .none):
cachePath = nil
cachePath = .none
case (false, .local):
cachePath = self.buildPath
case (false, .shared):
cachePath = try self.getCachePath().map{ $0.appending(component: "manifests") }
cachePath = try self.getSharedCacheDirectory().map{ Workspace.DefaultLocations.manifestsDirectory(at: $0) }
}

var extraManifestFlags = self.options.manifestFlags
var extraManifestFlags = self.options.manifestFlags
// Disable the implicit concurrency import if the compiler in use supports it to avoid warnings if we are building against an older SDK that does not contain a Concurrency module.
if SwiftTargetBuildDescription.checkSupportedFrontendFlags(flags: ["disable-implicit-concurrency-module-import"], fs: localFileSystem) {
extraManifestFlags += ["-Xfrontend", "-disable-implicit-concurrency-module-import"]
Expand Down
Loading