From b2ca7503095e0ca46516317b3f44233e2b8aa37a Mon Sep 17 00:00:00 2001 From: John Bute Date: Thu, 23 Oct 2025 15:47:42 -0400 Subject: [PATCH 1/3] first attempt at fixing add-dependency to multiple manifests --- .../PackageCommands/AddDependency.swift | 61 ++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/Sources/Commands/PackageCommands/AddDependency.swift b/Sources/Commands/PackageCommands/AddDependency.swift index 03c49fc9661..9461be55e73 100644 --- a/Sources/Commands/PackageCommands/AddDependency.swift +++ b/Sources/Commands/PackageCommands/AddDependency.swift @@ -231,6 +231,33 @@ extension SwiftPackageCommand { ) } + private func findAllManifests(packagePath: Basics.AbsolutePath, fileSystem: Basics.FileSystem) -> [Basics.AbsolutePath] { + var manifests: [Basics.AbsolutePath] = [] + + // Add standard manifest if it exists + let standardManifest = packagePath.appending(component: Manifest.filename) + let manifestContents: ByteString + if fileSystem.isFile(standardManifest) { + manifests.append(standardManifest) + } + + // Find version specific manifests + do { + let packageContents = try fileSystem.getDirectoryContents(packagePath) + let regexManifestFile = try! RegEx(pattern: #"^Package@swift-(\d+)(?:\.(\d+))?(?:\.(\d+))?.swift$"#) + + for file in packageContents { + if regexManifestFile.matchGroups(in: file).first != nil { + manifests.append(packagePath.appending(component: file)) + } + } + } catch { + // If we cannot read directory, just use standard manifest + } + return manifests + } + + private func applyEdits( packagePath: Basics.AbsolutePath, workspace: Workspace, @@ -238,7 +265,39 @@ extension SwiftPackageCommand { ) throws { // Load the manifest file let fileSystem = workspace.fileSystem - let manifestPath = packagePath.appending(component: Manifest.filename) + let packageManifests = findAllManifests(packagePath: packagePath, fileSystem: workspace.fileSystem) + + guard !packageManifests.isEmpty else { + throw StringError("cannot find package manifest in \(packagePath)") + } + + var successCount = 0 + var errors: [String] = [] + + for manifest in packageManifests { + do { + try applyEditsToSingleManifest(manifestPath: manifest, fileSystem: fileSystem, packageDependency: packageDependency) + successCount += 1 + } catch { + let errorMessage = "Failed to update \(manifest.basename)" + errors.append(errorMessage) + } + } + + if successCount == 0 { + throw StringError("Failed to update any manifest files:\n" + errors.joined(separator: "\n")) + } else if !errors.isEmpty { + print("Successfully updated \(successCount)/\(packageManifests.count) manifest files") + print("Warnings/Errors occured:\n" + errors.joined(separator: "\n")) + } + } + + private func applyEditsToSingleManifest( + manifestPath: Basics.AbsolutePath, + fileSystem: FileSystem, + packageDependency: PackageDependency + ) throws { + // Load the manifest file let manifestContents: ByteString do { manifestContents = try fileSystem.readFileContents(manifestPath) From bb75b13b49719572784b544e6857417f6c7cfa1e Mon Sep 17 00:00:00 2001 From: John Bute Date: Thu, 13 Nov 2025 14:02:20 -0500 Subject: [PATCH 2/3] added manifest filtering + tests --- .../PackageCommands/AddDependency.swift | 22 +- Tests/CommandsTests/PackageCommandTests.swift | 199 ++++++++++++++++-- 2 files changed, 207 insertions(+), 14 deletions(-) diff --git a/Sources/Commands/PackageCommands/AddDependency.swift b/Sources/Commands/PackageCommands/AddDependency.swift index 9461be55e73..dc726280372 100644 --- a/Sources/Commands/PackageCommands/AddDependency.swift +++ b/Sources/Commands/PackageCommands/AddDependency.swift @@ -57,6 +57,9 @@ extension SwiftPackageCommand { @Option(help: "Specify dependency type.") var type: DependencyType = .url + @Option(name: .customLong("package-manifests"), help: "Filter manifests by name pattern") + var packageManifests: [String] = [] + enum DependencyType: String, Codable, CaseIterable, ExpressibleByArgument { case url case path @@ -236,7 +239,6 @@ extension SwiftPackageCommand { // Add standard manifest if it exists let standardManifest = packagePath.appending(component: Manifest.filename) - let manifestContents: ByteString if fileSystem.isFile(standardManifest) { manifests.append(standardManifest) } @@ -254,10 +256,21 @@ extension SwiftPackageCommand { } catch { // If we cannot read directory, just use standard manifest } + + // Filter manifests by name patterns if specified + if !packageManifests.isEmpty { + manifests = manifests.filter { manifestPath in + let fileName = manifestPath.basename + return packageManifests.contains { pattern in + fileName.contains(pattern) || + NSPredicate(format: "SELF LIKE %@", pattern).evaluate(with: fileName) + } + } + } + return manifests } - private func applyEdits( packagePath: Basics.AbsolutePath, workspace: Workspace, @@ -279,6 +292,11 @@ extension SwiftPackageCommand { try applyEditsToSingleManifest(manifestPath: manifest, fileSystem: fileSystem, packageDependency: packageDependency) successCount += 1 } catch { + // For single manifest scenarios, rethrow error + if packageManifests.count == 1 { + throw error + } + // For multiple manifests, collect error message let errorMessage = "Failed to update \(manifest.basename)" errors.append(errorMessage) } diff --git a/Tests/CommandsTests/PackageCommandTests.swift b/Tests/CommandsTests/PackageCommandTests.swift index e7b1936c4b5..5eb45bf868a 100644 --- a/Tests/CommandsTests/PackageCommandTests.swift +++ b/Tests/CommandsTests/PackageCommandTests.swift @@ -34,14 +34,34 @@ import enum TSCBasic.JSON fileprivate func execute( _ args: [String] = [], packagePath: AbsolutePath? = nil, - manifest: String? = nil, + manifests: [String]? = nil, env: Environment? = nil, configuration: BuildConfiguration, buildSystem: BuildSystemProvider.Kind ) async throws -> (stdout: String, stderr: String) { var environment = env ?? [:] - if let manifest, let packagePath { - try localFileSystem.writeFileContents(packagePath.appending("Package.swift"), string: manifest) + if let manifests, let packagePath { + if manifests.count > 1 { + for (index, manifest) in manifests.enumerated() { + + guard index != manifests.count - 1 else { + let marker = "// swift-tools-version: " + guard let markerRange = manifest.range(of: marker) else { + continue + } + let startIndex = markerRange.upperBound + let endIndex = manifest.index(startIndex, offsetBy: 3, limitedBy: manifest.endIndex) ?? manifest.endIndex + + let toolsVersion = manifest[startIndex.. Void) throws { + let fs = localFileSystem + var manifestContents: [String] = [] + + let mainManifest = packagePath.appending("Package.swift") + if fs.exists(mainManifest) { + let contents: String = try fs.readFileContents(mainManifest) + manifestContents.append(contents) } + + let packageFiles = try fs.getDirectoryContents(packagePath) + let manifestFiles = packageFiles.filter { $0.hasPrefix("Package@swift-") && $0.hasSuffix(".swift") } + + for manifestFile in manifestFiles.sorted() { + let manifestPath = packagePath.appending(manifestFile) + let contents: String = try fs.readFileContents(manifestPath) + manifestContents.append(contents) + } + + try callback(manifestContents) } // Helper function to test adding a URL dependency and asserting the result private func executeAddURLDependencyAndAssert( packagePath: AbsolutePath, - initialManifest: String? = nil, + initialManifests: [String]? = nil, url: String, requirementArgs: [String], expectedManifestString: String, @@ -82,7 +125,7 @@ private func executeAddURLDependencyAndAssert( _ = try await execute( ["add-dependency", url] + requirementArgs, packagePath: packagePath, - manifest: initialManifest, + manifests: initialManifests, configuration: buildData.config, buildSystem: buildData.buildSystem, ) @@ -2005,6 +2048,138 @@ struct PackageCommandTests { } } + @Test( + arguments: getBuildData(for: SupportedBuildSystemOnAllPlatforms), + ) + func packageAddDependencyToMultipleManifests( + data: BuildData, + ) async throws { + try await testWithTemporaryDirectory { tmpPath in + let fs = localFileSystem + let path = tmpPath.appending("PackageA") + try fs.createDirectory(path) + + let url = "https://github.com/swiftlang/swift-syntax.git" + + let manifestA = """ + // swift-tools-version: 5.9 + import PackageDescription + let package = Package( + name: "client", + dependencies: [ + .package(url: "\(url)", exact: "601.0.1"), + ], + targets: [ .target(name: "client", dependencies: [ "library" ]) ] + ) + """ + let manifestB = """ + // swift-tools-version: 6.1 + import PackageDescription + let package = Package( + name: "client", + dependencies: [ + .package(url: "\(url)", exact: "601.0.1"), + ], + targets: [ .target(name: "client", dependencies: [ "library" ]) ] + ) + """ + + let expected = + #".package(url: "https://github.com/swiftlang/swift-syntax.git", exact: "601.0.1"),"# + + let manifests = [manifestA, manifestB] + try await executeAddURLDependencyAndAssert( + packagePath: path, + initialManifests: manifests, + url: url, + requirementArgs: ["--exact", "601.0.1"], + expectedManifestString: expected, + buildData: data, + ) + + try expectAllManifests(path) { manifestContents in + #expect(manifestContents.count == 2) + for manifest in manifestContents { + let components = manifest.components(separatedBy: expected) + #expect(components.count == 2, "Expected dependency to appear exactly once in each manifest, but found \(components.count - 1) occurrences") + } + } + + } + } + + @Test( + arguments: getBuildData(for: SupportedBuildSystemOnAllPlatforms), + ) + func packageAddDependencyToMultipleManifestsWithFilter( + data: BuildData, + ) async throws { + try await testWithTemporaryDirectory { tmpPath in + let fs = localFileSystem + let path = tmpPath.appending("PackageA") + try fs.createDirectory(path) + + let url = "https://github.com/swiftlang/swift-syntax.git" + + let manifestA = """ + // swift-tools-version: 5.9 + import PackageDescription + let package = Package( + name: "client", + dependencies: [ + .package(url: "\(url)", exact: "601.0.1"), + ], + targets: [ .target(name: "client", dependencies: [ "library" ]) ] + ) + """ + let manifestB = """ + // swift-tools-version: 6.1 + import PackageDescription + let package = Package( + name: "client", + dependencies: [ + .package(url: "\(url)", exact: "601.0.1"), + ], + targets: [ .target(name: "client", dependencies: [ "library" ]) ] + ) + """ + + let expected = + #".package(url: "https://github.com/swiftlang/swift-syntax.git", exact: "601.0.1"),"# + + let manifests = [manifestA, manifestB] + try await executeAddURLDependencyAndAssert( + packagePath: path, + initialManifests: manifests, + url: url, + requirementArgs: ["--exact", "601.0.1", "--package-manifests", "Package@swift-6.1.swift"], + expectedManifestString: expected, + buildData: data, + ) + + try expectAllManifests(path) { manifestContents in + #expect(manifestContents.count == 2) + + var mainManifestCount = 0 + var filteredManifestCount = 0 + + for manifest in manifestContents { + let components = manifest.components(separatedBy: expected) + if manifest.contains("// swift-tools-version: 5.9") { + mainManifestCount = components.count - 1 + } else if manifest.contains("// swift-tools-version: 6.1") { + filteredManifestCount = components.count - 1 + } + } + + #expect(mainManifestCount == 1, "Main manifest (5.9) should have exactly 1 dependency occurrence") + #expect(filteredManifestCount == 1, "Filtered manifest (6.1) should have exactly 1 dependency occurrence") + } + + } + } + + @Test( arguments: getBuildData(for: SupportedBuildSystemOnAllPlatforms), ) @@ -2033,7 +2208,7 @@ struct PackageCommandTests { try await executeAddURLDependencyAndAssert( packagePath: path, - initialManifest: manifest, + initialManifests: [manifest], url: url, requirementArgs: ["--exact", "601.0.1"], expectedManifestString: expected, @@ -2074,7 +2249,7 @@ struct PackageCommandTests { let expected = #".package(path: "../foo")"# try await executeAddURLDependencyAndAssert( packagePath: path, - initialManifest: manifest, + initialManifests: [manifest], url: depPath, requirementArgs: ["--type", "path"], expectedManifestString: expected, @@ -2115,7 +2290,7 @@ struct PackageCommandTests { let expected = #".package(id: "foo", exact: "1.0.0")"# try await executeAddURLDependencyAndAssert( packagePath: path, - initialManifest: manifest, + initialManifests: [manifest], url: registryId, requirementArgs: ["--type", "registry", "--exact", "1.0.0"], expectedManifestString: expected, @@ -2215,7 +2390,7 @@ struct PackageCommandTests { try await executeAddURLDependencyAndAssert( packagePath: path, - initialManifest: manifest, + initialManifests: [manifest], url: testData.url, requirementArgs: testData.requirementArgs, expectedManifestString: testData.expectedManifestString, @@ -2260,7 +2435,7 @@ struct PackageCommandTests { try await executeAddURLDependencyAndAssert( packagePath: path, - initialManifest: manifest, + initialManifests: [manifest], url: testData.url, requirementArgs: testData.requirementArgs, expectedManifestString: testData.expectedManifestString, @@ -2325,7 +2500,7 @@ struct PackageCommandTests { """ try await executeAddURLDependencyAndAssert( packagePath: path, - initialManifest: manifest, + initialManifests: [manifest], url: testData.url, requirementArgs: testData.requirementArgs, expectedManifestString: testData.expectedManifestString, From 7e46218f36b4c000ece5c6b6530516e520c9457e Mon Sep 17 00:00:00 2001 From: John Bute Date: Thu, 13 Nov 2025 14:21:39 -0500 Subject: [PATCH 3/3] refactored option for clarity + removed erroneous wildcard support --- Sources/Commands/PackageCommands/AddDependency.swift | 11 +++++------ Tests/CommandsTests/PackageCommandTests.swift | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Sources/Commands/PackageCommands/AddDependency.swift b/Sources/Commands/PackageCommands/AddDependency.swift index dc726280372..b1b7126bf99 100644 --- a/Sources/Commands/PackageCommands/AddDependency.swift +++ b/Sources/Commands/PackageCommands/AddDependency.swift @@ -57,8 +57,8 @@ extension SwiftPackageCommand { @Option(help: "Specify dependency type.") var type: DependencyType = .url - @Option(name: .customLong("package-manifests"), help: "Filter manifests by name pattern") - var packageManifests: [String] = [] + @Option(name: .customLong("filter-manifests"), help: "Filter manifests by name pattern") + var manifestFilter: [String] = [] enum DependencyType: String, Codable, CaseIterable, ExpressibleByArgument { case url @@ -258,12 +258,11 @@ extension SwiftPackageCommand { } // Filter manifests by name patterns if specified - if !packageManifests.isEmpty { + if !manifestFilter.isEmpty { manifests = manifests.filter { manifestPath in let fileName = manifestPath.basename - return packageManifests.contains { pattern in - fileName.contains(pattern) || - NSPredicate(format: "SELF LIKE %@", pattern).evaluate(with: fileName) + return manifestFilter.contains { pattern in + fileName == pattern } } } diff --git a/Tests/CommandsTests/PackageCommandTests.swift b/Tests/CommandsTests/PackageCommandTests.swift index 5eb45bf868a..aade84a176f 100644 --- a/Tests/CommandsTests/PackageCommandTests.swift +++ b/Tests/CommandsTests/PackageCommandTests.swift @@ -2152,7 +2152,7 @@ struct PackageCommandTests { packagePath: path, initialManifests: manifests, url: url, - requirementArgs: ["--exact", "601.0.1", "--package-manifests", "Package@swift-6.1.swift"], + requirementArgs: ["--exact", "601.0.1", "--filter-manifests", "Package@swift-6.1.swift"], expectedManifestString: expected, buildData: data, )