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

rsc re-uses DirectoryDigest from process execution rather than re-snapshotting #7861

Conversation

Projects
None yet
3 participants
@illicitonion
Copy link
Contributor

commented Jun 5, 2019

No description provided.

@illicitonion illicitonion requested review from stuhood and cosmicexplorer Jun 5, 2019

@illicitonion

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2019

Depends on #7858

@stuhood

stuhood approved these changes Jun 5, 2019

Copy link
Member

left a comment

Thanks!

@illicitonion illicitonion force-pushed the twitter:dwagnerhall/rsc/avoid-re-snapshotting-2 branch from a952759 to 399b9f0 Jun 6, 2019

@cosmicexplorer
Copy link
Contributor

left a comment

Wonderful!!!

@@ -678,14 +656,13 @@ def _runtool_nonhermetic(self, parent_workunit, classpath, main, tool_name, args
return runjava_workunit

def _runtool(self, args, distribution,
tgt=None, input_digest=None, output_dir=None):
tgt=None, input_digest=None, ctx=None):

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Jun 7, 2019

Contributor

Great change!

@@ -361,7 +337,7 @@ def work_for_vts_rsc(vts, ctx):
classpath_product = self.context.products.get_data('rsc_mixed_compile_classpath')
classpath_entries = classpath_product.get_classpath_entries_for_targets(dependencies_for_target)
for _conf, classpath_entry in classpath_entries:
classpath_paths.append(classpath_entry.path)
classpath_paths.append(fast_relpath(classpath_entry.path, get_buildroot()))

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Jun 7, 2019

Contributor

Keeping the path as absolute within the ClasspathEntry itself seems definitely like the right thing to do here -- thanks for making this consistent compared to the first diff I had posted!

@cosmicexplorer

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2019

I might note in the description that this fixes what is almost definitely a quadratic slowdown induced by filesystem traversals -- or perhaps link to #7838 / steal that PR's description.

@illicitonion

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2019

I might note in the description that this fixes what is almost definitely a quadratic slowdown induced by filesystem traversals -- or perhaps link to #7838 / steal that PR's description.

#7858 was the PR that did that; this just stops re-snapshotting one file, the output jar from rsc :)

@illicitonion illicitonion merged commit 42f162d into pantsbuild:master Jun 7, 2019

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details

@illicitonion illicitonion deleted the twitter:dwagnerhall/rsc/avoid-re-snapshotting-2 branch Jun 7, 2019

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.