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

expand: environment variables are no longer case insensitive on Windows #325

Closed
RossHammer opened this issue Nov 24, 2018 · 3 comments
Closed
Milestone

Comments

@RossHammer
Copy link

When I run commands on my system I get the following when trying to run any exe

gosh -c "go version"
"go": executable file not found in $PATH
exit status 127

After doing some digging I noticed my path environment variable is Path not PATH for some reason. This is a relatively fresh install of windows as well. Once I changed it to PATH everything started working better

@mvdan
Copy link
Owner

mvdan commented Nov 24, 2018

Interesting - thanks for the report. We already do test on Windows via Travis, but as you can imagine, Windows environments can vary quite a lot.

I understand that environment variables in Windows are case insensitive, whereas they are case sensitive on unix-like systems. That is, os.Getenv("PATH") should succeed on Windows even if one has only set Path.

We used to take this into account when constructing an interpreter, but it seems like that piece of code was dropped recently. I can't think why I'd remove it, so I think it was just a mistake. I'll re-add it and add a regression test.

@mvdan mvdan changed the title Cannot find any executables on windows expand: environment variables are no longer case insensitive on Windows Nov 24, 2018
@mvdan mvdan added this to the 2.6.2 milestone Nov 24, 2018
@mvdan mvdan closed this as completed in f069fb5 Nov 24, 2018
mvdan added a commit that referenced this issue Nov 24, 2018
We were doing this in the interp package before the code was extracted
into the expand package. Getting rid of that was a mistake, and led to
subtle bugs on plain Windows.

To prevent this from happening again, add a regression unit test, as
well as a full interpreter test via os.Setenv in the interp package.

Fixes #325.
@mvdan
Copy link
Owner

mvdan commented Nov 24, 2018

@RossHammer could you please test a master build? I'm fairly positive that the commit above should fix it for you, but I'd like confirmation from you before doing a bugfix release.

@RossHammer
Copy link
Author

That seems like it fixed it. 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

No branches or pull requests

2 participants