Clean up analysis.tmp usage between pants and zinc wrapper (Part 1)

Review Request #4245 - Created Sept. 19, 2016 and submitted

Information
Peiyu Wang
pants
3667, 3886
4246
Reviewers
pants-reviews
benjyw, jsirois, molsen, nhoward_tw, stuhood, zundel

We use a temp analysis file so in case when analysis update fails,
the main analysis file can remain intact.

Currently pants creates the temp file by copying from the original analysis
file before calling zinc, and renames the temp file after zinc call succeeds.
The way it is today however may cause: temp file and the original analysis file
that are passed to zinc are not in sync, which is at least one contributing
factor to https://github.com/pantsbuild/pants/issues/3667.

Say if zinc compile is successful but there are some unused deps. We would end
up with a good analysis file but compile workunit fails due to
_check_unused_deps, which means hash file is not going to be updated, which
then result next time should_compile_incrementally returns false. Now we will
pass an empty .analysis.tmp and a non-empty .analysis to zinc wrapper.

This review and rb/4246 eliminates
.analysis.tmp's usage in pants, hides its creation and renaming with a
SafeFileBasedStore, so between pants and zinc the analysis file is always
valid. Also there is no more copy penalty because .analysis is where all
reads go, .analysis.tmp is used only when write is necessary.

https://travis-ci.org/peiyuwang/pants/builds/161120149
https://travis-ci.org/peiyuwang/pants/builds/161707210

Issues

  • 0
  • 4
  • 0
  • 4
Description From Last Updated
Peiyu Wang
Benjy Weinberger
John Sirois
Ity Kaul
Peiyu Wang
Stu Hood
John Sirois
Stu Hood
Peiyu Wang
Review request changed

Status: Closed (submitted)

Change Summary:

Merged as c32700b1f3f3c19508d38d570e1d474fdb73bcc2

Loading...