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

Support using a "clean" environment for the terminal #70248

Closed
praenubilus opened this issue Mar 11, 2019 · 35 comments · Fixed by #75376
Closed

Support using a "clean" environment for the terminal #70248

praenubilus opened this issue Mar 11, 2019 · 35 comments · Fixed by #75376
Assignees
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities linux Issues with VS Code on Linux macos Issues with VS Code on MAC/OS X on-testplan terminal Integrated terminal issues
Milestone

Comments

@praenubilus
Copy link

praenubilus commented Mar 11, 2019

  • VSCode Version: 1.30, 1.31, 1.33
  • OS Version: Ubuntu 18.04/ MacOS 10.14 Mojave

Steps to Reproduce:

  1. From OS terminal, type echo $PATH, you may see values similar to following:
/Users/xxx/anaconda3/bin:/Users/xxx/anaconda3/condabin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin
  1. Open a vscode instance, in the integrated terminal type echo $PATH, now the value may look like following:
/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Users/xxx/anaconda3/bin:/Users/xxx/anaconda3/condabin

The CONTENT in two $PATH values are the same, BUT in different order. Problems it may cause: some system configuration depends on the first executable value they can find in the $PATH. Different $PATH value order will have different default program being used. In previous example, the value in step 1 will use python3 and pip3 from anaconda. But in vscode integrated terminal, the $PATH value in step 2 will use the python2 and pip2 in /usr/local/bin. Even you activate a virtualenv in integrated terminal, it will still use the wrong virtual env. More details and a proposed workaround can be seen in vscode-python #4434.

This has been observed on both MacOS and Ubuntu. I don't have windows computer, but I guess that may also have the same issue. Some related issues can be found in this issue vscode-python #4668.

Now I'm trying to fix this issue with a pull request. But I need some help to indicate me to the correct location in the code base. I also addressed this question on StackOverflow:

Through my own observation, I found the root cause may be: after some steps, somewhere during the create terminal process, the actual environment variable values(specific to PATH) has been reordered/changed. Maybe it is being processed for remove duplication.

I download the vscode codebase(1.33) and follow the debugging until method createProcess in source file src/vs/workbench/contrib/terminal/electron-browser/terminalProcessManager.ts. What I have found is: even after the terminal instance has been created, the env.PATH in the main process has NOT been changed. For ptyProcess, the constructor takes options(which contains env) as input, but I cannot find any env related property in created ptyProcess. And after the terminal instance created, the debugging ends, integrated terminal appears in the workbench. I get lost after terminal instance created, can someone from the core team or used to working on integrated terminal module indicate me to the right place in the codebase so I can try to solve the issue? There is not enough documents on internet or comments in the source code can help me find/locate the environments changing during/after the terminal instance creation.

@vscodebot vscodebot bot added the terminal Integrated terminal issues label Mar 11, 2019
@praenubilus
Copy link
Author

praenubilus commented Mar 11, 2019

Also, if I want to debug vscode platform source code, where should I expect a User Settings or Workspace Settings so I can add some customized stuff in it?

@DonJayamanne
Copy link
Contributor

Current workaround is:

"terminal.integrated.env.osx": {
		"PATH": ""
}

@Tyriar
Copy link
Member

Tyriar commented Mar 13, 2019

