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

Improve Snapshot determinism #4614

Merged
merged 8 commits into from May 23, 2017

Conversation

Projects
None yet
3 participants
@stuhood
Copy link
Member

stuhood commented May 22, 2017

Problem

In #4610, we determined that the file mode is taken into account while creating Snapshots. But since git does not enforce file modes with the exception of the execute bit, the umask on various machines ends up influencing the Snapshot fingerprints.

Solution

Implement a change to the Deterministic append mode upstream in tar-rs, and preemptively include it. The append mode will either be accepted as-is, or as a new append mode, in which case I'll update this review.

Result

File mode is not included in the fingerprint, and so cross-platform/machine fingerprints are stable. Tested with checkouts with differing umask values across OSX and Linux. Fixes #4610.

@stuhood stuhood requested review from kwlzn and JieGhost May 22, 2017

@kwlzn

kwlzn approved these changes May 23, 2017

Copy link
Member

kwlzn left a comment

lgtm!

@ity
Copy link
Member

ity left a comment

lgtm!

@stuhood stuhood merged commit 9a6145f into pantsbuild:master May 23, 2017

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@stuhood

This comment has been minimized.

Copy link
Member

stuhood commented May 23, 2017

There was some network flakiness in CI, and I accidentally restarted the whole job... merging as is.

stuhood added a commit that referenced this pull request May 23, 2017

Improve Snapshot determinism (#4614)
### Problem

In #4610, we determined that the file mode is taken into account while creating Snapshots. But since git does not enforce file modes with the exception of the `execute` bit, the `umask` on various machines ends up influencing the Snapshot fingerprints.

### Solution

Implement a change to the `Deterministic` append mode upstream in `tar-rs`, and preemptively include it. The append mode will either be accepted as-is, or as a new append mode, in which case I'll update this review.

### Result

File mode is not included in the fingerprint, and so cross-platform/machine fingerprints are stable. Tested with checkouts with differing `umask` values across OSX and Linux. Fixes #4610.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment