Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions src/git_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,9 +707,10 @@ impl GitManager {
SubmoduleError::GitoxideError(format!("GitOpsManager update failed: {e}"))
})?;

if self.verbose {
println!("✅ Updated {name} successfully");
}
// Name every submodule that was updated, not just a trailing count: with
// only the count, a multi-submodule `update` gives no way to tell which
// ones it actually touched.
println!("✅ Updated {name}");
Ok(())
}

Expand Down
28 changes: 23 additions & 5 deletions src/git_ops/gix_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,30 @@ use crate::utilities;
#[derive(Debug, Clone, PartialEq)]
pub struct GixOperations {
repo: gix::Repository,
/// Whether to surface the underlying gitoxide fetch report on stderr.
verbose: bool,
}
impl GixOperations {
/// Create a new `GixOperations` instance
/// Create a new `GixOperations` instance. Quiet by default; see
/// [`GixOperations::with_verbose`].
pub fn new(repo_path: Option<&Path>) -> Result<Self> {
let repo = match repo_path {
Some(path) => gix::open(path)
.with_context(|| format!("Failed to open repository at {}", path.display()))?,
None => gix::discover(".")
.with_context(|| "Failed to discover repository in current directory")?,
};
Ok(Self { repo })
Ok(Self {
repo,
verbose: false,
})
}

/// Set whether gitoxide's own fetch report is echoed to stderr.
#[must_use]
pub const fn with_verbose(mut self, verbose: bool) -> Self {
self.verbose = verbose;
self
}

/// Try to perform operation with gix, return error if not supported
Expand Down Expand Up @@ -457,8 +470,13 @@ impl GitOperations for GixOperations {
// Pass None to let gix resolve the default remote (which has refspecs configured).
// Passing the URL string would create a bare remote without refspecs.
let submodule_repo = gix::open(&submodule_path)?;
fetch_repo(submodule_repo, None, entry.shallow == Some(true))
.map_err(|e| anyhow::anyhow!("Failed to fetch submodule: {e}"))?;
fetch_repo(
submodule_repo,
None,
entry.shallow == Some(true),
self.verbose,
)
.map_err(|e| anyhow::anyhow!("Failed to fetch submodule: {e}"))?;
match opts.strategy {
crate::options::SerializableUpdate::Checkout
| crate::options::SerializableUpdate::Unspecified => {
Expand Down Expand Up @@ -696,7 +714,7 @@ impl GitOperations for GixOperations {
fn fetch_submodule(&self, path: &str) -> Result<()> {
// Pass None to let gix resolve the default remote (which has refspecs configured).
let submodule_repo = utilities::repo_from_path(&std::path::PathBuf::from(path))?;
fetch_repo(submodule_repo, None, false)
fetch_repo(submodule_repo, None, false, self.verbose)
.map_err(|e| anyhow::anyhow!("Failed to fetch submodule: {e}"))
}

Expand Down
6 changes: 4 additions & 2 deletions src/git_ops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@ pub struct GitOpsManager {
impl GitOpsManager {
/// Create a new `GitOpsManager` with automatic fallback
pub fn new(repo_path: Option<&Path>, verbose: bool) -> Result<Self> {
let gix_ops = GixOperations::new(repo_path).ok();
let gix_ops = GixOperations::new(repo_path)
.ok()
.map(|ops| ops.with_verbose(verbose));
let git2_ops = Git2Operations::new(repo_path)
.with_context(|| "Failed to initialize git2 operations")?;

Expand Down Expand Up @@ -266,7 +268,7 @@ impl GitOpsManager {
// gix is an optional optimistic backend — log failures but don't fail.
match GixOperations::new(Some(&workdir)) {
Ok(new_gix) => {
self.gix_ops = Some(new_gix);
self.gix_ops = Some(new_gix.with_verbose(self.verbose));
}
Err(e) => {
if self.verbose {
Expand Down
49 changes: 39 additions & 10 deletions src/git_ops/simple_gix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use gitoxide_core::repository::fetch::{
};
use gix::{features::progress, progress::prodash};
use prodash::render::line;
use std::io::{stderr, stdout};
use std::io::stderr;

/// A standard range for line renderer.
pub fn setup_line_renderer_range(
Expand Down Expand Up @@ -46,7 +46,14 @@ pub fn progress_tree(trace: bool) -> std::sync::Arc<prodash::tree::Root> {
.into()
}

/// Run a function with progress tracking, capturing output to stdout and stderr.
/// Run a function with progress tracking, returning its result alongside the
/// bytes it wrote to the `out` and `err` handles it was given.
///
/// `gitoxide-core` writes CLI-shaped reports to those handles — for `fetch`,
/// the refspec mapping and a per-ref status line. That is narration for someone
/// running `gix` directly, not part of `submod`'s output contract, so it is
/// captured and handed back rather than written to this process's own streams.
/// The caller decides where, and whether, it is shown.
pub fn get_progress<T>(
func_name: &str,
range: Option<std::ops::RangeInclusive<u8>>,
Expand All @@ -55,7 +62,7 @@ pub fn get_progress<T>(
&mut dyn std::io::Write,
&mut dyn std::io::Write,
) -> T,
) -> Result<T> {
) -> (T, Vec<u8>, Vec<u8>) {
let standard_range = 2..=2;
let range = range.unwrap_or_else(|| standard_range.clone());
let progress = progress_tree(false);
Expand All @@ -75,9 +82,7 @@ pub fn get_progress<T>(
});

handle.shutdown_and_wait();
std::io::Write::write_all(&mut stdout(), &out)?;
std::io::Write::write_all(&mut stderr(), &err)?;
Ok(result)
(result, out, err)
}

/// Fetch options for the `fetch` command, with an option for shallow fetching.
Expand All @@ -100,9 +105,33 @@ const fn fetch_options(remote: Option<String>, shallow: bool) -> FetchOptions {
}

/// Fetch updates from a remote repository.
pub fn fetch_repo(repo: gix::Repository, remote: Option<String>, shallow: bool) -> Result<()> {
let inner_result = get_progress("fetch", Some(FetchProgressRange), |progress, out, err| {
gitoxide_core::repository::fetch(repo, progress, out, err, fetch_options(remote, shallow))
})?;
///
/// The fetch report `gitoxide-core` produces never reaches stdout: stdout
/// carries `submod`'s own output, and callers parse it. The report is written to
/// stderr when the caller asked for verbose output, or when the fetch failed and
/// it is the only detail available to explain why.
pub fn fetch_repo(
repo: gix::Repository,
remote: Option<String>,
shallow: bool,
verbose: bool,
) -> Result<()> {
let (inner_result, out, err) =
get_progress("fetch", Some(FetchProgressRange), |progress, out, err| {
gitoxide_core::repository::fetch(
repo,
progress,
out,
err,
fetch_options(remote, shallow),
)
});

if verbose || inner_result.is_err() {
let mut sink = stderr();
std::io::Write::write_all(&mut sink, &out)?;
std::io::Write::write_all(&mut sink, &err)?;
}

inner_result.map_err(|e| anyhow::anyhow!("Fetch failed: {e}"))
}
16 changes: 13 additions & 3 deletions tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,23 @@ sparse_paths = ["src"]
.run_submod_success(&["update"])
.expect("Failed to run update");

// The one submodule in the config must be counted. `update` does not name
// the submodules it touched — it forwards the raw fetch output and then
// prints this summary — so the count is the assertable contract here.
// `update` must name each submodule it touched, then summarize the count.
assert!(
stdout.contains("✅ Updated update-lib"),
"update should name the submodule it updated; got: {stdout}"
);
assert!(
stdout.contains("Updated 1 submodule(s)"),
"update should report how many submodules it updated; got: {stdout}"
);

// gitoxide's fetch report is plumbing narration for `gix fetch`, not part
// of submod's output. Leaking it to stdout is what made the per-submodule
// line unassertable in the first place.
assert!(
!stdout.contains("refs/remotes/origin/"),
"update must not leak the raw fetch refspec report to stdout; got: {stdout}"
);
}

/// `update` must check the submodule worktree out to the commit recorded as
Expand Down
Loading