Skip to content
This repository has been archived by the owner before Nov 9, 2022. It is now read-only.

Use a specific mtime when packing, rather than none at all #20027

Merged
merged 1 commit into from Mar 12, 2018

Conversation

isaacs
Copy link
Contributor

@isaacs isaacs commented Mar 12, 2018

Thank god I found you. Listen, can you meet me at Twin Pines Mall tonight at 1:15? I've made a major breakthrough, I'll need your assistance.

@isaacs isaacs requested a review from a team as a code owner Mar 12, 2018
@isaacs isaacs changed the title Isaacs/pack timestamp Use a specific mtime when packing, rather than none at all Mar 12, 2018
@isaacs isaacs force-pushed the isaacs/pack-timestamp branch from 2daaac0 to e1a6068 Compare Mar 12, 2018
zkat
zkat approved these changes Mar 12, 2018
Copy link
Contributor

@zkat zkat left a comment

💯

@zkat zkat changed the base branch from latest to release-next Mar 12, 2018
> Thank god I found you.  Listen, can you meet me at Twin Pines Mall
> tonight at 1:15?  I've made a major breakthrough, I'll need your
> assistance.

Fixes: #19933
Fixes: #19968
PR-URL: #20027
Credit: @isaacs
Reviewed-By: @zkat
@zkat zkat force-pushed the isaacs/pack-timestamp branch from e1a6068 to 7302686 Compare Mar 12, 2018
@zkat
Copy link
Contributor

zkat commented Mar 12, 2018

cc @simonua

@zkat zkat merged this pull request into release-next Mar 12, 2018
@zkat zkat deleted the isaacs/pack-timestamp branch Mar 12, 2018
zkat pushed a commit that referenced this pull request Mar 12, 2018
> Thank god I found you.  Listen, can you meet me at Twin Pines Mall
> tonight at 1:15?  I've made a major breakthrough, I'll need your
> assistance.

Fixes: #19933
Fixes: #19968
PR-URL: #20027
Credit: @isaacs
Reviewed-By: @zkat
noMtime: true,
// Provide a specific date in the 1980s for the benefit of zip,
// which is confounded by files dated at the Unix epoch 0.
mtime: new Date('1985-10-26T08:15:00.000Z'),

Choose a reason for hiding this comment

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

when this tarball hits 88... we are going to see some serious 💩

Choose a reason for hiding this comment

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

Jokes aside, is there a reason to set a date in the past as opposed to using the original mtime of the file?

If I'm not mistaken older versions would maintain the original mtime, see googleapis/google-p12-pem#27 (comment)

Copy link
Contributor

@zkat zkat Mar 13, 2018

Choose a reason for hiding this comment

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

The reason for this is that, by setting mtimes like this, we're able to make it so two computers with two checkouts of the same project at the same commit, barring extra noise in the signal, will both generate tarballs with the exact same hash. It means npm packages can be made compliant with reproducible builds. Obvs, npm isn't the only part of this story, and this also depends on people's own run-scripts modifying things, but at least npm's packer no longer gets in the way of this.

For a while now, we were using the noMtime option because it allowed those stable times, but it turns out there's some implementations that can't handle beginning-of-epoch timestamps (or timestamps before 1980), so this is the compromise.

Choose a reason for hiding this comment

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

A++ explanation thanks.

I know that Node is interested in exploring reproducible builds, and timestamps are definitely an issue for this.

@simonua
Copy link
Contributor

simonua commented Mar 13, 2018

Thanks, Doc!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants