Add command sentry-cli debug-files bundle-jvm for bundling Java (and other JVM based languages) sources
#1551
Conversation
adinauer
changed the title
sentry-cli difutil create-source-bundle for bundling Java sourcessentry-cli debug-files create-jvm-based-bundle for bundling Java (and other JVM based languages) sources
|
I assume changelog is written during release process. If not I can add something do it. |
|
|
Co-authored-by: Sebastian Zivota <loewenheim@users.noreply.github.com>
|
@loewenheim I could if there was a way to unify The other test looks like I should be able to unify now that there's no more error message that differs. Will update. Any idea how to unify the one above? |
|
Ah, that's unfortunate :/ In that case I would just leave them separate. |
| let project = config.get_project(matches).ok(); | ||
| let api = Api::current(); | ||
| let chunk_upload_options = api.get_chunk_upload_options(&org)?; | ||
| let context = &UploadContext { |
There was a problem hiding this comment.
@loewenheim something that we should refactor soon (whole UploadContext should have a constructor or a Default implementation, or a builder.
|
You can write |
src/commands/debug_files/find.rs
Outdated
| @@ -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), | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
adinauer
changed the title
sentry-cli debug-files create-jvm-based-bundle for bundling Java (and other JVM based languages) sourcessentry-cli debug-files bundle-jvm for bundling Java (and other JVM based languages) sources
|
I've updated the PR according to feedback. Can you please give it anohter pass to make sure it's as you expected 🙏 |
|
lgtm! well done :) |
| @@ -21,6 +21,7 @@ pub enum DifType { | |||
| Pdb, | |||
| PortablePdb, | |||
| Wasm, | |||
| Jvm, | |||
There was a problem hiding this comment.
Do we really want this here? The file we create is just a SourceBundle, and no debug file on its own exists for JVM.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I guess it doesn’t hurt either :-)
There was a problem hiding this comment.
@adinauer walked me through the changes and it seems that there are no obvious problems, so lgtm
Takes a path of source files which is bundled up.
Required for fixing: getsentry/sentry-java#633