diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index 0e38b5de84..872228f95f 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -100,7 +100,7 @@ pub struct GitFetchArgs { /// /// By default, the specified name matches exactly. Use `glob:` prefix to /// expand `*` as a glob. The other wildcard characters aren't supported. - #[arg(long, value_parser = parse_string_pattern)] + #[arg(long, default_value = "glob:*", value_parser = parse_string_pattern)] branch: Vec, /// The remote to fetch from (only named remotes are supported, can be /// repeated) @@ -319,7 +319,7 @@ fn cmd_git_fetch( tx.mut_repo(), &git_repo, &remote, - (!args.branch.is_empty()).then_some(&args.branch), + &args.branch, cb, &command.settings().git_settings(), ) @@ -535,7 +535,7 @@ fn do_git_clone( fetch_tx.mut_repo(), &git_repo, remote_name, - None, + &[StringPattern::everything()], cb, &command.settings().git_settings(), ) diff --git a/lib/src/git.rs b/lib/src/git.rs index 3e77b7db3c..d2540d5c34 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -1001,16 +1001,10 @@ pub fn fetch( mut_repo: &mut MutableRepo, git_repo: &git2::Repository, remote_name: &str, - branch_names: Option<&[StringPattern]>, + branch_names: &[StringPattern], callbacks: RemoteCallbacks<'_>, git_settings: &GitSettings, ) -> Result { - let branch_name_filter = |branch: &str| { - branch_names.map_or(true, |patterns| { - patterns.iter().any(|pattern| pattern.matches(branch)) - }) - }; - // Perform a `git fetch` on the local git repo, updating the remote-tracking // branches in the git repo. let mut remote = git_repo.find_remote(remote_name).map_err(|err| { @@ -1026,27 +1020,28 @@ pub fn fetch( fetch_options.proxy_options(proxy_options); let callbacks = callbacks.into_git(); fetch_options.remote_callbacks(callbacks); - let refspecs = { - // If no globs have been given, import all branches - let globs = if let Some(patterns) = branch_names { - patterns - .iter() - .map(|pattern| pattern.to_glob()) - .collect::>>() - .ok_or(GitFetchError::InvalidBranchPattern)? - } else { - vec!["*".into()] + // At this point, we are only updating Git's remote tracking branches, not the + // local branches. + let refspecs: Vec<_> = branch_names + .iter() + .map(|pattern| { + pattern + .to_glob() + .filter(|glob| !glob.contains(INVALID_REFSPEC_CHARS)) + .map(|glob| format!("+refs/heads/{glob}:refs/remotes/{remote_name}/{glob}")) + }) + .collect::>() + .ok_or(GitFetchError::InvalidBranchPattern)?; + if refspecs.is_empty() { + // Don't fall back to the base refspecs. + let stats = GitFetchStats { + default_branch: None, + import_stats: GitImportStats { + abandoned_commits: vec![], + }, }; - if globs.iter().any(|g| g.contains(INVALID_REFSPEC_CHARS)) { - return Err(GitFetchError::InvalidBranchPattern); - } - // At this point, we are only updating Git's remote tracking branches, not the - // local branches. - globs - .iter() - .map(|glob| format!("+refs/heads/{glob}:refs/remotes/{remote_name}/{glob}")) - .collect_vec() - }; + return Ok(stats); + } tracing::debug!("remote.download"); remote.download(&refspecs, Some(&mut fetch_options))?; tracing::debug!("remote.prune"); @@ -1075,7 +1070,7 @@ pub fn fetch( tracing::debug!("import_refs"); let import_stats = import_some_refs(mut_repo, git_repo, git_settings, |ref_name| { to_remote_branch(ref_name, remote_name) - .map(branch_name_filter) + .map(|branch| branch_names.iter().any(|pattern| pattern.matches(branch))) .unwrap_or_else(|| matches!(ref_name, RefName::Tag(_))) })?; let stats = GitFetchStats { diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 9c0dc83901..1e9cd80eae 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -35,6 +35,7 @@ use jj_lib::op_store::{BranchTarget, RefTarget, RemoteRef, RemoteRefState}; use jj_lib::refs::BranchPushUpdate; use jj_lib::repo::{MutableRepo, ReadonlyRepo, Repo}; use jj_lib::settings::{GitSettings, UserSettings}; +use jj_lib::str_util::StringPattern; use jj_lib::workspace::Workspace; use maplit::{btreemap, hashset}; use tempfile::TempDir; @@ -1964,7 +1965,7 @@ fn test_fetch_empty_repo() { tx.mut_repo(), &test_data.git_repo, "origin", - None, + &[StringPattern::everything()], git::RemoteCallbacks::default(), &git_settings, ) @@ -1989,7 +1990,7 @@ fn test_fetch_initial_commit() { tx.mut_repo(), &test_data.git_repo, "origin", - None, + &[StringPattern::everything()], git::RemoteCallbacks::default(), &git_settings, ) @@ -2038,7 +2039,7 @@ fn test_fetch_success() { tx.mut_repo(), &test_data.git_repo, "origin", - None, + &[StringPattern::everything()], git::RemoteCallbacks::default(), &git_settings, ) @@ -2063,7 +2064,7 @@ fn test_fetch_success() { tx.mut_repo(), &test_data.git_repo, "origin", - None, + &[StringPattern::everything()], git::RemoteCallbacks::default(), &git_settings, ) @@ -2119,7 +2120,7 @@ fn test_fetch_prune_deleted_ref() { tx.mut_repo(), &test_data.git_repo, "origin", - None, + &[StringPattern::everything()], git::RemoteCallbacks::default(), &git_settings, ) @@ -2138,7 +2139,7 @@ fn test_fetch_prune_deleted_ref() { tx.mut_repo(), &test_data.git_repo, "origin", - None, + &[StringPattern::everything()], git::RemoteCallbacks::default(), &git_settings, ) @@ -2160,7 +2161,7 @@ fn test_fetch_no_default_branch() { tx.mut_repo(), &test_data.git_repo, "origin", - None, + &[StringPattern::everything()], git::RemoteCallbacks::default(), &git_settings, ) @@ -2183,7 +2184,7 @@ fn test_fetch_no_default_branch() { tx.mut_repo(), &test_data.git_repo, "origin", - None, + &[StringPattern::everything()], git::RemoteCallbacks::default(), &git_settings, ) @@ -2192,6 +2193,37 @@ fn test_fetch_no_default_branch() { assert_eq!(stats.default_branch, None); } +#[test] +fn test_fetch_empty_refspecs() { + let test_data = GitRepoData::create(); + let git_settings = GitSettings::default(); + empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); + + // Base refspecs shouldn't be respected + let mut tx = test_data + .repo + .start_transaction(&test_data.settings, "test"); + git::fetch( + tx.mut_repo(), + &test_data.git_repo, + "origin", + &[], + git::RemoteCallbacks::default(), + &git_settings, + ) + .unwrap(); + assert!(tx + .mut_repo() + .get_remote_branch("main", "origin") + .is_absent()); + // No remote refs should have been fetched + git::import_refs(tx.mut_repo(), &test_data.git_repo, &git_settings).unwrap(); + assert!(tx + .mut_repo() + .get_remote_branch("main", "origin") + .is_absent()); +} + #[test] fn test_fetch_no_such_remote() { let test_data = GitRepoData::create(); @@ -2203,7 +2235,7 @@ fn test_fetch_no_such_remote() { tx.mut_repo(), &test_data.git_repo, "invalid-remote", - None, + &[StringPattern::everything()], git::RemoteCallbacks::default(), &git_settings, );