Custom-control-args #122
Custom-control-args #122
Conversation
… that need custom command line options. We add the following options that are all prefixed by `--bcce-` (for Bazel-Compile-Command-Extractor): --bcce-color allows to enable/disable colored output. --bcce-compiler allows to override the compiler (this goes beyond the current apple ing). --bcce-copt allows to pass down additional args to the arg lists.
for more information, see https://pre-commit.ci
…l-compile-commands-extractor into custom-control-args
Hi, Marcus! Great to meet you. Thanks for your high-quality PR--and for being the kind of person who uses his talents to leave things better than he found them. Sorry I've been slower than you deserve; I'm doing my best to catch back up after a trip. Love the idea of helping people run this tool automatically. |
Since the only thing better than great flags is (also) automatically doing the right thing, I wanted to start by checking in with you on which of these we might be able to automate. Starting with color: What do you think about automatically disabling color if the output isn't a terminal, parallel to this implementation, but augmenting its inspection of the TERM environment variable to match bazel's? That'd also get us standard manual override with, e.g. NO_COLOR, perhaps removing the need for the flag. (Some other variants that crossed my mind, which seemed worse, but which I'll still list for your consideration: We could also check the COLORTERM variable, but I don't know that we really need it. We could try to support the same flag as Bazel so our color is in sync with it, trying to inspect bazelrc with --announce_rc, but I think it's not worth it--I'd be inclined to just match the standard environment variables. What a shame that the vscode output pane doesn't support color!) |
For argument addition and compiler override: I'd love to backtrace the need here--to understand what the problematic values are that you want to override. Ideally, of course, we'd automatically get good, de-bazeled commands for you, as we do with, e.g. unwrapping on Apple and the other platform es. (I'm confident we shouldn't recommend that everyone just always override with clang, though; that'll break things, for example, for folks who are cross compiling.) Some other quick notes:
Anyway, that's my quick look. Thanks again for being great :) and bring your experience to bear on this. |
Add further documentaton to the new flags.
Thanks for the response Chris, I like stuff to work automatically while allowing users control in case the automation is wrong. So I looked at automatic color detection while keeping a color flag that defaults to 'auto', very similar to bazel's way. Looking at the environment from the OUTPUT I get: VSCODE_AMD_ENTRYPOINT=vs/workbench/api/node/extensionHostProcess VSCODE_CRASH_REPORTER_PROCESS_TYPE=extensionHost VSCODE_CRASH_REPORTER_SANDBOXED_HINT=1 VSCODE_CWD=/ VSCODE_HANDLES_UNCAUGHT_ERRORS=true VSCODE_IPC_HOOK=/Users/marcus/Library/Application Support/Code/1.78-main.sock VSCODE_L10N_BUNDLE_LOCATION= VSCODE_NLS_CONFIG={"locale":"en-us","osLocale":"en-us","availableLanguages":{},"_languagePackSupport":true} VSCODE_PID=23867 But no TERM or similar var. From the "Terminal" I get: LANG=en_US.UTF-8 TERM=xterm-256color TERM_PROGRAM=vscode TERM_PROGRAM_VERSION=1.78.2 VSCODE_GIT_ASKPASS_EXTRA_ARGS=--ms-enable-electron-run-as-node VSCODE_GIT_ASKPASS_MAIN=/Applications/Visual Studio Code.app/Contents/Resources/app/extensions/git/dist/askpass-main.js VSCODE_GIT_ASKPASS_NODE=/Applications/Visual Studio Code.app/Contents/Frameworks/Code Helper (Plugin).app/Contents/MacOS/Code Helper (Plugin) VSCODE_GIT_IPC_HANDLE=/var/folders/lm/vprqmglx6zb56y6bx6wh23k80000gn/T/vscode-git-f92ff67fca.sock VSCODE_INJECTION=1 So we can check TERM and NO_COLOR for automation. As for compiler and option overrides, I will add further explanations. But there is a difference between using cland's config and overriding the values in the generated compile_commands.json file. They simply have different used cases. …On Fri, Jun 2, 2023 at 5:39 AM Chris Sauer ***@***.***> wrote: For argument addition and compiler override: I'd love to backtrace the need here--to understand what the problematic values are that you want to override. Ideally, of course, we'd automatically get good, de-bazeled commands for you, as we do with, e.g. unwrapping on Apple and the other platform es. (I'm confident we shouldn't recommend that everyone just always override with clang, though; that'll break things, for example, for folks who are cross compiling.) Some other quick notes: - If this is for clangd, and we *really* want to manually override, I think you can also probably override both with a user level clangd config <https://clangd.llvm.org/config>, if that'd suit your needs. (Project-level clangd config will fail for out-of-tree, system headers, I'm guessing.) Might be an alternative to adding flags here. - This is super minor, but I think we also might want to tighten the emeraldwalk.runonsave regex just a little. I'm guessing it's matching the file path, right? So we probably want an ending $, can drop the .*s. And I think there are a couple more we might want to match, like .star. Quick search from the vscode plugin <https://.com/search?q=repo%3Abazelbuild%2Fvscode-bazel%20bzl&type=code> . - Heads that there's a new feature probably coming (in a PR) soon that'll incrementally get/update commands for individual files. The idea is that it'd be called every time a file was opened in an editor, thereby accommodating folks with very large codebases where running the full extraction would be overwhelming. Anyway, that's my quick look. Thanks again for being great :) and bring your experience to bear on this. Chris — Reply to this email directly, view it on <#122 (comment)>, or unsubscribe <https://.com/notifications/unsubscribe-auth/ABQ7NSPSL7KBNOFICAHRWP3XJFN5JANCNFSM6AAAAAAYKQCJQE> . You are receiving this because you authored the thread.Message ID: ***@***.*** .com> |
for more information, see https://pre-commit.ci
Right there with you on "automatic but configurable" :) Thanks for your reply. Would still love to backtrace your needs on the other two--for my understanding if nothing else! Cheers and thanks! |
Here is how I ended up needing to control the compiler: - The extractor correctly finds `external/local_config_cc/cc_wrapper.sh` on my MacOS setup. It will select the correct compiler: ``` external/local_config_cc/cc_wrapper.sh --version Apple clang version 14.0.3 (clang-1403.0.22.14.1) Target: x86_64-apple-darwin22.5.0 Thread model: posix InstalledDir: /Library/Developer/CommandLineTools/usr/bin ``` I can set the correct compiler to use with my clangd setup in the `.clang` project file or a user/computer wide control file, say: ``` CompileFlags: Compiler: /usr/local/opt/llvm/bin/clang ``` Unfortunately I cannot just say `Compiler: clang` as it will find the wrong one then, so I have to actually provide the absolute path. Now the absolute path won't be the same for all users and all setups, so it cannot be shipped in the project's `.clang` file. On the other hand the extractor flag --bcce-compiler="$(which clang)" will work at least for any setup that uses clang. In particular this will work for projects that are invested in `clangd` or for projects that use https://.com/grailbio/bazel-toolchain. I could set the path correctly in a user-clangd settings file, but that could interfere with other projects if cross compile is in effect. So it appeared to me that the compiler override is fairly useful, even if only for edge or complex cases. And with that in mind I added the ability to provide additional flags as well. However I left out the ability to strip compiler flags as one can often just provide more clags to override prior flags. The most useful case for adding flags is actually to define or undefined macros when the code gets sent to `clangd`. For instance, if you switch between opt and dbg modes you might want to always enable debug symbols by providing the debug macro. Hope this clarifies why I added `--bcce-compiler` and `--bcce-copt` flags. cheers marcus …On Sat, Jun 3, 2023 at 2:26 AM Chris Sauer ***@***.***> wrote: Right there with you on "automatic but configurable" :) Thanks for your reply. Would still love to backtrace your needs on the other two--for my understanding if nothing else! (And I think I should ask if you'd still follow up on some of the other remaining details above, if that's not too annoying--maybe you just haven't gotten to them yet.) Cheers and thanks! Chris — Reply to this email directly, view it on <#122 (comment)>, or unsubscribe <https://.com/notifications/unsubscribe-auth/ABQ7NSPEGFE4FLB3CV4G3N3XJKAEJANCNFSM6AAAAAAYKQCJQE> . You are receiving this because you authored the thread.Message ID: ***@***.*** .com> |
Hey, Marcus! Sorry--there's still something I'm still not understanding. (And I need to make sure I understand needs before we extend the interface.) Won't $(which clang) will give the same result as clang? Looks like the wrapper is around macOS CommandLineTools's clang, but that the other path is (maybe) for brew-installed-llvm clang? |
Add flags for custom output control.
--bcce-color[=no]A boolean flag that enables or disables colored output. This is useful for environments where the color codes are not handled (e.g. VSCode output window).--bcce-compiler[=compiler]Allows to override the detected compiler. This is helpful if the compiler found in the editor environment is different from the compiler that should be used forcompile_commands.json.--bcce-copt[=option]Enables passing additionaloptions to arg lists incompile_commands.json(can be repeated).Unlike in other PRs here all flags are prefixed with '--bcce-' (for Bazel-Compile-Command-Extractor) so they can easily be distinguished and filtered out.