Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Unreleased

### Features

- (bundle-jvm) Allow running directly on a project root (including multi-module repos) by automatically collecting only JVM source files (`.java`, `.kt`, `.scala`, `.groovy`), respecting `.gitignore`, and excluding common build output directories ([#3260](https://github.com/getsentry/sentry-cli/pull/3260))
- (bundle-jvm) Add `--exclude` option for custom glob patterns to exclude files/directories from source collection ([#3260](https://github.com/getsentry/sentry-cli/pull/3260))

### Fixes

- Replace `eprintln!` with `log::info!` for progress bar completion messages when the progress bar is disabled (e.g. in CI). This avoids spurious stderr output that some CI systems treat as errors ([#3223](https://github.com/getsentry/sentry-cli/pull/3223)).
Expand Down
170 changes: 167 additions & 3 deletions src/commands/debug_files/bundle_jvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,70 @@ use crate::utils::file_upload::SourceFile;
use crate::utils::fs::path_as_url;
use crate::utils::source_bundle::{self, BundleContext};
use anyhow::{bail, Context as _, Result};
use clap::{Arg, ArgMatches, Command};
use clap::{Arg, ArgAction, ArgMatches, Command};
use log::debug;
use sentry::types::DebugId;
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::ffi::OsStr;
use std::fs;
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::str::FromStr as _;
use std::sync::Arc;
use symbolic::debuginfo::sourcebundle::SourceFileType;

/// File extensions for JVM-based languages.
const JVM_EXTENSIONS: &[&str] = &[
"java", "kt", "scala", "sc", "groovy", "gvy", "gy", "gsh", "clj", "cljc",
];
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
szokeasaurusrex marked this conversation as resolved.

/// Directory patterns that are always safe to exclude globally (can never be
/// valid JVM package names due to leading dots or conventions).
const SAFE_EXCLUDES: &[&str] = &[
"!.cxx",
"!.eclipse",
"!.fleet",
"!.gradle",
"!.idea",
"!.kotlin",
"!.mvn",
"!.settings",
"!.vscode",
"!node_modules",
];
Comment thread
cursor[bot] marked this conversation as resolved.

/// Directory names that are common build output dirs but could also be valid
/// JVM package names (e.g. `com.example.build`). These are only excluded when
/// they appear outside of `src/` directories to avoid filtering out legitimate
/// source packages.
const AMBIGUOUS_EXCLUDES: &[&str] = &["bin", "build", "out", "target"];

/// Returns true if the file should be excluded because it sits inside an
/// ambiguous build-output directory that is NOT under a `src/` ancestor.
///
/// We check *all* ambiguous directories in the path and exclude if any of them
/// is not under a `src/` ancestor. This handles nested cases like
/// `build/src/main/java/com/example/target/Foo.java` where the inner `target`
/// is under `src`, but the outer `build` is not — the file should be excluded.
fn is_in_ambiguous_build_dir(relative_path: &Path) -> bool {
for ancestor in relative_path.ancestors() {
let Some(name) = ancestor.file_name().and_then(|n| n.to_str()) else {
continue;
};
if AMBIGUOUS_EXCLUDES.contains(&name) {
// Check if any ancestor *above* this directory is named "src".
let has_src_above = ancestor
.ancestors()
.skip(1) // skip the ambiguous dir itself
.any(|a| a.file_name() == Some(OsStr::new("src")));
if !has_src_above {
return true;
}
}
}
false
}
Comment thread
sentry[bot] marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.

pub fn make_command(command: Command) -> Command {
command
.hide(true) // experimental for now
Expand Down Expand Up @@ -47,6 +102,17 @@ pub fn make_command(command: Command) -> Command {
.value_parser(DebugId::from_str)
.help("Debug ID (UUID) to use for the source bundle."),
)
.arg(
Arg::new("exclude")
.long("exclude")
.value_name("PATTERN")
.action(ArgAction::Append)
.help(
"Glob pattern to exclude files/directories. Can be repeated. \
By default, common build output and IDE directories are excluded \
(build, .gradle, target, .idea, .vscode, out, bin, etc.).",
),
)
}

pub fn execute(matches: &ArgMatches) -> Result<()> {
Expand Down Expand Up @@ -75,7 +141,36 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
))?;
}

let sources = ReleaseFileSearch::new(path.clone()).collect_files()?;
let user_excludes = matches
.get_many::<String>("exclude")
.into_iter()
.flatten()
.map(|v| format!("!{v}"));

let all_excludes = SAFE_EXCLUDES
.iter()
.copied()
.map(Cow::Borrowed)
.chain(user_excludes.map(Cow::Owned));

let sources = ReleaseFileSearch::new(path.clone())
.extensions(JVM_EXTENSIONS.iter().copied())
.ignores(all_excludes)
.respect_ignores(true)
.collect_files()?;

let sources: Vec<_> = sources
.into_iter()
.filter(|source| {
let relative = source.path.strip_prefix(&source.base_path).unwrap();
if is_in_ambiguous_build_dir(relative) {
debug!("excluding (build output): {}", source.path.display());
return false;
}
true
})
.collect();

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");
Expand All @@ -100,3 +195,72 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {

Ok(())
}

#[cfg(test)]
mod tests {
use super::*;
use std::path::Path;

#[test]
fn test_excludes_build_output_at_module_root() {
assert!(is_in_ambiguous_build_dir(Path::new(
"app/build/generated/Foo.java"
)));
assert!(is_in_ambiguous_build_dir(Path::new(
"build/generated/Foo.java"
)));
assert!(is_in_ambiguous_build_dir(Path::new(
"module/target/classes/Foo.java"
)));
assert!(is_in_ambiguous_build_dir(Path::new("bin/Foo.class")));
assert!(is_in_ambiguous_build_dir(Path::new(
"out/production/Foo.java"
)));
}

#[test]
fn test_keeps_source_packages_under_src() {
assert!(!is_in_ambiguous_build_dir(Path::new(
"src/main/java/com/example/build/Builder.java"
)));
assert!(!is_in_ambiguous_build_dir(Path::new(
"app/src/main/java/com/example/target/Target.java"
)));
assert!(!is_in_ambiguous_build_dir(Path::new(
"src/main/kotlin/com/example/out/Output.kt"
)));
}

#[test]
fn test_excludes_build_dir_containing_src() {
// build/src/... should still be excluded — src is *inside* build, not above it
assert!(is_in_ambiguous_build_dir(Path::new(
"build/src/main/java/Foo.java"
)));
assert!(is_in_ambiguous_build_dir(Path::new(
"app/build/src/generated/Foo.java"
)));
}

#[test]
fn test_excludes_nested_ambiguous_dirs_under_build() {
// build/src/.../target/ — inner `target` is under src, but outer `build` is not
assert!(is_in_ambiguous_build_dir(Path::new(
"build/src/main/java/com/example/target/Foo.java"
)));
assert!(is_in_ambiguous_build_dir(Path::new(
"target/src/main/java/com/example/out/Foo.java"
)));
}

#[test]
fn test_keeps_files_without_ambiguous_dirs() {
assert!(!is_in_ambiguous_build_dir(Path::new(
"src/main/java/com/example/Foo.java"
)));
assert!(!is_in_ambiguous_build_dir(Path::new("Foo.java")));
assert!(!is_in_ambiguous_build_dir(Path::new(
"app/src/main/java/Foo.java"
)));
}
}
17 changes: 12 additions & 5 deletions src/utils/file_search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub struct ReleaseFileSearch {
ignores: BTreeSet<String>,
ignore_file: Option<String>,
decompress: bool,
respect_ignores: bool,
}

#[derive(Eq, PartialEq, Hash)]
Expand All @@ -37,6 +38,7 @@ impl ReleaseFileSearch {
ignore_file: None,
ignores: BTreeSet::new(),
decompress: false,
respect_ignores: false,
}
}

Expand Down Expand Up @@ -78,6 +80,11 @@ impl ReleaseFileSearch {
self
}

pub fn respect_ignores(&mut self, respect: bool) -> &mut Self {
self.respect_ignores = respect;
self
}

pub fn collect_file(path: PathBuf) -> Result<ReleaseFileMatch> {
// NOTE: `collect_file` currently do not handle gzip decompression,
// as its mostly used for 3rd tools like xcode or gradle.
Expand Down Expand Up @@ -105,11 +112,11 @@ impl ReleaseFileSearch {
let mut collected = Vec::new();

let mut builder = WalkBuilder::new(&self.path);
builder
.follow_links(true)
.git_exclude(false)
.git_ignore(false)
.ignore(false);
builder.follow_links(true);

if !self.respect_ignores {
builder.git_exclude(false).git_ignore(false).ignore(false);
}

if !&self.extensions.is_empty() {
let mut types_builder = TypesBuilder::new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ Options:
--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]
--exclude <PATTERN> Glob pattern to exclude files/directories. Can be repeated. By
default, common build output and IDE directories are excluded
(build, .gradle, target, .idea, .vscode, out, bin, etc.).
--quiet Do not print any output while preserving correct exit code. This
flag is currently implemented only for selected subcommands.
[aliases: --silent]
Expand Down
Loading