Skip to content

Commit 71bb79c

Browse files
neonichuneonichu
authored
Include the compiler commandline in ManifestParseError.invalidManifestFormat (#5958)
This could make it easier to debug these type of errors.
1 parent f79acf3 commit 71bb79c

11 files changed

+47
-34
lines changed

‎Sources/PackageLoading/ManifestJSONParser.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ enum ManifestJSONParser {
165165
do {
166166
path = try AbsolutePath(validating: location)
167167
} catch PathValidationError.invalidAbsolutePath(let path) {
168-
throw ManifestParseError.invalidManifestFormat("'\(path)' is not a valid path for path-based dependencies; use relative or absolute path instead.", diagnosticFile: nil)
168+
throw ManifestParseError.invalidManifestFormat("'\(path)' is not a valid path for path-based dependencies; use relative or absolute path instead.", diagnosticFile: nil, compilerCommandLine: nil)
169169
}
170170
let identity = try identityResolver.resolveIdentity(for: path)
171171
return .fileSystem(identity: identity,
@@ -224,11 +224,11 @@ enum ManifestJSONParser {
224224
guard hostnameComponent.isEmpty else {
225225
if hostnameComponent == ".." {
226226
throw ManifestParseError.invalidManifestFormat(
227-
"file:// URLs cannot be relative, did you mean to use '.package(path:)'?", diagnosticFile: nil
227+
"file:// URLs cannot be relative, did you mean to use '.package(path:)'?", diagnosticFile: nil, compilerCommandLine: nil
228228
)
229229
}
230230
throw ManifestParseError.invalidManifestFormat(
231-
"file:// URLs with hostnames are not supported, are you missing a '/'?", diagnosticFile: nil
231+
"file:// URLs with hostnames are not supported, are you missing a '/'?", diagnosticFile: nil, compilerCommandLine: nil
232232
)
233233
}
234234
return try AbsolutePath(validating: location).pathString

‎Sources/PackageLoading/ManifestLoader.swift

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ public enum ManifestParseError: Swift.Error, Equatable {
2323
/// The manifest contains invalid format.
2424
case invalidManifestFormat(String, diagnosticFile: AbsolutePath?)
2525
// TODO: Test this error.
26+
case invalidManifestFormat(String, diagnosticFile: AbsolutePath?, compilerCommandLine: [String]?)
27+
2628
/// The manifest was successfully loaded by swift interpreter but there were runtime issues.
2729
case runtimeManifestErrors([String])
2830

@@ -36,8 +38,14 @@ extension ManifestParseError: CustomStringConvertible {
3638
switch self {
3739
case .emptyManifest(let manifestPath):
3840
return "'\(manifestPath)' is empty"
39-
case .invalidManifestFormat(let error, _):
40-
return "invalid manifest\n\(error)"
41+
case .invalidManifestFormat(let error, _, let compilerCommandLine):
42+
let suffix: String
43+
if let compilerCommandLine = compilerCommandLine {
44+
suffix = " (compiled with: \(compilerCommandLine))"
45+
} else {
46+
suffix = ""
47+
}
48+
return "Invalid manifest\(suffix)\n\(error)"
4149
case .runtimeManifestErrors(let errors):
4250
return "invalid manifest (evaluation failed)\n\(errors.joined(separator: "\n"))"
4351
case .importsRestrictedModules(let modules):
@@ -292,7 +300,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
292300
// Throw now if we weren't able to parse the manifest.
293301
guard let manifestJSON = result.manifestJSON, !manifestJSON.isEmpty else {
294302
let errors = result.errorOutput ?? result.compilerOutput ?? "Missing or empty JSON output from manifest compilation for \(packageIdentity)"
295-
throw ManifestParseError.invalidManifestFormat(errors, diagnosticFile: result.diagnosticFile)
303+
throw ManifestParseError.invalidManifestFormat(errors, diagnosticFile: result.diagnosticFile, compilerCommandLine: result.compilerCommandLine)
296304
}
297305

298306
// We should not have any fatal error at this point.
@@ -660,6 +668,8 @@ public final class ManifestLoader: ManifestLoaderProtocol {
660668
let compiledManifestFile = tmpDir.appending(component: "\(packageIdentity)-manifest\(executableSuffix)")
661669
cmd += ["-o", compiledManifestFile.pathString]
662670

671+
evaluationResult.compilerCommandLine = cmd
672+
663673
// Compile the manifest.
664674
TSCBasic.Process.popen(arguments: cmd, environment: self.toolchain.swiftCompilerEnvironment, queue: callbackQueue) { result in
665675
dispatchPrecondition(condition: .onQueue(callbackQueue))
@@ -898,6 +908,9 @@ extension ManifestLoader {
898908
/// The manifest in JSON format.
899909
var manifestJSON: String?
900910

911+
/// The command line used to compile the manifest
912+
var compilerCommandLine: [String]?
913+
901914
/// Any non-compiler error that might have occurred during manifest loading.
902915
///
903916
/// For e.g., we could have failed to spawn the process or create temporary file.

‎Tests/PackageLoadingTests/PD_4_0_LoadingTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ class PackageDescription4_0LoadingTests: PackageDescriptionLoadingTests {
311311

312312
let observability = ObservabilitySystem.makeForTesting()
313313
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
314-
if case ManifestParseError.invalidManifestFormat(let error, _) = error {
314+
if case ManifestParseError.invalidManifestFormat(let error, _, _) = error {
315315
XCTAssert(error.contains("error: 'package(url:version:)' is unavailable: use package(url:exact:) instead"), "\(error)")
316316
XCTAssert(error.contains("error: 'package(url:range:)' is unavailable: use package(url:_:) instead"), "\(error)")
317317
} else {

‎Tests/PackageLoadingTests/PD_4_2_LoadingTests.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests {
103103

104104
let observability = ObservabilitySystem.makeForTesting()
105105
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
106-
if case ManifestParseError.invalidManifestFormat(let message, _) = error {
106+
if case ManifestParseError.invalidManifestFormat(let message, _, _) = error {
107107
XCTAssertMatch(
108108
message,
109109
.and(
@@ -166,7 +166,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests {
166166

167167
let observability = ObservabilitySystem.makeForTesting()
168168
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
169-
if case ManifestParseError.invalidManifestFormat(let message, _) = error {
169+
if case ManifestParseError.invalidManifestFormat(let message, _, _) = error {
170170
XCTAssertMatch(message, .contains("is unavailable"))
171171
XCTAssertMatch(message, .contains("was introduced in PackageDescription 5"))
172172
} else {
@@ -188,7 +188,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests {
188188

189189
let observability = ObservabilitySystem.makeForTesting()
190190
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
191-
if case ManifestParseError.invalidManifestFormat(let message, _) = error {
191+
if case ManifestParseError.invalidManifestFormat(let message, _, _) = error {
192192
XCTAssertMatch(message, .contains("is unavailable"))
193193
XCTAssertMatch(message, .contains("was introduced in PackageDescription 5"))
194194
} else {
@@ -208,7 +208,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests {
208208

209209
let observability = ObservabilitySystem.makeForTesting()
210210
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
211-
if case ManifestParseError.invalidManifestFormat(let message, _) = error {
211+
if case ManifestParseError.invalidManifestFormat(let message, _, _) = error {
212212
XCTAssertMatch(message, .contains("is unavailable"))
213213
XCTAssertMatch(message, .contains("was introduced in PackageDescription 5"))
214214
} else {
@@ -239,7 +239,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests {
239239

240240
let observability = ObservabilitySystem.makeForTesting()
241241
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
242-
if case ManifestParseError.invalidManifestFormat(let message, _) = error {
242+
if case ManifestParseError.invalidManifestFormat(let message, _, _) = error {
243243
XCTAssertMatch(message, .contains("is unavailable"))
244244
XCTAssertMatch(message, .contains("was introduced in PackageDescription 5"))
245245
} else {
@@ -499,7 +499,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests {
499499

500500
let observability = ObservabilitySystem.makeForTesting()
501501
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
502-
if case ManifestParseError.invalidManifestFormat(let message, let diagnosticFile) = error {
502+
if case ManifestParseError.invalidManifestFormat(let message, let diagnosticFile, _) = error {
503503
XCTAssertNil(diagnosticFile)
504504
XCTAssertEqual(message, "'https://someurl.com' is not a valid path for path-based dependencies; use relative or absolute path instead.")
505505
} else {
@@ -519,9 +519,9 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests {
519519
case .invalidAbsolutePath:
520520
return nil
521521
case .relativePath:
522-
return .invalidManifestFormat("file:// URLs cannot be relative, did you mean to use '.package(path:)'?", diagnosticFile: nil)
522+
return .invalidManifestFormat("file:// URLs cannot be relative, did you mean to use '.package(path:)'?", diagnosticFile: nil, compilerCommandLine: nil)
523523
case .unsupportedHostname:
524-
return .invalidManifestFormat("file:// URLs with hostnames are not supported, are you missing a '/'?", diagnosticFile: nil)
524+
return .invalidManifestFormat("file:// URLs with hostnames are not supported, are you missing a '/'?", diagnosticFile: nil, compilerCommandLine: nil)
525525
}
526526
}
527527

‎Tests/PackageLoadingTests/PD_5_0_LoadingTests.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ class PackageDescription5_0LoadingTests: PackageDescriptionLoadingTests {
112112

113113
let observability = ObservabilitySystem.makeForTesting()
114114
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
115-
if case ManifestParseError.invalidManifestFormat(let message, _) = error {
115+
if case ManifestParseError.invalidManifestFormat(let message, _, _) = error {
116116
XCTAssertMatch(message, .contains("'v3' is unavailable"))
117117
XCTAssertMatch(message, .contains("'v3' was obsoleted in PackageDescription 5"))
118118
} else {
@@ -274,7 +274,7 @@ class PackageDescription5_0LoadingTests: PackageDescriptionLoadingTests {
274274

275275
let observability = ObservabilitySystem.makeForTesting()
276276
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
277-
if case ManifestParseError.invalidManifestFormat(let message, _) = error {
277+
if case ManifestParseError.invalidManifestFormat(let message, _, _) = error {
278278
XCTAssertMatch(message, .contains("error: 'v11' is unavailable"))
279279
XCTAssertMatch(message, .contains("note: 'v11' was introduced in PackageDescription 5.3"))
280280
XCTAssertMatch(message, .contains("note: 'v14' was introduced in PackageDescription 5.3"))
@@ -298,7 +298,7 @@ class PackageDescription5_0LoadingTests: PackageDescriptionLoadingTests {
298298

299299
let observability = ObservabilitySystem.makeForTesting()
300300
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
301-
if case ManifestParseError.invalidManifestFormat(let message, _) = error {
301+
if case ManifestParseError.invalidManifestFormat(let message, _, _) = error {
302302
XCTAssertMatch(message, .contains("error: 'v10_16' has been renamed to 'v11'"))
303303
XCTAssertMatch(message, .contains("note: 'v10_16' has been explicitly marked unavailable here"))
304304
XCTAssertMatch(message, .contains("note: 'v14' was introduced in PackageDescription 5.3"))
@@ -395,7 +395,7 @@ class PackageDescription5_0LoadingTests: PackageDescriptionLoadingTests {
395395
fileSystem: fs,
396396
observabilityScope: observability.topScope
397397
)
398-
} catch ManifestParseError.invalidManifestFormat(let error, let diagnosticFile) {
398+
} catch ManifestParseError.invalidManifestFormat(let error, let diagnosticFile, _) {
399399
XCTAssertMatch(error, .contains("expected expression in container literal"))
400400
let contents = try localFileSystem.readFileContents(diagnosticFile!)
401401
XCTAssertNotNil(contents)
@@ -506,7 +506,7 @@ class PackageDescription5_0LoadingTests: PackageDescriptionLoadingTests {
506506
do {
507507
let observability = ObservabilitySystem.makeForTesting()
508508
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
509-
if case ManifestParseError.invalidManifestFormat(let message, _) = error {
509+
if case ManifestParseError.invalidManifestFormat(let message, _, _) = error {
510510
XCTAssertMatch(message, .contains("is unavailable"))
511511
XCTAssertMatch(message, .contains("was introduced in PackageDescription 5.2"))
512512
} else {
@@ -553,7 +553,7 @@ class PackageDescription5_0LoadingTests: PackageDescriptionLoadingTests {
553553

554554
let observability = ObservabilitySystem.makeForTesting()
555555
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
556-
if case ManifestParseError.invalidManifestFormat(let message, _) = error {
556+
if case ManifestParseError.invalidManifestFormat(let message, _, _) = error {
557557
XCTAssertMatch(message, .contains("is unavailable"))
558558
XCTAssertMatch(message, .contains("was introduced in PackageDescription 5.2"))
559559
} else {

‎Tests/PackageLoadingTests/PD_5_2_LoadingTests.swift

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
4141

4242
let observability = ObservabilitySystem.makeForTesting()
4343
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
44-
if case ManifestParseError.invalidManifestFormat(let error, _) = error {
44+
if case ManifestParseError.invalidManifestFormat(let error, _, _) = error {
4545
XCTAssert(error.contains("error: \'product(name:package:)\' is unavailable: the 'package' argument is mandatory as of tools version 5.2"))
4646
} else {
4747
XCTFail("unexpected error: \(error)")
@@ -277,7 +277,7 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
277277

278278
let observability = ObservabilitySystem.makeForTesting()
279279
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
280-
if case ManifestParseError.invalidManifestFormat(let error, _) = error {
280+
if case ManifestParseError.invalidManifestFormat(let error, _, _) = error {
281281
XCTAssertMatch(error, .contains("is unavailable"))
282282
XCTAssertMatch(error, .contains("was introduced in PackageDescription 5.3"))
283283
} else {
@@ -303,7 +303,7 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
303303

304304
let observability = ObservabilitySystem.makeForTesting()
305305
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
306-
if case ManifestParseError.invalidManifestFormat(let error, _) = error {
306+
if case ManifestParseError.invalidManifestFormat(let error, _, _) = error {
307307
XCTAssertMatch(error, .contains("is unavailable"))
308308
XCTAssertMatch(error, .contains("was introduced in PackageDescription 5.3"))
309309
} else {
@@ -329,7 +329,7 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
329329

330330
let observability = ObservabilitySystem.makeForTesting()
331331
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
332-
if case ManifestParseError.invalidManifestFormat(let error, _) = error {
332+
if case ManifestParseError.invalidManifestFormat(let error, _, _) = error {
333333
XCTAssertMatch(error, .contains("is unavailable"))
334334
XCTAssertMatch(error, .contains("was introduced in PackageDescription 5.3"))
335335
} else {
@@ -359,7 +359,7 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
359359

360360
let observability = ObservabilitySystem.makeForTesting()
361361
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
362-
if case ManifestParseError.invalidManifestFormat(let error, _) = error {
362+
if case ManifestParseError.invalidManifestFormat(let error, _, _) = error {
363363
XCTAssertMatch(error, .contains("is unavailable"))
364364
XCTAssertMatch(error, .contains("was introduced in PackageDescription 5.3"))
365365
} else {
@@ -384,7 +384,7 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
384384

385385
let observability = ObservabilitySystem.makeForTesting()
386386
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
387-
if case ManifestParseError.invalidManifestFormat(let error, _) = error {
387+
if case ManifestParseError.invalidManifestFormat(let error, _, _) = error {
388388
XCTAssertMatch(error, .contains("is unavailable"))
389389
XCTAssertMatch(error, .contains("was introduced in PackageDescription 5.3"))
390390
} else {
@@ -415,7 +415,7 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
415415

416416
let observability = ObservabilitySystem.makeForTesting()
417417
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
418-
if case ManifestParseError.invalidManifestFormat(let error, _) = error {
418+
if case ManifestParseError.invalidManifestFormat(let error, _, _) = error {
419419
XCTAssertTrue(error.contains("Operation not permitted"), "unexpected error message: \(error)")
420420
} else {
421421
XCTFail("unexpected error: \(error)")

‎Tests/PackageLoadingTests/PD_5_3_LoadingTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ class PackageDescription5_3LoadingTests: PackageDescriptionLoadingTests {
541541

542542
let observability = ObservabilitySystem.makeForTesting()
543543
XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") { error in
544-
if case ManifestParseError.invalidManifestFormat(let error, _) = error {
544+
if case ManifestParseError.invalidManifestFormat(let error, _, _) = error {
545545
XCTAssertTrue(error.contains("Operation not permitted"), "unexpected error message: \(error)")
546546
} else {
547547
XCTFail("unexpected error: \(error)")

0 commit comments

Comments
 (0)