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

Add command sentry-cli debug-files bundle-jvm for bundling Java (and other JVM based languages) sources #1551

Merged
merged 32 commits into from
Apr 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
ccb8b39
Add command for bundling Java sources
adinauer Mar 29, 2023
77c49a1
remove commented out code
adinauer Mar 29, 2023
05f0c7e
Allow passing in debug_id; add tests; cleanup
adinauer Apr 6, 2023
1c8034c
Fix existing tests
adinauer Apr 6, 2023
6af257e
Test help
adinauer Apr 6, 2023
2adb20d
Merge branch 'master' into feat/java-source-bundling
adinauer Apr 6, 2023
7fff72d
Fix tests after merge
adinauer Apr 6, 2023
9fc5b01
Apply fmt
adinauer Apr 6, 2023
f1a1a0e
fix clippy
adinauer Apr 6, 2023
80462d8
Add tests for windows
adinauer Apr 6, 2023
696c658
fmt
adinauer Apr 6, 2023
a883b56
add comma
adinauer Apr 6, 2023
7fea001
fix windows test
adinauer Apr 6, 2023
280c47f
revert windows test
adinauer Apr 6, 2023
4241cc4
try to fix windows test
adinauer Apr 6, 2023
3644f88
another try
adinauer Apr 6, 2023
6084679
yet another try
adinauer Apr 6, 2023
907dea9
Apply suggestions from code review
adinauer Apr 7, 2023
958c8f4
Merge windows and non windows help test
adinauer Apr 7, 2023
c749169
clippy
adinauer Apr 7, 2023
76d8a16
Update expected test output after merging suggestions (caused by added)
adinauer Apr 7, 2023
73f0436
If out dir does not exist, create it
adinauer Apr 7, 2023
629ebdb
fix tests
adinauer Apr 7, 2023
06e615b
Expect success
adinauer Apr 7, 2023
f5a2ef3
Unify windows and non windows test
adinauer Apr 7, 2023
ba0227a
Remove commented out short options
adinauer Apr 11, 2023
624a52c
PR feedback; use clap value_parser; move code; etc
adinauer Apr 11, 2023
db04bee
Merge is-file tests for windows and non windows
adinauer Apr 11, 2023
c348d99
Rename command to bundle-jvm
adinauer Apr 11, 2023
e05eacb
Rename JvmBased to Jvm
adinauer Apr 11, 2023
b83271d
fmt
adinauer Apr 11, 2023
04418d6
Forgot to add renamed dir after move
adinauer Apr 11, 2023
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
110 changes: 110 additions & 0 deletions src/commands/debug_files/bundle_jvm.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
use std::fs;
use std::path::{PathBuf};
use std::str::FromStr;
use anyhow::{bail, Context, Result};
use clap::{Arg, ArgMatches, Command};
use sentry::types::DebugId;
use symbolic::debuginfo::sourcebundle::{SourceFileType};
use crate::api::Api;
use crate::config::Config;
use crate::utils::args::ArgExt;
use crate::utils::file_search::ReleaseFileSearch;
use crate::utils::file_upload::{FileUpload, SourceFile, UploadContext};
use crate::utils::fs::{path_as_url};

pub fn make_command(command: Command) -> Command {
command
.hide(true) // experimental for now
.about("Create a source bundle for the given JVM based source files (e.g. Java, Kotlin, ...)")
.org_arg()
.project_arg(false)
.arg(
Arg::new("path")
.value_name("PATH")
.required(true)
.value_parser(clap::builder::PathBufValueParser::new())
.help("The directory containing source files to bundle."),
)
.arg(
Arg::new("output")
.long("output")
.value_name("PATH")
.required(true)
.value_parser(clap::builder::PathBufValueParser::new())
.help("The path to the output folder."),
)
.arg(
Arg::new("debug_id")
.long("debug-id")
.value_name("UUID")
.required(true)
.value_parser(DebugId::from_str)
.help("Debug ID (UUID) to use for the source bundle."),
)
}

pub fn execute(matches: &ArgMatches) -> Result<()> {
let config = Config::current();
let org = config.get_org(matches)?;
let project = config.get_project(matches).ok();
let api = Api::current();
let chunk_upload_options = api.get_chunk_upload_options(&org)?;
let context = &UploadContext {
org: &org,
project: project.as_deref(),
release: None,
dist: None,
note: None,
wait: true,
dedupe: false,
chunk_upload_options: chunk_upload_options.as_ref(),
};
let path = matches.get_one::<PathBuf>("path").unwrap();
let output_path = matches.get_one::<PathBuf>("output").unwrap();
let debug_id = matches.get_one::<DebugId>("debug_id").unwrap();
let out = output_path.join(format!("{debug_id}.zip"));

if !path.exists() {
bail!("Given path does not exist: {}", path.display())
}

if !path.is_dir() {
bail!("Given path is not a directory: {}", path.display())
}

if !output_path.exists() {
fs::create_dir_all(output_path).context(format!("Failed to create output directory {}", output_path.display()))?;
}

let sources = ReleaseFileSearch::new(path.to_path_buf()).collect_files()?;
let files = sources
.iter()
.map(|source| {
let local_path = source.path.strip_prefix(&source.base_path).unwrap();
let local_path_jvm_ext = local_path.with_extension("jvm");
let url = format!("~/{}", path_as_url(&local_path_jvm_ext));
(
url.to_string(),
SourceFile {
url,
path: source.path.clone(),
contents: source.contents.clone(),
ty: SourceFileType::Source,
headers: vec![],
messages: vec![],
already_uploaded: false,
},
)
})
.collect();

let tempfile = FileUpload::new(context)
.files(&files)
.build_jvm_bundle(Some(*debug_id))
.context("Unable to create source bundle")?;

fs::copy(tempfile.path(), &out).context("Unable to write source bundle")?;
println!("Created {}", out.display());

Ok(())
}
1 change: 1 addition & 0 deletions src/commands/debug_files/find.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ fn find_ids(
DifType::SourceBundle => find_ids_for_sourcebundle(&dirent, &remaining),
DifType::Breakpad => find_ids_for_breakpad(&dirent, &remaining),
DifType::Proguard => find_ids_for_proguard(&dirent, &proguard_uuids),
DifType::Jvm => find_ids_for_sourcebundle(&dirent, &remaining),
DifType::Wasm => None,
})
.flatten()
Expand Down
2 changes: 2 additions & 0 deletions src/commands/debug_files/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use anyhow::Result;
use clap::{ArgMatches, Command};

