Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ we should rename this section to "Unreleased" -->

- Removed the `upload-proguard` subcommand's `--app-id`, `--version`, and `--version-code` arguments ([#2876](https://github.com/getsentry/sentry-cli/pull/2876)). Users using these arguments should stop using them, as they are unnecessary. The information passed to these arguments is no longer visible in Sentry.

### Improvements

- The `sentry-cli upload-proguard` command now uses chunked uploading by default ([#2918](https://github.com/getsentry/sentry-cli/pull/2918)). Users who previously set the `SENTRY_EXPERIMENTAL_PROGUARD_CHUNK_UPLOAD` environment variable to opt into this behavior no longer need to set the variable.

### Fixes

- Fixed misleading error message claiming the server doesn't support chunk uploading when the actual error was a non-existent organization ([#2930](https://github.com/getsentry/sentry-cli/pull/2930)).
Expand Down
96 changes: 22 additions & 74 deletions src/commands/upload_proguard.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::env;
use std::io;

use anyhow::{bail, Error, Result};
Expand All @@ -9,16 +8,13 @@ use symbolic::common::ByteView;
use uuid::Uuid;

use crate::api::Api;
use crate::api::ChunkUploadCapability;
use crate::config::Config;
use crate::utils::android::dump_proguard_uuids_as_properties;
use crate::utils::args::ArgExt as _;
use crate::utils::fs::TempFile;
use crate::utils::proguard;
use crate::utils::proguard::ProguardMapping;
use crate::utils::system::QuietExit;
use crate::utils::ui::{copy_with_progress, make_byte_progress_bar};

const CHUNK_UPLOAD_ENV_VAR: &str = "SENTRY_EXPERIMENTAL_PROGUARD_CHUNK_UPLOAD";

pub fn make_command(command: Command) -> Command {
command
Expand Down Expand Up @@ -167,79 +163,31 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
let api = Api::current();
let config = Config::current();

// Don't initialize these until we confirm the user did not pass the --no-upload flag,
// or if we are using chunked uploading. This is because auth token, org, and project
// are not needed for the no-upload case.
let authenticated_api;
let (org, project);

if env::var(CHUNK_UPLOAD_ENV_VAR) == Ok("1".into()) {
log::warn!(
"EXPERIMENTAL FEATURE: Uploading proguard mappings using chunked uploading. \
Some functionality may be unavailable when using chunked uploading. Please unset \
the {CHUNK_UPLOAD_ENV_VAR} variable if you encounter any \
problems."
);

authenticated_api = api.authenticated()?;
(org, project) = config.get_org_and_project(matches)?;

let chunk_upload_options = authenticated_api.get_chunk_upload_options(&org)?;

proguard::chunk_upload(&mappings, chunk_upload_options, &org, &project)?;
} else {
if mappings.is_empty() && matches.get_flag("require_one") {
println!();
eprintln!("{}", style("error: found no mapping files to upload").red());
return Err(QuietExit(1).into());
}

println!("{} compressing mappings", style(">").dim());
let tf = TempFile::create()?;
{
let mut zip = zip::ZipWriter::new(tf.open()?);
for mapping in &mappings {
let pb = make_byte_progress_bar(mapping.len() as u64);
zip.start_file(
format!("proguard/{}.txt", mapping.uuid()),
zip::write::SimpleFileOptions::default(),
)?;
copy_with_progress(&pb, &mut mapping.as_ref(), &mut zip)?;
pb.finish_and_clear();
}
}
if mappings.is_empty() && matches.get_flag("require_one") {
println!();
eprintln!("{}", style("error: found no mapping files to upload").red());
return Err(QuietExit(1).into());
}

// write UUIDs into the mapping file.
if let Some(p) = matches.get_one::<String>("write_properties") {
let uuids: Vec<_> = mappings.iter().map(|x| x.uuid()).collect();
dump_proguard_uuids_as_properties(p, &uuids)?;
}
// write UUIDs into the mapping file.
if let Some(p) = matches.get_one::<String>("write_properties") {
let uuids: Vec<_> = mappings.iter().map(|x| x.uuid()).collect();
dump_proguard_uuids_as_properties(p, &uuids)?;
}

if matches.get_flag("no_upload") {
println!("{} skipping upload.", style(">").dim());
return Ok(());
}
if matches.get_flag("no_upload") {
println!("{} skipping upload.", style(">").dim());
return Ok(());
}

println!("{} uploading mappings", style(">").dim());
(org, project) = config.get_org_and_project(matches)?;
let authenticated_api = api.authenticated()?;
let (org, project) = config.get_org_and_project(matches)?;

authenticated_api = api.authenticated()?;
let chunk_upload_options = authenticated_api.get_chunk_upload_options(&org)?;

let rv = authenticated_api
.region_specific(&org)
.upload_dif_archive(&project, tf.path())?;
println!(
"{} Uploaded a total of {} new mapping files",
style(">").dim(),
style(rv.len()).yellow()
);
if !rv.is_empty() {
println!("Newly uploaded debug symbols:");
for df in rv {
println!(" {}", style(&df.id()).dim());
}
}
if chunk_upload_options.supports(ChunkUploadCapability::Proguard) {
proguard::chunk_upload(&mappings, chunk_upload_options, &org, &project)
} else {
Err(anyhow::anyhow!("Server does not support uploading ProGuard mappings via chunked upload. Please update your Sentry server."))
}

Ok(())
}
5 changes: 0 additions & 5 deletions src/utils/proguard/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@ pub struct ProguardMapping<'a> {
}

impl<'a> ProguardMapping<'a> {
/// Get the length of the mapping in bytes.
pub fn len(&self) -> usize {
self.bytes.len()
}

/// Get the UUID of the mapping.
pub fn uuid(&self) -> Uuid {
self.uuid
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
$ sentry-cli upload-proguard tests/integration/_fixtures/proguard.txt --no-upload
? success
warning: ignoring proguard mapping 'tests/integration/_fixtures/proguard.txt': Proguard mapping does not contain line information
> compressing mappings
> skipping upload.

```

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@
"concurrency": 8,
"hashAlgorithm": "sha1",
"compression": ["gzip"],
"accept": ["debug_files", "release_files", "pdbs", "portablepdbs", "sources", "bcsymbolmaps"]
"accept": ["debug_files", "release_files", "pdbs", "portablepdbs", "sources", "bcsymbolmaps", "proguard"]
Copy link
Member Author

Choose a reason for hiding this comment

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

Sentry server declares ProGuard support since getsentry/sentry#82304.

}
6 changes: 0 additions & 6 deletions tests/integration/test_utils/test_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,6 @@ impl AssertCmdTestManager {
self
}

/// Set a custom environment variable for the test.
pub fn env(mut self, variable: impl AsRef<OsStr>, value: impl AsRef<OsStr>) -> Self {
self.command.env(variable, value);
self
}

/// Run the command and perform assertions.
///
/// This function asserts both the mocks and the command result.
Expand Down
11 changes: 1 addition & 10 deletions tests/integration/upload_proguard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,7 @@ use crate::integration::{MockEndpointBuilder, TestManager};

#[test]
fn command_upload_proguard() {
TestManager::new()
.mock_endpoint(
MockEndpointBuilder::new("POST", "/api/0/projects/wat-org/wat-project/files/dsyms/")
.with_response_body("[]"),
)
Comment on lines -12 to -15
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed this mock per Cursor's comment; mocking the chunk upload endpoints here is not needed, though, as other tests below check the upload

.register_trycmd_test("upload_proguard/*.trycmd")
.with_default_token();
TestManager::new().register_trycmd_test("upload_proguard/*.trycmd");
}

#[test]
Expand Down Expand Up @@ -72,7 +66,6 @@ fn chunk_upload_already_there() {
"tests/integration/_fixtures/upload_proguard/mapping.txt",
])
.with_default_token()
.env("SENTRY_EXPERIMENTAL_PROGUARD_CHUNK_UPLOAD", "1")
.run_and_assert(AssertCommand::Success)
}

Expand Down Expand Up @@ -166,7 +159,6 @@ fn chunk_upload_needs_upload() {
"tests/integration/_fixtures/upload_proguard/mapping.txt",
])
.with_default_token()
.env("SENTRY_EXPERIMENTAL_PROGUARD_CHUNK_UPLOAD", "1")
.run_and_assert(AssertCommand::Success)
}

Expand Down Expand Up @@ -289,6 +281,5 @@ fn chunk_upload_two_files() {
"tests/integration/_fixtures/upload_proguard/mapping-2.txt",
])
.with_default_token()
.env("SENTRY_EXPERIMENTAL_PROGUARD_CHUNK_UPLOAD", "1")
.run_and_assert(AssertCommand::Success)
}
Loading