-
-
Notifications
You must be signed in to change notification settings - Fork 235
feat(build): Add CLI version metadata to build upload archives #2890
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
base: master
Are you sure you want to change the base?
Conversation
|
| )?; | ||
| } | ||
|
|
||
| let options = SimpleFileOptions::default() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the same options as set in upload.rs (scroll up further in the file to see it)
Adds a `.sentry-cli-metadata.txt` file to all zip archives created during build uploads. This file contains the sentry-cli version for debugging purposes. The metadata file is added to both single-file uploads (APK, AAB) and directory uploads (XCArchive) with the same compression and timestamp settings as other files to maintain consistent checksums. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
9d2ccb4 to
caff448
Compare
|
|
||
| let options = SimpleFileOptions::default() | ||
| .compression_method(zip::CompressionMethod::Deflated) | ||
| .last_modified_time(DateTime::default()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Compression Inconsistency Causes Checksum Mismatch
The .sentry-cli-metadata.txt file is compressed with Deflated in normalize.rs but Stored in upload.rs. This inconsistency violates the intended goal of uniform compression and timestamp settings, which affects checksum consistency across different upload types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments/suggestions.
Also just wondering, were you able to test the changes manually? It would be cool if we could also have an automated test to check that the file is created and added to the ZIP, so we can ensure the behavior remains in CI
| ? success | ||
| WARN [..] EXPERIMENTAL: The build subcommand is experimental. The command is subject to breaking changes and may be removed without notice in any release. | ||
| > Preparing for upload completed in [..] | ||
| > Uploading completed in [..] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m: I don't see why this line of output is disappearing following this change; I would expect that it should stay the same.
Please double-check why this is changing, and whether the change is intentional.
| zip.start_file(".sentry-cli-metadata.txt", options)?; | ||
| writeln!(zip, "sentry-cli-version: {VERSION}")?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: To ensure we always generate the version information in the same way, I would suggest we extract this logic to a separate method, that we can also reuse in the commands/build/upload.rs file
|
|
||
| ## Unreleased | ||
|
|
||
| ### Various fixes & improvements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ### Various fixes & improvements | |
| ### Features |
|
|
||
| ### Various fixes & improvements | ||
|
|
||
| - Add CLI version metadata to build upload archives for debugging purposes ([#2890](https://github.com/getsentry/sentry-cli/pull/2890)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add more details to this changelog entry, specifically so that users understand the user-facing changes. I have been making user-readable changelog entries a priority in the last few Sentry CLI releases.
| > Uploading completed in [..] | ||
| Successfully uploaded 1 file to Sentry | ||
| - tests/integration/_fixtures/build/apk.apk (http[..]/wat-org/preprod/wat-project/42) | ||
| - tests/integration/_fixtures/build/apk.apk (http://sentry.io/wat-org/preprod/wat-project/42) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In any case, this line should remain the same
| - tests/integration/_fixtures/build/apk.apk (http://sentry.io/wat-org/preprod/wat-project/42) | |
| - tests/integration/_fixtures/build/apk.apk (http[..]/wat-org/preprod/wat-project/42) |
Summary
Adds CLI version metadata to all zip archives created during build uploads for debugging purposes.
Resolves EME-471
Changes
.sentry-cli-metadata.txtfile at the root of each build upload zip archivesentry-cli-version: <version>🤖 Generated with Claude Code