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 patching). --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 patches. (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 patches. (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://github.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 GitHub
<#122 (comment)>,
or unsubscribe
<https://github.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://github.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 GitHub
<#122 (comment)>,
or unsubscribe
<https://github.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.