Environment variables for setup and test tasks #215
Conversation
|
I'm not really sure if this is the right way to do it, so please be zealous in reviewing it 😄 |
|
any update here? |
|
Yeah... that's my bad. Let me get to it this week. |
thx |
| options: cp.ExecFileOptions = { shell: true }, | ||
| ) { | ||
| if (extraEnv) { | ||
| options.env = { ...(options.env ?? process.env), ...extraEnv }; |
There was a problem hiding this comment.
Maybe I don't understand the node API here, if we pass an environment object, does it not also inherit the parent environment?
There was a problem hiding this comment.
I'm not 100% sure. The documentation says, about the env option:
env: Environment key-value pairs. Default:process.env.
So because the default is process.env and not {}, I'd guess that options.env replaces the parent environment. (That also makes sense because otherwise, you couldn't delete environment variables.)
|
Just a few comments, and then it should be good to go. It would also be useful if you could add an entry in the CHANGELOG.md file. Otherwise, I can get to it after merge. |
This setting allows setting extra environment variables during the setup stage.
It's already possible to add environment variables in the 'options' argument — but child_process.execFile() does not treat options.env as extra environment variables to add. It replaces the environment with options.env entirely. So, it's convenient to have an extraEnv argument with variables to add. This will be necessary for the testEnvironment setting added in the next commit.
This setting allows setting extra environment variables while running meson test, as well as individual tests.
Consistently replace 'Meson setup' -> 'setup' as pointed out in code review. Improve phrasing for mesonbuild.buildFolder.
ptomato
force-pushed
the
9-environment-variables
branch
from
d02cfce to
f44631d
Compare
3 participants
This adds two options,
configureEnvironmentandtestEnvironment, allowing users to set particular environment variables duringmeson setupandmeson test.Closes: #9