Expand @filename arguments

[amlinux]

Currently the tool tails with assertion:

No source files found in compile args: .../clang-cl.exe @bazel-out/path/to/flags/file

This PR preprocesses clang command line by reading referenced files and injecting its content as the invocation flags.

[cpsauer]

Hey, Alex! Thanks for looking for ways to make this tool better. Sorry for being such a slowpoke over here--I got totally buried with urgent stuff.

There's some history here with param files that I should relay: We originally considered auto unpacking the param files for MSVC, but then you're dependent on the param files having been built already. If the param files haven't been generated or are out of date, clangd (or other tools) won't very good understanding of the code. That's not ideal; people often want to be able to keep editing even in states where the code is unbuildable or only partially built, especially on larger codebases. So instead, we'd switched over to trying to query bazel with param files disabled. That way we'd get the full flags, whether the code built or not. In refresh.template.py we try to to this with --features=-compiler_param_file, and then, if that'd have made the command too long at header-search time, we spill the parameters to a file in _subprocess_run_spilling_over_to_param_file_if_needed.

Now, clearly that's not working with your clangcl toolchain for some reason. Any idea why? Might be worth conforming to that feature just so other tooling works more seamlessly.

Thoughts? IMO I should probably ask you to try to add that feature to your toolchain regardless. We could still add param file handling here as a fallback, as you have, though it might cause confusion and frustration. What do you think? If we do go this route, we should probably also check whether we need to defend against the bzlmod-style @@ target syntax.

Cheers and thanks for reaching out, Chris

[amlinux]

Hi Chris. Thank you for your response.

Here is the full error I get:

$ bazelisk run @hedron_compile_commands//:refresh_all
INFO: Invocation ID: f8ba21d2-1b54-4599-a58d-eecb07c55ca4
INFO: Analyzed target @hedron_compile_commands//:refresh_all (88 packages loaded, 3513 targets configured).
INFO: Found 1 target...
Target @hedron_compile_commands//:refresh_all up-to-date:
  bazel-bin/external/hedron_compile_commands/refresh_all.zip
  bazel-bin/external/hedron_compile_commands/refresh_all.exe
  bazel-bin/external/hedron_compile_commands/refresh_all.check_python_version.py
  bazel-bin/external/hedron_compile_commands/refresh_all.py