pub mod bundle_jvm;
pub mod bundle_sources;
pub mod check;
pub mod find;
Expand All @@ -11,6 +12,7 @@ macro_rules! each_subcommand {
($mac:ident) => {
$mac!(bundle_sources);
$mac!(check);
$mac!(bundle_jvm);
$mac!(find);
$mac!(print_sources);
$mac!(upload);
Expand Down
1 change: 1 addition & 0 deletions src/commands/debug_files/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
"pe" => upload.filter_format(DifFormat::Object(FileFormat::Pe)),
"sourcebundle" => upload.filter_format(DifFormat::Object(FileFormat::SourceBundle)),
"portablepdb" => upload.filter_format(DifFormat::Object(FileFormat::PortablePdb)),
"jvm" => upload.filter_format(DifFormat::Object(FileFormat::SourceBundle)),
"bcsymbolmap" => {
upload.filter_format(DifFormat::BcSymbolMap);
upload.filter_format(DifFormat::PList)
Expand Down
6 changes: 6 additions & 0 deletions src/utils/dif.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub enum DifType {
Pdb,
PortablePdb,
Wasm,
Jvm,
Copy link
Member

Choose a reason for hiding this comment

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

Do we really want this here? The file we create is just a SourceBundle, and no debug file on its own exists for JVM.

Copy link
Member Author

Choose a reason for hiding this comment

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

In an internal discussion we didn't decide whether we actually need the new type but it was suggested to err on the side of defining a new one. Do you want me to reopen that conversation internally? Handling of jvm bundles is different from others in that files are renamed while bundling and lookup constructs the file path from module and abs_path if available.

Copy link
Member

Choose a reason for hiding this comment

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

I guess it doesn’t hurt either :-)

}

impl DifType {
Expand All @@ -35,6 +36,7 @@ impl DifType {
DifType::Breakpad => "breakpad",
DifType::Proguard => "proguard",
DifType::Wasm => "wasm",
DifType::Jvm => "jvm",
}
}

Expand All @@ -49,6 +51,7 @@ impl DifType {
DifType::Breakpad,
DifType::Proguard,
DifType::Wasm,
DifType::Jvm,
]
}

Expand All @@ -63,6 +66,7 @@ impl DifType {
"breakpad",
"proguard",
"wasm",
"jvm",
]
}
}
Expand All @@ -87,6 +91,7 @@ impl str::FromStr for DifType {
"breakpad" => Ok(DifType::Breakpad),
"proguard" => Ok(DifType::Proguard),
"wasm" => Ok(DifType::Wasm),
"jvm" => Ok(DifType::Jvm),
_ => bail!("Invalid debug info file type"),
}
}
Expand Down Expand Up @@ -251,6 +256,7 @@ impl DifFile<'static> {
Some(DifType::Wasm) => DifFile::open_object(path, FileFormat::Wasm),
Some(DifType::Breakpad) => DifFile::open_object(path, FileFormat::Breakpad),
Some(DifType::Proguard) => DifFile::open_proguard(path),
Some(DifType::Jvm) => DifFile::open_object(path, FileFormat::SourceBundle),
None => DifFile::try_open(path),
}
}
Expand Down
16 changes: 13 additions & 3 deletions src/utils/file_upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ impl<'a> FileUpload<'a> {
.map_or(DEFAULT_CONCURRENCY, |o| usize::from(o.concurrency));
upload_files_parallel(self.context, &self.files, concurrency)
}

pub fn build_jvm_bundle(&self, debug_id: Option<DebugId>) -> Result<TempFile> {
build_artifact_bundle(self.context, &self.files, debug_id)
}
}

