bazel-compile-commands-extractor icon indicating copy to clipboard operation
bazel-compile-commands-extractor copied to clipboard

Support for custom --output_user_root=foo/bar

Open lgulich opened this issue 2 years ago • 3 comments

Hi

First, thank you for creating this fantastic tool!

Are there any plans to support a custom output_user_root?

I am currently facing an issue with this because we use the compile commands extractor together with dazel (which just runs bazel in a docker container). The issue is that dazel calls bazel with a custom output_user_root via the cli argument --output_user_root=foo/bar. Unfortunately, this argument is not forwarded to the internal call to bazel aquery here.

I think the solution would be to query the current output_user_root and then forward this to the bazel aquery command. Unfortunately, I am not really sure how this is best done. I currently have a solution implemented here which works, but is really hacky. I take advantage of the fact that dazel calls bazel with sudo inside the docker container, thus the SUDO_COMMAND variable is set and I can parse the output_user_root out of it. Unfortunately, this only works for dazel and I had hoped for a solution that works for every use case where a custom output_user_root is set.

Do you have any ideas how this could be solved in a clean way? Would love to hear your ideas and I would also be happy to contribute this back to the main repo.

Thanks in advance! Lionel

lgulich avatar Sep 05 '22 07:09 lgulich

Hi! You're very welcome. Thanks for giving it a try, writing in, and being willing to experiment and contribute.

Is the path something you know in advance? If so, you can pass flags through to aquery, either from the command line (add -- --flags to Bazel run) or refresh_compile_commands rule (see readme) or as a startup option in your .bazelrc.

If not...I sadly don't know of a general way to reflect and have a command being run by Bazel get the options that built it. If you find one, boy do I want to know about it. Your sudo one is quite clever, I think :)

I'm also curious what all goes wrong if you set the path differently. I'd think the symlinks might still point through to the right place and that the compile_commands.json might come out intact. Clearly something goes wrong or you wouldn't be writing in, but I haven't figured it out just yet, and and need to crash. (It's late my time.)

Cheers! Chris

cpsauer avatar Sep 05 '22 08:09 cpsauer

So if I run it with dazel: dazel run @hedron_compile_commands//:refresh_all I get the following error: (For brevity I have replaced my HOME path with ~ below)

FATAL: mkdir('~/.cache/bazel/_bazel_lgulich'): (error: 13): Permission denied
Bazel aquery failed. Command: ['bazel', 'aquery', "mnemonic('(Objc|Cpp)Compile',deps(@//...))", '--output=jsonproto', '--include_artifacts=false', '--ui_event_filters=-info', '--noshow_progress', '--features=-compiler_param_file']
>>> Failed extracting commands for @//...
    Continuing gracefully...

The problem here is that ~/.cache/bazel (=the default output_user_root) is not mounted into the docker container. This is because Dazel internally calls bazel --output_user_root=~/.cache/dazel run @hedron_compile_commands//:refresh_all. Thus it mounts the custom output_user_root ~/.cache/dazel but not the default output_user_root ~/.cache/bazel which this tool uses.

Regarding your suggestion to pass through the flag: I believe this does not work because this just appends the flag to the command, but AFAIU the --output_user_root flag needs to be specified directly after bazel and before aquery and cannot be appended. (I tried this and it just throws the same error as above).

lgulich avatar Sep 05 '22 13:09 lgulich

Hey @lgulich! Sorry I didn't back to this with my usual speed. Ended up having to travel last week.

Any chance .bazelrc would solve it for you and keep you on the mainline? (i.e. add startup --output_user_root =$USER/.cache/dazel).

The other option is that we could add a startup flags section to refresh_compile_commands rule and supply them directly after bazel. (output_user_root being a startup action is why it didn't work after aquery. Sorry that didn't occur last time.) The only reason I haven't already done this and would suggest the bazelrc is that one usually wants to be really sure that one's startup options are consistent across runs.

Cheers, Chris

cpsauer avatar Sep 13 '22 02:09 cpsauer

@lgulich, any updates on which of these options would work best for you?

cpsauer avatar Sep 23 '22 18:09 cpsauer

@cpsauer Adding startup --output_user_root =$USER/.cache/dazel to .bazelrc works, thanks for the suggestion.

One problem that remains is that ideally, we would like to be able to build the workspace with both bazel and dazel and let the two use different output_user_root directories (i.e. the default one for bazel and our custom one from above with dazel). By adding the startup option to the .bazelrc of course now both use the same directory.

I think the ideal way to fix this would be if bazel would allow the following in the .bazelrc

startup:build_with_dazel --output_user_root =$USER/.cache/dazel

but unfortunately, bazel does not allow config specifications for startup options.

Thus I think what we will do is modify dazel, s.t. it automatically adds the necessary startup option to the .bazelrc instead of using the command line flag to set the output_user_root.

Thanks again for your help! Unless you have any better suggestion than modifying dazel feel free to close this issue.

lgulich avatar Oct 02 '22 15:10 lgulich

Yay! Glad that works. That sounds like the best fix to me, but with one possible tweak: What about trying to add a feature to dazel try to import a second, additional, .bazelrc, like a .dazelrc or something? Might be about equally easy and provide a nice means of auto configuring and extending things for the future. That or, maybe there's a way to have dazel use the standard user root?

Please do let me know how things go!


For others reading this in the future: Suggested course of action is to add startup options to .bazelrc, keeping everything consistent. If you really need different startup arguments to run this tool, say something (or send a PR).

cpsauer avatar Oct 05 '22 04:10 cpsauer