refactor: streamline BuildFlags initialization across multiple modules

src/source/git.rs

@@ -2,6 +2,7 @@
 
 use anyhow::{Context, Result};
 use git2::{Cred, CredentialType, FetchOptions, Oid, RemoteCallbacks, Repository};
+use indicatif::{ProgressBar, ProgressDrawTarget, ProgressStyle};
 use inquire::Password;
 use sha2::{Digest, Sha256};
 use std::fs;
@@ -47,8 +48,16 @@ pub fn checkout(
         .ok_or_else(|| anyhow::anyhow!("Invalid mirror path"))?;
 
     crate::log_info!("Cloning git source into {}...", checkout_dir.display());
-    Repository::clone(mirror_url, checkout_dir)
+    let checkout_progress = CheckoutProgress::new(format!("git {}", pkgname));
+    let mut checkout = git2::build::CheckoutBuilder::new();
+    checkout_progress.attach(&mut checkout);
+
+    let mut builder = git2::build::RepoBuilder::new();
+    builder.with_checkout(checkout);
+    builder
+        .clone(mirror_url, checkout_dir)
         .with_context(|| format!("Failed to clone from mirror for {}", url))?;
+    checkout_progress.finish("checkout complete");
 
     let repo = Repository::open(checkout_dir)?;
     checkout_rev(&repo, rev).with_context(|| format!("Failed to checkout revision '{}'", rev))?;
@@ -136,13 +145,15 @@ fn ensure_mirror(url: &str, mirror_dir: &Path, pkgname: &str) -> Result<()> {
     if !mirror_dir.exists() {
         crate::log_info!("Cloning git mirror for {} ({})...", pkgname, url);
         let mut fo = FetchOptions::new();
-        fo.remote_callbacks(remote_callbacks());
+        let transfer_progress = TransferProgress::new(format!("git {}", pkgname));
+        fo.remote_callbacks(remote_callbacks(Some(transfer_progress.bar()), url));
         let mut builder = git2::build::RepoBuilder::new();
         builder.fetch_options(fo);
         builder.bare(true);
         builder
             .clone(url, mirror_dir)
             .with_context(|| format!("Failed to clone git mirror: {}", url))?;
+        transfer_progress.finish("mirror ready");
         return Ok(());
     }
 
@@ -156,12 +167,14 @@ fn ensure_mirror(url: &str, mirror_dir: &Path, pkgname: &str) -> Result<()> {
         .with_context(|| format!("Failed to create remote for {}", url))?;
 
     let mut fo = FetchOptions::new();
-    fo.remote_callbacks(remote_callbacks());
+    let transfer_progress = TransferProgress::new(format!("git {}", pkgname));
+    fo.remote_callbacks(remote_callbacks(Some(transfer_progress.bar()), url));
 
     // Fetch all remote refs (tags + heads). Empty refspec uses default.
     remote
         .fetch(&[] as &[&str], Some(&mut fo), None)
         .with_context(|| format!("Failed to fetch updates for {}", url))?;
+    transfer_progress.finish("mirror updated");
 
     Ok(())
 }
@@ -261,17 +274,110 @@ fn resolve_remote_head_like<'a>(repo: &'a Repository) -> Result<Option<git2::Obj
     Ok(Some(repo.find_object(candidates[0].1, None)?))
 }
 
-fn remote_callbacks() -> RemoteCallbacks<'static> {
+fn remote_callbacks(progress_bar: Option<ProgressBar>, _label: &str) -> RemoteCallbacks<'static> {
     let mut callbacks = RemoteCallbacks::new();
     let mut credential_state = CredentialState::default();
 
     callbacks.credentials(move |url, username_from_url, allowed| {
         credential_state.provide(url, username_from_url, allowed)
     });
+    if let Some(progress_bar) = progress_bar {
+        callbacks.transfer_progress(move |stats| {
+            let total_objects = stats.total_objects() as u64;
+            if total_objects > 0 {
+                progress_bar.set_length(total_objects);
+                progress_bar.set_position(stats.received_objects() as u64);
+            }
+
+            progress_bar.set_message(format!(
+                "{} obj, {} delta, {} bytes",
+                stats.received_objects(),
+                stats.indexed_deltas(),
+                stats.received_bytes()
+            ));
+            true
+        });
+    }
 
     callbacks
 }
 