INFO: Elapsed time: 8.275s, Critical Path: 5.27s
INFO: 4 processes: 3 internal, 1 local.
INFO: Build completed successfully, 4 total actions
INFO: Running command line: bazel-bin/external/hedron_compile_commands/refresh_all.exe
>>> Analyzing commands used in @//...
WARNING: Running Bazel server needs to be killed, because the startup options are different.
Starting local Bazel server and connecting to it...
Traceback (most recent call last):
  File "C:\Users\aml\AppData\Local\Temp\Bazel.runfiles_kvnpjbpx\runfiles\_main\..\hedron_compile_commands\refresh_all.check_python_version.py", line 16, in <module>
    refresh_all.main()
  File "C:\Users\aml\AppData\Local\Temp\Bazel.runfiles_kvnpjbpx\runfiles\hedron_compile_commands\refresh_all.py", line 1074, in main
    compile_command_entries.extend(_get_commands(target, flags))
  File "C:\Users\aml\AppData\Local\Temp\Bazel.runfiles_kvnpjbpx\runfiles\hedron_compile_commands\refresh_all.py", line 935, in _get_commands
    yield from _convert_compile_commands(parsed_aquery_output)
  File "C:\Users\aml\AppData\Local\Temp\Bazel.runfiles_kvnpjbpx\runfiles\hedron_compile_commands\refresh_all.py", line 820, in _convert_compile_commands
    for source_files, header_files, compile_command_args in outputs:
  File "C:\Users\aml\AppData\Local\Temp\Bazel.runfiles_kvnpjbpx\runfiles\rules_python~0.25.0~python~python_3_11_x86_64-pc-windows-msvc\Lib\concurrent\futures\_base.py", line 619, in result_iterator
    yield _result_or_cancel(fs.pop())
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\aml\AppData\Local\Temp\Bazel.runfiles_kvnpjbpx\runfiles\rules_python~0.25.0~python~python_3_11_x86_64-pc-windows-msvc\Lib\concurrent\futures\_base.py", line 317, in _result_or_cancel
    return fut.result(timeout)
           ^^^^^^^^^^^^^^^^^^^
  File "C:\Users\aml\AppData\Local\Temp\Bazel.runfiles_kvnpjbpx\runfiles\rules_python~0.25.0~python~python_3_11_x86_64-pc-windows-msvc\Lib\concurrent\futures\_base.py", line 449, in result
    return self.__get_result()
           ^^^^^^^^^^^^^^^^^^^
  File "C:\Users\aml\AppData\Local\Temp\Bazel.runfiles_kvnpjbpx\runfiles\rules_python~0.25.0~python~python_3_11_x86_64-pc-windows-msvc\Lib\concurrent\futures\_base.py", line 401, in __get_result
    raise self._exception
  File "C:\Users\aml\AppData\Local\Temp\Bazel.runfiles_kvnpjbpx\runfiles\rules_python~0.25.0~python~python_3_11_x86_64-pc-windows-msvc\Lib\concurrent\futures\thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\aml\AppData\Local\Temp\Bazel.runfiles_kvnpjbpx\runfiles\hedron_compile_commands\refresh_all.py", line 787, in _get_cpp_command_for_files
    source_files, header_files = _get_files(compile_action)
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\aml\AppData\Local\Temp\Bazel.runfiles_kvnpjbpx\runfiles\hedron_compile_commands\refresh_all.py", line 576, in _get_files
    assert source_file_candidates, f"No source files found in compile args: {compile_action.arguments}.\nPlease file an issue with this information!"
AssertionError: No source files found in compile args: ['C:/Program Files/LLVM/bin/clang-cl.exe', '@bazel-out/x64_windows-opt-exec-ST-d610928244b0/bin/embed/_objs/generator/generator.obj.params'].
Please file an issue with this information!

I don't know why --compiler_param_file doesn't work for me. Perhaps not all toolchains support it correctly. I'm using windows + clang.

My .bazelrc looks like this:

startup --windows_enable_symlinks
common --enable_bzlmod --extra_toolchains=@local_config_cc//:cc-toolchain-x64_windows-clang-cl --extra_execution_platforms=//:x64_windows-clang-cl --incompatible_enable_cc_toolchain_resolution --enable_runfiles

Regarding codebases that don't build, I use the following command to rebuild compile_commands.json:

bazelisk build --keep_going ... ; bazelisk run @hedron_compile_commands//:refresh_all

--keep_going is the trick that allows me to generate everything needed for hedron_compile_commands to do its job even if the code doesn't compile. I have to run a build anyway, because my project heavily depends on code generation (protobufs at least), and doing this pre-pass is needed anyway.

[cpsauer]

Likewise :)

Sounds like Bazel's got a bug in their clang-cl toolchain, not conforming to --features=-compiler_param_file. Since it sounds like you're on Windows and have comparative advantage--and I'm overloaded trying to help folks here and elsewhere--would you be down to file an issue with them, cc me? (I ssh'd into my Windows box and checked that it does indeed seem to work as their docs would suggest for msvc and if you set it with --compiler. That might also be a good workaround for you.) If they fix the underlying issue, that, on its own, should fix things for you.

If we want to expand param files more generally (a good idea in case someone specifies them manually at the command line and for this in the meantime), I'm happy to still merge, but I'm going to need to ask you to make this a bit more bombproof. In addition to defending against the @@ label (bzlmod) case above, I'm seeing that we'll also need not crash when the param file doesn't yet exist, printing a nice error message the first time (like the one that steers people to --keep_going), and we should probably call this at the start of _all_platform_patch instead, so other flag patches can operate on the expanded arguments. There might be other cases to consider--that's just what I'm seeing on a quick pass--so I'm hoping you'll give it some careful thought. That way I can hit that green merge button and get you on the contributors board :)

