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

environment fixes in run #3191

Merged
merged 5 commits into from Dec 2, 2019
Merged

environment fixes in run #3191

merged 5 commits into from Dec 2, 2019

Conversation

@mkenigs
Copy link

mkenigs commented Nov 1, 2019

Code in #3172 which adds --ignore-environment to dev-shell is redundant because --ignore-environment is already implemented for run. I moved the related code in run to a separate function. If doing it this way works, I was thinking I could move it to a common class that both run and dev-shell can inherit from.

I also changed the way ignore environment is handled, creating a new char* array if ignoreEnvironment is set rather than calling clearEnv. The previous way of doing it had to getenv, clearenv, and setenv for every kept variable, and clearenv for all variables, so it seems like creating a new array might be a better way of doing it that just does one getenv for kept variables. Not sure if it makes the call to runProgram and handling of PATH too complicated.

@mkenigs mkenigs force-pushed the mkenigs:run-environment branch from f712427 to 20b4c1f Nov 5, 2019
@edolstra

This comment has been minimized.

Copy link
Member

edolstra commented Nov 5, 2019

Maybe we can factor out the environment handling in nix run into a MIxEnvironment class that could be used by nix dev-shell? That way the latter would also get the --keep and --unset flags.

mkenigs added 4 commits Nov 1, 2019
Move environment related code to a separate function. Create a new char** if ignoreEnvironment is set rather than calling clearEnv
@mkenigs mkenigs force-pushed the mkenigs:run-environment branch from 20b4c1f to 6419f50 Nov 7, 2019
@mkenigs

This comment has been minimized.

Copy link
Author

mkenigs commented Nov 7, 2019

Factored code out into MixEnvironment and used it in both run and dev-shell. Changes would close #3172 and #2813


for (const auto & var : keep) {
auto val = getenv(var.c_str());
if (val) stringEnv.emplace_back(fmt("%s=%s", var.c_str(), val));

This comment has been minimized.

Copy link
@edolstra

edolstra Nov 26, 2019

Member

This doesn't compile since stringEnv doesn't exist.

@edolstra edolstra merged commit 062012e into NixOS:flakes Dec 2, 2019
@edolstra

This comment has been minimized.

Copy link
Member

edolstra commented Dec 2, 2019

Thanks, merged!

@mkenigs mkenigs deleted the mkenigs:run-environment branch Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.