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

NailgunTasks execute java with consistent cwd. #7872

Merged
merged 1 commit into from Jun 9, 2019

Conversation

Projects
None yet
2 participants
@jsirois
Copy link
Member

commented Jun 8, 2019

Previously, nailgun-mode executions would run with the cwd set to the
buildroot but subprocess mode executions would inherit cwd. These
usually aligned since pants is run from the buildroot, but could
misalign in tests where a test buildroot is propped up.

Ensure cwd is always explicitly the buildroot for NailgunTasks and fix
executor cwd plumbing to effect this with a test added that proves this
all works.

Additionally, fix ScalaPlatform classpaths, which had been relativized
to the buildroot to support snapshotting. Maintain the snapshot relative
paths, but leave the in-memory classpath un-altered and populated with
absolute paths like all other runtime classpaths.

Prep work for #7866.

NailgunTasks execute java with consistent cwd.
Previously, nailgun-mode executions would run with the cwd set to the
buildroot but subprocess mode executions would inherit cwd. These
usually aligned since pants is run from the buildroot, but could
misalign in tests where a test buildroot is propped up.

Ensure cwd is always explicitly the buildroot for NailgunTasks and fix
executor cwd plumbing to effect this with a test added that proves this
all works.

Additionally, fix ScalaPlatform classpaths, which had been relativized
to the buildroot to support snapshotting. Maintain the snapshot relative
paths, but leave the in-memory classpath un-altered and populated with
absolute paths like all other runtime classpaths.

Prep work for #7866.
@jsirois

This comment has been minimized.

Copy link
Member Author

commented Jun 8, 2019

Reviewers - the two big things to weigh in on that weigh on me:

  1. There is 1 API change here in pants.java.executor.Executor.runner where the redundant and ultimately unused cwd argument was removed. Executor was marked :API: public but this API was truly ambiguous and there are no signs of use outside this repo that I can lay eyes on (fsqio & toolchain repo Pants plugins).
  2. The ScalaPlatform in-memory classpath fix. Please sanity check that its ok to have the in-memory classpath in use for a local run be different from the snapshot paired with it (absolute paths in-memory, buildroot relative paths in the snapshot).

jsirois added a commit to jsirois/pants that referenced this pull request Jun 8, 2019

Fix `DistributionLocator` dependency declarations.
This fixes all usages of `DistributionLocator` to declare a subsystem
dependency and removes all unused dependency declarations save for
those in the `Ivy` family where the tangles run deep.

Discovered working pantsbuild#7872.
@stuhood

stuhood approved these changes Jun 9, 2019

Copy link
Member

left a comment

The ScalaPlatform in-memory classpath fix. Please sanity check that its ok to have the in-memory classpath in use for a local run be different from the snapshot paired with it (absolute paths in-memory, buildroot relative paths in the snapshot).

Yea, this looks fine: thanks John!

@jsirois jsirois merged commit daf098d into pantsbuild:master Jun 9, 2019

1 check passed

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

@jsirois jsirois deleted the jsirois:issues/7865-uniform-ng-task-cwd branch Jun 9, 2019

jsirois added a commit to jsirois/pants that referenced this pull request Jun 9, 2019

Fix `DistributionLocator` dependency declarations.
This fixes all usages of `DistributionLocator` to declare a subsystem
dependency and removes all unused dependency declarations save for
those in the `Ivy` family where the tangles run deep.

Discovered working pantsbuild#7872.

jsirois added a commit that referenced this pull request Jun 9, 2019

Fix `DistributionLocator` dependency declarations. (#7875)
This fixes all usages of `DistributionLocator` to declare a subsystem
dependency and removes all unused dependency declarations save for
those in the `Ivy` family where the tangles run deep.

Discovered working #7872.

cattibrie added a commit to cattibrie/pants that referenced this pull request Jun 19, 2019

NailgunTasks execute java with consistent cwd. (pantsbuild#7872)
Previously, nailgun-mode executions would run with the cwd set to the
buildroot but subprocess mode executions would inherit cwd. These
usually aligned since pants is run from the buildroot, but could
misalign in tests where a test buildroot is propped up.

Ensure cwd is always explicitly the buildroot for NailgunTasks and fix
executor cwd plumbing to effect this with a test added that proves this
all works.

Additionally, fix ScalaPlatform classpaths, which had been relativized
to the buildroot to support snapshotting. Maintain the snapshot relative
paths, but leave the in-memory classpath un-altered and populated with
absolute paths like all other runtime classpaths.

Prep work for pantsbuild#7866.

cattibrie added a commit to cattibrie/pants that referenced this pull request Jun 19, 2019

Fix `DistributionLocator` dependency declarations. (pantsbuild#7875)
This fixes all usages of `DistributionLocator` to declare a subsystem
dependency and removes all unused dependency declarations save for
those in the `Ivy` family where the tangles run deep.

Discovered working pantsbuild#7872.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.