Skip to content

Suppress warnings for remote Clang targets #7423

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 27, 2024
Merged

Suppress warnings for remote Clang targets #7423

merged 2 commits into from
Mar 27, 2024

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Mar 25, 2024

This mirrors behavior of remote swift targets which suppress warnings as well.

@xedin
Copy link
Contributor Author

xedin commented Mar 25, 2024

@MaxDesiatov what would be the best way to test this? I couldn't find any tests with -suppress-warnings flag in the suite :/

@xedin
Copy link
Contributor Author

xedin commented Mar 25, 2024

This is actually a draft, "package" doesn't exist for clang targets yet, I'm about open a PR to introduce that.

@xedin xedin mentioned this pull request Mar 25, 2024
Merged
@xedin
Copy link
Contributor Author

xedin commented Mar 25, 2024

I think I figured it out - I can extend ClangTargetBuildDescriptionTests to check that by forming a remote package dependency.

@MaxDesiatov
Copy link
Contributor

@MaxDesiatov what would be the best way to test this? I couldn't find any tests with -suppress-warnings flag in the suite :/

This looks like something that belongs to BuildPlanTests or maybe worth creating a new ClangTargetBuildPlanTests as the existing BuildPlanTests is too big.

@MaxDesiatov MaxDesiatov added build system Changes to interactions with build systems diagnostics labels Mar 26, 2024
@MaxDesiatov
Copy link
Contributor

maybe worth creating a new ClangTargetBuildPlanTests as the existing BuildPlanTests is too big.

Turns out we already have ClangTargetBuildDescriptionTests.swift.

@MaxDesiatov MaxDesiatov added the needs tests This change needs test coverage label Mar 26, 2024
xedin added 2 commits March 27, 2024 10:25
@xedin
Suppress warnings for remote Clang targets
398dab8 
This mirrors behavior of remote swift targets which
suppress warnings as well.
@xedin
NFC: Add a test-case for -w in clang arguments for "remote" packages
08efdf5 
Remote in this case means local or remote source control or registry.
@xedin xedin force-pushed the suppress-warnings-for-remote-clang-targets branch from 9ff1739 to 08efdf5 Compare March 27, 2024 17:27
@xedin
Copy link
Contributor Author

xedin commented Mar 27, 2024

@swift-ci please test

@xedin
Copy link
Contributor Author

xedin commented Mar 27, 2024

@swift-ci please test Windows platform

@MaxDesiatov
Copy link
Contributor

@swift-ci test windows

@xedin xedin merged commit c4a3af1 into swiftlang:main Mar 27, 2024
5 checks passed
@xedin xedin removed the needs tests This change needs test coverage label Apr 12, 2024
@xedin xedin mentioned this pull request Apr 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaxDesiatov MaxDesiatov MaxDesiatov approved these changes

@bnbarham bnbarham Awaiting requested review from bnbarham bnbarham is a code owner

@tomerd tomerd Awaiting requested review from tomerd

Assignees
No one assigned
Labels
build system Changes to interactions with build systems diagnostics
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@xedin @MaxDesiatov