fn upload_files_parallel(
Expand Down Expand Up @@ -274,7 +278,7 @@ fn upload_files_chunked(
files: &SourceFiles,
options: &ChunkUploadOptions,
) -> Result<()> {
let archive = build_artifact_bundle(context, files)?;
let archive = build_artifact_bundle(context, files, None)?;

let progress_style =
ProgressStyle::default_spinner().template("{spinner} Optimizing bundle for upload...");
Expand Down Expand Up @@ -396,7 +400,11 @@ fn build_debug_id(files: &SourceFiles) -> DebugId {
DebugId::from_uuid(uuid::Builder::from_sha1_bytes(sha1_bytes).into_uuid())
}

fn build_artifact_bundle(context: &UploadContext, files: &SourceFiles) -> Result<TempFile> {
fn build_artifact_bundle(
context: &UploadContext,
files: &SourceFiles,
debug_id: Option<DebugId>,
) -> Result<TempFile> {
let progress_style = ProgressStyle::default_bar().template(
"{prefix:.dim} Bundling files for upload... {msg:.dim}\
\n{wide_bar} {pos}/{len}",
Expand All @@ -410,7 +418,9 @@ fn build_artifact_bundle(context: &UploadContext, files: &SourceFiles) -> Result
let mut bundle = SourceBundleWriter::start(BufWriter::new(archive.open()?))?;

// artifact bundles get a random UUID as debug id
bundle.set_attribute("debug_id", build_debug_id(files).to_string());
let debug_id = debug_id.unwrap_or_else(|| build_debug_id(files));
bundle.set_attribute("debug_id", debug_id.to_string());

if let Some(note) = context.note {
bundle.set_attribute("note", note.to_owned());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
```
$ sentry-cli debug-files bundle-jvm --help
? success
Create a source bundle for the given JVM based source files (e.g. Java, Kotlin, ...)

Usage: sentry-cli[EXE] debug-files bundle-jvm [OPTIONS] --output <PATH> --debug-id <UUID> <PATH>

Arguments:
<PATH> The directory containing source files to bundle.

Options:
-o, --org <ORG> The organization slug
--header <KEY:VALUE> Custom headers that should be attached to all requests
in key:value format.
-p, --project <PROJECT> The project slug.
--auth-token <AUTH_TOKEN> Use the given Sentry auth token.
--output <PATH> The path to the output folder.
--debug-id <UUID> Debug ID (UUID) to use for the source bundle.
--log-level <LOG_LEVEL> Set the log output verbosity. [possible values: trace, debug, info,
warn, error]
--quiet Do not print any output while preserving correct exit code. This
flag is currently implemented only for selected subcommands.
[aliases: silent]
-h, --help Print help

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
```
$ sentry-cli debug-files bundle-jvm --output . --debug-id 48dee70b-4f3f-4a49-9223-de441738f7cd empty-dir
? success
> Found 0 files
> Bundled 0 files for upload
Created ./48dee70b-4f3f-4a49-9223-de441738f7cd.zip

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
```
$ sentry-cli debug-files bundle-jvm --output `cwd` --debug-id 59144E0E-52C4-41F8-9111-0D6F8F14905B tests/integration/_fixtures/jvm/io/sentry/sample/MainActivity.java
? failed
error: Given path is not a directory: tests/integration/_fixtures/jvm/io/sentry/sample/MainActivity.java

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
```
$ sentry-cli debug-files bundle-jvm --output `cwd` --debug-id 7A575F05-0585-4DB0-95DE-D0C032D7C707 tests/integration/_fixtures/i-do-not-exist
? failed
error: Given path does not exist: tests/integration/_fixtures/i-do-not-exist

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
```
$ sentry-cli debug-files bundle-jvm --output . --debug-id not-a-valid-uuid io
? failed
error: invalid value 'not-a-valid-uuid' for '--debug-id <UUID>': invalid debug identifier

For more information, try '--help'.

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
```
$ sentry-cli debug-files bundle-jvm --output ./file.txt --debug-id D384DC3B-AB2F-4DC7-903D-2C851E27E094 ./io
? failed
> Found 2 files
> Bundled 2 files for upload
error: Unable to write source bundle
caused by: [..]

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
```
$ sentry-cli debug-files bundle-jvm --output i-do-not-exist --debug-id 0B693ABA-531C-4EB6-99E4-B7320C3C85DA tests/integration/_fixtures/jvm
? success
> Found 2 files
> Bundled 2 files for upload
Created i-do-not-exist/0b693aba-531c-4eb6-99e4-b7320c3c85da.zip

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
```
$ sentry-cli debug-files bundle-jvm --output . --debug-id a4368a48-0880-40d7-9a26-c9ef5a84d156 ./io
? success
> Found 2 files
> Bundled 2 files for upload
Created ./a4368a48-0880-40d7-9a26-c9ef5a84d156.zip

```
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Options:
-t, --type <TYPE> Only consider debug information files of the given type. By
default, all types are considered. [possible values: bcsymbolmap,
dsym, elf, pe, pdb, portablepdb, sourcebundle, breakpad, proguard,
wasm]
wasm, jvm]
--no-unwind Do not scan for stack unwinding information. Specify this flag for
builds with disabled FPO, or when stackwalking occurs on the
device. This usually excludes executables and dynamic libraries.
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/_cases/upload_dif/upload_dif-help.trycmd
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Options:
-t, --type <TYPE> Only consider debug information files of the given type. By
default, all types are considered. [possible values: bcsymbolmap,
dsym, elf, pe, pdb, portablepdb, sourcebundle, breakpad, proguard,
wasm]
wasm, jvm]
--no-unwind Do not scan for stack unwinding information. Specify this flag for
builds with disabled FPO, or when stackwalking occurs on the
device. This usually excludes executables and dynamic libraries.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package integration._fixtures.jvm.io.sentry.sample;

public class MainActivity {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package integration._fixtures.jvm.io.sentry.sample

class SampleActivity {
}
Loading