Skip to content

Preserve mtimes from input tar files in TarFileWriter output tars#155

Merged
aiuto merged 3 commits intobazelbuild:masterfrom
dannysullivan:preserve-tar-mtimes
Apr 21, 2020
Merged

Preserve mtimes from input tar files in TarFileWriter output tars#155
aiuto merged 3 commits intobazelbuild:masterfrom
dannysullivan:preserve-tar-mtimes

Conversation

@dannysullivan
Copy link
Copy Markdown
Collaborator

@dannysullivan dannysullivan commented Apr 16, 2020

This tool should not need to override mtimes for files contained in tar files that are added to the output of TarFileWriter, since these files are either constructed by other parts of the build process (in which case we can rely on the caching and reproducibility of those steps) or they are constructed outside of the bazel system, in which case we shouldn't need to alter them.

See discussion on this issue for more information.

Fixes #149

@dannysullivan dannysullivan requested a review from aiuto as a code owner April 16, 2020 20:14
@dannysullivan
Copy link
Copy Markdown
Collaborator Author

@aiuto please let me know if I missed something about your recommendation - my understanding is that removing this line gives us the behavior you were describing, where output tars always respect the mtimes of input tars.

@aiuto
Copy link
Copy Markdown
Collaborator

aiuto commented Apr 16, 2020

So this is roughly https://github.com/bazelbuild/bazel/pull/11028/files
Let's err on the safe side and provide the option to update the mtime, but turn if off by default. Just like the suggestion there. Or, to put it differently. Change the behavior so it preserves mtime, but leave an escape valve.

Copy link
Copy Markdown
Collaborator

@aiuto aiuto left a comment

Choose a reason for hiding this comment

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

Thanks for your patience while I was backlogged.

@dannysullivan
Copy link
Copy Markdown
Collaborator Author

No problem! This PR should be up to date with that requested change now.

@aiuto aiuto merged commit 0761c40 into bazelbuild:master Apr 21, 2020
dannysullivan added a commit to dannysullivan/rules_docker that referenced this pull request May 6, 2020
This fixes a mistake in bazelbuild/rules_pkg#155 - I missed one instance of hardcoding the value False. This PR also adds a test of this configuration option.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: TarFileWriter tool optionally preserves input file mtimes

2 participants