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
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Allow passing in debug_id; add tests; cleanup
adinauer committed Apr 6, 2023

Verified

This commit was signed with the committer’s verified signature.
mikz Michal Cichra
commit 05f0c7e826f9b0c2a102dfeb517be8be992b0e05
112 changes: 112 additions & 0 deletions src/commands/debug_files/create_jvm_based_bundle.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
use std::fs;
use std::path::{Path};
use anyhow::{bail, Result};
use clap::{Arg, ArgMatches, Command};
use sentry::types::DebugId;
use symbolic::debuginfo::sourcebundle::{SourceFileType};
use uuid::{Uuid};
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)
.help("The directory containing source files to bundle."),
)
.arg(
Arg::new("output")
// .short('o')
.long("output")
.value_name("PATH")
.required(true)
.help("The path to the output folder."),
)
.arg(
Arg::new("debug_id")
// .short('d')
.long("debug-id")
.value_name("UUID")
.required(true)
.help("Debug ID 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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

@loewenheim something that we should refactor soon (whole UploadContext should have a constructor or a Default implementation, or a builder.

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 = Path::new(matches.get_one::<String>("path").unwrap());
let output_path = Path::new(matches.get_one::<String>("output").unwrap());
let debug_id_arg = matches.get_one::<String>("debug_id").unwrap();
let debug_id_uuid = Uuid::parse_str(debug_id_arg);
if debug_id_uuid.is_err() {
bail!("Given debug_id is invalid: {}", debug_id_arg)
}
let debug_id = DebugId::from_uuid(debug_id_uuid.unwrap());
let mut debug_id_string = debug_id.to_string().to_owned();
debug_id_string.push_str(".zip");
let out = output_path.join(Path::new(&debug_id_string));

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

if path.is_dir() {
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 copy_result = match FileUpload::new(context).files(&files).build_jvm_based_bundle(Some(debug_id)) {
Ok(tempfile) => fs::copy(tempfile.path(), &out),
Err(e) => bail!("Unable to create source bundle: {}", e),
};
match copy_result {
Ok(_) => println!("Created {}", out.to_string_lossy()),
Err(e) => bail!("Unable to write source bundle: {}", e)
}
Ok(())
} else {
bail!("Given path is not a directory: {}", path.to_string_lossy())
}
}
184 changes: 0 additions & 184 deletions src/commands/debug_files/create_source_bundle.rs

This file was deleted.

1 change: 1 addition & 0 deletions src/commands/debug_files/find.rs
Original file line number Diff line number Diff line change
@@ -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::JvmBased => find_ids_for_sourcebundle(&dirent, &remaining),
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about the whole jvmbased naming. We dont use PdbBased, ElfBased, ProguardBasd etc. Might use JvmBundle or just Jvm instead? wdyt @Swatinem

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I guess there's little risk in people actually thinking they need to upload a JVM. Can rename it here and in other PRs as this was suggested before. Happy to rename and make it easier to use.

Copy link
Member

Choose a reason for hiding this comment

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

Well, naming is only one of the hardest things in software… :-D I’m okay with the naming. It clearly states that we have are dealing with any of the myriad of languages that compile to java bytecode and run on the jvm.

DifType::Wasm => None,
})
.flatten()
4 changes: 2 additions & 2 deletions src/commands/debug_files/mod.rs
Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@ use clap::{ArgMatches, Command};

pub mod bundle_sources;
pub mod check;
pub mod create_source_bundle;
pub mod create_jvm_based_bundle;
pub mod find;
pub mod print_sources;
pub mod upload;
@@ -12,7 +12,7 @@ macro_rules! each_subcommand {
($mac:ident) => {
$mac!(bundle_sources);
$mac!(check);
$mac!(create_source_bundle);
$mac!(create_jvm_based_bundle);
$mac!(find);
$mac!(print_sources);
$mac!(upload);
1 change: 1 addition & 0 deletions src/commands/debug_files/upload.rs
Original file line number Diff line number Diff line change
@@ -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)),
"jvmbased" => upload.filter_format(DifFormat::Object(FileFormat::SourceBundle)),
"bcsymbolmap" => {
upload.filter_format(DifFormat::BcSymbolMap);
upload.filter_format(DifFormat::PList)
6 changes: 6 additions & 0 deletions src/utils/dif.rs
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@ pub enum DifType {
Pdb,
PortablePdb,
Wasm,
JvmBased,
}

impl DifType {
@@ -35,6 +36,7 @@ impl DifType {
DifType::Breakpad => "breakpad",
DifType::Proguard => "proguard",
DifType::Wasm => "wasm",
DifType::JvmBased => "jvmbased",
}
}

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

@@ -63,6 +66,7 @@ impl DifType {
"breakpad",
"proguard",
"wasm",
"jvmbased",
]
}
}
@@ -87,6 +91,7 @@ impl str::FromStr for DifType {
"breakpad" => Ok(DifType::Breakpad),
"proguard" => Ok(DifType::Proguard),
"wasm" => Ok(DifType::Wasm),
"jvmbased" => Ok(DifType::JvmBased),
_ => bail!("Invalid debug info file type"),
}
}
@@ -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::JvmBased) => DifFile::open_object(path, FileFormat::SourceBundle),
None => DifFile::try_open(path),
}
}
Loading