Skip to content

Commit 3fedbbd

Browse files
aciidghaciidgh
committed
[PackageLoading] Filter versions containing older tools version
We were only doing this for root packages, we also need to do this check during dependency resolution. <rdar://problem/44395499> Alamofire as a dependency shows manifest parse error instead of tools version message
1 parent a56c33d commit 3fedbbd

File tree

13 files changed

+88
-22
lines changed

13 files changed

+88
-22
lines changed

‎Sources/Commands/SwiftPackageTool.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,14 +210,14 @@ public class SwiftPackageTool: SwiftTool<PackageToolOptions> {
210210
// FIXME: Probably lift this error defination to ToolsVersion.
211211
throw ToolsVersionLoader.Error.malformed(specifier: value, file: pkg)
212212
}
213-
try writeToolsVersion(at: pkg, version: toolsVersion, fs: &localFileSystem)
213+
try writeToolsVersion(at: pkg, version: toolsVersion, fs: localFileSystem)
214214

215215
case .setCurrent:
216216
// Write the tools version with current version but with patch set to zero.
217217
// We do this to avoid adding unnecessary constraints to patch versions, if
218218
// the package really needs it, they can do it using --set option.
219219
try writeToolsVersion(
220-
at: pkg, version: ToolsVersion.currentToolsVersion.zeroedPatch, fs: &localFileSystem)
220+
at: pkg, version: ToolsVersion.currentToolsVersion.zeroedPatch, fs: localFileSystem)
221221
}
222222

223223
case .generateXcodeproj:

‎Sources/PackageGraph/RepositoryPackageContainerProvider.swift

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,15 @@ public class RepositoryPackageContainer: BasePackageContainer, CustomStringConve
268268

269269
// Otherwise, compute and cache the result.
270270
let isValid = (try? self.toolsVersion(for: $0)).flatMap({
271-
self.currentToolsVersion >= $0
271+
guard $0 >= ToolsVersion.minimumRequired else {
272+
return false
273+
}
274+
275+
guard self.currentToolsVersion >= $0 else {
276+
return false
277+
}
278+
279+
return true
272280
}) ?? false
273281
self.validToolsVersionsCache[$0] = isValid
274282

‎Sources/PackageLoading/PackageBuilder.swift

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,6 @@ public enum ModuleError: Swift.Error {
4545
/// We found multiple LinuxMain.swift files.
4646
case multipleLinuxMainFound(package: String, linuxMainFiles: [AbsolutePath])
4747

48-
/// The package should support version 3 compiler but doesn't.
49-
case mustSupportSwift3Compiler(package: String)
50-
5148
/// The tools version in use is not compatible with target's sources.
5249
case incompatibleToolsVersions(package: String, required: [SwiftLanguageVersion], current: ToolsVersion)
5350

@@ -82,8 +79,6 @@ extension ModuleError: CustomStringConvertible {
8279
case .multipleLinuxMainFound(let package, let linuxMainFiles):
8380
let files = linuxMainFiles.map({ $0.asString }).sorted().joined(separator: ", ")
8481
return "package '\(package)' has multiple linux main files: \(files)"
85-
case .mustSupportSwift3Compiler(let package):
86-
return "package '\(package)' must support Swift 3 because its minimum tools version is 3"
8782
case .incompatibleToolsVersions(let package, let required, let current):
8883
if required.isEmpty {
8984
return "package '\(package)' supported Swift language versions is empty"

‎Sources/PackageModel/ToolsVersion.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ public struct ToolsVersion: CustomStringConvertible, Comparable {
2525
"\(Versioning.currentVersion.minor)." +
2626
"\(Versioning.currentVersion.patch)")!
2727

28+
/// The minimum tools version that is required by the package manager.
29+
public static let minimumRequired: ToolsVersion = .v4
30+
2831
/// Regex pattern to parse tools version. The format is SemVer 2.0 with an
2932
/// addition that specifying the patch version is optional.
3033
static let toolsVersionRegex = try! NSRegularExpression(pattern: "^" +

‎Sources/TestSupport/TestWorkspace.swift

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ public final class TestWorkspace {
7878
let sourcesDir = packagePath.appending(component: "Sources")
7979
let url = (isRoot ? packagePath : packagesDir.appending(RelativePath(package.path ?? package.name))).asString
8080
let specifier = RepositorySpecifier(url: url)
81+
let manifestPath = packagePath.appending(component: Manifest.filename)
8182

8283
// Create targets on disk.
8384
let repo = repoProvider.specifierMap[specifier] ?? InMemoryGitRepository(path: packagePath, fs: fs as! InMemoryFileSystem)
@@ -86,14 +87,18 @@ public final class TestWorkspace {
8687
try repo.createDirectory(targetDir, recursive: true)
8788
try repo.writeFileContents(targetDir.appending(component: "file.swift"), bytes: "")
8889
}
90+
if let toolsVersion = package.toolsVersion {
91+
try repo.writeFileContents(manifestPath, bytes: "")
92+
try writeToolsVersion(at: packagePath, version: toolsVersion, fs: repo)
93+
}
8994
repo.commit()
9095

9196
let versions: [String?] = isRoot ? [nil] : package.versions
9297
for version in versions {
9398
let v = version.flatMap(Version.init(string:))
9499
manifests[.init(url: url, version: v)] = Manifest(
95100
name: package.name,
96-
path: packagePath.appending(component: Manifest.filename),
101+
path: manifestPath,
97102
url: url,
98103
version: v,
99104
manifestVersion: .v4,
@@ -466,21 +471,25 @@ public struct TestPackage {
466471
public let products: [TestProduct]
467472
public let dependencies: [TestDependency]
468473
public let versions: [String?]
474+
// FIXME: This should be per-version.
475+
public let toolsVersion: ToolsVersion?
469476

470477
public init(
471478
name: String,
472479
path: String? = nil,
473480
targets: [TestTarget],
474481
products: [TestProduct],
475482
dependencies: [TestDependency] = [],
476-
versions: [String?] = []
483+
versions: [String?] = [],
484+
toolsVersion: ToolsVersion? = nil
477485
) {
478486
self.name = name
479487
self.path = path
480488
self.targets = targets
481489
self.products = products
482490
self.dependencies = dependencies
483491
self.versions = versions
492+
self.toolsVersion = toolsVersion
484493
}
485494

486495
public static func genericPackage1(named name: String) -> TestPackage {

‎Sources/Workspace/InitPackage.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ public final class InitPackage {
137137

138138
// Write the current tools version.
139139
try writeToolsVersion(
140-
at: manifest.parentDirectory, version: version, fs: &localFileSystem)
140+
at: manifest.parentDirectory, version: version, fs: localFileSystem)
141141
}
142142

143143
private func writeREADMEFile() throws {

‎Sources/Workspace/ToolsVersionWriter.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import Utility
1818
/// - Parameters:
1919
/// - path: The path of the package.
2020
/// - version: The version to write.
21-
public func writeToolsVersion(at path: AbsolutePath, version: ToolsVersion, fs: inout FileSystem) throws {
21+
public func writeToolsVersion(at path: AbsolutePath, version: ToolsVersion, fs: FileSystem) throws {
2222
let file = path.appending(component: Manifest.filename)
2323
assert(fs.isFile(file), "Tools version file not present")
2424

‎Sources/Workspace/Workspace.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1033,7 +1033,7 @@ extension Workspace {
10331033
at: packagePath, fileSystem: fileSystem)
10341034

10351035
// Make sure the package has the right minimum tools version.
1036-
guard toolsVersion >= .v4 else {
1036+
guard toolsVersion >= ToolsVersion.minimumRequired else {
10371037
throw WorkspaceDiagnostics.IncompatibleToolsVersion(
10381038
rootPackagePath: packagePath,
10391039
requiredToolsVersion: .v4,

‎Tests/FunctionalTests/VersionSpecificTests.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ class VersionSpecificTests: XCTestCase {
5050

5151
// Create the version to test against.
5252
try fs.writeFileContents(depPath.appending(component: "Package.swift")) {
53+
// FIXME: We end up filtering this manifest if it has an invalid
54+
// tools version as they're assumed to be v3 manifests. Should we
55+
// do something better?
56+
$0 <<< "// swift-tools-version:4.2\n"
5357
$0 <<< "NOT_A_VALID_PACKAGE"
5458
}
5559
try fs.writeFileContents(depPath.appending(component: "foo.swift")) {

‎Tests/PackageGraphTests/RepositoryPackageContainerProviderTests.swift

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -280,14 +280,18 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
280280
repo.commit()
281281
try repo.tag(name: "1.0.0")
282282

283-
try repo.writeFileContents(filePath, bytes: "// swift-tools-version:3.1.0;hello\n")
283+
try repo.writeFileContents(filePath, bytes: "// swift-tools-version:4.0")
284284
repo.commit()
285285
try repo.tag(name: "1.0.1")
286286

287-
try repo.writeFileContents(filePath, bytes: "// swift-tools-version:4.0.0\n")
287+
try repo.writeFileContents(filePath, bytes: "// swift-tools-version:4.2.0;hello\n")
288288
repo.commit()
289289
try repo.tag(name: "1.0.2")
290290

291+
try repo.writeFileContents(filePath, bytes: "// swift-tools-version:4.2.0\n")
292+
repo.commit()
293+
try repo.tag(name: "1.0.3")
294+
291295
let inMemRepoProvider = InMemoryGitRepositoryProvider()
292296
inMemRepoProvider.add(specifier: specifier, repository: repo)
293297

@@ -307,21 +311,21 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
307311
}
308312

309313
do {
310-
let provider = createProvider(ToolsVersion(version: "3.1.0"))
314+
let provider = createProvider(ToolsVersion(version: "4.0.0"))
311315
let ref = PackageReference(identity: "foo", path: specifier.url)
312316
let container = try await { provider.getContainer(for: ref, completion: $0) }
313317
let v = container.versions(filter: { _ in true }).map{$0}
314-
XCTAssertEqual(v, ["1.0.1", "1.0.0"])
318+
XCTAssertEqual(v, ["1.0.1"])
315319
}
316320

317321
do {
318-
let provider = createProvider(ToolsVersion(version: "4.0.0"))
322+
let provider = createProvider(ToolsVersion(version: "4.2.0"))
319323
let ref = PackageReference(identity: "foo", path: specifier.url)
320324
let container = try await { provider.getContainer(for: ref, completion: $0) }
321325
XCTAssertEqual((container as! RepositoryPackageContainer).validToolsVersionsCache, [:])
322326
let v = container.versions(filter: { _ in true }).map{$0}
323-
XCTAssertEqual((container as! RepositoryPackageContainer).validToolsVersionsCache, ["1.0.1": true, "1.0.0": true, "1.0.2": true])
324-
XCTAssertEqual(v, ["1.0.2", "1.0.1", "1.0.0"])
327+
XCTAssertEqual((container as! RepositoryPackageContainer).validToolsVersionsCache, ["1.0.1": true, "1.0.0": false, "1.0.3": true, "1.0.2": true])
328+
XCTAssertEqual(v, ["1.0.3", "1.0.2", "1.0.1"])
325329
}
326330

327331
do {

‎Tests/WorkspaceTests/ToolsVersionWriterTests.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,15 +106,15 @@ class ToolsVersionWriterTests: XCTestCase {
106106
_ result: (ByteString) -> Void
107107
) {
108108
do {
109-
var fs: FileSystem = InMemoryFileSystem()
109+
let fs: FileSystem = InMemoryFileSystem()
110110

111111
let file = AbsolutePath("/pkg/Package.swift")
112112

113113
try fs.createDirectory(file.parentDirectory, recursive: true)
114114
try fs.writeFileContents(file, bytes: stream.bytes)
115115

116116
try writeToolsVersion(
117-
at: file.parentDirectory, version: version, fs: &fs)
117+
at: file.parentDirectory, version: version, fs: fs)
118118

119119
result(try fs.readFileContents(file))
120120
} catch {

‎Tests/WorkspaceTests/WorkspaceTests.swift

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,6 +1237,48 @@ final class WorkspaceTests: XCTestCase {
12371237
}
12381238
}
12391239

1240+
func testMinimumRequiredToolsVersionInDependencyResolution() throws {
1241+
let sandbox = AbsolutePath("/tmp/ws/")
1242+
let fs = InMemoryFileSystem()
1243+
1244+
let workspace = try TestWorkspace(
1245+
sandbox: sandbox,
1246+
fs: fs,
1247+
roots: [
1248+
TestPackage(
1249+
name: "Root",
1250+
targets: [
1251+
TestTarget(name: "Root", dependencies: ["Foo"]),
1252+
],
1253+
products: [],
1254+
dependencies: [
1255+
TestDependency(name: "Foo", requirement: .upToNextMajor(from: "1.0.0")),
1256+
]
1257+
),
1258+
],
1259+
packages: [
1260+
TestPackage(
1261+
name: "Foo",
1262+
targets: [
1263+
TestTarget(name: "Foo"),
1264+
],
1265+
products: [
1266+
TestProduct(name: "Foo", targets: ["Foo"]),
1267+
],
1268+
versions: ["1.0.0"],
1269+
toolsVersion: .v3
1270+
),
1271+
]
1272+
)
1273+
1274+
workspace.checkPackageGraph(roots: ["Root"]) { (graph, diagnostics) in
1275+
DiagnosticsEngineTester(diagnostics) { result in
1276+
result.check(diagnostic: .contains("/tmp/ws/pkgs/Foo @ 1.0.0..<2.0.0"), behavior: .error)
1277+
result.check(diagnostic: .contains("product dependency 'Foo' not found"), behavior: .error, location: "'Root' /tmp/ws/roots/Root")
1278+
}
1279+
}
1280+
}
1281+
12401282
func testToolsVersionRootPackages() throws {
12411283
let sandbox = AbsolutePath("/tmp/ws/")
12421284
let fs = InMemoryFileSystem()

‎Tests/WorkspaceTests/XCTestManifests.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ extension WorkspaceTests {
6464
("testLocalDependencyWithPackageUpdate", testLocalDependencyWithPackageUpdate),
6565
("testLocalLocalSwitch", testLocalLocalSwitch),
6666
("testLocalVersionSwitch", testLocalVersionSwitch),
67+
("testMinimumRequiredToolsVersionInDependencyResolution", testMinimumRequiredToolsVersionInDependencyResolution),
6768
("testMissingEditCanRestoreOriginalCheckout", testMissingEditCanRestoreOriginalCheckout),
6869
("testMultipleRootPackages", testMultipleRootPackages),
6970
("testPackageMirror", testPackageMirror),

0 commit comments

Comments
 (0)