diff --git a/Sources/PackageLoading/ManifestJSONParser.swift b/Sources/PackageLoading/ManifestJSONParser.swift index 76497dd2d5c..84f1d26660a 100644 --- a/Sources/PackageLoading/ManifestJSONParser.swift +++ b/Sources/PackageLoading/ManifestJSONParser.swift @@ -165,7 +165,7 @@ enum ManifestJSONParser { do { path = try AbsolutePath(validating: location) } catch PathValidationError.invalidAbsolutePath(let path) { - throw ManifestParseError.invalidManifestFormat("'\(path)' is not a valid path for path-based dependencies; use relative or absolute path instead.", diagnosticFile: nil) + throw ManifestParseError.invalidManifestFormat("'\(path)' is not a valid path for path-based dependencies; use relative or absolute path instead.", diagnosticFile: nil, compilerCommandLine: nil) } let identity = try identityResolver.resolveIdentity(for: path) return .fileSystem(identity: identity, @@ -224,11 +224,11 @@ enum ManifestJSONParser { guard hostnameComponent.isEmpty else { if hostnameComponent == ".." { throw ManifestParseError.invalidManifestFormat( - "file:// URLs cannot be relative, did you mean to use '.package(path:)'?", diagnosticFile: nil + "file:// URLs cannot be relative, did you mean to use '.package(path:)'?", diagnosticFile: nil, compilerCommandLine: nil ) } throw ManifestParseError.invalidManifestFormat( - "file:// URLs with hostnames are not supported, are you missing a '/'?", diagnosticFile: nil + "file:// URLs with hostnames are not supported, are you missing a '/'?", diagnosticFile: nil, compilerCommandLine: nil ) } return try AbsolutePath(validating: location).pathString diff --git a/Sources/PackageLoading/ManifestLoader.swift b/Sources/PackageLoading/ManifestLoader.swift index 1120c9878d8..479e58107d1 100644 --- a/Sources/PackageLoading/ManifestLoader.swift +++ b/Sources/PackageLoading/ManifestLoader.swift @@ -23,6 +23,8 @@ public enum ManifestParseError: Swift.Error, Equatable { /// The manifest contains invalid format. case invalidManifestFormat(String, diagnosticFile: AbsolutePath?) // TODO: Test this error. + case invalidManifestFormat(String, diagnosticFile: AbsolutePath?, compilerCommandLine: [String]?) + /// The manifest was successfully loaded by swift interpreter but there were runtime issues. case runtimeManifestErrors([String]) @@ -36,8 +38,14 @@ extension ManifestParseError: CustomStringConvertible { switch self { case .emptyManifest(let manifestPath): return "'\(manifestPath)' is empty" - case .invalidManifestFormat(let error, _): - return "invalid manifest\n\(error)" + case .invalidManifestFormat(let error, _, let compilerCommandLine): + let suffix: String + if let compilerCommandLine = compilerCommandLine { + suffix = " (compiled with: \(compilerCommandLine))" + } else { + suffix = "" + } + return "Invalid manifest\(suffix)\n\(error)" case .runtimeManifestErrors(let errors): return "invalid manifest (evaluation failed)\n\(errors.joined(separator: "\n"))" case .importsRestrictedModules(let modules): @@ -292,7 +300,7 @@ public final class ManifestLoader: ManifestLoaderProtocol { // Throw now if we weren't able to parse the manifest. guard let manifestJSON = result.manifestJSON, !manifestJSON.isEmpty else { let errors = result.errorOutput ?? result.compilerOutput ?? "Missing or empty JSON output from manifest compilation for \(packageIdentity)" - throw ManifestParseError.invalidManifestFormat(errors, diagnosticFile: result.diagnosticFile) + throw ManifestParseError.invalidManifestFormat(errors, diagnosticFile: result.diagnosticFile, compilerCommandLine: result.compilerCommandLine) } // We should not have any fatal error at this point. @@ -660,6 +668,8 @@ public final class ManifestLoader: ManifestLoaderProtocol { let compiledManifestFile = tmpDir.appending(component: "\(packageIdentity)-manifest\(executableSuffix)") cmd += ["-o", compiledManifestFile.pathString] + evaluationResult.compilerCommandLine = cmd + // Compile the manifest. TSCBasic.Process.popen(arguments: cmd, environment: self.toolchain.swiftCompilerEnvironment, queue: callbackQueue) { result in dispatchPrecondition(condition: .onQueue(callbackQueue)) @@ -898,6 +908,9 @@ extension ManifestLoader { /// The manifest in JSON format. var manifestJSON: String? + /// The command line used to compile the manifest + var compilerCommandLine: [String]? + /// Any non-compiler error that might have occurred during manifest loading. /// /// For e.g., we could have failed to spawn the process or create temporary file. diff --git a/Tests/PackageLoadingTests/PD_4_0_LoadingTests.swift b/Tests/PackageLoadingTests/PD_4_0_LoadingTests.swift index cfe359da90a..ff35e81e8de 100644 --- a/Tests/PackageLoadingTests/PD_4_0_LoadingTests.swift +++ b/Tests/PackageLoadingTests/PD_4_0_LoadingTests.swift @@ -311,7 +311,7 @@ class PackageDescription4_0LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let error, _) = error { + if case ManifestParseError.invalidManifestFormat(let error, _, _) = error { XCTAssert(error.contains("error: 'package(url:version:)' is unavailable: use package(url:exact:) instead"), "\(error)") XCTAssert(error.contains("error: 'package(url:range:)' is unavailable: use package(url:_:) instead"), "\(error)") } else { diff --git a/Tests/PackageLoadingTests/PD_4_2_LoadingTests.swift b/Tests/PackageLoadingTests/PD_4_2_LoadingTests.swift index 7757cb40c0f..605807c232a 100644 --- a/Tests/PackageLoadingTests/PD_4_2_LoadingTests.swift +++ b/Tests/PackageLoadingTests/PD_4_2_LoadingTests.swift @@ -103,7 +103,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let message, _) = error { + if case ManifestParseError.invalidManifestFormat(let message, _, _) = error { XCTAssertMatch( message, .and( @@ -166,7 +166,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let message, _) = error { + if case ManifestParseError.invalidManifestFormat(let message, _, _) = error { XCTAssertMatch(message, .contains("is unavailable")) XCTAssertMatch(message, .contains("was introduced in PackageDescription 5")) } else { @@ -188,7 +188,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let message, _) = error { + if case ManifestParseError.invalidManifestFormat(let message, _, _) = error { XCTAssertMatch(message, .contains("is unavailable")) XCTAssertMatch(message, .contains("was introduced in PackageDescription 5")) } else { @@ -208,7 +208,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let message, _) = error { + if case ManifestParseError.invalidManifestFormat(let message, _, _) = error { XCTAssertMatch(message, .contains("is unavailable")) XCTAssertMatch(message, .contains("was introduced in PackageDescription 5")) } else { @@ -239,7 +239,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let message, _) = error { + if case ManifestParseError.invalidManifestFormat(let message, _, _) = error { XCTAssertMatch(message, .contains("is unavailable")) XCTAssertMatch(message, .contains("was introduced in PackageDescription 5")) } else { @@ -499,7 +499,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let message, let diagnosticFile) = error { + if case ManifestParseError.invalidManifestFormat(let message, let diagnosticFile, _) = error { XCTAssertNil(diagnosticFile) XCTAssertEqual(message, "'https://someurl.com' is not a valid path for path-based dependencies; use relative or absolute path instead.") } else { @@ -519,9 +519,9 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests { case .invalidAbsolutePath: return nil case .relativePath: - return .invalidManifestFormat("file:// URLs cannot be relative, did you mean to use '.package(path:)'?", diagnosticFile: nil) + return .invalidManifestFormat("file:// URLs cannot be relative, did you mean to use '.package(path:)'?", diagnosticFile: nil, compilerCommandLine: nil) case .unsupportedHostname: - return .invalidManifestFormat("file:// URLs with hostnames are not supported, are you missing a '/'?", diagnosticFile: nil) + return .invalidManifestFormat("file:// URLs with hostnames are not supported, are you missing a '/'?", diagnosticFile: nil, compilerCommandLine: nil) } } diff --git a/Tests/PackageLoadingTests/PD_5_0_LoadingTests.swift b/Tests/PackageLoadingTests/PD_5_0_LoadingTests.swift index 6bbe17b566e..2597ebb360f 100644 --- a/Tests/PackageLoadingTests/PD_5_0_LoadingTests.swift +++ b/Tests/PackageLoadingTests/PD_5_0_LoadingTests.swift @@ -112,7 +112,7 @@ class PackageDescription5_0LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let message, _) = error { + if case ManifestParseError.invalidManifestFormat(let message, _, _) = error { XCTAssertMatch(message, .contains("'v3' is unavailable")) XCTAssertMatch(message, .contains("'v3' was obsoleted in PackageDescription 5")) } else { @@ -274,7 +274,7 @@ class PackageDescription5_0LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let message, _) = error { + if case ManifestParseError.invalidManifestFormat(let message, _, _) = error { XCTAssertMatch(message, .contains("error: 'v11' is unavailable")) XCTAssertMatch(message, .contains("note: 'v11' was introduced in PackageDescription 5.3")) XCTAssertMatch(message, .contains("note: 'v14' was introduced in PackageDescription 5.3")) @@ -298,7 +298,7 @@ class PackageDescription5_0LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let message, _) = error { + if case ManifestParseError.invalidManifestFormat(let message, _, _) = error { XCTAssertMatch(message, .contains("error: 'v10_16' has been renamed to 'v11'")) XCTAssertMatch(message, .contains("note: 'v10_16' has been explicitly marked unavailable here")) XCTAssertMatch(message, .contains("note: 'v14' was introduced in PackageDescription 5.3")) @@ -395,7 +395,7 @@ class PackageDescription5_0LoadingTests: PackageDescriptionLoadingTests { fileSystem: fs, observabilityScope: observability.topScope ) - } catch ManifestParseError.invalidManifestFormat(let error, let diagnosticFile) { + } catch ManifestParseError.invalidManifestFormat(let error, let diagnosticFile, _) { XCTAssertMatch(error, .contains("expected expression in container literal")) let contents = try localFileSystem.readFileContents(diagnosticFile!) XCTAssertNotNil(contents) @@ -506,7 +506,7 @@ class PackageDescription5_0LoadingTests: PackageDescriptionLoadingTests { do { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let message, _) = error { + if case ManifestParseError.invalidManifestFormat(let message, _, _) = error { XCTAssertMatch(message, .contains("is unavailable")) XCTAssertMatch(message, .contains("was introduced in PackageDescription 5.2")) } else { @@ -553,7 +553,7 @@ class PackageDescription5_0LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let message, _) = error { + if case ManifestParseError.invalidManifestFormat(let message, _, _) = error { XCTAssertMatch(message, .contains("is unavailable")) XCTAssertMatch(message, .contains("was introduced in PackageDescription 5.2")) } else { diff --git a/Tests/PackageLoadingTests/PD_5_2_LoadingTests.swift b/Tests/PackageLoadingTests/PD_5_2_LoadingTests.swift index c5fcdd405f6..6f19194bf6b 100644 --- a/Tests/PackageLoadingTests/PD_5_2_LoadingTests.swift +++ b/Tests/PackageLoadingTests/PD_5_2_LoadingTests.swift @@ -41,7 +41,7 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let error, _) = error { + if case ManifestParseError.invalidManifestFormat(let error, _, _) = error { XCTAssert(error.contains("error: \'product(name:package:)\' is unavailable: the 'package' argument is mandatory as of tools version 5.2")) } else { XCTFail("unexpected error: \(error)") @@ -277,7 +277,7 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let error, _) = error { + if case ManifestParseError.invalidManifestFormat(let error, _, _) = error { XCTAssertMatch(error, .contains("is unavailable")) XCTAssertMatch(error, .contains("was introduced in PackageDescription 5.3")) } else { @@ -303,7 +303,7 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let error, _) = error { + if case ManifestParseError.invalidManifestFormat(let error, _, _) = error { XCTAssertMatch(error, .contains("is unavailable")) XCTAssertMatch(error, .contains("was introduced in PackageDescription 5.3")) } else { @@ -329,7 +329,7 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let error, _) = error { + if case ManifestParseError.invalidManifestFormat(let error, _, _) = error { XCTAssertMatch(error, .contains("is unavailable")) XCTAssertMatch(error, .contains("was introduced in PackageDescription 5.3")) } else { @@ -359,7 +359,7 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let error, _) = error { + if case ManifestParseError.invalidManifestFormat(let error, _, _) = error { XCTAssertMatch(error, .contains("is unavailable")) XCTAssertMatch(error, .contains("was introduced in PackageDescription 5.3")) } else { @@ -384,7 +384,7 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let error, _) = error { + if case ManifestParseError.invalidManifestFormat(let error, _, _) = error { XCTAssertMatch(error, .contains("is unavailable")) XCTAssertMatch(error, .contains("was introduced in PackageDescription 5.3")) } else { @@ -415,7 +415,7 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let error, _) = error { + if case ManifestParseError.invalidManifestFormat(let error, _, _) = error { XCTAssertTrue(error.contains("Operation not permitted"), "unexpected error message: \(error)") } else { XCTFail("unexpected error: \(error)") diff --git a/Tests/PackageLoadingTests/PD_5_3_LoadingTests.swift b/Tests/PackageLoadingTests/PD_5_3_LoadingTests.swift index 1979534786b..79b1f6b7b5f 100644 --- a/Tests/PackageLoadingTests/PD_5_3_LoadingTests.swift +++ b/Tests/PackageLoadingTests/PD_5_3_LoadingTests.swift @@ -541,7 +541,7 @@ class PackageDescription5_3LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let error, _) = error { + if case ManifestParseError.invalidManifestFormat(let error, _, _) = error { XCTAssertTrue(error.contains("Operation not permitted"), "unexpected error message: \(error)") } else { XCTFail("unexpected error: \(error)") diff --git a/Tests/PackageLoadingTests/PD_5_4_LoadingTests.swift b/Tests/PackageLoadingTests/PD_5_4_LoadingTests.swift index 06fe0a05f92..38dfdf3c8cc 100644 --- a/Tests/PackageLoadingTests/PD_5_4_LoadingTests.swift +++ b/Tests/PackageLoadingTests/PD_5_4_LoadingTests.swift @@ -59,7 +59,7 @@ class PackageDescription5_4LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let message, _) = error { + if case ManifestParseError.invalidManifestFormat(let message, _, _) = error { XCTAssertMatch(message, .contains("is unavailable")) XCTAssertMatch(message, .contains("was introduced in PackageDescription 5.5")) } else { diff --git a/Tests/PackageLoadingTests/PD_5_7_LoadingTests .swift b/Tests/PackageLoadingTests/PD_5_7_LoadingTests .swift index 356322dc0b1..96857d969eb 100644 --- a/Tests/PackageLoadingTests/PD_5_7_LoadingTests .swift +++ b/Tests/PackageLoadingTests/PD_5_7_LoadingTests .swift @@ -110,7 +110,7 @@ class PackageDescription5_7LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in - if case ManifestParseError.invalidManifestFormat(let error, _) = error { + if case ManifestParseError.invalidManifestFormat(let error, _, _) = error { XCTAssertMatch(error, .contains("when(platforms:)' was obsoleted")) } else { XCTFail("unexpected error: \(error)") @@ -137,7 +137,7 @@ class PackageDescription5_7LoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope)) { error in - if case ManifestParseError.invalidManifestFormat(let message, _) = error { + if case ManifestParseError.invalidManifestFormat(let message, _, _) = error { XCTAssertMatch(message, .contains("error: 'productItem(name:package:condition:)' is unavailable: use .product(name:package:condition) instead.")) } else { XCTFail("unexpected error: \(error)") diff --git a/Tests/PackageLoadingTests/PD_Next_LoadingTests.swift b/Tests/PackageLoadingTests/PD_Next_LoadingTests.swift index 85d8c1bdb2c..4c0d6da3644 100644 --- a/Tests/PackageLoadingTests/PD_Next_LoadingTests.swift +++ b/Tests/PackageLoadingTests/PD_Next_LoadingTests.swift @@ -32,7 +32,7 @@ class PackageDescriptionNextLoadingTests: PackageDescriptionLoadingTests { let observability = ObservabilitySystem.makeForTesting() XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { - if case ManifestParseError.invalidManifestFormat(let error, _) = $0 { + if case ManifestParseError.invalidManifestFormat(let error, _, _) = $0 { XCTAssertMatch(error, .contains("cannot find 'FileManager' in scope")) } else { XCTFail("unexpected error: \($0)") diff --git a/Tests/WorkspaceTests/WorkspaceTests.swift b/Tests/WorkspaceTests/WorkspaceTests.swift index 015f423e125..5c7ccc94646 100644 --- a/Tests/WorkspaceTests/WorkspaceTests.swift +++ b/Tests/WorkspaceTests/WorkspaceTests.swift @@ -215,7 +215,7 @@ final class WorkspaceTests: XCTestCase { XCTAssert(rootManifests.count == 0, "\(rootManifests)") testDiagnostics(observability.diagnostics) { result in - let diagnostic = result.check(diagnostic: .prefix("invalid manifest\n\(path.appending(components: "MyPkg", "Package.swift")):4:8: error: An error in MyPkg"), severity: .error) + let diagnostic = result.check(diagnostic: .contains("\(path.appending(components: "MyPkg", "Package.swift")):4:8: error: An error in MyPkg"), severity: .error) XCTAssertEqual(diagnostic?.metadata?.packageIdentity, .init(path: pkgDir)) XCTAssertEqual(diagnostic?.metadata?.packageKind, .root(pkgDir)) }