environment fixes in run - #3191
Merged
Merged
Conversation
mkenigs
force-pushed
the
run-environment
branch
from
November 5, 2019 00:45
f712427 to
20b4c1f
Compare
Member
|
Maybe we can factor out the environment handling in |
Move environment related code to a separate function. Create a new char** if ignoreEnvironment is set rather than calling clearEnv
mkenigs
force-pushed
the
run-environment
branch
from
November 7, 2019 23:23
20b4c1f to
6419f50
Compare
Contributor
Author
edolstra
reviewed
Nov 26, 2019
|
|
||
| for (const auto & var : keep) { | ||
| auto val = getenv(var.c_str()); | ||
| if (val) stringEnv.emplace_back(fmt("%s=%s", var.c_str(), val)); |
Member
There was a problem hiding this comment.
This doesn't compile since stringEnv doesn't exist.
Member
|
Thanks, merged! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.