-
Notifications
You must be signed in to change notification settings - Fork 121
Dependencies: support clang modules when parsing dependency validation info #778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dependencies: support clang modules when parsing dependency validation info #778
Conversation
e2cd92e
to
99d122c
Compare
@swift-ci Please test |
99d122c
to
908f676
Compare
@swift-ci Please test |
Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift
Outdated
Show resolved
Hide resolved
Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift
Outdated
Show resolved
Hide resolved
@@ -542,7 +542,7 @@ fileprivate struct DependencyValidationTests: CoreBasedTests { | |||
|
|||
// Expect complaint about undeclared dependency | |||
try await tester.checkBuild(parameters: BuildParameters(configuration: "Debug"), runDestination: .host, persistent: true) { results in | |||
results.checkError(.contains("Missing entries in MODULE_DEPENDENCIES: Foundation (for task")) | |||
results.checkError(.contains("Missing entry in MODULE_DEPENDENCIES: Foundation (for task")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm why would this change? I thought the top level diag was always pluralized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test fails for me locally with "entries", I think it's because the "(for task" substring is at the end of the diagnostic which will have the singular messages. So the whole diagnostic is like:
Missing entries in MODULE_DEPENDENCIES: Foundation Foundation
/private/var/folders/5h/9y9ls7ms0fs8gdmlfwklkwpm0000gn/T/swbuild.tmp.x8Tuqtfi/Data.noindex/Test/aProject/Sources/CoreFoo.m: Missing entry in MODULE_DEPENDENCIES: Foundation
/private/var/folders/5h/9y9ls7ms0fs8gdmlfwklkwpm0000gn/T/swbuild.tmp.x8Tuqtfi/Data.noindex/Test/aProject/Sources/CoreBar.m: Missing entry in MODULE_DEPENDENCIES: Foundation (for task: [\"ValidateDependencies\"])
If you want I could change this to look for Missing entries in MODULE_DEPENDENCIES: Foundation Foundation\n
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The missing entries are supposed to be de-duplicated (which is why the test checks for "(for task" at the end). It sounds like that's not working.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I think there's an issue with the deduplication in my implementation of parsing v1 traces that I need to fix. However, even after fixing that the diagnostic looks like this:
Missing entries in MODULE_DEPENDENCIES: Foundation
Missing entry in MODULE_DEPENDENCIES: Foundation (for task: [\"ValidateDependencies\"])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I think my last commit should fix the deduplication issue, the first line of the diagnostic has only one mention of Foundation
. However, Foundation (for task
is still only in the last line which has singular entry
.
@@ -542,7 +542,7 @@ fileprivate struct DependencyValidationTests: CoreBasedTests { | |||
|
|||
// Expect complaint about undeclared dependency | |||
try await tester.checkBuild(parameters: BuildParameters(configuration: "Debug"), runDestination: .host, persistent: true) { results in | |||
results.checkError(.contains("Missing entries in MODULE_DEPENDENCIES: Foundation (for task")) | |||
results.checkError(.contains("Missing entry in MODULE_DEPENDENCIES: Foundation (for task")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The missing entries are supposed to be de-duplicated (which is why the test checks for "(for task" at the end). It sounds like that's not working.
@swift-ci test |
1 similar comment
@swift-ci test |
b06bce2
to
6a44f6a
Compare
@swift-ci test |
This PR adds support for the new Clang
-header-include-format=direct-per-file
output, which now tracks Clang modules. The logic should still support the old format for backwards compatibility if it doesn't detect a version number in the trace file. The related LLVM PR is: llvm/llvm-project#156756