microsoft / vscode-makefile-tools

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

Changing integrated terminal breaks build commands #527

Closed ingarpedersen closed 6 months ago

ingarpedersen commented 7 months ago

I have changed the integrated terminal to cmder. After I made this change makefile tools build target command fails:

 *  Executing task: ""E:\Proj\testproj\lib\make.exe" "all"" 

'D:\Program' is not recognized as an internal or external command,
operable program or batch file.

E:\Proj\testproj\lib>

If I switch back to command prompt as terminal build works just fine. If I run "make all" in cmder it builds just fine.

So it appears makefile tools picks up some strange settings when using cmder as integrated terminal. The makefile tools output does not contain an error, the last 3 lines are:

Executing task: "Makefile Tools Build Task" with quoting style "Strong"
 command name: E:\Proj\testproj\lib\make.exe
 command args all

And it is stuck there until I cancel the build using the cancel button in the popup window. I have enabled the other log file options, but nothing is written to them.

Makefile tools: v0.7.0 VS Code: 1.84.1

Possibly related issues: https://github.com/microsoft/vscode-makefile-tools/issues/493 https://github.com/microsoft/vscode-makefile-tools/issues/481 https://github.com/microsoft/vscode-makefile-tools/issues/390

gcampbell-msft commented 7 months ago

@ingarpedersen Could you please provide a minimal repro project for us to investigate this with?

It seems like you might be right that something about the environment, or possibly our string escaping in other terminals, isn't working. However, since the other logging options didn't help, we'll likely need to be able to reproduce this ourselves to investigate.

Also, do you happen to know where the "D\Program" is coming from? Is that something that might be coming from our extension? Or is that maybe coming from a makefile or project setting of yours?

Any information and repro project that you can provide would help us investigate this. Thanks!

ingarpedersen commented 7 months ago

I have search through the project several time scanning for any reference to D: and/or Program without finding anything. I do see that if I run a "Make configure" the dry run completes without errors, so apparently, this only happens with a make all.

I have also made a new project, and stripped the makefile down to:

    all: Makefile
    @echo "Making all"

make all in the terminal still works fine, but trying to build with the makefile tools gives the same error as described above, so it would seem the issue is not in any of my files, it has to come from somewhere else.

ingarpedersen commented 7 months ago

I have found the problem... As stated I had installed cmder and replaced it as my terminal of choice in VSCode. cmder was installed in: "D:\Program Files\cmder" and in the integrated terminals list cmder was defined as:

"Cmder": {
            "name": "Cmder",
            "path": [
                "${env:windir}\\Sysnative\\cmd.exe",
                "${env:windir}\\System32\\cmd.exe"
            ],
            "args": ["/k", "${env:cmder-root}\\vendor\\bin\\vscode_init.cmd"],
            "icon": "terminal-cmd",
            "color": "terminal.ansiGreen"
        },

cmder-root was defined as "D:\Program Files\cmder". If I replaced the cmder-root variable with the actual path, and added \" to the path to encapsulate the space, in other words, I changed the args line above to: "args": ["/k", "D:\\\"Program Files\"\\cmder\\vendor\\bin\\vscode_init.cmd"] It works again. I could probably also have modified the cmder-root variable in the same way.

Ingar

ingarpedersen commented 7 months ago

So the original path works in vscode (can start new terminal, debug in terminal etc), but the makefile tools extension appears to handle the path differently.

Ingar

gcampbell-msft commented 7 months ago

@ingarpedersen Ahh, this is very useful information! This may have something to do with how we handle our paths, we may need to add quotes around the path we get to make sure that it's properly handled. Thanks for the additional information.

gcampbell-msft commented 6 months ago

@ingarpedersen To help me investigate this, could you provide a repro sample? Currently I don't believe we necessarily support modifying the terminal (as I've mentioned in the other bugs referenced), but it may be that we have a small amount of support. The easiest way to tell would be for you to create a repro and share it with me so that I can know the exact scenario that you're hitting.

Thanks!

ingarpedersen commented 6 months ago

Should be fairly easy to reproduce. This is my list of profiles from the settings.json:

    "terminal.integrated.profiles.windows": {
        "Cmder": {
            "name": "Cmder",
            "path": [
                "${env:windir}\\Sysnative\\cmd.exe",
                "${env:windir}\\System32\\cmd.exe"
            ],
            "args": ["/k", "D:\\\"Program Files\"\\cmder\\vendor\\bin\\vscode_init.cmd"],
            "icon": "terminal-cmd",
            "color": "terminal.ansiGreen"
        },
        "PowerShell": {
            "source": "PowerShell",
            "icon": "terminal-powershell"
        },
        "Command Prompt": {
            "path": [
                "${env:windir}\\Sysnative\\cmd.exe",
                "${env:windir}\\System32\\cmd.exe"
            ],
            "args": [],
            "icon": "terminal-cmd"
        },
        "Git Bash": {
            "source": "Git Bash"
        }
    },

Selecting "Cmder" as terminal, and not escaping the path in args, like in my settings above, triggers the issue. I have not tested if the same is the case if the path contains space. It could be that it only affect args...(?)

If you want me to send you more let me know exactly what you need.

gcampbell-msft commented 6 months ago

@ingarpedersen If you add \" before and after the ${env:cmder-root}, does this fix your issue?

gcampbell-msft commented 6 months ago

Either way, I have tested this with your settings.json setting from this comment: https://github.com/microsoft/vscode-makefile-tools/issues/527#issuecomment-1841476068, and I also tested by adding a reference to cmder-root to the "Command Prompt" profile, and it also fails there. Also, I found these two links: https://github.com/Microsoft/vscode/issues/36733, https://stackoverflow.com/questions/67171114/vscode-shell-task-escaping-single-quotes, and it seems like the issue is actually in how VSCode interprets it's variables. Hopefully the stack overflow link can help you workaround this?

Let me know if these suggestions helped at all!

ingarpedersen commented 6 months ago

Adding \" before and after ${env:cmder-root} does not appear to work. I think I'll stick with an escaped fixed path, which does work. I tried a couple of the suggestions from the SO link, but could not get those to work either, but did not spend to much time on it since I do have something that does work.

From https://github.com/microsoft/vscode/issues/36733 it would appear that escaping of arguments is left to the user.

Thanks.

gcampbell-msft commented 6 months ago

@ingarpedersen Thanks for the follow up. Since it seems that vscode leaves the escaping of arguments to the user, I'll close this issue.

There are other issues on this repo that we're tracking for adding support for using the terminal profiles for things like build. Thanks!