[zinc] Record the canonical path that was fingerprinted, rather than the input path

Review Request #3692 - Created April 14, 2016 and submitted

Information
Stu Hood
pants
3193
Reviewers
pants-reviews
gmalmquist, nhoward_tw, patricklaw

Record the canonical path that was fingerprinted, rather than the input path.

It is expected for inputs to change in the presence of the stable symlink: we feed zinc .pants.d/compile/zinc/$taskversion/$targetid/current/* as the classes/analysis paths, and so during multiple incremental compiles the canonical path of the analysis will change.

zinc creates a FileFPrint for the analysis inputs, and uses it as a cache key. Before this change it was recording the symlink path for the file, rather than the canonical location. This was incorrect, because the analysis would not be reloaded if the symlink had changed. Luckily, this was detected by validation in AnalysisMap.get, and we observed exceptions like:

java.io.IOException: Analysis at (6480500942a8e2f9a0e76e0612e750f2: $BUILD_SANDBOX/.pants.d/compile/zinc/252d64521cf9/util.util-core.src.main.scala.scala/current/util.util-core.src.main.scala.scala.analysis) has changed since startup!

Because the cache key contained the stable name, when we failed to hit for the cache key, we finally detected that it had changed (much earlier).

This was integration tested internally at Twitter, and passes our full sandbox suite.

https://travis-ci.org/pantsbuild/pants/builds/123158795

Stu Hood
Matt Olsen
Garrett Malmquist
Stu Hood
Review request changed

Status: Closed (submitted)

Change Summary:

Merged as 61c01d4a76039cc9b88ff9cbe684e1989f79acf5

Patrick Lawson
Ship It!
Eric Ayers

We just updated to 0.0.81, are devs going to start running into issues without this patch?

  1. @Eric: This bug was in effect between 4d1796ca7964e7bce89b6f7a8f5d4ca2195e1bd0 (~0.0.77) and 2152846c3ceb34a1f737590be76cf2f8d24e6185. We only noticed it because we're just wrapping up merging internally for the first time since then.

    So, yes: but if you haven't already experienced errors, it's possible that you are not experiencing the issue in your repo for some reason. Either way, worth bumping to this zinc release in your BUILD.tools, or via a patch like: https://rbcommons.com/s/twitter/r/3693/

  2. OK, thanks. I haven't heard reports of this. We should be upgrading again soon.

  3. We have been on 0.0.79+ since it came out. I believe that I hit one report of this - although I can't find the slack conversation to confirm. It is not common, at least.

Loading...