Skip to content
Open
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
51 changes: 40 additions & 11 deletions Sources/Commands/PackageCommands/Config.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,16 @@ extension SwiftPackageCommand.Config {
@Option(name: .customLong("mirror-url"), help: .hidden)
var _deprecate_mirrorURL: String?

@Flag(help: "Apply settings to all projects for this user.")
var global: Bool = false

@Option(help: "The original url or identity.")
var original: String?

@Option(help: "The mirror url or identity.")
var mirror: String?

func run(_ swiftCommandState: SwiftCommandState) throws {
let config = try getMirrorsConfig(swiftCommandState)

if self._deprecate_packageURL != nil {
swiftCommandState.observabilityScope.emit(
warning: "'--package-url' option is deprecated; use '--original' instead"
Expand All @@ -81,8 +82,15 @@ extension SwiftPackageCommand.Config {
throw ExitCode.failure
}

try config.applyLocal { mirrors in
try mirrors.set(mirror: mirror, for: original)
let config = try getMirrorsConfig(swiftCommandState, global: self.global)
if self.global {
try config.applyShared { mirrors in
try mirrors.set(mirror: mirror, for: original)
}
} else {
try config.applyLocal { mirrors in
try mirrors.set(mirror: mirror, for: original)
}
}
}
}
Expand All @@ -104,15 +112,16 @@ extension SwiftPackageCommand.Config {
@Option(name: .customLong("mirror-url"), help: .hidden)
var _deprecate_mirrorURL: String?

@Flag(help: "Apply settings to all projects for this user.")
var global: Bool = false

@Option(help: "The original url or identity.")
var original: String?

@Option(help: "The mirror url or identity.")
var mirror: String?

func run(_ swiftCommandState: SwiftCommandState) throws {
let config = try getMirrorsConfig(swiftCommandState)

if self._deprecate_packageURL != nil {
swiftCommandState.observabilityScope.emit(
warning: "'--package-url' option is deprecated; use '--original' instead"
Expand All @@ -136,8 +145,15 @@ extension SwiftPackageCommand.Config {
throw ExitCode.failure
}

try config.applyLocal { mirrors in
try mirrors.unset(originalOrMirror: originalOrMirror)
let config = try getMirrorsConfig(swiftCommandState, global: self.global)
if self.global {
try config.applyShared { mirrors in
try mirrors.unset(originalOrMirror: originalOrMirror)
}
} else {
try config.applyLocal { mirrors in
try mirrors.unset(originalOrMirror: originalOrMirror)
}
}
}
}
Expand All @@ -155,12 +171,13 @@ extension SwiftPackageCommand.Config {
@Option(name: .customLong("original-url"), help: .hidden)
var _deprecate_originalURL: String?

@Flag(help: "Read only settings applied to all projects for this user.")
var global: Bool = false

@Option(help: "The original url or identity.")
var original: String?

func run(_ swiftCommandState: SwiftCommandState) throws {
let config = try getMirrorsConfig(swiftCommandState)

if self._deprecate_packageURL != nil {
swiftCommandState.observabilityScope.emit(
warning: "'--package-url' option is deprecated; use '--original' instead"
Expand All @@ -177,6 +194,7 @@ extension SwiftPackageCommand.Config {
throw ExitCode.failure
}

let config = try getMirrorsConfig(swiftCommandState, global: self.global)
if let mirror = config.mirrors.mirror(for: original) {
print(mirror)
} else {
Expand All @@ -187,7 +205,18 @@ extension SwiftPackageCommand.Config {
}
}

static func getMirrorsConfig(_ swiftCommandState: SwiftCommandState) throws -> Workspace.Configuration.Mirrors {
static func getMirrorsConfig(_ swiftCommandState: SwiftCommandState, global: Bool) throws -> Workspace.Configuration.Mirrors {
if global {
let sharedMirrorsFile = Workspace.DefaultLocations.mirrorsConfigurationFile(
at: swiftCommandState.sharedConfigurationDirectory
)
// Workspace not needed when working with user-level mirrors config
return try .init(
fileSystem: swiftCommandState.fileSystem,
localMirrorsFile: .none,
sharedMirrorsFile: sharedMirrorsFile
)
}
let workspace = try swiftCommandState.getActiveWorkspace()
return try .init(
fileSystem: swiftCommandState.fileSystem,
Expand Down
20 changes: 14 additions & 6 deletions Sources/Workspace/Workspace+Configuration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ extension Workspace.Configuration {

extension Workspace.Configuration {
public struct Mirrors {
private let localMirrors: MirrorsStorage
private let localMirrors: MirrorsStorage?
private let sharedMirrors: MirrorsStorage?
private let fileSystem: FileSystem

Expand Down Expand Up @@ -511,10 +511,16 @@ extension Workspace.Configuration {
/// - sharedMirrorsFile: Path to the shared mirrors configuration file, defaults to the standard location.
public init(
fileSystem: FileSystem,
localMirrorsFile: AbsolutePath,
localMirrorsFile: AbsolutePath?,
sharedMirrorsFile: AbsolutePath?
) throws {
self.localMirrors = .init(path: localMirrorsFile, fileSystem: fileSystem, deleteWhenEmpty: true)
// At least one of local or shared is required
if localMirrorsFile == nil, sharedMirrorsFile == nil {
throw StringError("No mirrors configuration provided")
}

self.localMirrors = localMirrorsFile
.map { .init(path: $0, fileSystem: fileSystem, deleteWhenEmpty: true) }
self.sharedMirrors = sharedMirrorsFile
.map { .init(path: $0, fileSystem: fileSystem, deleteWhenEmpty: false) }
self.fileSystem = fileSystem
Expand All @@ -525,7 +531,10 @@ extension Workspace.Configuration {

@discardableResult
public func applyLocal(handler: (inout DependencyMirrors) throws -> Void) throws -> DependencyMirrors {
try self.localMirrors.apply(handler: handler)
guard let localMirrors else {
throw InternalError("local mirrors not configured")
}
try localMirrors.apply(handler: handler)
try self.computeMirrors()
return self.mirrors
}
Expand All @@ -547,8 +556,7 @@ extension Workspace.Configuration {
self._mirrors.removeAll()

// prefer local mirrors to shared ones
let local = try self.localMirrors.get()
if !local.isEmpty {
if let local = try self.localMirrors?.get(), !local.isEmpty {
try self._mirrors.append(contentsOf: local)
return
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Though unrelated to this PR, I'm curious as to why we return here, instead of merging the two configuration files. The code for package registries use both files, merging the information with local configuration having higher priority. https://github.com/swiftlang/swift-package-manager/blob/main/Sources/Workspace/Workspace%2BConfiguration.swift#L732-L738 Is there reason why we don't do it here?
Would it be considered breaking to change mirrors to also merge instead of disregarding the global configuration?
If you have a global configuration with many values, and want to add some override locally, it's a bit inconvenient (and someway unexpected, to me) that you have to duplicate all other global configuration locally.

}
Expand Down
51 changes: 41 additions & 10 deletions Tests/CommandsTests/PackageCommandTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3747,9 +3747,12 @@ struct PackageCommandTests {
let fs = localFileSystem
let packageRoot = fixturePath.appending("Foo")
let configOverride = fixturePath.appending("configoverride")
let configFile = Workspace.DefaultLocations.mirrorsConfigurationFile(
let localConfigFile = Workspace.DefaultLocations.mirrorsConfigurationFile(
forRootPackage: packageRoot
)
let sharedConfigFile = Workspace.DefaultLocations.mirrorsConfigurationFile(
at: try fs.swiftPMConfigurationDirectory
)

fs.createEmptyFiles(
at: packageRoot,
Expand Down Expand Up @@ -3780,7 +3783,19 @@ struct PackageCommandTests {
configuration: config,
buildSystem: buildSystem,
)
#expect(fs.isFile(configFile))
#expect(fs.isFile(localConfigFile))

// Test writing.
try await execute(
[
"config", "set-mirror", "--global", "--original", "https://github.com/foo/bar", "--mirror",
"https://globalgithub.com/foo/bar",
],
packagePath: packageRoot,
configuration: config,
buildSystem: buildSystem,
)
#expect(fs.isFile(sharedConfigFile))

// Test env override.
try await execute(
Expand All @@ -3805,6 +3820,13 @@ struct PackageCommandTests {
buildSystem: buildSystem,
)
#expect(stdout.spm_chomp() == "https://mygithub.com/foo/bar")
(stdout, _) = try await execute(
["config", "get-mirror", "--global", "--original", "https://github.com/foo/bar"],
packagePath: packageRoot,
configuration: config,
buildSystem: buildSystem,
)
#expect(stdout.spm_chomp() == "https://globalgithub.com/foo/bar")
(stdout, _) = try await execute(
[
"config", "get-mirror", "--original",
Expand All @@ -3830,6 +3852,14 @@ struct PackageCommandTests {
buildSystem: buildSystem,
)
}
await check(stderr: "not found\n") {
try await execute(
["config", "get-mirror", "--global", "--original", "git@github.com:swiftlang/swift-package-manager.git"],
packagePath: packageRoot,
configuration: config,
buildSystem: buildSystem,
)
}

// Test deletion.
try await execute(
Expand All @@ -3848,14 +3878,15 @@ struct PackageCommandTests {
buildSystem: buildSystem,
)

await check(stderr: "not found\n") {
try await execute(
["config", "get-mirror", "--original", "https://github.com/foo/bar"],
packagePath: packageRoot,
configuration: config,
buildSystem: buildSystem,
)
}
// Still found via global
(stdout, _) = try await execute(
["config", "get-mirror", "--original", "https://github.com/foo/bar"],
packagePath: packageRoot,
configuration: config,
buildSystem: buildSystem,
)
#expect(stdout.spm_chomp() == "https://globalgithub.com/foo/bar")

await check(stderr: "not found\n") {
try await execute(
[
Expand Down
49 changes: 49 additions & 0 deletions Tests/WorkspaceTests/MirrorsConfigurationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,19 @@ fileprivate struct MirrorsConfigurationTests {
#expect(try config.get().isEmpty)
}

@Test
func throwsWhenBothLocalAndSharedAreNil() throws {
let fs = InMemoryFileSystem()

#expect(throws: StringError("No mirrors configuration provided")) {
try Workspace.Configuration.Mirrors(
fileSystem: fs,
localMirrorsFile: nil,
sharedMirrorsFile: nil
)
}
}

@Test
func localAndShared() throws {
let fs = InMemoryFileSystem()
Expand Down Expand Up @@ -152,4 +165,40 @@ fileprivate struct MirrorsConfigurationTests {
#expect(config.mirrors.mirror(for: original1URL) == nil)
#expect(config.mirrors.original(for: mirror1URL) == nil)
}

@Test
func onlyShared() throws {
let fs = InMemoryFileSystem()
let sharedConfigFile = AbsolutePath("/config/shared-mirrors.json")

let config = try Workspace.Configuration.Mirrors(
fileSystem: fs,
localMirrorsFile: nil,
sharedMirrorsFile: sharedConfigFile
)

// can write to shared location

let original1URL = "https://github.com/apple/swift-argument-parser.git"
let mirror1URL = "https://github.com/mona/swift-argument-parser.git"

try config.applyShared { mirrors in
try mirrors.set(mirror: mirror1URL, for: original1URL)
}

// cannot write to local location

let original2URL = "https://github.com/apple/swift-nio.git"
let mirror2URL = "https://github.com/mona/swift-nio.git"

#expect(throws: (any Error).self) {
try config.applyLocal { mirrors in
try mirrors.set(mirror: mirror2URL, for: original2URL)
}
}

#expect(config.mirrors.count == 1)
#expect(config.mirrors.mirror(for: original1URL) == mirror1URL)
#expect(config.mirrors.original(for: mirror1URL) == original1URL)
}
}
Loading