The reason this happens is because on mac and Linux there are two ways to launch VS Code:

  • From a terminal (ie. the environment has already been setup by running .bashrc/.bash_profile
  • From the "dash" in which case VS Code runs $SHELL -ilc to source the "development environment"

This sets up the main process environment which is inherited by each VS Code window, its tasks, debug sessions and terminals. Below explain this in detail:

Ubuntu launching from terminal

  • Ubuntu (log in sources ~/.bash_profile)
    • Terminal (sources ~/.bashrc)
      • VS Code main process
        • VS Code renderer process*
          • VS Code terminal (sources ~/.bashrc)
          • VS Code task*
          • VS Code debug session*

Ubuntu launching from dash

  • Ubuntu (log in sources ~/.bash_profile)
    • VS Code main process (sources ~/.bash_profile because VSCODE_CLI is not set)
      • VS Code renderer process*
        • VS Code terminal (sources ~/.bashrc)
        • VS Code task*
        • VS Code debug session*

macOS launching from terminal

  • macOS
    • Terminal (sources ~/.bash_profile)
      • VS Code main process
        • VS Code renderer process**
          • VS Code terminal (sources ~/.bash_profile)
          • VS Code task**
          • VS Code debug session**

macOS launching from dock

  • macOS
    • VS Code main process (sources ~/.bash_profile because VSCODE_CLI is not set)
      • VS Code renderer process**
        • VS Code terminal (sources ~/.bash_profile)
        • VS Code task**
        • VS Code debug session**

* needs ~/.bashrc already sourced
** needs ~/.bash_profile already sourced

The underlying problem is:

When a terminal is launched, it needs to run its launch script again; .bashrc on Linux and .bash_profile on macOS (because terminal.integrated.osx.shellArgs defaults to [ "-l" ]). This will shuffle the environment based on what the user's scripts does, and can mess things up if their script is not idempotent.

So as for fixing this issue once and for all this will be a little tricky, unfortunately it's not just as easy as holding onto the original environment and using that for the terminal instead of the one generated by getShellEnvironment, that will only work when launching VS Code from the dock/dash but not when run through the terminal.

I propose we fetch the environment of the parent process of the code executable which will probably be systemd on Linux or the root process on macOS, we can do this on Linux by running and parsing cat /proc/<pid>/environ on Linux, and ps eww <pid> on macOS.

It's worth noting that this could break some people's set up and potentially cause some other unexpected issues as before it was an assumption that the terminal process uses the environment of VS Code's window. Maybe this should even be an option in settings, something like "terminal.integrated.useWindowEnvironment": false?

@Tyriar Tyriar added the feature-request Request for new features or functionality label Mar 13, 2019
@Tyriar Tyriar added this to the Backlog milestone Mar 13, 2019
@Tyriar Tyriar changed the title Inconsistent Environment Variable Values in Integrated Terminal Support using a "clean" environment for the terminal Mar 13, 2019
@Tyriar
Copy link
Member

Tyriar commented Mar 13, 2019

@praenubilus if you wanted to put together a PR for this I would stick the fetching of the environment code insider electron-browser/terminalService and grab that inside TerminalProcessManager instead of basing it on process.env here:

https://github.com/Microsoft/vscode/blob/d3443fd68d625c980325e1571066850e943dfbef/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts#L128

I think for Windows we always want the old behavior.

@Tyriar Tyriar added the help wanted Issues identified as good community contribution opportunities label Mar 13, 2019
@praenubilus
Copy link
Author

@Tyriar Thanks for the information. Can you answer my another question: if I want to debug with customized settings, (for example "terminal.integrated.env.osx": { "PATH": ""} ) where should I expect a User Settings or Workspace Settings file I can play with?

@Tyriar
Copy link
Member

Tyriar commented Mar 14, 2019

@praenubilus I haven't touched any debugging stuff with extensions, this might be relevant?

https://github.com/Microsoft/vscode/blob/1615a36eb45182de5e34c53cfdd5b3bce8cddbf3/src/vs/vscode.d.ts#L8484-L8500

@auchenberg
Copy link
Contributor

Ping @DonJayamanne

@DonJayamanne

This comment has been minimized.

@bandrei
Copy link

bandrei commented Mar 19, 2019

Not sure if this helps with your assessment @Tyriar however, my bash_profile sources my bashrc file. If VSCode is to load bash_profile in either scenarios (dock vs terminal), why is the VSCODE_CLI variable not being set affect the execution order of the sourcing?

@Tyriar
Copy link
Member

Tyriar commented Mar 21, 2019

@DonJayamanne can you elaborate a little? Why don't you do that now? Do you expect users to setup the venv in a terminal and launch code from the terminal or something?

@bandrei the issues typically arise because the scripts are run multiple times, or at least a different number of times to what a standalone terminal does.

@praenubilus
Copy link
Author

praenubilus commented Mar 21, 2019

@DonJayamanne can you elaborate a little? Why don't you do that now? Do you expect users to setup the venv in a terminal and launch code from the terminal or something?

@bandrei the issues typically arise because the scripts are run multiple times, or at least a different number of times to what a standalone terminal does.

@Tyriar , I get what @bandrei is talking about. In my example at the beginning of the issue, my zshrc or bashrc only have the following command to operate on the $PATH environment variable.

export PATH="/home/xxx/anaconda3/bin:$PATH"

For this statement, even it has been run multiple times, /home/xxx/anaconda3/bin should always appear at the beginning of the $PATH in integrated terminal.

And the line/place you indicate me,

vscode/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts

Line 128 in d3

I have debugged tons of times and quite sure at that line the environment variable value has not been changed. And actually the environment variable is always in correct/expected order even after the createProcess function invocation. My own observation for the environment variable works in vscode terminal is:
Main UI->Terminal Tab->Terminal Process->ptyProcess. Then I got lost after createProcess function invocation. Since after that, everything has set, I don't know where to monitor the change of environment variables other than the env in main process(but I guess it will not being used by terminal anymore). However, the final PATH environment variable value order has been changed when the UI appears. So I have two questions(they are the same as what I want to ask in my original question):

  1. WHERE is the final environment variables we use in the integrated terminal stored? Can you indicate me to the exact class/variable?
  2. After terminal tab created, is there any possible places in the code will re-order the environment variable?

@DonJayamanne
Copy link
Contributor

@Tyriar apologies for the previous comment, please ignore that. I was replying to another comment if yours that wasn't for me.

I propose we fetch the environment of the parent process of the code executable which will probably be systemd on Linux or the root process on macOS, we can do this on Linux by running and parsing cat /proc//environ on Linux, and ps eww on macOS.

Sounds great

It's worth noting that this could break some people's set up and potentially cause some other unexpected issues as before it was an assumption that the terminal process uses the environment of VS Code's window. Maybe this should even be an option in settings, something like "terminal.integrated.useWindowEnvironment": false?

I like this approach.

@Tyriar
Copy link
Member

Tyriar commented Mar 23, 2019

Then I got lost after createProcess function invocation

@praenubilus createProcess creates the process and the shell scripts launch. To gleam some insights into what's happening after, you can pepper some echo calls throughout your .bashrc/.bash_profile/.profile/etc.

@Tyriar
Copy link
Member

Tyriar commented Mar 23, 2019

This could also solve #47816 by reloading the environment, we could also support doing this on Windows by fetching a clean environment block.

@ladyrick
Copy link

The underlying problem is:

When a terminal is launched, it needs to run its launch script again; .bashrc on Linux and .bash_profile on macOS (because terminal.integrated.osx.shellArgs defaults to [ "-l" ]). This will shuffle the environment based on what the user's scripts does, and can mess things up if their script is not idempotent.

So as for fixing this issue once and for all this will be a little tricky, unfortunately it's not just as easy as holding onto the original environment and using that for the terminal instead of the one generated by getShellEnvironment, that will only work when launching VS Code from the dock/dash but not when run through the terminal.

I propose we fetch the environment of the parent process of the code executable which will probably be systemd on Linux or the root process on macOS, we can do this on Linux by running and parsing cat /proc/<pid>/environ on Linux, and ps eww <pid> on macOS.

@Tyriar Hi, I noticed something new. See this video: https://ladyrick.github.io/2/index.html.
According to this video, I think maybe this problem is nothing to do with whether the .zshrc is idempotent or not and its father process. Probably something is wrong while starting vscode and parsing .zshrc.

And this problem also exists when no extensions are enabled.

@Tyriar
Copy link
Member

Tyriar commented Apr 16, 2019

@ladyrick the difference could depend on whether the external terminal launches as a login shell or not and what terminal.integrated.shellArgs.osx is set to.

Probably something is wrong while starting vscode and parsing .zshrc.

VS Code itself doesn't touch .zshrc, it simply launches zsh with a set of args (which is in an environment that has already had some script sourced as described above).

@Tyriar Tyriar added the macos Issues with VS Code on MAC/OS X label Jun 12, 2019
@Tyriar Tyriar modified the milestones: Backlog, June 2019 Jun 12, 2019
@Tyriar Tyriar closed this as completed in 86aa7ae Jun 12, 2019
@Tyriar
Copy link
Member

Tyriar commented Jun 12, 2019

@DonJayamanne just exposed "terminal.integrated.inheritEnv" which currently only affects mac and Linux (Windows deferred to #47816). It currently defaults to true, let me know if setting it to false works, seems like it should do the trick. If all is good we should probably switch the default over in a later version, after warning in the release notes of the upcoming change.

One caveat with this is that on macOS I couldn't find a reliable way of pulling the environment, because of this I just use the root environment which always seems to be {}. In other words, if you launch a terminal, export some env in there, the terminal won't pick that up.

@Tyriar
Copy link
Member

Tyriar commented Jun 13, 2019

Actually we do need to detect for mac, looking into it but this is ready to test on Linux.

@Tyriar Tyriar reopened this Jun 13, 2019
@Tyriar
Copy link
Member

Tyriar commented Jun 13, 2019

Couldn't figure it out for mac, doesn't seem possible to get the environment before the launch CLI has run (and tampered with the environment) and /system/launchd's environment always says it's nothing with ps e -o command. I did a workaround to cherry pick a small set of environment variables off the current process.

@Tyriar Tyriar closed this as completed Jun 13, 2019
Tyriar added a commit that referenced this issue Jun 13, 2019
@hologerry
Copy link

hologerry commented Jun 14, 2019

After set

"terminal.integrated.env.osx": {
		"PATH": ""
}

I can not build cpp file:
Screen Shot 2019-06-15 at 00 29 29

@Tyriar
Copy link
Member

Tyriar commented Jun 14, 2019

@hologerry try setting terminal.integrated.inheritEnv to false using the Insiders build and let me know if that works.

@hologerry
Copy link

@Tyriar With the Insiders build and setting terminal.integrated.inheritEnv to false.
It works! Thanks!

@Tyriar
Copy link
Member

Tyriar commented Jun 17, 2019

While inheritEnv false on macOS is better than the current workaround (setting PATH to ""), I suspect that it probably isn't good enough of a fix to switch the default over as the initial PATH will be completely empty, so things like sh won't be available (unless the login shell adds them).

@Tyriar
Copy link
Member

Tyriar commented Jun 17, 2019

Actually scratch that, I think that's how it's meant to be and it's the reason why the default shell always has the absolute path (/bin/sh) instead of the shell name (sh). path_helper pulls the $PATH in from /etc/paths via /etc/profile.

@lwabish
Copy link

lwabish commented Jun 19, 2019

At first I used bash+vscode+conda,this problem appears and I fixed it by setting "terminal.integrated.env.osx": {"PATH": ""} in vscode and it works.
But this time I just change my main shell to zsh(also in vscode),this method has failed : vscode prompt something like "zsh -l can't start".
@Tyriar I tried using vscode insider edition and set "terminal.integrated.inheritEnv": false,,this works

@ieipi
Copy link

ieipi commented Jul 5, 2019

using vscode on mac and set "terminal.integrated.inheritEnv": false,,this works;

@ladyrick
Copy link

using vscode on mac and set "terminal.integrated.inheritEnv": false,,this works;

But sometimes I really need to inheritEnv.
inheritEnv: false will make something goes wrong.

@DonJayamanne
Copy link
Contributor

inheritEnv: false will make something goes wrong.

Please could you provide more details on what this something is.

@ladyrick
Copy link

Please could you provide more details on what this something is.

In remote development, "inheritEnv: false" will make "$PATH" wrong.
Don't know why.
Maybe it's a bug of remote dev?

@Tyriar
Copy link
Member

Tyriar commented Jul 19, 2019

@ladyrick what's your OS/version?

@praenubilus
Copy link
Author

Please could you provide more details on what this something is.

In remote development, "inheritEnv: false" will make "$PATH" wrong.
Don't know why.
Maybe it's a bug of remote dev?

Can you add the following line at the end of your zshrc or bashrc?

echo $PATH

with this line, every time integrated terminal will display PATH value if the script being sourced/run, if the terminal has been printed more than once, and it may help you to locate the problem.

@vscodebot vscodebot bot locked and limited conversation to collaborators Jul 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities linux Issues with VS Code on Linux macos Issues with VS Code on MAC/OS X on-testplan terminal Integrated terminal issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

11 participants