Fix tar upload content-length mismatch (#1086)#1089
Fix tar upload content-length mismatch (#1086)#1089noamzbr wants to merge 2 commits intoe2b-dev:mainfrom
Conversation
Use portable mode for deterministic gzip output
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2be4177c0c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Top-level portable strips executable bits; gzip option only affects header
|
I don't think we can do it this due to how hashes are calculated. Is there another way to ensure these are same? |
|
Note I've fixed it from tar But sure, if you prefer avoiding changing the archive mechanism entirely - can you tell me more about what you had in mind? If the presigned PUT requires Content-Length (SDK currently assumes it does), we can’t really stream once to upload + counter because headers must be sent before the body. Did you mean switching the upload to chunked (no Content-Length), making the two-pass tar generation stable (e.g. normalize/strip volatile pax atime/ctime via onWriteEntry) so both passes produce identical bytes. or something else? |
|
Okay, I see. As far as I understand with For the upload of the files we are using pre-signed URLs that require Content-Length to be specified before the upload. My initial idea was to have 2 streams - one for counting the archive length and another for actual upload. |
yes, gzip: { portable: true } will not change file modes (permissions) |
|
sounds good, did you verify this fixes #1086 for you? |
yes, being using it for around a couple of days now |
<!-- CURSOR_SUMMARY -->
> [!NOTE]
> Ensures deterministic tar.gz archives during upload to prevent
content-length mismatches.
>
> - In `tarFileStream`, switch gzip option from `true` to `gzip: {
portable: true }` to produce stable gzip headers without altering file
modes
> - Adds a changeset noting a patch release for this behavior change
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
ec90756. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
---------
Co-authored-by: noamzbr <noamzbr@users.noreply.github.com>
|
Thanks for your contribution, we merged it here #1095 |
Fixes #1086
Use portable mode for deterministic gzip output