+struct TransferProgress {
+    bar: ProgressBar,
+}
+
+impl TransferProgress {
+    fn new(prefix: String) -> Self {
+        let bar = ProgressBar::new(1);
+        bar.set_draw_target(progress_draw_target());
+        bar.set_style(
+            ProgressStyle::default_bar()
+                .template("{prefix:.bold} [{bar:40.cyan/blue}] {pos}/{len} {msg}")
+                .unwrap_or_else(|_| ProgressStyle::default_bar())
+                .progress_chars("#>-"),
+        );
+        bar.set_prefix(prefix);
+        bar.set_message("starting transfer");
+        Self { bar }
+    }
+
+    fn bar(&self) -> ProgressBar {
+        self.bar.clone()
+    }
+
+    fn finish(&self, message: &str) {
+        self.bar.finish_and_clear();
+        crate::log_info!("{}", message);
+    }
+}
+
+struct CheckoutProgress {
+    bar: ProgressBar,
+}
+
+impl CheckoutProgress {
+    fn new(prefix: String) -> Self {
+        let bar = ProgressBar::new(1);
+        bar.set_draw_target(progress_draw_target());
+        bar.set_style(
+            ProgressStyle::default_bar()
+                .template("{prefix:.bold} [{bar:40.cyan/blue}] {pos}/{len} {msg}")
+                .unwrap_or_else(|_| ProgressStyle::default_bar())
+                .progress_chars("#>-"),
+        );
+        bar.set_prefix(prefix);
+        bar.set_message("preparing checkout");
+        Self { bar }
+    }
+
+    fn attach(&self, checkout: &mut git2::build::CheckoutBuilder<'static>) {
+        let bar = self.bar.clone();
+        checkout.progress(move |path, current, total| {
+            let total = total as u64;
+            if total > 0 {
+                bar.set_length(total);
+                bar.set_position(current as u64);
+            }
+            if let Some(path) = path {
+                bar.set_message(path.display().to_string());
+            }
+        });
+    }
+
+    fn finish(&self, message: &str) {
+        self.bar.finish_and_clear();
+        crate::log_info!("{}", message);
+    }
+}
+
+fn progress_draw_target() -> ProgressDrawTarget {
+    if io::stderr().is_terminal() {
+        ProgressDrawTarget::stderr()
+    } else {
+        ProgressDrawTarget::hidden()
+    }
+}
+
 #[derive(Default)]
 struct CredentialState {
     username: Option<String>,
@@ -458,7 +564,7 @@ mod tests {
         .unwrap();
 
         checkout_rev(&repo, "HEAD").unwrap();
-        assert_eq!(repo.head_detached().unwrap(), true);
+        assert!(repo.head_detached().unwrap());
         assert_eq!(repo.head().unwrap().target().unwrap(), commit_oid);
     }
 

src/source/mod.rs

@@ -313,6 +313,29 @@ fn prepare_one(
         .map(|rev| spec.expand_vars(rev))
         .collect();
 
+    // Treat `<url>#<rev>` and bare `*.git` sources as git before local file://
+    // handling so `file://...repo.git#tag` resolves through the git checkout path.
+    if let Some((base, rev)) = split_git_url(&url) {
+        let checkout_dir = build_dir.join(&extract_dir_name);
+        if checkout_dir.exists() && checkout_dir.join(".depot_state").exists() {
+            crate::log_info!(
+                "Resuming build in existing git directory: {}",
+                checkout_dir.display()
+            );
+            return Ok(checkout_dir);
+        }
+        git::checkout(
+            &base,
+            &rev,
+            &checkout_dir,
+            &cache_dir.join("git"),
+            &spec.package.name,
+            &cherry_pick_revs,
+        )?;
+        hooks::post_extract(spec, source, &checkout_dir, cache_dir)?;
+        return Ok(checkout_dir);
+    }
+
     // Local file:// handling (directories or archives)
     if let Some(path_str) = url.strip_prefix("file://") {
         let local_path = PathBuf::from(path_str);
@@ -354,29 +377,6 @@ fn prepare_one(
         }
     }
 
-    // Heuristic: if the URL contains '#', treat it as a git URL with a revision.
-    // (except when it clearly looks like an archive URL)
-    if let Some((base, rev)) = split_git_url(&url) {
-        let checkout_dir = build_dir.join(&extract_dir_name);
-        if checkout_dir.exists() && checkout_dir.join(".depot_state").exists() {
-            crate::log_info!(
-                "Resuming build in existing git directory: {}",
-                checkout_dir.display()
-            );
-            return Ok(checkout_dir);
-        }
-        git::checkout(
-            &base,
-            &rev,
-            &checkout_dir,
-            &cache_dir.join("git"),
-            &spec.package.name,
-            &cherry_pick_revs,
-        )?;
-        hooks::post_extract(spec, source, &checkout_dir, cache_dir)?;
-        return Ok(checkout_dir);
-    }
-
     if !source.cherry_pick.is_empty() {
         bail!(
             "source.cherry_pick is only supported for git sources (got URL: {})",
@@ -459,6 +459,89 @@ mod tests {
         Alternatives, Build, BuildFlags, BuildType, Dependencies, ManualSource, PackageInfo,
         PackageSpec, Source,
     };
+    use git2::{Oid, Repository};
+    use std::path::Path;
+
+    fn commit_file(repo: &Repository, workdir: &Path, rel: &str, data: &str) -> Oid {
+        let full_path = workdir.join(rel);
+        if let Some(parent) = full_path.parent() {
+            std::fs::create_dir_all(parent).unwrap();
+        }
+        std::fs::write(&full_path, data).unwrap();
+
+        let mut index = repo.index().unwrap();
+        index.add_path(Path::new(rel)).unwrap();
+        let tree_id = index.write_tree().unwrap();
+        let tree = repo.find_tree(tree_id).unwrap();
+        let sig = git2::Signature::now("depot-test", "depot@example.test").unwrap();
+        let mut parents = Vec::new();
+        if let Ok(head) = repo.head()
+            && let Some(oid) = head.target()
+        {
+            parents.push(repo.find_commit(oid).unwrap());
+        }
+        let parent_refs: Vec<&git2::Commit<'_>> = parents.iter().collect();
+
+        repo.commit(Some("HEAD"), &sig, &sig, "test", &tree, &parent_refs)
+            .unwrap()
+    }
+
+    fn make_git_source_spec(source_url: String, extract_dir: &str) -> PackageSpec {
+        PackageSpec {
+            package: PackageInfo {
+                name: "foo".into(),
+                version: "1.0".into(),
+                revision: 1,
+                description: "d".into(),
+                homepage: "h".into(),
+                license: vec!["MIT".into()],
+            },
+            packages: Vec::new(),
+            alternatives: Alternatives::default(),
+            manual_sources: Vec::new(),
+            source: vec![Source {
+                url: source_url,
+                sha256: "skip".into(),
+                extract_dir: extract_dir.into(),
+                patches: Vec::new(),
+                post_extract: Vec::new(),
+                cherry_pick: Vec::new(),
+            }],
+            build: Build {
+                build_type: BuildType::Custom,
+                flags: BuildFlags::default(),
+            },
+            dependencies: Dependencies::default(),
+            package_alternatives: Default::default(),
+            package_dependencies: Default::default(),
+            spec_dir: PathBuf::from("."),
+        }
+    }
+
+    fn make_remote_git_repo() -> (tempfile::TempDir, String, Oid, Oid) {
+        let tmp = tempfile::tempdir().unwrap();
+        let remote_dir = tmp.path().join("origin.git");
+        let workdir = tmp.path().join("work");
+
+        Repository::init_bare(&remote_dir).unwrap();
+        let repo = Repository::init(&workdir).unwrap();
+        let tagged = commit_file(&repo, &workdir, "README", "tagged\n");
+        let tag_target = repo.find_object(tagged, None).unwrap();
+        repo.tag_lightweight("v1.0.0", &tag_target, false).unwrap();
+        let hashed = commit_file(&repo, &workdir, "README", "hashed\n");
+
+        let branch_ref = repo.head().unwrap().name().unwrap().to_string();
+        let mut remote = repo.remote("origin", remote_dir.to_str().unwrap()).unwrap();
+        let push_specs = [
+            format!("{branch_ref}:{branch_ref}"),
+            "refs/tags/v1.0.0:refs/tags/v1.0.0".to_string(),
+        ];
+        let push_spec_refs: Vec<&String> = push_specs.iter().collect();
+        remote.push(&push_spec_refs, None).unwrap();
+
+        let remote_url = url::Url::from_file_path(&remote_dir).unwrap().to_string();
+        (tmp, remote_url, tagged, hashed)
+    }
 
     fn mk_spec_with_manuals(spec_dir: PathBuf, manuals: Vec<ManualSource>) -> PackageSpec {
         PackageSpec {
@@ -579,6 +662,42 @@ mod tests {
         );
     }
 
+    #[test]
+    fn prepare_one_checks_out_git_tag_revision() {
+        let (_tmp, remote_url, tagged, _hashed) = make_remote_git_repo();
+        let cache_dir = tempfile::tempdir().unwrap();
+        let build_dir = tempfile::tempdir().unwrap();
+        let spec = make_git_source_spec(format!("{remote_url}#v1.0.0"), "src-tag");
+
+        let checkout_dir =
+            prepare_one(&spec, &spec.source[0], cache_dir.path(), build_dir.path()).unwrap();
+        let repo = Repository::open(&checkout_dir).unwrap();
+
+        assert_eq!(repo.head().unwrap().target().unwrap(), tagged);
+        assert_eq!(
+            std::fs::read_to_string(checkout_dir.join("README")).unwrap(),
+            "tagged\n"
+        );
+    }
+
+    #[test]
+    fn prepare_one_checks_out_git_commit_hash_revision() {
+        let (_tmp, remote_url, _tagged, hashed) = make_remote_git_repo();
+        let cache_dir = tempfile::tempdir().unwrap();
+        let build_dir = tempfile::tempdir().unwrap();
+        let spec = make_git_source_spec(format!("{remote_url}#{hashed}"), "src-hash");
+
+        let checkout_dir =
+            prepare_one(&spec, &spec.source[0], cache_dir.path(), build_dir.path()).unwrap();
+        let repo = Repository::open(&checkout_dir).unwrap();
+
+        assert_eq!(repo.head().unwrap().target().unwrap(), hashed);
+        assert_eq!(
+            std::fs::read_to_string(checkout_dir.join("README")).unwrap(),
+            "hashed\n"
+        );
+    }
+
     #[test]
     fn verify_file_hash_accepts_multiple_algorithms() {
         use sha2::{Digest, Sha256, Sha512};