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

Something in PATH is deleted in Container When using Remote-Container #544

Closed
yejunjin opened this issue Jun 2, 2019 · 18 comments
Closed
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release containers Issue in vscode-remote containers important Issue identified as high-priority verified Verification succeeded
Milestone

Comments

@yejunjin
Copy link

yejunjin commented Jun 2, 2019

  • VSCode Version: VSCode insider 1.35.0
  • Local OS Version: Windows10 Pro
  • Remote OS Version: Debian with Rust
  • Remote Extension/Connection Type: Docker

Steps to Reproduce:

  1. Follow the Try a development container tutorial
  2. when I choose try rust, open folder in container after container build, the cargo, rls are not in PATH. I found that PATH is changed by the Remote-Container plugin.

When I start to run a new container generated by the same image in Powershell, the $PATH is:
/usr/local/cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin

When I open folder in Container and using internal terminal in vscode , the $PATH is:
/root/.vscode-server-insiders/bin/0284236851a94b116f468345f6e98688a737015d/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin

Does this issue occur when you try this locally?: Yes
Does this issue occur when you try this locally and all extensions are disabled?: Yes

@yejunjin yejunjin changed the title Something in PATH is deleted in Container When using Remote-Container Something in PATH is deleted in Container When using Remote-Container #container Jun 2, 2019
@yejunjin yejunjin changed the title Something in PATH is deleted in Container When using Remote-Container #container Something in PATH is deleted in Container When using Remote-Container Jun 2, 2019
@egamma
Copy link
Member

egamma commented Jun 3, 2019

@Chuxel I assume this is you, since the last commit is from you

@egamma egamma added the containers Issue in vscode-remote containers label Jun 3, 2019
@chrmarti
Copy link
Contributor

chrmarti commented Jun 3, 2019

Caused by https://github.com/microsoft/vscode-distro/commit/f1b3a1410f60f9fb123b1b8d2b27af9137009783 overwriting the PATH. This needs a more thinking.

@chrmarti chrmarti self-assigned this Jun 3, 2019
@chrmarti chrmarti added the bug Issue identified by VS Code Team member as probable bug label Jun 3, 2019
@chrmarti
Copy link
Contributor

chrmarti commented Jun 3, 2019

(/fyi @roblourens @aeschli PATH being overwritten unexpectedly.)

@roblourens
Copy link
Member

Where is the cargo path added, and do you know why the spawned shell doesn't have that path?

Do we need to fix this for May?

@roblourens
Copy link
Member

The agent's path is explicitly copied to the spawned process, so I'm confused as to why it's lost by this process. I can't reproduce it in a standalone script (but I can reproduce the issue with a devcontainer)

@roblourens
Copy link
Member

And the other variables like CARGO_HOME are there, just not PATH.

This is because shellEnv.ts is spawning a login shell which starts with fresh values for the general environment variables, and inherits the rest. https://wiki.debian.org/EnvironmentVariables

This means that we have to merge PATH manually. Will have to split the path, dedupe the components, put them together in the right order. But if the user's environment removed something from the PATH which exists in the agent's PATH, that would be an issue.

@roblourens roblourens added this to the May 2019 milestone Jun 3, 2019
@roblourens roblourens self-assigned this Jun 3, 2019
@roblourens roblourens added the candidate Issue identified as probable candidate for fixing in the next release label Jun 3, 2019
@roblourens
Copy link
Member

Discussing with @Tyriar, we think this only really affects containers which can set the global environment outside of .bashrc or .profile, etc. Of the ones we checked, Rust is the only one that uses that feature. e.g. you can see docker image inspect rust:1 sets config.env.PATH. The easiest and lowest impact fix would be to disable this user environment merging entirely for containers with a flag to the agent.

And I think that the container extension never launched server.sh with -il the way that ssh and wsl did, so disabling this would just put us exactly back to where we were before my change. Is that correct @chrmarti? If so that would be a good fix for May. I don't want to break everyone's Rust container.

@roblourens
Copy link
Member

And other alternatives, include a flag in devcontainer.json to disable this, or merge PATH. But I'm not sure it's useful in general in containers.

@Tyriar
Copy link
Member

Tyriar commented Jun 3, 2019

We also talked about whether the flag should be an entry in the devcontainer.json but I think this is just how docker works and you're now meant to launch a login shell there. @chrmarti was containers ever impacted by #437?

@Chuxel
Copy link
Member

Chuxel commented Jun 3, 2019

Of the ones we checked, Rust is the only one that uses that feature. e.g. you can see docker image inspect rust:1 sets config.env.PATH

@roblourens @Tyriar This looks like it affects a number of things. Java also appears to be affected. If I use the vscode-remote-try-java repo, which java returns nothing. The reason it appears to work is JAVA_HOME is set and picked up by VS Code, but the terminal won't work.

In addition, some of these may be falling back to other locations that make them appear to work but are not (e.g. there's an OS installed python) .

Rust is also mentioned in Hanselman's blog post and our getting started docs, so we definitely want this to work. The vscode-remote-try-rust repo is broken right now. Java and others are likely not behaving the way they are supposed to. Adding "important".

@Chuxel Chuxel added the important Issue identified as high-priority label Jun 3, 2019
@yejunjin
Copy link
Author

yejunjin commented Jun 4, 2019

I also find if I do not add ENV SHELL /bin/bash in Dockerfile, the PATH variable is ok.

@chrmarti chrmarti closed this as completed Jun 4, 2019
@isidorn
Copy link

isidorn commented Jun 4, 2019

Verified by checking that cargo is now on the path. Adding verified label
Screenshot 2019-06-04 at 15 04 52

@isidorn isidorn added the verified Verification succeeded label Jun 4, 2019
@yejunjin
Copy link
Author

yejunjin commented Jun 4, 2019

Verified by checking that cargo is now on the path. Adding verified label
Screenshot 2019-06-04 at 15 04 52

Hi ! The PATH is ok if you login container using sh instead of bash even if the bug is not repaired! try to use /bin/bash to login instead and verify it.

@isidorn
Copy link

isidorn commented Jun 4, 2019

Also works with sh.
By the way you can also verify using latest vscode insiders @eadren https://code.visualstudio.com/insiders/

Screenshot 2019-06-04 at 15 40 56

@scotty-c
Copy link

I am having a similar issue with the vscode terminal and the golang remote container that go was no longer in the PATH. I was able to troubleshoot the issue and found that it was the bash settings I had set in vscode for my local terminal being copied over to the container and overriding the PATH.

The settings in my user settings are

 "terminal.integrated.shell.linux": "/usr/bin/env",
 "terminal.integrated.shellArgs.linux": ["bash",  "-l"],

Is there a way I can stop the terminal from loading my local settings in the remote container ?

@Tyriar
Copy link
Member

Tyriar commented Jun 24, 2019

@scotty-c as I understand it, that is the problem that the new "machine-scoped" settings are meant to solve. However, categorizing settings as machine-scoped will prevent workspace settings from working which cannot happen for the terminal settings as it would be a change in behavior since setting your terminal config in workspace settings is perfectly valid.

Your only workaround is to define the settings in your remote settings.json if you define them in your user settings right now as they would then be overridden. FYI @sandy081

@scotty-c
Copy link

@Tyriar Do you have an example of how I would set the terminal settings back to default in the remote settings.json ?

@Tyriar
Copy link
Member

Tyriar commented Jun 25, 2019

@scotty-c setting to null should do it, that's the default value in now

@vscodebot vscodebot bot locked and limited conversation to collaborators Jul 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release containers Issue in vscode-remote containers important Issue identified as high-priority verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

8 participants