Preserve mtime explicitly when creating tar artifacts #18091
Conversation
When building tar archives, explicitly set mtime. This avoids losing that information in the minitar `Writer.add_file_simple` method https://github.com/halostatue/minitar/blob/a531136b17b9efdddf0a0f39537845b454c2371e/lib/minitar/writer.rb#L139
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
|
This pull request does not have a backport label. Could you fix it @donoghuc? 🙏
|
|
| @@ -669,7 +669,7 @@ namespace "artifact" do | |||
| elsif stat.symlink? | |||
| tar.symlink(path_in_tar, File.readlink(path), :mode => stat.mode) | |||
| else | |||
| tar.add_file_simple(path_in_tar, :mode => stat.mode, :size => stat.size) do |io| | |||
| tar.add_file_simple(path_in_tar, :mode => stat.mode, :size => stat.size, :mtime => stat.mtime&.to_i || 0) do |io| | |||
There was a problem hiding this comment.
For the reviewer here is the source for this method https://github.com/halostatue/minitar/blob/a531136b17b9efdddf0a0f39537845b454c2371e/lib/minitar/writer.rb#L104-L177
Idea is that to avoid it defaulting to nil (and resulting in a 0 for linux epoch time) we explicitly pass through the mtime of the file on the builder (or 0 if for some reason that cant be computed).
One use case #17925 on upgrade is relying on rsync defaults where it would be convenient that for each logstash release all files in an artifact have a "newer" mtime than the previous release. I believe this would ensure that.
There was a problem hiding this comment.
Should we then just do :mtime => stat.mtime&.to_i || Time.now.to_i ? seems better than epoch, there's already some expectation that dates of files in the archive will be similar to the date of when the files were put into the archive.
💚 Build Succeeded
|
Release notes
Improvement to logstash release artifacts file metadata: mtime is preserved when buiilding tar archives
What does this PR do?
When building tar archives, explicitly set mtime. This avoids losing that information in the minitar
Writer.add_file_simplemethodhttps://github.com/halostatue/minitar/blob/a531136b17b9efdddf0a0f39537845b454c2371e/lib/minitar/writer.rb#L139
Why is it important/What is the impact to the user?
Files containing mtime 0 lead to undesireable behavior in some use cases #17925 and do not show any clues to modification time. Avoiding setting mtime to 0 in many cases will help provide artifacts with the least amount of surprise to users.
Checklist
Related issues