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

Try to fix paths in clang-tidy output [DIP-356] #117

Merged
merged 2 commits into from
Dec 7, 2023

Conversation

woodfell
Copy link
Contributor

@woodfell woodfell commented Oct 6, 2023

Not elegant, but look for bazel cache-y paths in the output of clang-tidy and strip out the prefix so that errors relate back to real files in the repository.

An example error output from before the change:

ERROR: /home/matt/work/clean/starling-core/public_types/BUILD.bazel:14:17: Run clang-tidy on public_types/src/snprintf.cc failed: (Exit 1): clang_tidy failed: error executing command (from target //public_types:public_types) bazel-out/k8-fastbuild/bin/external/rules_swiftnav/clang_tidy/clang_tidy external/x86_64-linux-llvm/bin/clang-tidy external/rules_swiftnav/clang_tidy/.clang-tidy ... (remaining 80 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
1543 warnings generated.
Suppressed 1558 warnings (1541 in non-user code, 16 NOLINT, 1 with check filters).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
/home/matt/.cache/bazel/_bazel_matt/88ca1cd2a9e383e37ba5e842f72d57ea/sandbox/linux-sandbox/150/execroot/starling-core/public_types/src/snprintf.cc:101:19: warning: consider replacing 'short' with 'int16' [google-runtime-int]
struct arg_traits<signed short> final {

And after:

ERROR: /home/matt/work/clean/starling-core/public_types/BUILD.bazel:14:17: Run clang-tidy on public_types/src/snprintf.cc failed: (Exit 1): clang_tidy failed: error executing command (from target //public_types:public_types) bazel-out/k8-fastbuild/bin/external/rules_swiftnav/clang_tidy/clang_tidy external/x86_64-linux-llvm/bin/clang-tidy external/rules_swiftnav/clang_tidy/.clang-tidy ... (remaining 80 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
1543 warnings generated.
Suppressed 1558 warnings (1541 in non-user code, 16 NOLINT, 1 with check filters).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
public_types/src/snprintf.cc:101:19: warning: consider replacing 'short' with 'int16' [google-runtime-int]
struct arg_traits<signed short> final {

Note how the first message mentions a file in ~/.cache/bazel/.... which is helpfully deleted as soon as the command fails. By stripping out this prefix and leaving just the relative path within the repository then, assuming the command is run from the repository root, the path matches the actual real file rather than bazel's copy. IDEs and other tooling can parse out the message and trace it back to where it actually came from

@woodfell woodfell changed the title Try to fix paths in clang-tidy output Try to fix paths in clang-tidy output [DIP-356] Oct 6, 2023
@woodfell woodfell requested a review from jungleraptor October 6, 2023 04:36
@woodfell
Copy link
Contributor Author

woodfell commented Oct 6, 2023

@isaactorz as discussed in the build sync. Probably not an idiomatic solution but it solves my problem and makes the output of clang-tidy useable by other tooling

@jungleraptor
Copy link
Contributor

@isaactorz as discussed in the build sync. Probably not an idiomatic solution but it solves my problem and makes the output of clang-tidy useable by other tooling

pinging myself to remember to take a look at it.

At first glance seems perfectly reasonable just want to take a closer look before approving.

@woodfell woodfell marked this pull request as ready for review November 12, 2023 23:23
@woodfell woodfell requested a review from a team as a code owner November 12, 2023 23:23
@@ -22,6 +22,7 @@ fi
touch $OUTPUT
truncate -s 0 $OUTPUT

"${CLANG_TIDY_BIN}" "$@"
"${CLANG_TIDY_BIN}" "$@" | sed "s;.*execroot/[^/]*/;;"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we leave a small comment here as a reminder of what this was for?

@woodfell woodfell merged commit 3b975b6 into main Dec 7, 2023
1 check passed
@woodfell woodfell deleted the woodfell/better_clang_tidy_paths branch December 7, 2023 20:34
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.

2 participants