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

env_clear does not work on Windows #31259

Closed
Tracked by #3
nagisa opened this issue Jan 28, 2016 · 10 comments · Fixed by #86467
Closed
Tracked by #3

env_clear does not work on Windows #31259

nagisa opened this issue Jan 28, 2016 · 10 comments · Fixed by #86467
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. O-windows Operating system: Windows

Comments

@nagisa
Copy link
Member

nagisa commented Jan 28, 2016

Running Command with env_clear will reproducibly result in Error with OS code 87.

> cargo script -e '::std::process::Command::new(\"gcc.exe\").env_clear().output(
).unwrap()'
   Compiling expr v0.1.0 (file:///C:/Users/drk/AppData/Local/Cargo/script-cache/
expr-37b14ef2470ffe18)
thread '<main>' panicked at 'called `Result::unwrap()` on an `Err` value: Error
{ repr: Os { code: 87, message: "The parameter is incorrect." } }', ../src/libco
re\result.rs:688
An unknown error occurred

if env_clear is omitted, it works:

To learn more, run the command again with --verbose.
> cargo script -e '::std::process::Command::new(\"gcc.exe\").output().unwrap()'
   Compiling expr v0.1.0 (file:///C:/Users/drk/AppData/Local/Cargo/script-cache/
expr-67fa84126914720e)
Output { status: ExitStatus(ExitStatus(1)), stdout: "", stderr: "gcc.exe: fatal
error: no input files\r\ncompilation terminated.\r\n" }
@alexcrichton
Copy link
Member

This is expected behavior because it appears that there are basically some required environment variables on Windows to get subprocesses to work.

For example libuv has an explanation of this and a list of required variables which seems to match cygwin's list of required variables as well

@nagisa
Copy link
Member Author

nagisa commented Jan 28, 2016

Should we ensure that on windows we pass these variables through regardless of env_clear being specified? Otherwise, we should at least document this behaviour.

@alexcrichton
Copy link
Member

Perhaps yeah. In the past our spawning tended to mirror libuv closely (where it all originated from), but we've since backpedaled a bit from that original design. From a systems-y perspective I would prefer that env_clear actually clear the environment, but from a "let's make this ergonomic to use" perspective it'd make more sense to only clear non-required environment variables.

I'd probably personally be fine either way

@retep998
Copy link
Member

CreateEnvironmentBlock is used to create a new environment block. If FALSE is passed for bInherit then the returned block will "start from a clean state". If we're going to preserve a few required environment variables, might as well get them straight from the system rather than inheriting them.

@alexcrichton
Copy link
Member

Sounds reasonable to me!

@Mark-Simulacrum
Copy link
Member

I feel like this isn't a change we can make backwards compatibly. What are the considerations that are involved in making this change? Can we make it? We at least need documentation for this inconsistency, but I'm not sure what tag to use for those issues: T-doc?

@alexcrichton
Copy link
Member

I think we've got leeway to change the implementation on Windows to preserve some environment variables. If a process can just literally not be spawned without an environment variable then we're basically guaranteed to not break anyone if we start preserving it by default on env_clear. Existing working code would override it anyway (as it'd specify a value for the variable)

@sanmai-NL
Copy link

See the discussion in PowerShell/PowerShell#4671.

@meskill
Copy link

meskill commented Aug 11, 2023

The issue is closed, but with the fix related to running process on windows with empty environment that seems not quite exactly as described previously

Using env_clear on windows is still unusable in most cases and leads to the cryptic errors like this

@ChrisDenton
Copy link
Member

Can you open a new issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. O-windows Operating system: Windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants