Skip to content
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

Set the mtime fields in the tarball correctly #61

Merged
merged 2 commits into from
Aug 29, 2023
Merged

Conversation

edolstra
Copy link
Contributor

This makes FlakeHub flakes behave consistently with GitHub flakes. Unfortunately, this requires a patched tar-rs crate.

This makes FlakeHub flakes behave consistently with GitHub
flakes. Unfortunately, this requires a patched tar-rs crate.
@@ -17,7 +17,8 @@ serde = { version = "1.0.164", features = ["derive"] }
serde_json = "1.0.97"
gix = { version = "0.46.0", features = ["async-network-client", "serde"] }
gix-ref = { version = "0.30.0", features = ["serde"] }
tar = { version = "0.4.38", features = ["xattr"] }
#tar = { version = "0.4.38", features = ["xattr"] }
tar = { git = "https://github.com/DeterminateSystems/tar-rs.git", branch = "force-mtime", features = ["xattr"] }
Copy link
Member

Choose a reason for hiding this comment

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

Are we planning to try to upstream this? Otherwise, we'll need to keep this in sync somehow (I don't know how active the upstream is but...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's pretty ad hoc. Something more general (e.g. a filter to massage the FS metadata in arbitrary ways) might be more acceptable to upstream.

Upstream is not moving super fast: https://github.com/alexcrichton/tar-rs/tags so I don't think it's a lot of effort to rebase from time to time. It's not like the tar format is changing a lot...

Copy link
Member

Choose a reason for hiding this comment

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

Let's send a PR anyway and see what they say. We can keep pulling from our fork in the meantime.

Copy link
Member

Choose a reason for hiding this comment

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

(That is done in alexcrichton/tar-rs#337)

@lheckemann
Copy link
Contributor

Just one question: how will we remember to return to upstream if a change that addresses our need gets merged?

@edolstra edolstra merged commit abb112e into main Aug 29, 2023
5 checks passed
@edolstra edolstra deleted the edolstra/ds-1171 branch August 29, 2023 14:18
detsys-pr-bot pushed a commit to detsys-pr-bot/flakehub-push that referenced this pull request Aug 26, 2024
…m DeterminateSystems/use-coalesce-for-array` (`e8f6e8f54d85aa0fd3d0b694dd3279a21497a33b`)
grahamc added a commit that referenced this pull request Aug 26, 2024
…d85aa0fd3d0b694dd3279a21497a33b

Update `detsys-ts`: Merge pull request #61 from DeterminateSystems/use-coalesce-for-array
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.

4 participants