Skip to content

Conversation

neonichu
Copy link
Collaborator

@neonichu neonichu commented Aug 4, 2025

We want to group diagnostics to avoid duplicates, but also avoid building the entire target if we already know about errors. So for C compiles specifically, we'll continue emitting diagnostics per-task if errors are enabled.

Additionally, this PR fixes up the signature data related to dependency diagnostics. The ValidateDependencies task needs to depend on the MODULE_DEPENDENCIES build setting, but the Swift planning task shouldn't anymore. For Clang tasks, this is now also dependent on whether errors are enabled or not.

We want to group diagnostics to avoid duplicates, but also avoid building the entire target if we already know about errors. So for C compiles specifically, we'll continue emitting diagnostics per-task if errors are enabled.

Additionally, this PR fixes up the signature data related to dependency diagnostics. The `ValidateDependencies` task needs to depend on the `MODULE_DEPENDENCIES` build setting, but the Swift planning task shouldn't anymore. For Clang tasks, this is now also dependent on whether errors are enabled or not.
@neonichu
Copy link
Collaborator Author

neonichu commented Aug 4, 2025

@swift-ci please test

delegate.error("Error serializing \(payload): \(error)")
return
}
guard let signature = String(data: jsonData, encoding: .utf8) else {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always use String(decoding: jsonData, as: UTF8.self) which is non-failable

@neonichu
Copy link
Collaborator Author

neonichu commented Aug 6, 2025

Had some offline discussion about this and we decided that this isn't really worth it considering it actually needs more work to make sure rebuilds work well in all cases.

Instead I opened #710 to keep the signature fixes that were included in this PR.

@neonichu neonichu closed this Aug 6, 2025
@neonichu neonichu deleted the module-deps-early-errors-for-clang branch August 6, 2025 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants