Skip to content
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

Don't Panic and clang-tidy everything if there are no eligible files #78272

Merged
merged 1 commit into from
Dec 1, 2024

Conversation

kevingranade
Copy link
Member

Summary

None

Purpose of change

I was investigating some clang-tidy CI runs that were running over the whole repo and don't look like they should, and I found this logic/"y u do that coretutils" error.
Basically if there are no eligible files to tidy clang-tidy.sh thinks there's an error and then tidies everything instead of nothing.
The specific thing that happens is if you pass an empty string to grep (which means there are no lines to match against), grep thinks it's grep's fault and returns an error status.

Describe the solution

We're just calling grep to filter out paths in src/third-party, so it doesn't ned to be involved in that sanity check in the first place.
So capture the previous output to a variable and do the check, then filter out paths with "third-party" afterward.
After that filtering, if we end up with an empty string, exit (successfully) instead of running clang-tidy on everything.

Testing

I checked all the scripting logic locally, but I don't have a CI runner set up to test it fully.
Also as this is editing clang-tidy.sh that WILL trigger a tidy of everything, so check back some time tomorrow :/

@github-actions github-actions bot added Code: Build Issues regarding different builds and build environments astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Dec 1, 2024
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Dec 1, 2024
@akrieger akrieger merged commit 273ef69 into master Dec 1, 2024
18 of 25 checks passed
echo unknown )"

tidyable_cpp_files="$(echo -n "$tidyable_cpp_files" | grep -v )"
Copy link
Member

Choose a reason for hiding this comment

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

Wait, grep -v what? Should this still say grep -v third-party?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

@kevingranade kevingranade Dec 1, 2024

Choose a reason for hiding this comment

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

Yes it definitely should >_<

@kevingranade kevingranade deleted the dont-tidy-nothing branch December 1, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions Code: Build Issues regarding different builds and build environments json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants