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

[pantsd] Set the remote environment for pantsd-runner and child processes. #5508

Merged
merged 7 commits into from Feb 23, 2018

Conversation

Projects
None yet
3 participants
@kwlzn
Copy link
Member

kwlzn commented Feb 23, 2018

Problem

Currently, env vars set in post-pantsd-launching runs that use the daemon fail to inherit env vars correctly in child processes of pantsd-runners. Anything that relies on this will be broken with the daemon enabled for ./pants run, ./pants test, etc.

Additionally, env vars that are set during the pantsd-launching run will currently leak stale env state into child processes of pantsd-runners.

Solution

Set the remote environment as the local environment for DaemonPantsRunner from a clean state.

Result

Tests pass.

@kwlzn kwlzn requested review from stuhood and illicitonion Feb 23, 2018

kwlzn added some commits Feb 23, 2018

@illicitonion
Copy link
Contributor

illicitonion left a comment

Approving because it preserves backwards compatibility, but I think long term this is the wrong direction to go. I would much prefer that everything which could invalidate caches be forced to be flags to pants in some way, rather than passing env around verbatim.

@kwlzn

This comment has been minimized.

Copy link
Member

kwlzn commented Feb 23, 2018

I would much prefer that everything which could invalidate caches be forced to be flags to pants in some way, rather than passing env around verbatim.

agreed, but apparently customers are already depending on e.g. env-var affected tests. :(

@kwlzn kwlzn merged commit 6d87acd into pantsbuild:master Feb 23, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@stuhood stuhood added this to the 1.4.x milestone Feb 23, 2018

stuhood added a commit that referenced this pull request Feb 23, 2018

[pantsd] Set the remote environment for pantsd-runner and child proce…
…sses. (#5508)

Problem
Currently, env vars set in post-pantsd-launching runs that use the daemon fail to inherit env vars correctly in child processes of pantsd-runners. Anything that relies on this will be broken with the daemon enabled for ./pants run, ./pants test, etc.

Additionally, env vars that are set during the pantsd-launching run will currently leak stale env state into child processes of pantsd-runners.

Solution
Set the remote environment as the local environment for DaemonPantsRunner from a clean state.

Result
Tests pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment