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

Always capture and cache Digests in coursier and ivy #7835

Merged

Conversation

stuhood
Copy link
Sponsor Member

@stuhood stuhood commented Jun 2, 2019

Problem

Capturing digests for coursier and ivy artifacts is currently optional, presumably due to the performance impact of not being cached run over run.

Solution

#7241 added support for stashing Digests next to digested files, and support for providing a Digest hint that will skip snapshotting if the Digest is already stored. We use that support here to always-snapshot 3rdparty inputs (and to skip re-snapshotting if a Digest was stashed).

Result

One fewer option, and slightly better performance when re-running.

Copy link
Contributor

@cosmicexplorer cosmicexplorer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extremely cool!

@stuhood
Copy link
Sponsor Member Author

stuhood commented Jun 2, 2019

Lots of network flakiness (and one actual flake: #7836), but previous runs of those shards were good, and would be unaffected by the most recent change. Merging.

@stuhood stuhood merged commit 944b42c into pantsbuild:master Jun 2, 2019
@stuhood stuhood deleted the stuhood/capture-3rdparty-digests-by-default branch June 2, 2019 19:24
illicitonion added a commit that referenced this pull request Jul 9, 2019
stuhood added a commit that referenced this pull request Jul 28, 2019
### Problem

When hermetic execution is used for `zinc` or `rsc` and `DirectoryDigests` are not available for input classpath entries, we currently trigger a warning. But this being only a warning (and not an error) definitely masks a few different potential bugs, and might allow more to slip in in the future.

### Solution

Convert the warning into an error, and fix all of the issues exposed by that change:
* In some `Task` configurations, `ResourcesTask` could run before the compilers, meaning that its resources needed to be digested. This also prepares for remote execution of tests.
* When `--no-use-classpath-jars` was in use:
  * `ClasspathEntry`s having digests was preventing them from having an equality match in order to be removed from the classpath.
  * `rsc` was universally adding the jar for a target to the classpath, even though it was empty.
  * Both the loose classpath and output jar needed to be captured in hermetic execution.
* Tools that were bootstrapped were not propagating the digests that had already been captured for them in #7835, which caused "global" scalac plugins to not have digests.

### Result

Hermetic execution has improved validation that should help to catch additional bugs in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants