microsoft / vscode-makefile-tools

MAKE integration in Visual Studio Code
Other
184 stars 55 forks source link

Account for redirection characters when parsing compiler commands (we already do for linker commands) #518

Open b-spencer opened 8 months ago

b-spencer commented 8 months ago

When running with a Bourne-like shell such as bash, it is possible to prefix commands with a per-command environment variable, as in:

LANG=C $(CXX) -o file.o -c file.cc

or, perhaps more convincingly

INCLUDE="dir1;dir2;dir3" cl.exe

or dare I say

WINEDEBUG=-all wine someWindowsSpecificCompiler.exe

Sometimes, there's good reason to do this in a Makefile recipe. Unfortunately, it seems that Makefile Tools does not expect such command lines. It omits any such commands from compile_commands.json.

To be clear, I do not expect Makefile Tools to capture the environment variable or set it in compile_commands.json. I doubt that any language servers can make use of them. For some of my examples, a compilation database (CDB) without the environment variables applies will have incomplete or incorrect commands, but at least the commands are present. Having these commands silently ignored makes project setup more difficult. And, sometimes, ignoring the environment variables is fine and the resulting CDB will work.

I am not sure if Makefile Tools ignores such commands intentionally. If so, I didn't find documentation of this.

I put together a tiny project that shows the issue.

  1. Run "Makefile: Configure Clean" on the project.

  2. Observe that .vscode/compile_commands.json contains only rules for prog.cc:

$ grep '"command"' .vscode/compile_commands.json
        "command": "g++ -c -o prog.o prog.cc",
  1. Erase the VARIABLE=value part from the Makefile (on line 14).

  2. Re-run "Makefile: Configure Clean".

  3. Observe that .vscode/compile_commands.json now contains rules for both prog.cc and other.cc.

$ grep '"command"' .vscode/compile_commands.json
        "command": "g++ -c -o prog.o prog.cc",
        "command": "g++ -c -o other.o other.cc",

Should Makefile Tools handle such lines? Perhaps it should capture the line without the variable-setting pieces (when such a remainder looks like a compilation command).

gcampbell-msft commented 8 months ago

@b-spencer Thanks for letting us know! I think this is worthing checking out when are next available, I've made sure that it is triaged and ready for assignment at our next available chance, based on priority with other issues.

andreeis commented 8 months ago

@b-spencer , thank you for pointing out this scenario. Indeed it is not covered. The extension sees something before the gcc and we ignore. Usually gcc needs to be first in line, or we have a few other special cases of what we allow to be in front of. Definitely, we will add this scenario to the list. I will double check later but in the meantime, if you use & or && in between the variable setting and the gcc invocation, the extension would see the compiler line. I'll refresh my knowledge of how & and && work regarding if the variable setting would be persisted by the time gcc is run... but you can also quickly confirm if this unblocks you until we fix our parser to recognize a variable setting right before a compiler invocation.

b-spencer commented 8 months ago

Thanks for considering changes to handle this case!

When setting variables in this way, they affect only the individual command (i.e. a single process started by the shell), so separating the variable setting from the command with && won't have the same effect. (Note that the variables are not exported into the shell's environment; they're just added the the environment of the single command.)

I'm not blocked. I opened this issue so that others (including future me) wouldn't be as puzzled as to why some of the compilation commands were omitted from the generated CDB.

BTW I did not check if command-line capture does, or should, end at a pipe. Sometimes there are good reasons why compilation steps have their output piped. (Looking at you, MSVC's -showIncludes.)

andreeis commented 8 months ago

@b-spencer , thank you for clarifying the && scenario. I found another issue reported earlier on GitHub https://github.com/microsoft/vscode-makefile-tools/issues/185 and I would resolve this one as duplicate but your last comment made me look at our parsers again. We stop for pipe when parsing linker commands but not when parsing compiler commands. We need to do a similar check (also >, 1>, 2> besides |) for compiler commands. So let's leave this work item open for this exact issue and I'm renaming the title.