Cheers--and thanks! Chris

[amlinux]

Thanks for the pointers!

Since it sounds like you're on Windows and have comparative advantage

I would not call it an advantage, it's rather a curse :-)

I was about to file a bug to Bazel, but when preparing a min repro, I couldn't convince myself that it actually was. I found that the key element that doesn't work is binaries for the host target, those that are used by genrules. hedron generator doesn't work with either of msvc, --compiler=clang-cl or new --incompatible_enable_cc_toolchain_resolution.

Here is the repro: https://github.com/amlinux/hedron-repro.

Run one of:

bazelisk build --keep_going --remote_cache= ... ; bazelisk run --remote_cache= @hedron_compile_commands//:refresh_all -- --remote_cache=

bazelisk build --config clang-cl --keep_going --remote_cache= ... ; bazelisk run --config clang-cl --remote_cache= @hedron_compile_commands//:refresh_all -- --config clang-cl --remote_cache=

bazelisk build --config cc_toolchain_resolution --keep_going --remote_cache= ... ; bazelisk run --config cc_toolchain_resolution --remote_cache= @hedron_compile_commands//:refresh_all -- --config cc_toolchain_resolution --remote_cache=

All of them fail with the same error:

AssertionError: No source files found in compile args: ['C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.33.31629/bin/HostX64/x64/cl.exe', '@bazel-out/x64_windows-opt-exec-ST-13d3ddad9198/bin/_objs/test-generator/test-generator.obj.params'].

At the same time, when I run bazelisk aquery --features=-compiler_param_file ..., I don't see that test-generator.obj.params is generated anywhere. So it looks like bazel behaves correctly and I would rather suspect a bug in the hedron tool.

WDYT?

[cpsauer]

Aha! I think I've got it, thanks to your scoping. Looks like Bazel has since added a new flag, --host_features, so we need to add --host_features=-compiler_param_file. Would you be down to try running with that, and if it works, we'll bake it in automatically?

(Backtracking my misdiagnosis: The quick test I cooked up didn't have any host tools in it.)

Could you also tell me which version of bazel you're using? (from a quick git search looks like they added in 6.1, flipped on by default in 7?)

I would not call it an advantage, it's rather a curse :-)

Heh, required by work or something? (Advantage in dealing with Windows compared to me, I meant, ofc.)

[amlinux]

Would you be down to try running with that, and if it works, we'll bake it in automatically?

It worked! Thank you!

(Backtracking my misdiagnosis: The quick test I cooked up didn't have any host tools in it.)

I attached a demo repo to my previous comment. I'll keep it around, just in case you need to test it yourself.

Heh, required by work or something?

Yes. I'm doing PC game development.

[cpsauer]

Awesome! Thanks for working with me and being so great. Just pushed it up, so things should work automatically at HEAD, no manual flags required.

It should support 7.0 and the latest in v6, so I'm thinking we only hack in backwards compatibility only if we hear that it's breaking someone. But could you please give me any quick sense you have of which Bazel version it appeared in for you? Do you use rolling? Does your experience square with 7.0 or were you seeing it in 6.something?

Yes. I'm doing PC game development.

Super cool! Is this you? Taking a break from google for some fun game dev? Would love to hear about what you're building if you're willing to share.

Also, still want to merge the param file expansion functionality? I'm back leaning very weakly no, thinking it only really covers the case where people are manually specifying param files, which seems unlikely vs defending against this case, but you lmk. Happy to merge if you want, w/ those fixes.

[amlinux]

Now it works like a charm! Thank you very much for the fix! I think this PR can be dropped now.

[cpsauer]

I'll close it down then--but thanks again for being great to work with and being down to contribute! Hope I'll see you around here again :)