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

Use $HOME instead of ~ in PATH env variable #367

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

blanquicet
Copy link
Member

Hi folks,

I was running some experiments with the InnovationEngine project, which leverage the CloudShell to execute the documentation, and I faced a problem with the the PATH env variable. In particular, I noticed some paths within PATH are using ~ instead of $HOME and some tools don't parse ~ thus they aren't able to execute the binaries located on those paths. An example of the tools impacted is kubectl. Here the issue reported upstream. The solution is to use $HOME instead of ~, which shouldn't have any impact on tools already working with ~. I tried in other online shells and systems I could try, and none of them are using ~ within PATH.

Thanks

@blanquicet blanquicet marked this pull request as draft December 1, 2023 15:16
@blanquicet blanquicet force-pushed the jose/do-not-use-tilde-in-path branch from 7169fa0 to ca91064 Compare December 1, 2023 15:46
@darrentu
Copy link
Contributor

darrentu commented Dec 6, 2023

Hi @blanquicet, thank you for making a PR. We will look into this and get back to you!

@blanquicet
Copy link
Member Author

Hi @blanquicet, thank you for making a PR. We will look into this and get back to you!

Thanks @darrentu. I haven't marked it as ready because I'm still trying to understand how to fully test it and verify that after this PR I will be able to execute binaries within $HOME/.local/bin/. Any suggestion?

@darrentu
Copy link
Contributor

darrentu commented Dec 6, 2023

@blanquicet
Copy link
Member Author

blanquicet commented Dec 7, 2023

The test cases are passing: https://github.com/Azure/CloudShell/actions/runs/7062144530/job/19225319429.

Yep, but they are only testing that the PATH contains the correct paths, not that the binaries inside those paths can be executed. Let me investigate a little bit more this. I think I found an issue.

@blanquicet blanquicet marked this pull request as ready for review December 7, 2023 17:38
@blanquicet blanquicet marked this pull request as draft December 7, 2023 20:18
@blanquicet blanquicet force-pushed the jose/do-not-use-tilde-in-path branch from ca91064 to b9c28cf Compare December 7, 2023 23:49
Signed-off-by: Jose Blanquicet <josebl@microsoft.com>
@blanquicet blanquicet force-pushed the jose/do-not-use-tilde-in-path branch from b9c28cf to 2918057 Compare December 8, 2023 00:40
@blanquicet
Copy link
Member Author

blanquicet commented Dec 8, 2023

Hi @darrentu, I found the issue in my previous approach. I was adding the $HOME string literally in the PATH env variable and it was not being evaluated within the container. Now, I'm adding it with the .bashrc script which is executed every time a new shell is opened within the container so it will be able to resolve $HOME correctly for each user.

Now the PR is ready.

@blanquicet blanquicet marked this pull request as ready for review December 8, 2023 01:22
@darrentu darrentu self-assigned this Dec 10, 2023
@blanquicet
Copy link
Member Author

Hi @darrentu, friendly ping to have this PR merged 🙂. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants