Fix: #258 make _get_files treat sourceless commands gracefully#262
Open
izashchelkin wants to merge 1 commit intohedronvision:mainfrom
Open
Fix: #258 make _get_files treat sourceless commands gracefully#262izashchelkin wants to merge 1 commit intohedronvision:mainfrom
izashchelkin wants to merge 1 commit intohedronvision:mainfrom
Conversation
saser
added a commit
to saser/code
that referenced
this pull request
Sep 13, 2025
This represents a pretty significant amount of work to move away from the old and deprecated `WORKSPACE` way of managing Bazel external dependencies, to using `MODULE.bazel` aka Bzlmod. At this point, I have `bazel build //...` and `bazel test //...` up and running smoothly. The only thing I am aware of that doesn't work is `bazel run @hedron_compile_commands//:refresh_all`, but it seems that the issue I am running into is getting fixed [soon](hedronvision/bazel-compile-commands-extractor#262). In the process of doing this I did a few things to get the migration over the line. **Migrated from `rules_docker` to `rules_oci`.** The former is more or less deprecated and there's no Bazel module for it in BCR. There is a module for `rules_oci` though, which seems to be the spiritual successor to `rules_docker`. They do mostly the same things, although in different ways. **Bumped version of most dependencies.** The main reason for doing so was to get on versions of dependencies that likely had much better support for Bzlmod. **Struggled a lot to get the dependencies on the Docker Go libraries working**, and ended up refactoring my old version of `dockertest` in favor of one based on `github.com/ory/dockertest/v3` which I know from professional experience works perfectly fine and doesn't require me to do some of the lower-level Docker stuff myself. **Removed a bunch of the existing Docker-related stuff**, since I couldn't be bothered to fix it and will likely not be using it in the near future anyway. **Removed the `./bazel` script**, and chose to just rely on Bazelisk being installed on the system instead. Probably more things I'm forgetting, but they're probably not that important if I can't think of them right now.
|
I've tested this locally and can confirm it fixes #258 for me |
|
For anyone lazy that just wants it to work: |
|
Hey @izashchelkin and @eklitzke! 👋 I've tested this fix in production with Envoy proxy (10k+ C++ files) and it works great! One small suggestion: We could make this more robust by specifically detecting header compilation actions instead of silently skipping ALL sourceless commands. I'd suggest something like: if len(source_file_candidates) == 0:
# Check if this is a known header-only compilation case
is_header_compile = '-xc++-header' in compile_action.arguments
if is_header_compile:
return None # Expected case: skip header precompilation
# For unexpected cases, keep assertion for debugging
assert False, f"No source files found in compile args: {compile_action.arguments}..." This way:
Happy to submit this as an additional commit to your PR if you'd like! Thanks for working on this fix! 🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: #258