Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto --query-driver #539

Open
sam-mccall opened this issue Sep 23, 2020 · 8 comments
Open

Auto --query-driver #539

sam-mccall opened this issue Sep 23, 2020 · 8 comments
Labels
enhancement New feature or request

Comments

@sam-mccall
Copy link
Member

sam-mccall commented Sep 23, 2020

Lots of people seem to need --query-driver and have trouble understanding how/when to use it.
We also have people expecting it to work in situations where it won't. Can we automate some of this?

If:

  • the compile command has an absolute argv0
  • we get missing-include diagnostics
  • the missing include is spelled with an angle bracket (or some stdlib heuristic)

Then:

  • prompt the user for permission to query the driver (window/showMessagePrompt)
  • add it to the whitelist and invalidate the cache
  • tell the user how to make it permanent (window/showMessage)

Not sure if we'll get to this soon, but it's a fun idea... @kadircet

@sam-mccall sam-mccall added the enhancement New feature or request label Sep 23, 2020
@kadircet
Copy link
Member

Yes I totally agree on making this smoother via window/showMessageRequest and permanent per-project via config stuff, rather than through command-line options (well, we can even take the liberty of updating/creating the config ourselves!!!).

I believe the conditions you mentioned are definitely the cases that users want this to automagically work. I would even go one step further and relax the first condition to argv0 exists and is executable.

I believe we didn't have the show message request at the time, hence we went down this path. As long as we get user consent, we should always lean towards the way easier for the user.

@cpsauer
Copy link

cpsauer commented Feb 3, 2021

#654 morphed into a duplicate of this issue. Definitely think there's a need here.

The main point raised (x2) there but not mentioned here: Query driver being default off may well not really provide the security benefit anticipated. For example, in VSCode, one could just set the flag in the Project/.vscode/settings.json and invoke it on startup. (Discussion evolved after I'd raised just turning the query driver on by default.)

Coincidentally, just noticed that a missing --query-driver is currently breaking Google's Bazel<->IntelliJ/Android Studio integration. Was able to spot that thanks to Sam's great explanations!

@cpsauer
Copy link

cpsauer commented Jun 23, 2021

A new vscode update I happened to see: There's now a popup when you open a directory, asking if you trust the code to execute files ("workspace trust")

@cpsauer
Copy link

cpsauer commented Mar 25, 2022

For anyone reading, looking to get query-driver to run automatically:
You can now just set --query-driver=** (e.g. in VSCode workspace settings) and have it always run by default.

@i-ky
Copy link

i-ky commented Mar 25, 2022

You can now just set --query-driver=/**/* (e.g. in VSCode workspace settings) and have it always run by default.

You have to mention that it isn't very secure.

@cpsauer
Copy link

cpsauer commented Mar 25, 2022

I mean, no more or less secure in the VSCode case, right, since the flag can be specified in a settings file in the project directory--all gated behind a trust prompt? (As above/linked).

More generally, presumably they're going to run the driver anyway to build the code and (in most cases) to work around the header flag issues. Seems worth it to me to have things work out of the box! We need --query-driver pretty often over on the Bazel-adapter side of things to pierce wrapper scripts. Though obviously super happy if there's a better solution.

@i-ky
Copy link

i-ky commented Oct 10, 2022

A new vscode update I happened to see: There's now a popup when you open a directory, asking if you trust the code to execute files ("workspace trust")

clangd/vscode-clangd#318 also talks about "workspace trust" feature.

@jlmxyz
Copy link

jlmxyz commented Feb 19, 2023

there could be a simple approach : if the executable match a *gcc *clangd *cc *c++ ... and so on (there aren't a lot of compilers, intel, gcc, clang... and anyway, you have to adapt to compiler specificities like -dM -E to get builtin defines, even if compilers tends to be more and more interchangeable) then treat it as the corresponding compiler (*gcc -> gcc...)

and if you want to add safety :

  • prevent execute program into source tree,
  • verify that the executable outside a source tree is a known workable compiler (by building a simple program) before adding it to a safe list...

Lunderberg added a commit to Lunderberg/settings that referenced this issue Aug 25, 2023
Because clangd does not automatically query the driver (see
clangd/clangd#539), must explicitly give it
permission to call the underlying compiler.  Even though that compiler
had just been executed anyways, and clangd is running under the exact
same user permissions as would be able to execute the actual compiler.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants