diff --git a/crates/gitcomet-core/src/services.rs b/crates/gitcomet-core/src/services.rs index dfa7793b..d65f9ef6 100644 --- a/crates/gitcomet-core/src/services.rs +++ b/crates/gitcomet-core/src/services.rs @@ -113,6 +113,19 @@ pub enum RemoteUrlKind { Push, } +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct SubmoduleTrustTarget { + pub submodule_path: PathBuf, + pub display_source: String, + pub local_source_path: PathBuf, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum SubmoduleTrustDecision { + Proceed, + Prompt { sources: Vec }, +} + #[derive(Clone, Debug, Eq, PartialEq)] pub struct BlameLine { pub commit_id: Arc, @@ -541,13 +554,40 @@ pub trait GitRepository: Send + Sync { ))) } - fn add_submodule_with_output(&self, _url: &str, _path: &Path) -> Result { + fn check_submodule_add_trust( + &self, + _url: &str, + _path: &Path, + ) -> Result { + Err(Error::new(ErrorKind::Unsupported( + "submodule trust checks are not implemented for this backend", + ))) + } + + fn check_submodule_update_trust(&self) -> Result { + Err(Error::new(ErrorKind::Unsupported( + "submodule trust checks are not implemented for this backend", + ))) + } + + fn add_submodule_with_output( + &self, + _url: &str, + _path: &Path, + _branch: Option<&str>, + _name: Option<&str>, + _force: bool, + _approved_sources: &[SubmoduleTrustTarget], + ) -> Result { Err(Error::new(ErrorKind::Unsupported( "submodule add is not implemented for this backend", ))) } - fn update_submodules_with_output(&self) -> Result { + fn update_submodules_with_output( + &self, + _approved_sources: &[SubmoduleTrustTarget], + ) -> Result { Err(Error::new(ErrorKind::Unsupported( "submodule update is not implemented for this backend", ))) diff --git a/crates/gitcomet-git-gix/src/repo/mod.rs b/crates/gitcomet-git-gix/src/repo/mod.rs index 45378193..fe3b812a 100644 --- a/crates/gitcomet-git-gix/src/repo/mod.rs +++ b/crates/gitcomet-git-gix/src/repo/mod.rs @@ -9,7 +9,7 @@ use gitcomet_core::error::{Error, ErrorKind}; use gitcomet_core::git_ops_trace::{self, GitOpTraceKind}; use gitcomet_core::services::{ BlameLine, CommandOutput, ConflictFileStages, ConflictSide, GitRepository, MergetoolResult, - PullMode, RemoteUrlKind, ResetMode, Result, + PullMode, RemoteUrlKind, ResetMode, Result, SubmoduleTrustDecision, SubmoduleTrustTarget, }; use std::path::{Path, PathBuf}; use std::process::Command; @@ -529,12 +529,31 @@ impl GitRepository for GixRepo { self.list_submodules_impl() } - fn add_submodule_with_output(&self, url: &str, path: &Path) -> Result { - self.add_submodule_with_output_impl(url, path) + fn check_submodule_add_trust(&self, url: &str, path: &Path) -> Result { + self.check_submodule_add_trust_impl(url, path) } - fn update_submodules_with_output(&self) -> Result { - self.update_submodules_with_output_impl() + fn check_submodule_update_trust(&self) -> Result { + self.check_submodule_update_trust_impl() + } + + fn add_submodule_with_output( + &self, + url: &str, + path: &Path, + branch: Option<&str>, + name: Option<&str>, + force: bool, + approved_sources: &[SubmoduleTrustTarget], + ) -> Result { + self.add_submodule_with_output_impl(url, path, branch, name, force, approved_sources) + } + + fn update_submodules_with_output( + &self, + approved_sources: &[SubmoduleTrustTarget], + ) -> Result { + self.update_submodules_with_output_impl(approved_sources) } fn remove_submodule_with_output(&self, path: &Path) -> Result { diff --git a/crates/gitcomet-git-gix/src/repo/submodules.rs b/crates/gitcomet-git-gix/src/repo/submodules.rs index 1b1fc603..c29cd450 100644 --- a/crates/gitcomet-git-gix/src/repo/submodules.rs +++ b/crates/gitcomet-git-gix/src/repo/submodules.rs @@ -1,11 +1,28 @@ use super::GixRepo; use super::history::gix_head_id_or_none; -use crate::util::run_git_with_output; +use crate::util::{ + bytes_to_text_preserving_utf8, git_workdir_cmd_for, run_git_simple, run_git_with_output, +}; use gitcomet_core::domain::{CommitId, Submodule, SubmoduleStatus}; -use gitcomet_core::error::{Error, ErrorKind}; -use gitcomet_core::services::{CommandOutput, Result}; +use gitcomet_core::error::{Error, ErrorKind, GitFailure}; +use gitcomet_core::path_utils::canonicalize_or_original; +use gitcomet_core::services::{ + CommandOutput, Result, SubmoduleTrustDecision, SubmoduleTrustTarget, +}; +use gix::bstr::ByteSlice as _; use std::collections::BTreeMap; +use std::fs; use std::path::{Path, PathBuf}; +use std::process::Command; +use std::thread; +use std::time::Duration; + +fn allow_file_submodule_transport(cmd: &mut Command) { + // `git submodule` blocks local-path remotes unless `protocol.file.allow` is enabled. + // Use per-command config so local workflows keep working without disabling `https`/`ssh`. + cmd.arg("-c").arg("protocol.file.allow=always"); +} + impl GixRepo { pub(super) fn list_submodules_impl(&self) -> Result> { let repo = self.reopen_repo()?; @@ -15,34 +32,121 @@ impl GixRepo { Ok(submodules) } + pub(super) fn check_submodule_add_trust_impl( + &self, + url: &str, + path: &Path, + ) -> Result { + let repo = self.reopen_repo()?; + let Some(target) = + trust_target_from_raw_source(repo_workdir_for_submodule_trust(&repo), path, url)? + else { + return Ok(SubmoduleTrustDecision::Proceed); + }; + + if submodule_source_trusted(repo_workdir_for_submodule_trust(&repo), &target)? { + Ok(SubmoduleTrustDecision::Proceed) + } else { + Ok(SubmoduleTrustDecision::Prompt { + sources: vec![target], + }) + } + } + + pub(super) fn check_submodule_update_trust_impl(&self) -> Result { + let repo = self.reopen_repo()?; + let trust_root = repo_workdir_for_submodule_trust(&repo); + let mut sources = BTreeMap::new(); + collect_repo_untrusted_submodule_sources(&repo, trust_root, Path::new(""), &mut sources)?; + if sources.is_empty() { + Ok(SubmoduleTrustDecision::Proceed) + } else { + Ok(SubmoduleTrustDecision::Prompt { + sources: sources.into_values().collect(), + }) + } + } + pub(super) fn add_submodule_with_output_impl( &self, url: &str, path: &Path, + branch: Option<&str>, + name: Option<&str>, + force: bool, + approved_sources: &[SubmoduleTrustTarget], ) -> Result { + let repo = self.reopen_repo()?; + let trust_root = repo_workdir_for_submodule_trust(&repo); + let git_dir = repo.git_dir().to_path_buf(); + persist_submodule_trust_approvals(trust_root, approved_sources)?; + let mut cmd = self.git_workdir_cmd(); - cmd.arg("submodule") - .arg("add") - .arg(url) - .arg(path) - // Local-path submodule URLs are used in tests and supported workflows. - // Explicitly allow `file` transport for this command. - .env("GIT_ALLOW_PROTOCOL", "file"); - run_git_with_output(cmd, &format!("git submodule add {url} {}", path.display())) + if let Some(target) = trust_target_from_raw_source(trust_root, path, url)? { + if !submodule_source_trusted(trust_root, &target)? { + return Err(untrusted_local_submodule_error(&target, "add")); + } + allow_file_submodule_transport(&mut cmd); + } + let logical_name = name + .map(PathBuf::from) + .unwrap_or_else(|| path.to_path_buf()); + + cmd.arg("submodule").arg("add"); + let mut command = "git submodule add".to_string(); + if let Some(branch) = branch { + cmd.arg("--branch").arg(branch); + command.push_str(&format!(" --branch {branch}")); + } + if force { + cmd.arg("--force"); + command.push_str(" --force"); + } + if let Some(name) = name { + cmd.arg("--name").arg(name); + command.push_str(&format!(" --name {name}")); + } + cmd.arg(url).arg(path); + command.push_str(&format!(" {url} {}", path.display())); + match run_git_with_output(cmd, &command) { + Ok(output) => Ok(output), + Err(err) => Err(cleanup_failed_submodule_add_error( + trust_root, + &git_dir, + path, + &logical_name, + err, + )), + } } - pub(super) fn update_submodules_with_output_impl(&self) -> Result { - let mut cmd = self.git_workdir_cmd(); - cmd.arg("submodule") - .arg("update") - .arg("--init") - .arg("--recursive") - // Keep behavior consistent with add: allow local-path submodule URLs. - .env("GIT_ALLOW_PROTOCOL", "file"); - run_git_with_output(cmd, "git submodule update --init --recursive") + pub(super) fn update_submodules_with_output_impl( + &self, + approved_sources: &[SubmoduleTrustTarget], + ) -> Result { + let repo = self.reopen_repo()?; + let trust_root = repo_workdir_for_submodule_trust(&repo).to_path_buf(); + persist_submodule_trust_approvals(&trust_root, approved_sources)?; + + let mut outputs = Vec::new(); + update_repo_submodules_recursive(&repo, &trust_root, Path::new(""), &mut outputs)?; + + if outputs.is_empty() { + Ok(CommandOutput::empty_success( + "git submodule update --init --recursive", + )) + } else { + Ok(combine_submodule_update_outputs(outputs)) + } } pub(super) fn remove_submodule_with_output_impl(&self, path: &Path) -> Result { + let repo = self.reopen_repo()?; + let workdir = repo_workdir_for_submodule_trust(&repo).to_path_buf(); + let git_dir = repo.git_dir().to_path_buf(); + let logical_name = + resolve_submodule_logical_name(&repo, path)?.unwrap_or_else(|| path.to_path_buf()); + let mut cmd1 = self.git_workdir_cmd(); cmd1.arg("submodule") .arg("deinit") @@ -56,6 +160,13 @@ impl GixRepo { cmd2.arg("rm").arg("-f").arg("--").arg(path); let out2 = run_git_with_output(cmd2, &format!("git rm -f {}", path.display()))?; + cleanup_removed_submodule_metadata(&workdir, &git_dir, &logical_name).map_err(|err| { + Error::new(ErrorKind::Backend(format!( + "Removed submodule '{}' from the worktree and index, but failed to clean metadata: {err}", + path.display() + ))) + })?; + Ok(CommandOutput { command: format!("Remove submodule {}", path.display()), stdout: [out1.stdout.trim_end(), out2.stdout.trim_end()] @@ -142,6 +253,90 @@ fn collect_repo_submodules( Ok(()) } +fn collect_repo_untrusted_submodule_sources( + repo: &gix::Repository, + trust_root: &Path, + prefix: &Path, + out: &mut BTreeMap, +) -> Result<()> { + let Some(submodules) = repo + .submodules() + .map_err(|e| Error::new(ErrorKind::Backend(format!("gix submodules: {e}"))))? + else { + return Ok(()); + }; + + let current_workdir = repo_workdir_for_submodule_trust(repo); + for submodule in submodules { + let relative_path = submodule + .path() + .map_err(|e| Error::new(ErrorKind::Backend(format!("gix submodule path: {e}")))) + .and_then(|path| pathbuf_from_gix_path(path.as_ref()))?; + let full_path = prefix.join(&relative_path); + + if let Some(target) = trust_target_from_submodule(current_workdir, &full_path, &submodule)? + && !submodule_source_trusted(trust_root, &target)? + { + out.insert(full_path.clone(), target); + } + + if let Some(nested_repo) = open_configured_submodule_repo(&submodule)? { + collect_repo_untrusted_submodule_sources(&nested_repo, trust_root, &full_path, out)?; + } + } + + Ok(()) +} + +fn update_repo_submodules_recursive( + repo: &gix::Repository, + trust_root: &Path, + prefix: &Path, + outputs: &mut Vec, +) -> Result<()> { + let Some(submodules) = repo + .submodules() + .map_err(|e| Error::new(ErrorKind::Backend(format!("gix submodules: {e}"))))? + else { + return Ok(()); + }; + + let current_workdir = repo_workdir_for_submodule_trust(repo); + for submodule in submodules { + let relative_path = submodule + .path() + .map_err(|e| Error::new(ErrorKind::Backend(format!("gix submodule path: {e}")))) + .and_then(|path| pathbuf_from_gix_path(path.as_ref()))?; + let full_path = prefix.join(&relative_path); + + let local_target = trust_target_from_submodule(current_workdir, &full_path, &submodule)?; + + let mut cmd = git_workdir_cmd_for(current_workdir); + if let Some(target) = local_target.as_ref() { + if !submodule_source_trusted(trust_root, target)? { + return Err(untrusted_local_submodule_error(target, "update")); + } + allow_file_submodule_transport(&mut cmd); + } + + cmd.arg("submodule") + .arg("update") + .arg("--init") + .arg("--") + .arg(&relative_path); + outputs.push(run_git_with_output( + cmd, + &format!("git submodule update --init -- {}", full_path.display()), + )?); + + if let Some(nested_repo) = open_gitlink_repo(repo, &relative_path)? { + update_repo_submodules_recursive(&nested_repo, trust_root, &full_path, outputs)?; + } + } + + Ok(()) +} + fn configured_submodule_row( repo: &gix::Repository, submodule: gix::Submodule<'_>, @@ -159,16 +354,7 @@ fn configured_submodule_row( )); } - let state = submodule - .state() - .map_err(|e| Error::new(ErrorKind::Backend(format!("gix submodule state: {e}"))))?; - let nested_repo = if state.repository_exists && state.worktree_checkout { - submodule - .open() - .map_err(|e| Error::new(ErrorKind::Backend(format!("gix submodule open: {e}"))))? - } else { - None - }; + let nested_repo = open_configured_submodule_repo(&submodule)?; let Some(nested_repo) = nested_repo else { return Ok(( Submodule { @@ -200,6 +386,294 @@ fn configured_submodule_row( )) } +fn resolve_submodule_logical_name(repo: &gix::Repository, path: &Path) -> Result> { + let Some(submodules) = repo + .submodules() + .map_err(|e| Error::new(ErrorKind::Backend(format!("gix submodules: {e}"))))? + else { + return Ok(None); + }; + + for submodule in submodules { + let relative_path = submodule + .path() + .map_err(|e| Error::new(ErrorKind::Backend(format!("gix submodule path: {e}")))) + .and_then(|path| pathbuf_from_gix_path(path.as_ref()))?; + if relative_path == path { + return pathbuf_from_gix_path(submodule.name()).map(Some); + } + } + + Ok(None) +} + +fn cleanup_failed_submodule_add_error( + workdir: &Path, + git_dir: &Path, + path: &Path, + logical_name: &Path, + err: Error, +) -> Error { + let clone_only_state = match failed_submodule_add_left_clone_only_state(workdir, path) { + Ok(value) => value, + Err(probe_err) => { + return append_failed_submodule_add_note( + err, + &format!("GitComet could not inspect failed submodule add state: {probe_err}"), + ); + } + }; + + if !clone_only_state { + return err; + } + + match cleanup_failed_submodule_add_leftovers(workdir, git_dir, path, logical_name) { + Ok(()) => err, + Err(cleanup_err) => append_failed_submodule_add_note( + err, + &format!("Cleanup after failed submodule add also failed: {cleanup_err}"), + ), + } +} + +fn failed_submodule_add_left_clone_only_state(workdir: &Path, path: &Path) -> Result { + let repo = gix::open(workdir).map_err(|e| { + Error::new(ErrorKind::Backend(format!( + "open repo after failed submodule add {}: {e}", + workdir.display() + ))) + })?; + Ok(!submodule_path_registered(&repo, path)?) +} + +fn submodule_path_registered(repo: &gix::Repository, path: &Path) -> Result { + if configured_submodule_path_exists(repo, path)? { + return Ok(true); + } + Ok(collect_gitlinks(repo)?.contains_key(path)) +} + +fn configured_submodule_path_exists(repo: &gix::Repository, path: &Path) -> Result { + let Some(submodules) = repo + .submodules() + .map_err(|e| Error::new(ErrorKind::Backend(format!("gix submodules: {e}"))))? + else { + return Ok(false); + }; + + for submodule in submodules { + let relative_path = submodule + .path() + .map_err(|e| Error::new(ErrorKind::Backend(format!("gix submodule path: {e}")))) + .and_then(|path| pathbuf_from_gix_path(path.as_ref()))?; + if relative_path == path { + return Ok(true); + } + } + + Ok(false) +} + +fn cleanup_failed_submodule_add_leftovers( + workdir: &Path, + git_dir: &Path, + path: &Path, + logical_name: &Path, +) -> Result<()> { + remove_failed_submodule_checkout(workdir, git_dir, path, logical_name)?; + cleanup_removed_submodule_metadata(workdir, git_dir, logical_name) +} + +fn cleanup_removed_submodule_metadata( + workdir: &Path, + git_dir: &Path, + logical_name: &Path, +) -> Result<()> { + remove_local_submodule_config_section_if_present(workdir, logical_name)?; + remove_submodule_git_dir(git_dir, logical_name)?; + Ok(()) +} + +fn remove_failed_submodule_checkout( + workdir: &Path, + git_dir: &Path, + submodule_path: &Path, + logical_name: &Path, +) -> Result<()> { + let checkout_path = submodule_worktree_path(workdir, submodule_path)?; + if !checkout_path.exists() { + return Ok(()); + } + + let expected_git_dir = canonicalize_or_original(git_dir.join("modules").join(logical_name)); + let Some(actual_git_dir) = checkout_git_dir_reference(&checkout_path)? else { + return Err(Error::new(ErrorKind::Backend(format!( + "refusing to remove failed submodule checkout {} because it is not linked to {}", + checkout_path.display(), + expected_git_dir.display() + )))); + }; + + if actual_git_dir != expected_git_dir { + return Err(Error::new(ErrorKind::Backend(format!( + "refusing to remove failed submodule checkout {} because it points to {} instead of {}", + checkout_path.display(), + actual_git_dir.display(), + expected_git_dir.display() + )))); + } + + fs::remove_dir_all(&checkout_path).map_err(|e| { + Error::new(ErrorKind::Backend(format!( + "remove failed submodule checkout {}: {e}", + checkout_path.display() + ))) + }) +} + +fn submodule_worktree_path(workdir: &Path, submodule_path: &Path) -> Result { + if submodule_path.is_absolute() { + if submodule_path.starts_with(workdir) { + return Ok(submodule_path.to_path_buf()); + } + return Err(Error::new(ErrorKind::Backend(format!( + "refusing to clean failed submodule add outside repository workdir: {}", + submodule_path.display() + )))); + } + Ok(workdir.join(submodule_path)) +} + +fn checkout_git_dir_reference(checkout_path: &Path) -> Result> { + let dot_git = checkout_path.join(".git"); + let metadata = match fs::metadata(&dot_git) { + Ok(metadata) => metadata, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(err) => return Err(Error::new(ErrorKind::Io(err.kind()))), + }; + + if metadata.is_dir() { + return Ok(Some(canonicalize_or_original(dot_git))); + } + + let bytes = fs::read(&dot_git).map_err(|e| Error::new(ErrorKind::Io(e.kind())))?; + let text = bytes_to_text_preserving_utf8(&bytes); + let Some(git_dir) = text.strip_prefix("gitdir:") else { + return Ok(None); + }; + + let git_dir = PathBuf::from(git_dir.trim()); + let resolved = if git_dir.is_absolute() { + git_dir + } else { + checkout_path.join(git_dir) + }; + Ok(Some(canonicalize_or_original(resolved))) +} + +fn append_failed_submodule_add_note(err: Error, note: &str) -> Error { + match err.kind() { + ErrorKind::Git(failure) => Error::new(ErrorKind::Git(GitFailure::new( + failure.command(), + failure.id(), + failure.exit_code(), + failure.stdout().to_vec(), + failure.stderr().to_vec(), + Some(match failure.detail() { + Some(detail) if !detail.is_empty() => format!("{detail}\n\n{note}"), + _ => note.to_string(), + }), + ))), + _ => Error::new(ErrorKind::Backend(format!("{err}\n\n{note}"))), + } +} + +fn remove_local_submodule_config_section_if_present( + workdir: &Path, + logical_name: &Path, +) -> Result<()> { + let Some(logical_name) = logical_name.to_str() else { + return Err(Error::new(ErrorKind::Unsupported( + "submodule logical name is not valid UTF-8", + ))); + }; + let section = format!("submodule.{logical_name}"); + + let mut cmd = git_workdir_cmd_for(workdir); + cmd.arg("config") + .arg("--local") + .arg("--remove-section") + .arg(§ion); + let output = cmd + .output() + .map_err(|err| Error::new(ErrorKind::Io(err.kind())))?; + if output.status.success() { + return Ok(()); + } + + let stderr = bytes_to_text_preserving_utf8(&output.stderr); + if stderr.contains("no such section") { + return Ok(()); + } + + Err(Error::new(ErrorKind::Backend(format!( + "git config --local --remove-section {section} failed: {}", + stderr.trim() + )))) +} + +fn remove_submodule_git_dir(git_dir: &Path, logical_name: &Path) -> Result<()> { + let modules_root = git_dir.join("modules"); + let module_dir = modules_root.join(logical_name); + if !module_dir.exists() { + return Ok(()); + } + + fs::remove_dir_all(&module_dir).map_err(|e| { + Error::new(ErrorKind::Backend(format!( + "remove submodule git dir {}: {e}", + module_dir.display() + ))) + })?; + prune_empty_module_parent_dirs(&modules_root, &module_dir) +} + +fn prune_empty_module_parent_dirs(modules_root: &Path, removed_dir: &Path) -> Result<()> { + let mut current = removed_dir.parent(); + while let Some(dir) = current { + if dir == modules_root || !dir.starts_with(modules_root) { + break; + } + + let mut entries = fs::read_dir(dir).map_err(|e| { + Error::new(ErrorKind::Backend(format!( + "read module metadata dir {}: {e}", + dir.display() + ))) + })?; + match entries.next() { + None => { + fs::remove_dir(dir).map_err(|e| { + Error::new(ErrorKind::Backend(format!( + "remove empty module metadata dir {}: {e}", + dir.display() + ))) + })?; + current = dir.parent(); + } + Some(Ok(_)) => break, + Some(Err(e)) => { + return Err(Error::new(ErrorKind::Backend(format!( + "read module metadata dir entry {}: {e}", + dir.display() + )))); + } + } + } + Ok(()) +} + fn collect_gitlinks(repo: &gix::Repository) -> Result> { let index = repo .index_or_load_from_head_or_empty() @@ -246,6 +720,268 @@ fn open_gitlink_repo( } } +fn open_configured_submodule_repo( + submodule: &gix::Submodule<'_>, +) -> Result> { + let state = submodule + .state() + .map_err(|e| Error::new(ErrorKind::Backend(format!("gix submodule state: {e}"))))?; + if !(state.repository_exists && state.worktree_checkout) { + return Ok(None); + } + submodule + .open() + .map_err(|e| Error::new(ErrorKind::Backend(format!("gix submodule open: {e}")))) +} + +fn trust_target_from_submodule( + current_repo_workdir: &Path, + full_submodule_path: &Path, + submodule: &gix::Submodule<'_>, +) -> Result> { + let url = submodule + .url() + .map_err(|e| Error::new(ErrorKind::Backend(format!("gix submodule url: {e}"))))?; + trust_target_from_url(current_repo_workdir, full_submodule_path, &url) +} + +fn trust_target_from_raw_source( + current_repo_workdir: &Path, + submodule_path: &Path, + raw_source: &str, +) -> Result> { + let url = gix::url::parse(raw_source.as_bytes().as_bstr()).map_err(|e| { + Error::new(ErrorKind::Backend(format!( + "invalid submodule source {raw_source:?}: {e}" + ))) + })?; + let display_source = raw_source.trim().to_string(); + trust_target_from_parsed_url(current_repo_workdir, submodule_path, &url, display_source) +} + +fn trust_target_from_url( + current_repo_workdir: &Path, + submodule_path: &Path, + url: &gix::Url, +) -> Result> { + let display_source = bytes_to_text_preserving_utf8(url.to_bstring().as_ref()); + trust_target_from_parsed_url(current_repo_workdir, submodule_path, url, display_source) +} + +fn trust_target_from_parsed_url( + current_repo_workdir: &Path, + submodule_path: &Path, + url: &gix::Url, + display_source: String, +) -> Result> { + if url.scheme != gix::url::Scheme::File { + return Ok(None); + } + + let local_source_path = canonicalize_or_original(resolve_local_file_transport_path( + current_repo_workdir, + url, + )?); + Ok(Some(SubmoduleTrustTarget { + submodule_path: submodule_path.to_path_buf(), + display_source, + local_source_path, + })) +} + +fn resolve_local_file_transport_path( + current_repo_workdir: &Path, + url: &gix::Url, +) -> Result { + let mut path = pathbuf_from_gix_path(url.path.as_ref())?; + if let Some(host) = url.host.as_deref() + && !host.eq_ignore_ascii_case("localhost") + { + let host_path = PathBuf::from(format!("//{host}")).join(&path); + path = host_path; + } + if path.is_relative() { + path = current_repo_workdir.join(path); + } + Ok(path) +} + +fn persist_submodule_trust_approvals( + trust_root: &Path, + approved_sources: &[SubmoduleTrustTarget], +) -> Result<()> { + const GIT_CONFIG_LOCK_RETRIES: usize = 6; + + for source in approved_sources { + let key = submodule_file_transport_consent_key(trust_root, &source.local_source_path); + if git_config_get_bool_global(trust_root, &key)?.unwrap_or(false) { + continue; + } + + let mut last_err = None; + for attempt in 0..GIT_CONFIG_LOCK_RETRIES { + let mut cmd = git_workdir_cmd_for(trust_root); + cmd.arg("config").arg("--global").arg(&key).arg("true"); + match run_git_simple(cmd, &format!("git config --global {key} true")) { + Ok(()) => { + last_err = None; + break; + } + Err(err) => { + if git_config_get_bool_global(trust_root, &key)?.unwrap_or(false) { + last_err = None; + break; + } + let retryable = + attempt + 1 < GIT_CONFIG_LOCK_RETRIES && is_git_config_lock_error(&err); + last_err = Some(err); + if retryable { + thread::sleep(Duration::from_millis(25)); + continue; + } + break; + } + } + } + if let Some(err) = last_err { + return Err(err); + } + } + Ok(()) +} + +fn is_git_config_lock_error(err: &Error) -> bool { + let text = match err.kind() { + ErrorKind::Git(failure) => format!( + "{}{}{}", + failure.detail().unwrap_or_default(), + String::from_utf8_lossy(failure.stderr()), + String::from_utf8_lossy(failure.stdout()) + ), + _ => err.to_string(), + }; + text.contains("could not lock config file") +} + +fn submodule_source_trusted(trust_root: &Path, source: &SubmoduleTrustTarget) -> Result { + let key = submodule_file_transport_consent_key(trust_root, &source.local_source_path); + Ok(git_config_get_bool_global(trust_root, &key)?.unwrap_or(false)) +} + +fn untrusted_local_submodule_error(source: &SubmoduleTrustTarget, action: &str) -> Error { + Error::new(ErrorKind::Backend(format!( + "Refusing to {action} local submodule '{}' from '{}'. Explicit trust is required before enabling file transport.", + source.submodule_path.display(), + source.display_source + ))) +} + +fn combine_submodule_update_outputs(outputs: Vec) -> CommandOutput { + CommandOutput { + command: "git submodule update --init --recursive".to_string(), + stdout: outputs + .iter() + .map(|output| output.stdout.trim_end()) + .filter(|text| !text.is_empty()) + .collect::>() + .join("\n"), + stderr: outputs + .iter() + .map(|output| output.stderr.trim_end()) + .filter(|text| !text.is_empty()) + .collect::>() + .join("\n"), + exit_code: Some(0), + } +} + +fn repo_workdir_for_submodule_trust(repo: &gix::Repository) -> &Path { + repo.workdir().unwrap_or_else(|| repo.git_dir()) +} + +fn submodule_file_transport_consent_key(trust_root: &Path, source_path: &Path) -> String { + let root = canonicalize_or_original(trust_root.to_path_buf()); + let source = canonicalize_or_original(source_path.to_path_buf()); + + let mut bytes = stable_path_bytes(&root); + bytes.push(0); + bytes.extend_from_slice(&stable_path_bytes(&source)); + format!( + "gitcomet.submodule.allowfiletransport-{:016x}", + fnv1a_64(&bytes) + ) +} + +fn git_config_get_bool_global(trust_root: &Path, key: &str) -> Result> { + let mut cmd = git_workdir_cmd_for(trust_root); + cmd.arg("config") + .arg("--global") + .arg("--type=bool") + .arg("--get") + .arg(key); + + let output = cmd + .output() + .map_err(|err| Error::new(ErrorKind::Io(err.kind())))?; + + if output.status.success() { + let value = bytes_to_text_preserving_utf8(&output.stdout); + return match value.trim() { + "true" => Ok(Some(true)), + "false" => Ok(Some(false)), + other => Err(Error::new(ErrorKind::Backend(format!( + "Invalid boolean value for git config {key}: {:?}. Expected true or false.", + other + )))), + }; + } + + if output.status.code() == Some(1) { + return Ok(None); + } + + Err(Error::new(ErrorKind::Backend(format!( + "git config --global --type=bool --get {key} failed: {}", + bytes_to_text_preserving_utf8(&output.stderr).trim() + )))) +} + +fn stable_path_bytes(path: &Path) -> Vec { + #[cfg(unix)] + { + use std::os::unix::ffi::OsStrExt as _; + + path.as_os_str().as_bytes().to_vec() + } + + #[cfg(windows)] + { + use std::os::windows::ffi::OsStrExt as _; + + let mut bytes = Vec::new(); + for unit in path.as_os_str().encode_wide() { + bytes.extend_from_slice(&unit.to_le_bytes()); + } + bytes + } + + #[cfg(not(any(unix, windows)))] + { + path.to_str() + .map(|text| text.as_bytes().to_vec()) + .unwrap_or_else(|| format!("{path:?}").into_bytes()) + } +} + +fn fnv1a_64(bytes: &[u8]) -> u64 { + let mut hash = 0xcbf29ce484222325u64; + for byte in bytes { + hash ^= u64::from(*byte); + hash = hash.wrapping_mul(0x100000001b3); + } + hash +} + fn pathbuf_from_gix_path(path: &gix::bstr::BStr) -> Result { gix::path::try_from_bstr(path) .map(|path| path.into_owned()) @@ -255,3 +991,48 @@ fn pathbuf_from_gix_path(path: &gix::bstr::BStr) -> Result { fn object_id_to_commit_id(id: gix::ObjectId) -> CommitId { CommitId(id.to_string().into()) } + +#[cfg(test)] +mod tests { + use super::allow_file_submodule_transport; + use super::submodule_file_transport_consent_key; + use std::ffi::OsStr; + use std::path::Path; + use std::process::Command; + + #[test] + fn allow_file_submodule_transport_uses_git_config_not_protocol_allowlist() { + let mut cmd = Command::new("git"); + + allow_file_submodule_transport(&mut cmd); + + let args = cmd + .get_args() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect::>(); + assert_eq!(args, ["-c", "protocol.file.allow=always"]); + assert!( + !cmd.get_envs() + .any(|(key, _)| key == OsStr::new("GIT_ALLOW_PROTOCOL")) + ); + } + + #[test] + fn consent_key_depends_on_root_and_source_path() { + let a = submodule_file_transport_consent_key( + Path::new("/repo-a"), + Path::new("/sources/local-one"), + ); + let b = submodule_file_transport_consent_key( + Path::new("/repo-a"), + Path::new("/sources/local-two"), + ); + let c = submodule_file_transport_consent_key( + Path::new("/repo-b"), + Path::new("/sources/local-one"), + ); + + assert_ne!(a, b); + assert_ne!(a, c); + } +} diff --git a/crates/gitcomet-git-gix/src/util.rs b/crates/gitcomet-git-gix/src/util.rs index 104b7372..50dd044f 100644 --- a/crates/gitcomet-git-gix/src/util.rs +++ b/crates/gitcomet-git-gix/src/util.rs @@ -169,7 +169,7 @@ fn apply_test_git_command_environment(cmd: &mut Command) { cmd.env("HOME", &env.home_dir); cmd.env("XDG_CONFIG_HOME", &env.xdg_config_home); cmd.env("GNUPGHOME", &env.gnupg_home); - cmd.env("GIT_ALLOW_PROTOCOL", "file"); + cmd.arg("-c").arg("protocol.file.allow=always"); } pub(crate) fn git_workdir_cmd_for(workdir: &Path) -> Command { diff --git a/crates/gitcomet-git-gix/tests/submodules_integration.rs b/crates/gitcomet-git-gix/tests/submodules_integration.rs index 72c61b6b..d0b9b8d9 100644 --- a/crates/gitcomet-git-gix/tests/submodules_integration.rs +++ b/crates/gitcomet-git-gix/tests/submodules_integration.rs @@ -1,5 +1,5 @@ use gitcomet_core::domain::SubmoduleStatus; -use gitcomet_core::services::GitBackend; +use gitcomet_core::services::{GitBackend, SubmoduleTrustDecision}; use gitcomet_git_gix::GixBackend; #[path = "support/test_git_env.rs"] mod test_git_env; @@ -43,6 +43,78 @@ fn git_stdout(repo: &Path, args: &[&str]) -> String { .to_string() } +fn local_submodule_config_entries(repo: &Path) -> Vec { + let output = git_output(repo, &["config", "--list", "--local"]); + assert!(output.status.success(), "git config --list --local failed"); + String::from_utf8(output.stdout) + .expect("git stdout is utf-8") + .lines() + .filter(|line| line.starts_with("submodule.")) + .map(ToOwned::to_owned) + .collect() +} + +fn add_submodule_raw(parent_repo: &Path, sub_repo: &Path, path: &Path, name: Option<&str>) { + let mut cmd = git_command(); + cmd.arg("-C") + .arg(parent_repo) + .arg("-c") + .arg("protocol.file.allow=always") + .arg("submodule") + .arg("add"); + if let Some(name) = name { + cmd.arg("--name").arg(name); + } + let status = cmd + .arg(sub_repo) + .arg(path) + .status() + .expect("git submodule add to run"); + assert!(status.success(), "git submodule add failed"); +} + +fn run_git_with_path(repo: &Path, args: &[&str], path: &Path) { + let status = git_command() + .arg("-C") + .arg(repo) + .args(args) + .arg(path) + .status() + .expect("git command to run"); + assert!(status.success(), "git {:?} {:?} failed", args, path); +} + +fn create_stale_submodule_git_dir( + parent_repo: &Path, + sub_repo: &Path, + path: &Path, + name: Option<&str>, +) { + add_submodule_raw(parent_repo, sub_repo, path, name); + run_git( + parent_repo, + &[ + "-c", + "commit.gpgsign=false", + "commit", + "-m", + "add submodule", + ], + ); + run_git_with_path(parent_repo, &["submodule", "deinit", "-f", "--"], path); + run_git_with_path(parent_repo, &["rm", "-f", "--"], path); + run_git( + parent_repo, + &[ + "-c", + "commit.gpgsign=false", + "commit", + "-m", + "remove submodule", + ], + ); +} + fn init_repo_with_seed(repo: &Path, file: &str, contents: &str, message: &str) { run_git(repo, &["init"]); run_git(repo, &["config", "user.email", "you@example.com"]); @@ -520,14 +592,27 @@ fn submodule_add_update_remove_round_trip() { init_repo_with_seed(&sub_repo, "file.txt", "hello\n", "seed submodule"); init_repo_with_seed(&parent_repo, "seed.txt", "seed\n", "seed parent"); - run_git(&parent_repo, &["config", "protocol.file.allow", "always"]); let backend = GixBackend; let opened = backend.open(&parent_repo).expect("open parent repository"); let submodule_path = Path::new("mods/sub-one"); + let approved_sources = match opened + .check_submodule_add_trust(sub_repo.to_string_lossy().as_ref(), submodule_path) + .expect("check local submodule trust") + { + SubmoduleTrustDecision::Prompt { sources } => sources, + other => panic!("expected trust prompt for local submodule, got {other:?}"), + }; let add_output = opened - .add_submodule_with_output(sub_repo.to_string_lossy().as_ref(), submodule_path) + .add_submodule_with_output( + sub_repo.to_string_lossy().as_ref(), + submodule_path, + None, + None, + false, + &approved_sources, + ) .expect("add submodule"); assert_eq!(add_output.exit_code, Some(0)); @@ -551,8 +636,15 @@ fn submodule_add_update_remove_round_trip() { ); assert_eq!(listed[0].head.as_ref().len(), 40); + assert_eq!( + opened + .check_submodule_update_trust() + .expect("check update trust after approval"), + SubmoduleTrustDecision::Proceed + ); + let update_output = opened - .update_submodules_with_output() + .update_submodules_with_output(&[]) .expect("update submodules"); assert_eq!(update_output.exit_code, Some(0)); @@ -567,4 +659,514 @@ fn submodule_add_update_remove_round_trip() { .expect("list submodules after remove"); assert!(listed_after_remove.is_empty()); assert!(!parent_repo.join("mods/sub-one").exists()); + assert!(local_submodule_config_entries(&parent_repo).is_empty()); + assert!(!parent_repo.join(".git/modules/mods/sub-one").exists()); + assert!(!parent_repo.join(".git/modules/mods").exists()); +} + +#[test] +fn add_submodule_does_not_restrict_https_or_ssh_transports() { + if !require_git_shell_for_submodule_tests() { + return; + } + let dir = tempfile::tempdir().expect("create tempdir"); + let repo = dir.path().join("parent"); + fs::create_dir_all(&repo).expect("create parent repository directory"); + init_repo_with_seed(&repo, "seed.txt", "seed\n", "seed parent"); + + let backend = GixBackend; + let opened = backend.open(&repo).expect("open parent repository"); + + for (url, blocked_transport) in [ + ("https://127.0.0.1:1/repo.git", "https"), + ("ssh://git@127.0.0.1:1/repo.git", "ssh"), + ] { + let err = opened + .add_submodule_with_output(url, Path::new("mods/sub-one"), None, None, false, &[]) + .expect_err("dummy remote should fail without a reachable server"); + let rendered = err.to_string(); + assert!( + !rendered.contains(&format!("transport '{blocked_transport}' not allowed")), + "unexpected protocol allowlist failure for {url}: {rendered}" + ); + } +} + +#[test] +fn add_local_submodule_requires_explicit_trust() { + if !require_git_shell_for_submodule_tests() { + return; + } + let dir = tempfile::tempdir().expect("create tempdir"); + let root = dir.path(); + + let sub_repo = root.join("sub"); + let parent_repo = root.join("parent"); + fs::create_dir_all(&sub_repo).expect("create sub repository directory"); + fs::create_dir_all(&parent_repo).expect("create parent repository directory"); + + init_repo_with_seed(&sub_repo, "file.txt", "hello\n", "seed submodule"); + init_repo_with_seed(&parent_repo, "seed.txt", "seed\n", "seed parent"); + + let backend = GixBackend; + let opened = backend.open(&parent_repo).expect("open parent repository"); + + let submodule_path = Path::new("mods/sub"); + let trust = opened + .check_submodule_add_trust(sub_repo.to_string_lossy().as_ref(), submodule_path) + .expect("check local submodule trust"); + let approved_sources = match trust { + SubmoduleTrustDecision::Prompt { sources } => sources, + other => panic!("expected trust prompt for local submodule, got {other:?}"), + }; + + let err = opened + .add_submodule_with_output( + sub_repo.to_string_lossy().as_ref(), + submodule_path, + None, + None, + false, + &[], + ) + .expect_err("local submodule should fail without trust"); + assert!( + err.to_string().contains("Explicit trust is required"), + "unexpected error: {err}" + ); + + let add_output = opened + .add_submodule_with_output( + sub_repo.to_string_lossy().as_ref(), + submodule_path, + None, + None, + false, + &approved_sources, + ) + .expect("add trusted local submodule"); + assert_eq!(add_output.exit_code, Some(0)); +} + +#[test] +fn add_submodule_supports_branch_selection() { + if !require_git_shell_for_submodule_tests() { + return; + } + let dir = tempfile::tempdir().expect("create tempdir"); + let root = dir.path(); + + let sub_repo = root.join("sub"); + let parent_repo = root.join("parent"); + fs::create_dir_all(&sub_repo).expect("create sub repository directory"); + fs::create_dir_all(&parent_repo).expect("create parent repository directory"); + + init_repo_with_seed(&sub_repo, "file.txt", "hello\n", "seed submodule"); + run_git(&sub_repo, &["branch", "feature"]); + init_repo_with_seed(&parent_repo, "seed.txt", "seed\n", "seed parent"); + + let backend = GixBackend; + let opened = backend.open(&parent_repo).expect("open parent repository"); + let submodule_path = Path::new("mods/sub"); + let approved_sources = match opened + .check_submodule_add_trust(sub_repo.to_string_lossy().as_ref(), submodule_path) + .expect("check local submodule trust") + { + SubmoduleTrustDecision::Prompt { sources } => sources, + other => panic!("expected trust prompt for local submodule, got {other:?}"), + }; + + let add_output = opened + .add_submodule_with_output( + sub_repo.to_string_lossy().as_ref(), + submodule_path, + Some("feature"), + None, + false, + &approved_sources, + ) + .expect("add submodule with branch"); + assert_eq!(add_output.exit_code, Some(0)); + assert!(add_output.command.contains("--branch feature")); + + let gitmodules = fs::read_to_string(parent_repo.join(".gitmodules")).expect("read .gitmodules"); + assert!(gitmodules.contains("branch = feature")); + assert_eq!( + git_stdout(&parent_repo.join("mods/sub"), &["branch", "--show-current"]), + "feature" + ); +} + +#[test] +fn add_submodule_supports_multiple_branches_from_same_source() { + if !require_git_shell_for_submodule_tests() { + return; + } + let dir = tempfile::tempdir().expect("create tempdir"); + let root = dir.path(); + + let sub_repo = root.join("sub"); + let parent_repo = root.join("parent"); + fs::create_dir_all(&sub_repo).expect("create sub repository directory"); + fs::create_dir_all(&parent_repo).expect("create parent repository directory"); + + init_repo_with_seed(&sub_repo, "file.txt", "hello\n", "seed submodule"); + let default_branch = git_stdout(&sub_repo, &["symbolic-ref", "--short", "HEAD"]); + run_git(&sub_repo, &["checkout", "-b", "feature"]); + fs::write(sub_repo.join("file.txt"), "feature\n").expect("write feature contents"); + run_git( + &sub_repo, + &[ + "-c", + "commit.gpgsign=false", + "commit", + "-am", + "feature commit", + ], + ); + run_git(&sub_repo, &["checkout", &default_branch]); + init_repo_with_seed(&parent_repo, "seed.txt", "seed\n", "seed parent"); + + let backend = GixBackend; + let opened = backend.open(&parent_repo).expect("open parent repository"); + + let main_path = Path::new("mods/main"); + let approved_main = match opened + .check_submodule_add_trust(sub_repo.to_string_lossy().as_ref(), main_path) + .expect("check local submodule trust for main") + { + SubmoduleTrustDecision::Prompt { sources } => sources, + other => panic!("expected trust prompt for local submodule, got {other:?}"), + }; + let main_output = opened + .add_submodule_with_output( + sub_repo.to_string_lossy().as_ref(), + main_path, + Some(default_branch.as_str()), + None, + false, + &approved_main, + ) + .expect("add default-branch submodule"); + assert_eq!(main_output.exit_code, Some(0)); + + let feature_path = Path::new("mods/feature"); + let approved_feature = match opened + .check_submodule_add_trust(sub_repo.to_string_lossy().as_ref(), feature_path) + .expect("check local submodule trust for feature") + { + SubmoduleTrustDecision::Prompt { sources } => sources, + SubmoduleTrustDecision::Proceed => Vec::new(), + }; + let feature_output = opened + .add_submodule_with_output( + sub_repo.to_string_lossy().as_ref(), + feature_path, + Some("feature"), + None, + false, + &approved_feature, + ) + .expect("add feature-branch submodule"); + assert_eq!(feature_output.exit_code, Some(0)); + + let listed = opened.list_submodules().expect("list added submodules"); + assert_eq!(listed.len(), 2); + assert_eq!( + git_stdout( + &parent_repo.join("mods/main"), + &["branch", "--show-current"] + ), + default_branch + ); + assert_eq!( + git_stdout( + &parent_repo.join("mods/feature"), + &["branch", "--show-current"] + ), + "feature" + ); + + let gitmodules = fs::read_to_string(parent_repo.join(".gitmodules")).expect("read .gitmodules"); + assert!(gitmodules.contains(&format!("branch = {default_branch}"))); + assert!(gitmodules.contains("branch = feature")); +} + +#[test] +fn add_submodule_failed_branch_checkout_cleans_partial_clone_and_metadata() { + if !require_git_shell_for_submodule_tests() { + return; + } + let dir = tempfile::tempdir().expect("create tempdir"); + let root = dir.path(); + + let sub_repo = root.join("sub"); + let parent_repo = root.join("parent"); + fs::create_dir_all(&sub_repo).expect("create sub repository directory"); + fs::create_dir_all(&parent_repo).expect("create parent repository directory"); + + init_repo_with_seed(&sub_repo, "file.txt", "hello\n", "seed submodule"); + init_repo_with_seed(&parent_repo, "seed.txt", "seed\n", "seed parent"); + + let backend = GixBackend; + let opened = backend.open(&parent_repo).expect("open parent repository"); + let submodule_path = Path::new("mods/sub"); + let approved_sources = match opened + .check_submodule_add_trust(sub_repo.to_string_lossy().as_ref(), submodule_path) + .expect("check local submodule trust") + { + SubmoduleTrustDecision::Prompt { sources } => sources, + other => panic!("expected trust prompt for local submodule, got {other:?}"), + }; + + let err = opened + .add_submodule_with_output( + sub_repo.to_string_lossy().as_ref(), + submodule_path, + Some("does-not-exist"), + None, + false, + &approved_sources, + ) + .expect_err("add with missing branch should fail"); + let rendered = err.to_string(); + assert!( + rendered.contains("does-not-exist"), + "unexpected branch failure error: {rendered}" + ); + + assert!( + !parent_repo.join("mods/sub").exists(), + "expected failed submodule checkout to be removed" + ); + assert!( + !parent_repo.join(".git/modules/mods/sub").exists(), + "expected failed submodule metadata to be removed" + ); + assert!( + !parent_repo.join(".gitmodules").exists(), + "expected no .gitmodules entry after failed add" + ); + assert!(local_submodule_config_entries(&parent_repo).is_empty()); + assert!( + git_stdout(&parent_repo, &["submodule"]).is_empty(), + "expected git submodule to report no registered submodules" + ); + assert!( + opened + .list_submodules() + .expect("list submodules") + .is_empty(), + "expected failed submodule add not to be listed" + ); +} + +#[test] +fn add_submodule_failed_branch_checkout_cleans_partial_clone_with_custom_name() { + if !require_git_shell_for_submodule_tests() { + return; + } + let dir = tempfile::tempdir().expect("create tempdir"); + let root = dir.path(); + + let sub_repo = root.join("sub"); + let parent_repo = root.join("parent"); + fs::create_dir_all(&sub_repo).expect("create sub repository directory"); + fs::create_dir_all(&parent_repo).expect("create parent repository directory"); + + init_repo_with_seed(&sub_repo, "file.txt", "hello\n", "seed submodule"); + init_repo_with_seed(&parent_repo, "seed.txt", "seed\n", "seed parent"); + + let backend = GixBackend; + let opened = backend.open(&parent_repo).expect("open parent repository"); + let submodule_path = Path::new("mods/sub"); + let approved_sources = match opened + .check_submodule_add_trust(sub_repo.to_string_lossy().as_ref(), submodule_path) + .expect("check local submodule trust") + { + SubmoduleTrustDecision::Prompt { sources } => sources, + other => panic!("expected trust prompt for local submodule, got {other:?}"), + }; + + let err = opened + .add_submodule_with_output( + sub_repo.to_string_lossy().as_ref(), + submodule_path, + Some("does-not-exist"), + Some("custom-name"), + false, + &approved_sources, + ) + .expect_err("add with missing branch and custom name should fail"); + let rendered = err.to_string(); + assert!( + rendered.contains("does-not-exist"), + "unexpected branch failure error: {rendered}" + ); + + assert!( + !parent_repo.join("mods/sub").exists(), + "expected failed submodule checkout to be removed" + ); + assert!( + !parent_repo.join(".git/modules/custom-name").exists(), + "expected failed custom-name metadata to be removed" + ); + assert!( + !parent_repo.join(".gitmodules").exists(), + "expected no .gitmodules entry after failed add" + ); + assert!(local_submodule_config_entries(&parent_repo).is_empty()); +} + +#[test] +fn add_submodule_supports_custom_logical_name_for_local_git_dir_collision() { + if !require_git_shell_for_submodule_tests() { + return; + } + let dir = tempfile::tempdir().expect("create tempdir"); + let root = dir.path(); + + let sub_repo = root.join("sub"); + let parent_repo = root.join("parent"); + fs::create_dir_all(&sub_repo).expect("create sub repository directory"); + fs::create_dir_all(&parent_repo).expect("create parent repository directory"); + + init_repo_with_seed(&sub_repo, "file.txt", "hello\n", "seed submodule"); + init_repo_with_seed(&parent_repo, "seed.txt", "seed\n", "seed parent"); + + let submodule_path = Path::new("sm"); + create_stale_submodule_git_dir(&parent_repo, &sub_repo, submodule_path, None); + assert!( + parent_repo.join(".git/modules/sm").exists(), + "expected stale local submodule git dir" + ); + + let backend = GixBackend; + let opened = backend.open(&parent_repo).expect("open parent repository"); + let approved_sources = match opened + .check_submodule_add_trust(sub_repo.to_string_lossy().as_ref(), submodule_path) + .expect("check local submodule trust") + { + SubmoduleTrustDecision::Prompt { sources } => sources, + other => panic!("expected trust prompt for local submodule, got {other:?}"), + }; + + let err = opened + .add_submodule_with_output( + sub_repo.to_string_lossy().as_ref(), + submodule_path, + None, + None, + false, + &approved_sources, + ) + .expect_err("add without custom name or force should fail"); + let rendered = err.to_string(); + assert!( + rendered.contains("If you want to reuse this local git directory") + || rendered.contains("use the '--force' option") + || rendered.contains("choose another name with the '--name' option"), + "unexpected collision error: {rendered}" + ); + + let add_output = opened + .add_submodule_with_output( + sub_repo.to_string_lossy().as_ref(), + submodule_path, + None, + Some("sm-renamed"), + false, + &approved_sources, + ) + .expect("add submodule with custom logical name"); + assert_eq!(add_output.exit_code, Some(0)); +} + +#[test] +fn add_submodule_supports_force_for_local_git_dir_collision() { + if !require_git_shell_for_submodule_tests() { + return; + } + let dir = tempfile::tempdir().expect("create tempdir"); + let root = dir.path(); + + let sub_repo = root.join("sub"); + let parent_repo = root.join("parent"); + fs::create_dir_all(&sub_repo).expect("create sub repository directory"); + fs::create_dir_all(&parent_repo).expect("create parent repository directory"); + + init_repo_with_seed(&sub_repo, "file.txt", "hello\n", "seed submodule"); + init_repo_with_seed(&parent_repo, "seed.txt", "seed\n", "seed parent"); + + let submodule_path = Path::new("sm"); + create_stale_submodule_git_dir(&parent_repo, &sub_repo, submodule_path, None); + assert!( + parent_repo.join(".git/modules/sm").exists(), + "expected stale local submodule git dir" + ); + + let backend = GixBackend; + let opened = backend.open(&parent_repo).expect("open parent repository"); + let approved_sources = match opened + .check_submodule_add_trust(sub_repo.to_string_lossy().as_ref(), submodule_path) + .expect("check local submodule trust") + { + SubmoduleTrustDecision::Prompt { sources } => sources, + other => panic!("expected trust prompt for local submodule, got {other:?}"), + }; + + let add_output = opened + .add_submodule_with_output( + sub_repo.to_string_lossy().as_ref(), + submodule_path, + None, + None, + true, + &approved_sources, + ) + .expect("add submodule with force"); + assert_eq!(add_output.exit_code, Some(0)); +} + +#[test] +fn remove_submodule_cleans_custom_logical_name_metadata() { + if !require_git_shell_for_submodule_tests() { + return; + } + let dir = tempfile::tempdir().expect("create tempdir"); + let root = dir.path(); + + let sub_repo = root.join("sub"); + let parent_repo = root.join("parent"); + fs::create_dir_all(&sub_repo).expect("create sub repository directory"); + fs::create_dir_all(&parent_repo).expect("create parent repository directory"); + + init_repo_with_seed(&sub_repo, "file.txt", "hello\n", "seed submodule"); + init_repo_with_seed(&parent_repo, "seed.txt", "seed\n", "seed parent"); + add_submodule_raw( + &parent_repo, + &sub_repo, + Path::new("mods/sub"), + Some("custom"), + ); + run_git( + &parent_repo, + &[ + "-c", + "commit.gpgsign=false", + "commit", + "-m", + "add submodule", + ], + ); + + let backend = GixBackend; + let opened = backend.open(&parent_repo).expect("open parent repository"); + let remove_output = opened + .remove_submodule_with_output(Path::new("mods/sub")) + .expect("remove submodule"); + assert_eq!(remove_output.exit_code, Some(0)); + assert!(local_submodule_config_entries(&parent_repo).is_empty()); + assert!(!parent_repo.join(".git/modules/custom").exists()); } diff --git a/crates/gitcomet-git-gix/tests/support/test_git_env.rs b/crates/gitcomet-git-gix/tests/support/test_git_env.rs index a00923c7..a2e1ec18 100644 --- a/crates/gitcomet-git-gix/tests/support/test_git_env.rs +++ b/crates/gitcomet-git-gix/tests/support/test_git_env.rs @@ -10,6 +10,7 @@ struct IsolatedGitConfigEnv { home_dir: PathBuf, xdg_config_home: PathBuf, global_config: PathBuf, + gnupg_home: PathBuf, } fn isolated_git_config_env() -> &'static IsolatedGitConfigEnv { @@ -19,16 +20,34 @@ fn isolated_git_config_env() -> &'static IsolatedGitConfigEnv { let home_dir = root.path().join("home"); let xdg_config_home = root.path().join("xdg"); let global_config = root.path().join("global.gitconfig"); + let gnupg_home = root.path().join("gnupg"); fs::create_dir_all(&home_dir).expect("create isolated HOME directory"); fs::create_dir_all(&xdg_config_home).expect("create isolated XDG_CONFIG_HOME directory"); + fs::create_dir_all(&gnupg_home).expect("create isolated GNUPGHOME directory"); fs::write(&global_config, "").expect("create isolated global git config file"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + + fs::set_permissions(&gnupg_home, fs::Permissions::from_mode(0o700)) + .expect("set isolated GNUPGHOME permissions"); + } + + gitcomet_git_gix::install_test_git_command_environment( + global_config.clone(), + home_dir.clone(), + xdg_config_home.clone(), + gnupg_home.clone(), + ); + IsolatedGitConfigEnv { _root: root, home_dir, xdg_config_home, global_config, + gnupg_home, } }) } @@ -39,5 +58,6 @@ pub(crate) fn apply(cmd: &mut Command) { .env("GIT_CONFIG_GLOBAL", &env.global_config) .env("HOME", &env.home_dir) .env("XDG_CONFIG_HOME", &env.xdg_config_home) + .env("GNUPGHOME", &env.gnupg_home) .env_remove("GIT_CONFIG_SYSTEM"); } diff --git a/crates/gitcomet-git/src/noop_backend.rs b/crates/gitcomet-git/src/noop_backend.rs index 58c84c83..400836a3 100644 --- a/crates/gitcomet-git/src/noop_backend.rs +++ b/crates/gitcomet-git/src/noop_backend.rs @@ -290,8 +290,17 @@ mod tests { assert_unsupported(repo.remove_worktree_with_output(path)); assert_unsupported(repo.force_remove_worktree_with_output(path)); assert_unsupported(repo.list_submodules()); - assert_unsupported(repo.add_submodule_with_output("https://example.com/repo.git", path)); - assert_unsupported(repo.update_submodules_with_output()); + assert_unsupported(repo.check_submodule_add_trust("https://example.com/repo.git", path)); + assert_unsupported(repo.check_submodule_update_trust()); + assert_unsupported(repo.add_submodule_with_output( + "https://example.com/repo.git", + path, + None, + None, + false, + &[], + )); + assert_unsupported(repo.update_submodules_with_output(&[])); assert_unsupported(repo.remove_submodule_with_output(path)); } } diff --git a/crates/gitcomet-state/src/model.rs b/crates/gitcomet-state/src/model.rs index 2060da15..d76a4b1e 100644 --- a/crates/gitcomet-state/src/model.rs +++ b/crates/gitcomet-state/src/model.rs @@ -6,7 +6,7 @@ use gitcomet_core::conflict_session::{ }; use gitcomet_core::domain::*; use gitcomet_core::process::GitRuntimeState; -use gitcomet_core::services::BlameLine; +use gitcomet_core::services::{BlameLine, SubmoduleTrustTarget}; use serde::{Deserialize, Serialize}; use std::collections::VecDeque; use std::path::PathBuf; @@ -275,6 +275,7 @@ pub struct AppState { pub notifications: Vec, pub banner_error: Option, pub auth_prompt: Option, + pub submodule_trust_prompt: Option, pub git_runtime: GitRuntimeState, pub git_log_settings: GitLogSettings, } @@ -322,6 +323,25 @@ pub struct AuthPromptState { pub operation: AuthRetryOperation, } +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum SubmoduleTrustPromptOperation { + Add { + url: String, + path: PathBuf, + branch: Option, + name: Option, + force: bool, + }, + Update, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct SubmoduleTrustPromptState { + pub repo_id: RepoId, + pub operation: SubmoduleTrustPromptOperation, + pub sources: Vec, +} + #[derive(Clone, Debug, Eq, PartialEq)] pub struct AppNotification { pub time: SystemTime, @@ -347,6 +367,12 @@ pub struct CloneOpState { pub output_tail: VecDeque, } +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct SubmoduleAddProgressState { + pub url: String, + pub path: PathBuf, +} + #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum CloneProgressStage { Loading, @@ -560,6 +586,7 @@ pub struct RepoState { pub worktrees_rev: u64, pub submodules: Loadable>>, pub submodules_rev: u64, + pub submodule_add_in_flight: Option, pub sidebar_data_request: SidebarDataRequest, /// Invalidates cached branch-sidebar rows when any sidebar-relevant source changes. pub branch_sidebar_rev: u64, @@ -630,6 +657,7 @@ impl RepoState { worktrees_rev: 0, submodules: Loadable::NotLoaded, submodules_rev: 0, + submodule_add_in_flight: None, sidebar_data_request: SidebarDataRequest::default(), branch_sidebar_rev: 0, diff_state: DiffState::default(), diff --git a/crates/gitcomet-state/src/msg/effect.rs b/crates/gitcomet-state/src/msg/effect.rs index 89984257..0d1ffc59 100644 --- a/crates/gitcomet-state/src/msg/effect.rs +++ b/crates/gitcomet-state/src/msg/effect.rs @@ -1,7 +1,9 @@ use crate::model::{ConflictFileLoadMode, RepoId}; use gitcomet_core::auth::StagedGitAuth; use gitcomet_core::domain::*; -use gitcomet_core::services::{ConflictSide, PullMode, RemoteUrlKind, ResetMode}; +use gitcomet_core::services::{ + ConflictSide, PullMode, RemoteUrlKind, ResetMode, SubmoduleTrustTarget, +}; use std::path::PathBuf; use super::RepoPathList; @@ -199,14 +201,30 @@ pub enum Effect { repo_id: RepoId, path: PathBuf, }, + CheckSubmoduleAddTrust { + repo_id: RepoId, + url: String, + path: PathBuf, + branch: Option, + name: Option, + force: bool, + }, + CheckSubmoduleUpdateTrust { + repo_id: RepoId, + }, AddSubmodule { repo_id: RepoId, url: String, path: PathBuf, + branch: Option, + name: Option, + force: bool, + approved_sources: Vec, auth: Option, }, UpdateSubmodules { repo_id: RepoId, + approved_sources: Vec, auth: Option, }, RemoveSubmodule { diff --git a/crates/gitcomet-state/src/msg/message.rs b/crates/gitcomet-state/src/msg/message.rs index 3b131a6e..a9004ad1 100644 --- a/crates/gitcomet-state/src/msg/message.rs +++ b/crates/gitcomet-state/src/msg/message.rs @@ -5,7 +5,10 @@ use gitcomet_core::domain::*; use gitcomet_core::error::Error; use gitcomet_core::process::GitRuntimeState; use gitcomet_core::services::GitRepository; -use gitcomet_core::services::{CommandOutput, ConflictSide, PullMode, RemoteUrlKind, ResetMode}; +use gitcomet_core::services::{ + CommandOutput, ConflictSide, PullMode, RemoteUrlKind, ResetMode, SubmoduleTrustDecision, + SubmoduleTrustTarget, +}; use std::path::PathBuf; use std::sync::Arc; @@ -264,10 +267,28 @@ pub enum Msg { repo_id: RepoId, url: String, path: PathBuf, + branch: Option, + name: Option, + force: bool, + }, + AddSubmoduleTrusted { + repo_id: RepoId, + url: String, + path: PathBuf, + branch: Option, + name: Option, + force: bool, + approved_sources: Vec, }, UpdateSubmodules { repo_id: RepoId, }, + UpdateSubmodulesTrusted { + repo_id: RepoId, + approved_sources: Vec, + }, + ConfirmSubmoduleTrustPrompt, + CancelSubmoduleTrustPrompt, RemoveSubmodule { repo_id: RepoId, path: PathBuf, @@ -603,6 +624,19 @@ pub enum InternalMsg { repo_id: RepoId, result: Result, Error>, }, + SubmoduleAddTrustChecked { + repo_id: RepoId, + url: String, + path: PathBuf, + branch: Option, + name: Option, + force: bool, + result: Result, + }, + SubmoduleUpdateTrustChecked { + repo_id: RepoId, + result: Result, + }, CommitDetailsLoaded { repo_id: RepoId, commit_id: CommitId, diff --git a/crates/gitcomet-state/src/msg/message_debug.rs b/crates/gitcomet-state/src/msg/message_debug.rs index fbb9f0b4..99b175b6 100644 --- a/crates/gitcomet-state/src/msg/message_debug.rs +++ b/crates/gitcomet-state/src/msg/message_debug.rs @@ -166,6 +166,29 @@ impl std::fmt::Debug for InternalMsg { .field("repo_id", repo_id) .field("result", result) .finish(), + InternalMsg::SubmoduleAddTrustChecked { + repo_id, + url, + path, + branch, + name, + force, + result, + } => f + .debug_struct("SubmoduleAddTrustChecked") + .field("repo_id", repo_id) + .field("url", url) + .field("path", path) + .field("branch", branch) + .field("name", name) + .field("force", force) + .field("result", result) + .finish(), + InternalMsg::SubmoduleUpdateTrustChecked { repo_id, result } => f + .debug_struct("SubmoduleUpdateTrustChecked") + .field("repo_id", repo_id) + .field("result", result) + .finish(), InternalMsg::CommitDetailsLoaded { repo_id, commit_id, diff --git a/crates/gitcomet-state/src/msg/repo_command_kind.rs b/crates/gitcomet-state/src/msg/repo_command_kind.rs index d268a7de..37b00a3c 100644 --- a/crates/gitcomet-state/src/msg/repo_command_kind.rs +++ b/crates/gitcomet-state/src/msg/repo_command_kind.rs @@ -1,5 +1,7 @@ use gitcomet_core::domain::CommitId; -use gitcomet_core::services::{ConflictSide, PullMode, RemoteUrlKind, ResetMode}; +use gitcomet_core::services::{ + ConflictSide, PullMode, RemoteUrlKind, ResetMode, SubmoduleTrustTarget, +}; use std::path::PathBuf; #[derive(Clone, Debug, Eq, PartialEq)] @@ -111,8 +113,14 @@ pub enum RepoCommandKind { AddSubmodule { url: String, path: PathBuf, + branch: Option, + name: Option, + force: bool, + approved_sources: Vec, + }, + UpdateSubmodules { + approved_sources: Vec, }, - UpdateSubmodules, RemoveSubmodule { path: PathBuf, }, diff --git a/crates/gitcomet-state/src/store/effects.rs b/crates/gitcomet-state/src/store/effects.rs index 3662be02..28df314d 100644 --- a/crates/gitcomet-state/src/store/effects.rs +++ b/crates/gitcomet-state/src/store/effects.rs @@ -203,6 +203,30 @@ fn send_unavailable_git_effect_result( result: Err(git_unavailable_error(runtime)), })) } + Effect::CheckSubmoduleAddTrust { + repo_id, + url, + path, + branch, + name, + force, + } => send(Msg::Internal( + crate::msg::InternalMsg::SubmoduleAddTrustChecked { + repo_id, + url, + path, + branch, + name, + force, + result: Err(git_unavailable_error(runtime)), + }, + )), + Effect::CheckSubmoduleUpdateTrust { repo_id } => send(Msg::Internal( + crate::msg::InternalMsg::SubmoduleUpdateTrustChecked { + repo_id, + result: Err(git_unavailable_error(runtime)), + }, + )), Effect::LoadRebaseAndMergeState { repo_id } => { send(Msg::Internal(crate::msg::InternalMsg::RebaseStateLoaded { repo_id, @@ -398,18 +422,36 @@ fn send_unavailable_git_effect_result( }, )), Effect::AddSubmodule { - repo_id, url, path, .. + repo_id, + url, + path, + branch, + name, + force, + approved_sources, + .. } => send(Msg::Internal( crate::msg::InternalMsg::RepoCommandFinished { repo_id, - command: RepoCommandKind::AddSubmodule { url, path }, + command: RepoCommandKind::AddSubmodule { + url, + path, + branch, + name, + force, + approved_sources, + }, result: Err(git_unavailable_error(runtime)), }, )), - Effect::UpdateSubmodules { repo_id, .. } => send(Msg::Internal( + Effect::UpdateSubmodules { + repo_id, + approved_sources, + .. + } => send(Msg::Internal( crate::msg::InternalMsg::RepoCommandFinished { repo_id, - command: RepoCommandKind::UpdateSubmodules, + command: RepoCommandKind::UpdateSubmodules { approved_sources }, result: Err(git_unavailable_error(runtime)), }, )), @@ -955,18 +997,60 @@ pub(super) fn schedule_effect( Effect::ForceRemoveWorktree { repo_id, path } => { repo_commands::schedule_force_remove_worktree(executor, repos, msg_tx, repo_id, path); } + Effect::CheckSubmoduleAddTrust { + repo_id, + url, + path, + branch, + name, + force, + } => { + repo_commands::schedule_check_submodule_add_trust( + executor, repos, msg_tx, repo_id, url, path, branch, name, force, + ); + } + Effect::CheckSubmoduleUpdateTrust { repo_id } => { + repo_commands::schedule_check_submodule_update_trust(executor, repos, msg_tx, repo_id); + } Effect::AddSubmodule { repo_id, url, path, + branch, + name, + force, + approved_sources, auth, } => { repo_commands::schedule_add_submodule( - executor, repos, msg_tx, repo_id, url, path, auth, + executor, + repos, + msg_tx, + repo_id, + repo_commands::AddSubmoduleRequest { + url, + path, + branch, + name, + force, + approved_sources, + auth, + }, ); } - Effect::UpdateSubmodules { repo_id, auth } => { - repo_commands::schedule_update_submodules(executor, repos, msg_tx, repo_id, auth); + Effect::UpdateSubmodules { + repo_id, + approved_sources, + auth, + } => { + repo_commands::schedule_update_submodules( + executor, + repos, + msg_tx, + repo_id, + approved_sources, + auth, + ); } Effect::RemoveSubmodule { repo_id, path } => { repo_commands::schedule_remove_submodule(executor, repos, msg_tx, repo_id, path); diff --git a/crates/gitcomet-state/src/store/effects/repo_commands.rs b/crates/gitcomet-state/src/store/effects/repo_commands.rs index 435e956d..9eda38e5 100644 --- a/crates/gitcomet-state/src/store/effects/repo_commands.rs +++ b/crates/gitcomet-state/src/store/effects/repo_commands.rs @@ -5,6 +5,7 @@ use gitcomet_core::auth::{ use gitcomet_core::error::{Error, ErrorKind}; use gitcomet_core::services::{ CommandOutput, ConflictSide, GitRepository, PullMode, RemoteUrlKind, ResetMode, + SubmoduleTrustTarget, }; use std::path::{Component, Path, PathBuf}; use std::sync::{Arc, mpsc}; @@ -78,6 +79,16 @@ fn run_with_git_auth( } } +pub(super) struct AddSubmoduleRequest { + pub(super) url: String, + pub(super) path: PathBuf, + pub(super) branch: Option, + pub(super) name: Option, + pub(super) force: bool, + pub(super) approved_sources: Vec, + pub(super) auth: Option, +} + pub(super) fn schedule_save_worktree_file( executor: &TaskExecutor, repos: &RepoMap, @@ -230,12 +241,22 @@ pub(super) fn schedule_add_submodule( repos: &RepoMap, msg_tx: mpsc::Sender, repo_id: RepoId, - url: String, - path: PathBuf, - auth: Option, + request: AddSubmoduleRequest, ) { + let AddSubmoduleRequest { + url, + path, + branch, + name, + force, + approved_sources, + auth, + } = request; let command_url = url.clone(); let command_path = path.clone(); + let command_branch = branch.clone(); + let command_name = name.clone(); + let command_sources = approved_sources.clone(); schedule_repo_command( executor, repos, @@ -244,8 +265,23 @@ pub(super) fn schedule_add_submodule( RepoCommandKind::AddSubmodule { url: command_url, path: command_path, + branch: command_branch, + name: command_name, + force, + approved_sources: command_sources, + }, + move |repo| { + run_with_git_auth(auth, || { + repo.add_submodule_with_output( + &url, + &path, + branch.as_deref(), + name.as_deref(), + force, + &approved_sources, + ) + }) }, - move |repo| run_with_git_auth(auth, || repo.add_submodule_with_output(&url, &path)), ); } @@ -254,18 +290,69 @@ pub(super) fn schedule_update_submodules( repos: &RepoMap, msg_tx: mpsc::Sender, repo_id: RepoId, + approved_sources: Vec, auth: Option, ) { + let command_sources = approved_sources.clone(); schedule_repo_command( executor, repos, msg_tx, repo_id, - RepoCommandKind::UpdateSubmodules, - move |repo| run_with_git_auth(auth, || repo.update_submodules_with_output()), + RepoCommandKind::UpdateSubmodules { + approved_sources: command_sources, + }, + move |repo| { + run_with_git_auth(auth, || { + repo.update_submodules_with_output(&approved_sources) + }) + }, ); } +pub(super) fn schedule_check_submodule_add_trust( + executor: &TaskExecutor, + repos: &RepoMap, + msg_tx: mpsc::Sender, + repo_id: RepoId, + url: String, + path: PathBuf, + branch: Option, + name: Option, + force: bool, +) { + spawn_with_repo(executor, repos, repo_id, msg_tx, move |repo, msg_tx| { + let result = repo.check_submodule_add_trust(&url, &path); + send_or_log( + &msg_tx, + Msg::Internal(crate::msg::InternalMsg::SubmoduleAddTrustChecked { + repo_id, + url, + path, + branch, + name, + force, + result, + }), + ); + }); +} + +pub(super) fn schedule_check_submodule_update_trust( + executor: &TaskExecutor, + repos: &RepoMap, + msg_tx: mpsc::Sender, + repo_id: RepoId, +) { + spawn_with_repo(executor, repos, repo_id, msg_tx, move |repo, msg_tx| { + let result = repo.check_submodule_update_trust(); + send_or_log( + &msg_tx, + Msg::Internal(crate::msg::InternalMsg::SubmoduleUpdateTrustChecked { repo_id, result }), + ); + }); +} + pub(super) fn schedule_remove_submodule( executor: &TaskExecutor, repos: &RepoMap, diff --git a/crates/gitcomet-state/src/store/reducer.rs b/crates/gitcomet-state/src/store/reducer.rs index f552ca51..4833b7f2 100644 --- a/crates/gitcomet-state/src/store/reducer.rs +++ b/crates/gitcomet-state/src/store/reducer.rs @@ -8,6 +8,7 @@ mod util; use crate::model::{ AppState, AuthPromptState, AuthRetryOperation, BannerErrorState, PendingCommitRetry, RepoId, + SubmoduleAddProgressState, SubmoduleTrustPromptOperation, SubmoduleTrustPromptState, }; use crate::msg::{ConflictRegionChoice, Effect, Msg, RepoCommandKind, RepoPath, RepoPathList}; use gitcomet_core::auth::StagedGitAuth; @@ -59,6 +60,20 @@ fn begin_commit_action(state: &mut AppState, repo_id: RepoId) { } } +fn start_submodule_add_progress( + state: &mut AppState, + repo_id: RepoId, + url: &str, + path: &std::path::Path, +) { + if let Some(repo_state) = state.repos.iter_mut().find(|r| r.id == repo_id) { + repo_state.submodule_add_in_flight = Some(SubmoduleAddProgressState { + url: url.to_string(), + path: path.to_path_buf(), + }); + } +} + pub(crate) fn msg_requires_available_git(msg: &Msg) -> bool { matches!( msg, @@ -337,8 +352,26 @@ fn retry_msg_for_repo_command(repo_id: RepoId, command: RepoCommandKind) -> Opti }, RepoCommandKind::RemoveWorktree { path } => Msg::RemoveWorktree { repo_id, path }, RepoCommandKind::ForceRemoveWorktree { path } => Msg::ForceRemoveWorktree { repo_id, path }, - RepoCommandKind::AddSubmodule { url, path } => Msg::AddSubmodule { repo_id, url, path }, - RepoCommandKind::UpdateSubmodules => Msg::UpdateSubmodules { repo_id }, + RepoCommandKind::AddSubmodule { + url, + path, + branch, + name, + force, + approved_sources, + } => Msg::AddSubmoduleTrusted { + repo_id, + url, + path, + branch, + name, + force, + approved_sources, + }, + RepoCommandKind::UpdateSubmodules { approved_sources } => Msg::UpdateSubmodulesTrusted { + repo_id, + approved_sources, + }, RepoCommandKind::RemoveSubmodule { path } => Msg::RemoveSubmodule { repo_id, path }, // Not replayable because command metadata does not retain original content. RepoCommandKind::SaveWorktreeFile { .. } @@ -761,13 +794,89 @@ pub(super) fn reduce( }; actions_emit_effects::force_remove_worktree(repo_id, normalized_path) } - Msg::AddSubmodule { repo_id, url, path } => { + Msg::AddSubmodule { + repo_id, + url, + path, + branch, + name, + force, + } => { + state.submodule_trust_prompt = None; + vec![Effect::CheckSubmoduleAddTrust { + repo_id, + url, + path, + branch, + name, + force, + }] + } + Msg::AddSubmoduleTrusted { + repo_id, + url, + path, + branch, + name, + force, + approved_sources, + } => { begin_local_action(state, repo_id); - actions_emit_effects::add_submodule(repo_id, url, path) + start_submodule_add_progress(state, repo_id, &url, &path); + actions_emit_effects::add_submodule( + repo_id, + url, + path, + branch, + name, + force, + approved_sources, + ) } Msg::UpdateSubmodules { repo_id } => { + state.submodule_trust_prompt = None; + vec![Effect::CheckSubmoduleUpdateTrust { repo_id }] + } + Msg::UpdateSubmodulesTrusted { + repo_id, + approved_sources, + } => { begin_local_action(state, repo_id); - actions_emit_effects::update_submodules(repo_id) + actions_emit_effects::update_submodules(repo_id, approved_sources) + } + Msg::ConfirmSubmoduleTrustPrompt => { + let Some(prompt) = state.submodule_trust_prompt.take() else { + return Vec::new(); + }; + match prompt.operation { + SubmoduleTrustPromptOperation::Add { + url, + path, + branch, + name, + force, + } => { + begin_local_action(state, prompt.repo_id); + start_submodule_add_progress(state, prompt.repo_id, &url, &path); + actions_emit_effects::add_submodule( + prompt.repo_id, + url, + path, + branch, + name, + force, + prompt.sources, + ) + } + SubmoduleTrustPromptOperation::Update => { + begin_local_action(state, prompt.repo_id); + actions_emit_effects::update_submodules(prompt.repo_id, prompt.sources) + } + } + } + Msg::CancelSubmoduleTrustPrompt => { + state.submodule_trust_prompt = None; + Vec::new() } Msg::RemoveSubmodule { repo_id, path } => { begin_local_action(state, repo_id); @@ -1116,6 +1225,73 @@ pub(super) fn reduce( Msg::Internal(crate::msg::InternalMsg::SubmodulesLoaded { repo_id, result }) => { effects::submodules_loaded(state, repo_id, result) } + Msg::Internal(crate::msg::InternalMsg::SubmoduleAddTrustChecked { + repo_id, + url, + path, + branch, + name, + force, + result, + }) => match result { + Ok(gitcomet_core::services::SubmoduleTrustDecision::Proceed) => { + begin_local_action(state, repo_id); + start_submodule_add_progress(state, repo_id, &url, &path); + actions_emit_effects::add_submodule( + repo_id, + url, + path, + branch, + name, + force, + Vec::new(), + ) + } + Ok(gitcomet_core::services::SubmoduleTrustDecision::Prompt { sources }) => { + state.submodule_trust_prompt = Some(SubmoduleTrustPromptState { + repo_id, + operation: SubmoduleTrustPromptOperation::Add { + url, + path, + branch, + name, + force, + }, + sources, + }); + Vec::new() + } + Err(error) => { + state.banner_error = Some(BannerErrorState { + repo_id: Some(repo_id), + message: util::format_failure_summary("Submodule trust check", &error), + }); + Vec::new() + } + }, + Msg::Internal(crate::msg::InternalMsg::SubmoduleUpdateTrustChecked { repo_id, result }) => { + match result { + Ok(gitcomet_core::services::SubmoduleTrustDecision::Proceed) => { + begin_local_action(state, repo_id); + actions_emit_effects::update_submodules(repo_id, Vec::new()) + } + Ok(gitcomet_core::services::SubmoduleTrustDecision::Prompt { sources }) => { + state.submodule_trust_prompt = Some(SubmoduleTrustPromptState { + repo_id, + operation: SubmoduleTrustPromptOperation::Update, + sources, + }); + Vec::new() + } + Err(error) => { + state.banner_error = Some(BannerErrorState { + repo_id: Some(repo_id), + message: util::format_failure_summary("Submodule trust check", &error), + }); + Vec::new() + } + } + } Msg::Internal(crate::msg::InternalMsg::CommitDetailsLoaded { repo_id, commit_id, diff --git a/crates/gitcomet-state/src/store/reducer/actions_emit_effects.rs b/crates/gitcomet-state/src/store/reducer/actions_emit_effects.rs index b62e0330..d8d4ae7c 100644 --- a/crates/gitcomet-state/src/store/reducer/actions_emit_effects.rs +++ b/crates/gitcomet-state/src/store/reducer/actions_emit_effects.rs @@ -117,18 +117,34 @@ pub(super) fn force_remove_worktree(repo_id: RepoId, path: PathBuf) -> Vec Vec { +pub(super) fn add_submodule( + repo_id: RepoId, + url: String, + path: PathBuf, + branch: Option, + name: Option, + force: bool, + approved_sources: Vec, +) -> Vec { vec![Effect::AddSubmodule { repo_id, url, path, + branch, + name, + force, + approved_sources, auth: None, }] } -pub(super) fn update_submodules(repo_id: RepoId) -> Vec { +pub(super) fn update_submodules( + repo_id: RepoId, + approved_sources: Vec, +) -> Vec { vec![Effect::UpdateSubmodules { repo_id, + approved_sources, auth: None, }] } @@ -623,7 +639,7 @@ fn tracks_local_actions_in_flight(command: &RepoCommandKind) -> bool { | RepoCommandKind::ExportPatch { .. } | RepoCommandKind::ApplyPatch { .. } | RepoCommandKind::AddSubmodule { .. } - | RepoCommandKind::UpdateSubmodules + | RepoCommandKind::UpdateSubmodules { .. } | RepoCommandKind::RemoveSubmodule { .. } | RepoCommandKind::StageHunk | RepoCommandKind::UnstageHunk @@ -646,7 +662,7 @@ pub(super) fn repo_command_finished( let refresh_submodules = matches!( &command, RepoCommandKind::AddSubmodule { .. } - | RepoCommandKind::UpdateSubmodules + | RepoCommandKind::UpdateSubmodules { .. } | RepoCommandKind::RemoveSubmodule { .. } ) && result.is_ok(); let command_succeeded = result.is_ok(); @@ -688,6 +704,10 @@ pub(super) fn repo_command_finished( _ => {} } + if matches!(&command, RepoCommandKind::AddSubmodule { .. }) { + repo_state.submodule_add_in_flight = None; + } + match result { Ok(output) => { repo_state.last_error = None; diff --git a/crates/gitcomet-state/src/store/reducer/util.rs b/crates/gitcomet-state/src/store/reducer/util.rs index e6b03010..a631c400 100644 --- a/crates/gitcomet-state/src/store/reducer/util.rs +++ b/crates/gitcomet-state/src/store/reducer/util.rs @@ -862,7 +862,7 @@ fn summarize_command( | RepoCommandKind::RemoveWorktree { .. } | RepoCommandKind::ForceRemoveWorktree { .. } => "Worktree", RepoCommandKind::AddSubmodule { .. } - | RepoCommandKind::UpdateSubmodules + | RepoCommandKind::UpdateSubmodules { .. } | RepoCommandKind::RemoveSubmodule { .. } => "Submodule", RepoCommandKind::StageHunk | RepoCommandKind::UnstageHunk => "Hunk", RepoCommandKind::ApplyWorktreePatch { reverse } => { @@ -1054,7 +1054,7 @@ fn summarize_command( RepoCommandKind::AddSubmodule { path, .. } => { format!("Submodule added → {}", path.display()) } - RepoCommandKind::UpdateSubmodules => "Submodules: Updated".to_string(), + RepoCommandKind::UpdateSubmodules { .. } => "Submodules: Updated".to_string(), RepoCommandKind::RemoveSubmodule { path } => { format!("Submodule removed → {}", path.display()) } diff --git a/crates/gitcomet-state/src/store/tests/actions_emit_effects.rs b/crates/gitcomet-state/src/store/tests/actions_emit_effects.rs index a3055600..5d362520 100644 --- a/crates/gitcomet-state/src/store/tests/actions_emit_effects.rs +++ b/crates/gitcomet-state/src/store/tests/actions_emit_effects.rs @@ -642,6 +642,10 @@ fn submodule_commands_reload_submodules_on_success() { }, )); state.repos[0].set_submodules(Loadable::Ready(Vec::new())); + state.repos[0].submodule_add_in_flight = Some(crate::model::SubmoduleAddProgressState { + url: "https://example.com/sub.git".to_string(), + path: PathBuf::from("submodule"), + }); let effects = reduce( &mut repos, @@ -652,12 +656,17 @@ fn submodule_commands_reload_submodules_on_success() { command: RepoCommandKind::AddSubmodule { url: "https://example.com/sub.git".to_string(), path: PathBuf::from("submodule"), + branch: None, + name: None, + force: false, + approved_sources: Vec::new(), }, result: Ok(CommandOutput::empty_success("git submodule add")), }), ); assert!(state.repos[0].submodules.is_loading()); + assert!(state.repos[0].submodule_add_in_flight.is_none()); assert!( effects .iter() @@ -671,7 +680,9 @@ fn submodule_commands_reload_submodules_on_success() { &mut state, Msg::Internal(crate::msg::InternalMsg::RepoCommandFinished { repo_id, - command: RepoCommandKind::UpdateSubmodules, + command: RepoCommandKind::UpdateSubmodules { + approved_sources: Vec::new(), + }, result: Ok(CommandOutput::empty_success( "git submodule update --init --recursive", )), @@ -2214,10 +2225,19 @@ fn repo_command_finished_error_summaries_cover_additional_labels() { RepoCommandKind::AddSubmodule { url: "https://example.com/sub.git".to_string(), path: PathBuf::from("mods/sub"), + branch: None, + name: None, + force: false, + approved_sources: Vec::new(), + }, + "Submodule", + ), + ( + RepoCommandKind::UpdateSubmodules { + approved_sources: Vec::new(), }, "Submodule", ), - (RepoCommandKind::UpdateSubmodules, "Submodule"), ( RepoCommandKind::RemoveSubmodule { path: PathBuf::from("mods/sub"), @@ -2327,10 +2347,19 @@ fn repo_command_finished_success_summaries_cover_additional_commands() { RepoCommandKind::AddSubmodule { url: "https://example.com/sub.git".to_string(), path: PathBuf::from("mods/sub"), + branch: None, + name: None, + force: false, + approved_sources: Vec::new(), }, "Submodule added → mods/sub", ), - (RepoCommandKind::UpdateSubmodules, "Submodules: Updated"), + ( + RepoCommandKind::UpdateSubmodules { + approved_sources: Vec::new(), + }, + "Submodules: Updated", + ), ( RepoCommandKind::RemoveSubmodule { path: PathBuf::from("mods/sub"), @@ -2481,15 +2510,22 @@ fn checkout_branch_and_submodule_messages_emit_effects() { repo_id: RepoId(1), url: "https://example.com/sub.git".to_string(), path: PathBuf::from("mods/sub"), + branch: Some("feature".to_string()), + name: None, + force: false, }, ); assert!(matches!( add_submodule.as_slice(), - [Effect::AddSubmodule { + [Effect::CheckSubmoduleAddTrust { repo_id: RepoId(1), + url, path, + branch, .. - }] if path == &PathBuf::from("mods/sub") + }] if url == "https://example.com/sub.git" + && path == &PathBuf::from("mods/sub") + && branch.as_deref() == Some("feature") )); let update_submodules = reduce( @@ -2500,10 +2536,7 @@ fn checkout_branch_and_submodule_messages_emit_effects() { ); assert!(matches!( update_submodules.as_slice(), - [Effect::UpdateSubmodules { - repo_id: RepoId(1), - .. - }] + [Effect::CheckSubmoduleUpdateTrust { repo_id: RepoId(1) }] )); let remove_submodule = reduce( @@ -2524,6 +2557,232 @@ fn checkout_branch_and_submodule_messages_emit_effects() { )); } +#[test] +fn local_submodule_add_trust_prompt_confirms_into_add_effect() { + let mut repos: HashMap> = HashMap::default(); + let id_alloc = AtomicU64::new(1); + let mut state = AppState::default(); + let repo_id = RepoId(1); + state.repos.push(RepoState::new_opening( + repo_id, + RepoSpec { + workdir: PathBuf::from("/tmp/repo"), + }, + )); + + let source = gitcomet_core::services::SubmoduleTrustTarget { + submodule_path: PathBuf::from("mods/sub"), + display_source: "../local-sub".to_string(), + local_source_path: PathBuf::from("/tmp/local-sub"), + }; + + let effects = reduce( + &mut repos, + &id_alloc, + &mut state, + Msg::Internal(crate::msg::InternalMsg::SubmoduleAddTrustChecked { + repo_id, + url: "../local-sub".to_string(), + path: PathBuf::from("mods/sub"), + branch: Some("feature".to_string()), + name: None, + force: false, + result: Ok(gitcomet_core::services::SubmoduleTrustDecision::Prompt { + sources: vec![source.clone()], + }), + }), + ); + assert!(effects.is_empty()); + assert_eq!( + state.submodule_trust_prompt, + Some(crate::model::SubmoduleTrustPromptState { + repo_id, + operation: crate::model::SubmoduleTrustPromptOperation::Add { + url: "../local-sub".to_string(), + path: PathBuf::from("mods/sub"), + branch: Some("feature".to_string()), + name: None, + force: false, + }, + sources: vec![source.clone()], + }) + ); + + let effects = reduce( + &mut repos, + &id_alloc, + &mut state, + Msg::ConfirmSubmoduleTrustPrompt, + ); + assert!(state.submodule_trust_prompt.is_none()); + assert!(matches!( + effects.as_slice(), + [Effect::AddSubmodule { + repo_id: RepoId(1), + url, + path, + branch, + approved_sources, + .. + }] if url == "../local-sub" + && path == &PathBuf::from("mods/sub") + && branch.as_deref() == Some("feature") + && approved_sources == &vec![source] + )); + assert_eq!( + state.repos[0].submodule_add_in_flight, + Some(crate::model::SubmoduleAddProgressState { + url: "../local-sub".to_string(), + path: PathBuf::from("mods/sub"), + }) + ); +} + +#[test] +fn submodule_add_progress_starts_when_trust_check_proceeds() { + let mut repos: HashMap> = HashMap::default(); + let id_alloc = AtomicU64::new(1); + let mut state = AppState::default(); + let repo_id = RepoId(1); + state.repos.push(RepoState::new_opening( + repo_id, + RepoSpec { + workdir: PathBuf::from("/tmp/repo"), + }, + )); + + let effects = reduce( + &mut repos, + &id_alloc, + &mut state, + Msg::Internal(crate::msg::InternalMsg::SubmoduleAddTrustChecked { + repo_id, + url: "https://example.com/sub.git".to_string(), + path: PathBuf::from("mods/sub"), + branch: None, + name: Some("deps/sub".to_string()), + force: true, + result: Ok(gitcomet_core::services::SubmoduleTrustDecision::Proceed), + }), + ); + + assert!(matches!( + effects.as_slice(), + [Effect::AddSubmodule { + repo_id: RepoId(1), + url, + path, + name, + force, + .. + }] if url == "https://example.com/sub.git" + && path == &PathBuf::from("mods/sub") + && name.as_deref() == Some("deps/sub") + && *force + )); + assert_eq!( + state.repos[0].submodule_add_in_flight, + Some(crate::model::SubmoduleAddProgressState { + url: "https://example.com/sub.git".to_string(), + path: PathBuf::from("mods/sub"), + }) + ); +} + +#[test] +fn submodule_add_progress_clears_on_failed_add_command() { + let mut repos: HashMap> = HashMap::default(); + let id_alloc = AtomicU64::new(1); + let mut state = AppState::default(); + let repo_id = RepoId(1); + state.repos.push(RepoState::new_opening( + repo_id, + RepoSpec { + workdir: PathBuf::from("/tmp/repo"), + }, + )); + state.repos[0].submodule_add_in_flight = Some(crate::model::SubmoduleAddProgressState { + url: "https://example.com/sub.git".to_string(), + path: PathBuf::from("mods/sub"), + }); + + let effects = reduce( + &mut repos, + &id_alloc, + &mut state, + Msg::Internal(crate::msg::InternalMsg::RepoCommandFinished { + repo_id, + command: RepoCommandKind::AddSubmodule { + url: "https://example.com/sub.git".to_string(), + path: PathBuf::from("mods/sub"), + branch: None, + name: None, + force: false, + approved_sources: Vec::new(), + }, + result: Err(gitcomet_core::error::Error::new( + gitcomet_core::error::ErrorKind::Backend("submodule add failed".to_string()), + )), + }), + ); + + assert!(state.repos[0].submodule_add_in_flight.is_none()); + assert!( + !effects.iter().any( + |effect| matches!(effect, Effect::LoadSubmodules { repo_id: id } if *id == repo_id) + ) + ); +} + +#[test] +fn local_submodule_update_trust_prompt_cancels_cleanly() { + let mut repos: HashMap> = HashMap::default(); + let id_alloc = AtomicU64::new(1); + let mut state = AppState::default(); + let repo_id = RepoId(1); + state.repos.push(RepoState::new_opening( + repo_id, + RepoSpec { + workdir: PathBuf::from("/tmp/repo"), + }, + )); + + let source = gitcomet_core::services::SubmoduleTrustTarget { + submodule_path: PathBuf::from("mods/sub"), + display_source: "../local-sub".to_string(), + local_source_path: PathBuf::from("/tmp/local-sub"), + }; + + reduce( + &mut repos, + &id_alloc, + &mut state, + Msg::Internal(crate::msg::InternalMsg::SubmoduleUpdateTrustChecked { + repo_id, + result: Ok(gitcomet_core::services::SubmoduleTrustDecision::Prompt { + sources: vec![source], + }), + }), + ); + assert!(matches!( + state.submodule_trust_prompt, + Some(crate::model::SubmoduleTrustPromptState { + repo_id: RepoId(1), + operation: crate::model::SubmoduleTrustPromptOperation::Update, + .. + }) + )); + + let effects = reduce( + &mut repos, + &id_alloc, + &mut state, + Msg::CancelSubmoduleTrustPrompt, + ); + assert!(effects.is_empty()); + assert!(state.submodule_trust_prompt.is_none()); +} + #[test] fn pull_branch_and_push_variants_mark_in_flight_when_repo_is_opened() { let mut repos: HashMap> = HashMap::default(); diff --git a/crates/gitcomet-state/src/store/tests/effects.rs b/crates/gitcomet-state/src/store/tests/effects.rs index 832dced8..5121ffc6 100644 --- a/crates/gitcomet-state/src/store/tests/effects.rs +++ b/crates/gitcomet-state/src/store/tests/effects.rs @@ -3895,12 +3895,17 @@ fn schedule_effect_dispatches_many_variants_with_repo_present() { repo_id, url: "https://example.com/repo.git".to_string(), path: PathBuf::from("sub"), + branch: None, + name: None, + force: false, + approved_sources: Vec::new(), auth: None, }, 1, ), ( Effect::UpdateSubmodules { + approved_sources: Vec::new(), repo_id, auth: None, }, diff --git a/crates/gitcomet-ui-gpui/src/view/clone_progress.rs b/crates/gitcomet-ui-gpui/src/view/clone_progress.rs index a55cd34c..0351f3a3 100644 --- a/crates/gitcomet-ui-gpui/src/view/clone_progress.rs +++ b/crates/gitcomet-ui-gpui/src/view/clone_progress.rs @@ -3,6 +3,10 @@ use crate::theme::AppTheme; use gitcomet_state::model::{CloneOpState, CloneOpStatus, CloneProgressStage}; use std::path::Path; +pub(crate) fn clone_progress_loading_color(theme: AppTheme) -> gpui::Rgba { + with_alpha(theme.colors.text, if theme.is_dark { 0.42 } else { 0.34 }) +} + pub(crate) fn clone_progress_title(op: &CloneOpState) -> &'static str { match op.status { CloneOpStatus::Cancelling => "Aborting clone…", @@ -26,9 +30,7 @@ pub(crate) fn clone_progress_color(theme: AppTheme, op: &CloneOpState) -> gpui:: with_alpha(theme.colors.text, if theme.is_dark { 0.60 } else { 0.48 }) } _ => match op.progress.stage { - CloneProgressStage::Loading => { - with_alpha(theme.colors.text, if theme.is_dark { 0.42 } else { 0.34 }) - } + CloneProgressStage::Loading => clone_progress_loading_color(theme), CloneProgressStage::RemoteObjects => { with_alpha(theme.colors.text, if theme.is_dark { 0.78 } else { 0.62 }) } @@ -120,6 +122,10 @@ mod tests { clone_progress_color(theme, &cancelling), with_alpha(theme.colors.text, 0.48) ); + assert_eq!( + clone_progress_loading_color(theme), + with_alpha(theme.colors.text, 0.34) + ); } #[test] @@ -145,6 +151,10 @@ mod tests { clone_progress_color(theme, &cancelling), with_alpha(theme.colors.text, 0.60) ); + assert_eq!( + clone_progress_loading_color(theme), + with_alpha(theme.colors.text, 0.42) + ); } #[test] diff --git a/crates/gitcomet-ui-gpui/src/view/mod.rs b/crates/gitcomet-ui-gpui/src/view/mod.rs index 9f9d06d3..f6eb26c9 100644 --- a/crates/gitcomet-ui-gpui/src/view/mod.rs +++ b/crates/gitcomet-ui-gpui/src/view/mod.rs @@ -13,7 +13,7 @@ use gitcomet_core::process::refresh_git_runtime; use gitcomet_core::services::{PullMode, RemoteUrlKind, ResetMode}; use gitcomet_state::model::{ AppNotificationKind, AppState, AuthPromptKind, CloneOpState, CloneOpStatus, DiagnosticKind, - Loadable, RepoId, RepoState, + Loadable, RepoId, RepoState, SubmoduleTrustPromptOperation, }; use gitcomet_state::msg::{Msg, RepoExternalChange, StoreEvent}; use gitcomet_state::session; @@ -145,6 +145,8 @@ const HISTORY_COL_DATE_MAX_PX: f32 = 240.0; const HISTORY_COL_SHA_MIN_PX: f32 = 60.0; const HISTORY_COL_SHA_MAX_PX: f32 = 160.0; const HISTORY_COL_MESSAGE_MIN_PX: f32 = 220.0; +const ERROR_BANNER_OVERFLOW_HINT_MIN_LINES: usize = 8; +const ERROR_BANNER_OVERFLOW_HINT_MIN_CHARS: usize = 240; const HISTORY_GRAPH_COL_GAP_PX: f32 = 16.0; const HISTORY_GRAPH_MARGIN_X_PX: f32 = 10.0; @@ -829,6 +831,7 @@ impl GitCometView { pending_pull_reconcile_prompt: None, pending_force_delete_branch_prompt: None, pending_force_remove_worktree_prompt: None, + pending_submodule_trust_prompt: None, pending_worktree_branch_removals: HashMap::default(), startup_crash_report, #[cfg(target_os = "macos")] @@ -1479,6 +1482,11 @@ impl GitCometView { (Some(command.into()), collapsed.join("\n").into()) } + fn should_show_error_banner_overflow_hint(err_text: &str) -> bool { + err_text.lines().count() > ERROR_BANNER_OVERFLOW_HINT_MIN_LINES + || err_text.len() > ERROR_BANNER_OVERFLOW_HINT_MIN_CHARS + } + fn should_render_generic_error_banner(auth_prompt_active: bool) -> bool { !auth_prompt_active } @@ -1638,6 +1646,17 @@ impl Render for GitCometView { ); } + if let Some(prompt) = self.pending_submodule_trust_prompt.take() + && self.active_repo_id() == Some(prompt.repo_id) + { + self.open_popover_at( + PopoverKind::submodule(prompt.repo_id, SubmodulePopoverKind::TrustConfirm), + self.last_mouse_pos, + window, + cx, + ); + } + let decorations = window.window_decorations(); let (tiling, client_inset) = match decorations { Decorations::Client { tiling } => (Some(tiling), CLIENT_SIDE_DECORATION_INSET), @@ -1924,6 +1943,8 @@ impl Render for GitCometView { if let Some(err_text) = banner_error { let (error_command, display_error) = Self::split_error_banner_message(err_text.as_ref()); + let show_overflow_hint = + Self::should_show_error_banner_overflow_hint(err_text.as_ref()); self.error_banner_input.update(cx, |input, cx| { input.set_theme(theme, cx); input.set_text(display_error.clone(), cx); @@ -1981,6 +2002,15 @@ impl Render for GitCometView { .child(self.error_banner_input.clone()), ), ) + .when(show_overflow_hint, |this| { + this.child( + div() + .mt_1() + .text_xs() + .text_color(theme.colors.text_muted) + .child("Scroll for full output"), + ) + }) .child(div().absolute().top(px(6.0)).right(px(6.0)).child(dismiss)), ); } diff --git a/crates/gitcomet-ui-gpui/src/view/mod_helpers.rs b/crates/gitcomet-ui-gpui/src/view/mod_helpers.rs index 789c6dc6..3be3762d 100644 --- a/crates/gitcomet-ui-gpui/src/view/mod_helpers.rs +++ b/crates/gitcomet-ui-gpui/src/view/mod_helpers.rs @@ -2366,6 +2366,7 @@ pub(super) enum SubmodulePopoverKind { SectionMenu, Menu { path: std::path::PathBuf }, AddPrompt, + TrustConfirm, OpenPicker, RemovePicker, RemoveConfirm { path: std::path::PathBuf }, @@ -2863,6 +2864,8 @@ pub struct GitCometView { pub(super) pending_force_delete_branch_prompt: Option<(RepoId, String)>, pub(super) pending_force_remove_worktree_prompt: Option<(RepoId, std::path::PathBuf, Option)>, + pub(super) pending_submodule_trust_prompt: + Option, pub(super) pending_worktree_branch_removals: HashMap<(RepoId, std::path::PathBuf), String>, pub(super) startup_crash_report: Option, #[cfg(target_os = "macos")] diff --git a/crates/gitcomet-ui-gpui/src/view/panels/popover.rs b/crates/gitcomet-ui-gpui/src/view/panels/popover.rs index 69742758..0c47b966 100644 --- a/crates/gitcomet-ui-gpui/src/view/panels/popover.rs +++ b/crates/gitcomet-ui-gpui/src/view/panels/popover.rs @@ -33,6 +33,7 @@ mod submodule_add_prompt; mod submodule_open_picker; mod submodule_remove_confirm; mod submodule_remove_picker; +mod submodule_trust_confirm; mod worktree_add_prompt; mod worktree_open_picker; mod worktree_remove_confirm; @@ -96,6 +97,10 @@ pub(in super::super) struct PopoverHost { worktree_ref_input: Entity, submodule_url_input: Entity, submodule_path_input: Entity, + submodule_branch_input: Entity, + submodule_name_input: Entity, + submodule_add_advanced_expanded: bool, + submodule_force_enabled: bool, } impl PopoverHost { @@ -461,6 +466,34 @@ impl PopoverHost { ) }); + let submodule_name_input = cx.new(|cx| { + components::TextInput::new( + components::TextInputOptions { + placeholder: "submodule-logical-name".into(), + multiline: false, + read_only: false, + chromeless: false, + soft_wrap: false, + }, + window, + cx, + ) + }); + + let submodule_branch_input = cx.new(|cx| { + components::TextInput::new( + components::TextInputOptions { + placeholder: "feature".into(), + multiline: false, + read_only: false, + chromeless: false, + soft_wrap: false, + }, + window, + cx, + ) + }); + let context_menu_focus_handle = cx.focus_handle().tab_index(0).tab_stop(false); Self { @@ -512,6 +545,10 @@ impl PopoverHost { worktree_ref_input, submodule_url_input, submodule_path_input, + submodule_branch_input, + submodule_name_input, + submodule_add_advanced_expanded: false, + submodule_force_enabled: false, } } @@ -546,6 +583,10 @@ impl PopoverHost { .update(cx, |input, cx| input.set_theme(theme, cx)); self.submodule_path_input .update(cx, |input, cx| input.set_theme(theme, cx)); + self.submodule_branch_input + .update(cx, |input, cx| input.set_theme(theme, cx)); + self.submodule_name_input + .update(cx, |input, cx| input.set_theme(theme, cx)); if let Some(input) = &self.repo_picker_search_input { input.update(cx, |input, cx| input.set_theme(theme, cx)); @@ -1045,6 +1086,8 @@ impl PopoverHost { .. } => { let theme = self.theme; + self.submodule_add_advanced_expanded = false; + self.submodule_force_enabled = false; self.submodule_url_input.update(cx, |input, cx| { input.set_theme(theme, cx); input.set_text("", cx); @@ -1055,11 +1098,25 @@ impl PopoverHost { input.set_text("", cx); cx.notify(); }); + self.submodule_branch_input.update(cx, |input, cx| { + input.set_theme(theme, cx); + input.set_text("", cx); + cx.notify(); + }); + self.submodule_name_input.update(cx, |input, cx| { + input.set_theme(theme, cx); + input.set_text("", cx); + cx.notify(); + }); let focus = self .submodule_url_input .read_with(cx, |i, _| i.focus_handle()); window.focus(&focus, cx); } + PopoverKind::Repo { + kind: RepoPopoverKind::Submodule(SubmodulePopoverKind::TrustConfirm), + .. + } => {} PopoverKind::Repo { repo_id, kind: @@ -1367,6 +1424,7 @@ impl PopoverHost { kind: RepoPopoverKind::Submodule( SubmodulePopoverKind::AddPrompt + | SubmodulePopoverKind::TrustConfirm | SubmodulePopoverKind::OpenPicker | SubmodulePopoverKind::RemovePicker | SubmodulePopoverKind::RemoveConfirm { .. }, @@ -1514,6 +1572,9 @@ impl PopoverHost { SubmodulePopoverKind::AddPrompt => { submodule_add_prompt::panel(self, repo_id, cx) } + SubmodulePopoverKind::TrustConfirm => { + submodule_trust_confirm::panel(self, repo_id, cx) + } SubmodulePopoverKind::OpenPicker => { submodule_open_picker::panel(self, repo_id, cx) } diff --git a/crates/gitcomet-ui-gpui/src/view/panels/popover/fingerprint.rs b/crates/gitcomet-ui-gpui/src/view/panels/popover/fingerprint.rs index 89c026f5..05c9af66 100644 --- a/crates/gitcomet-ui-gpui/src/view/panels/popover/fingerprint.rs +++ b/crates/gitcomet-ui-gpui/src/view/panels/popover/fingerprint.rs @@ -56,6 +56,43 @@ pub(super) fn notify_fingerprint(state: &AppState, popover: &PopoverKind) -> u64 view_fingerprint::hash_loadable_kind(&repo.open, &mut hasher); } } + PopoverKind::Repo { + kind: RepoPopoverKind::Submodule(SubmodulePopoverKind::TrustConfirm), + .. + } => { + if let Some(repo) = repo_for_popover(state, popover) { + hash_repo_for_popover(repo, popover, &mut hasher); + } else { + state.active_repo.hash(&mut hasher); + } + if let Some(prompt) = state.submodule_trust_prompt.as_ref() { + prompt.repo_id.hash(&mut hasher); + match &prompt.operation { + SubmoduleTrustPromptOperation::Add { + url, + path, + branch, + name, + force, + } => { + 0u8.hash(&mut hasher); + url.hash(&mut hasher); + path.hash(&mut hasher); + branch.hash(&mut hasher); + name.hash(&mut hasher); + force.hash(&mut hasher); + } + SubmoduleTrustPromptOperation::Update => { + 1u8.hash(&mut hasher); + } + } + for source in &prompt.sources { + source.submodule_path.hash(&mut hasher); + source.display_source.hash(&mut hasher); + source.local_source_path.hash(&mut hasher); + } + } + } _ => { if let Some(repo) = repo_for_popover(state, popover) { hash_repo_for_popover(repo, popover, &mut hasher); @@ -551,6 +588,10 @@ fn hash_repo_popover_kind(repo_id: RepoId, kind: &RepoPopoverKind, ha 24u8.hash(hasher); repo_id.hash(hasher); } + SubmodulePopoverKind::TrustConfirm => { + 28u8.hash(hasher); + repo_id.hash(hasher); + } SubmodulePopoverKind::OpenPicker => { 25u8.hash(hasher); repo_id.hash(hasher); diff --git a/crates/gitcomet-ui-gpui/src/view/panels/popover/submodule_add_prompt.rs b/crates/gitcomet-ui-gpui/src/view/panels/popover/submodule_add_prompt.rs index 9d33ba77..14dd54e4 100644 --- a/crates/gitcomet-ui-gpui/src/view/panels/popover/submodule_add_prompt.rs +++ b/crates/gitcomet-ui-gpui/src/view/panels/popover/submodule_add_prompt.rs @@ -1,11 +1,68 @@ use super::*; +fn advanced_toggle(theme: AppTheme, expanded: bool) -> gpui::Stateful { + div() + .id("submodule_add_advanced_toggle") + .debug_selector(|| "submodule_add_advanced_toggle".to_string()) + .w_full() + .px_2() + .py_1() + .flex() + .items_center() + .justify_between() + .rounded(px(theme.radii.row)) + .cursor(CursorStyle::PointingHand) + .hover(move |s| s.bg(theme.colors.hover)) + .active(move |s| s.bg(theme.colors.active)) + .child(div().text_sm().child("Advanced")) + .child( + div() + .text_sm() + .font_family(UI_MONOSPACE_FONT_FAMILY) + .text_color(theme.colors.text_muted) + .child(if expanded { "^" } else { "v" }), + ) +} + +fn force_toggle(theme: AppTheme, enabled: bool) -> gpui::Stateful { + div() + .id("submodule_add_force_toggle") + .debug_selector(|| "submodule_add_force_toggle".to_string()) + .w_full() + .px_2() + .py_1() + .flex() + .items_center() + .justify_between() + .rounded(px(theme.radii.row)) + .cursor(CursorStyle::PointingHand) + .hover(move |s| s.bg(theme.colors.hover)) + .active(move |s| s.bg(theme.colors.active)) + .child( + div() + .text_sm() + .child("Force reuse / bypass collision checks"), + ) + .child( + div() + .text_sm() + .text_color(if enabled { + theme.colors.success + } else { + theme.colors.text_muted + }) + .child(if enabled { "On" } else { "Off" }), + ) +} + pub(super) fn panel( this: &mut PopoverHost, repo_id: RepoId, cx: &mut gpui::Context, ) -> gpui::Div { let theme = this.theme; + let advanced_expanded = this.submodule_add_advanced_expanded; + let force_enabled = this.submodule_force_enabled; div() .flex() @@ -52,6 +109,65 @@ pub(super) fn panel( .min_w(px(0.0)) .child(this.submodule_path_input.clone()), ) + .child( + div() + .px_2() + .py_1() + .text_xs() + .text_color(theme.colors.text_muted) + .child("Branch (optional)"), + ) + .child( + div() + .px_2() + .pb_1() + .w_full() + .min_w(px(0.0)) + .child(this.submodule_branch_input.clone()), + ) + .child( + advanced_toggle(theme, advanced_expanded).on_click(cx.listener(|this, _e: &ClickEvent, _w, cx| { + this.submodule_add_advanced_expanded = !this.submodule_add_advanced_expanded; + cx.notify(); + })), + ) + .when(advanced_expanded, |this_panel| { + this_panel + .child( + div() + .px_2() + .py_1() + .text_xs() + .text_color(theme.colors.text_muted) + .child("Logical name (optional)"), + ) + .child( + div() + .px_2() + .pb_1() + .w_full() + .min_w(px(0.0)) + .child(this.submodule_name_input.clone()), + ) + .child( + force_toggle(theme, force_enabled).on_click(cx.listener( + |this, _e: &ClickEvent, _w, cx| { + this.submodule_force_enabled = !this.submodule_force_enabled; + cx.notify(); + }, + )), + ) + .child( + div() + .px_2() + .pb_1() + .text_xs() + .text_color(theme.colors.text_muted) + .child( + "Force reuses an existing local submodule git dir or bypasses Git's normal collision refusal.", + ), + ) + }) .child(div().border_t_1().border_color(theme.colors.border)) .child( div() @@ -79,6 +195,15 @@ pub(super) fn panel( let path_text = this .submodule_path_input .read_with(cx, |i, _| i.text().trim().to_string()); + let branch = this.submodule_branch_input.read_with(cx, |i, _| { + let text = i.text().trim().to_string(); + if text.is_empty() { None } else { Some(text) } + }); + let name = this.submodule_name_input.read_with(cx, |i, _| { + let text = i.text().trim().to_string(); + if text.is_empty() { None } else { Some(text) } + }); + let force = this.submodule_force_enabled; if url.is_empty() || path_text.is_empty() { this.push_toast( components::ToastKind::Error, @@ -91,6 +216,9 @@ pub(super) fn panel( repo_id, url, path: std::path::PathBuf::from(path_text), + branch, + name, + force, }); this.popover = None; this.popover_anchor = None; diff --git a/crates/gitcomet-ui-gpui/src/view/panels/popover/submodule_trust_confirm.rs b/crates/gitcomet-ui-gpui/src/view/panels/popover/submodule_trust_confirm.rs new file mode 100644 index 00000000..39e5d6e4 --- /dev/null +++ b/crates/gitcomet-ui-gpui/src/view/panels/popover/submodule_trust_confirm.rs @@ -0,0 +1,229 @@ +use super::*; + +const SUBMODULE_TRUST_CVE_URL: &str = + "https://github.blog/open-source/git/git-security-vulnerabilities-announced/#cve-2022-39253"; + +pub(super) fn panel( + this: &mut PopoverHost, + repo_id: RepoId, + cx: &mut gpui::Context, +) -> gpui::Div { + let theme = this.theme; + let Some(prompt) = this + .state + .submodule_trust_prompt + .as_ref() + .filter(|prompt| prompt.repo_id == repo_id) + .cloned() + else { + return div(); + }; + + let (title, confirm_label, cancel_label) = match &prompt.operation { + SubmoduleTrustPromptOperation::Add { .. } => { + ("Trust local submodule?", "Trust and add", "Back") + } + SubmoduleTrustPromptOperation::Update => ( + "Trust local submodule sources?", + "Trust and update", + "Cancel", + ), + }; + let (add_branch, add_name, add_force) = match &prompt.operation { + SubmoduleTrustPromptOperation::Add { + branch, + name, + force, + .. + } => (branch.clone(), name.clone(), *force), + SubmoduleTrustPromptOperation::Update => (None, None, false), + }; + let sources = prompt.sources.clone(); + let operation = prompt.operation.clone(); + + div() + .flex() + .flex_col() + .w(px(640.0)) + .child( + div() + .px_2() + .py_1() + .text_sm() + .font_weight(FontWeight::BOLD) + .child(title), + ) + .child(div().border_t_1().border_color(theme.colors.border)) + .child( + div() + .px_2() + .pt_1() + .text_sm() + .text_color(theme.colors.text_muted) + .child("Git blocks local file transport for submodules by default. Trusting these sources will allow GitComet to enable file transport only for this repo/source pair."), + ) + .child( + div() + .px_2() + .pb_1() + .child( + components::Button::new( + "submodule_trust_cve_link", + "Read about CVE-2022-39253", + ) + .style(components::ButtonStyle::Filled) + .borderless() + .no_hover_border() + .end_slot(div().font_family(UI_MONOSPACE_FONT_FAMILY).child("->")) + .on_click(theme, cx, |_this, _e, _window, cx| { + cx.open_url(SUBMODULE_TRUST_CVE_URL); + }), + ), + ) + .children(sources.into_iter().map(|source| { + div() + .px_2() + .pb_1() + .flex() + .flex_col() + .gap_0p5() + .child( + div() + .text_xs() + .text_color(theme.colors.text_muted) + .child(format!( + "Submodule: {}", + source.submodule_path.display() + )), + ) + .child( + div() + .text_sm() + .font_family(crate::font_preferences::EDITOR_MONOSPACE_FONT_FAMILY) + .child(source.display_source), + ) + .child( + div() + .text_xs() + .font_family(crate::font_preferences::EDITOR_MONOSPACE_FONT_FAMILY) + .text_color(theme.colors.text_muted) + .child(format!( + "Local path: {}", + source.local_source_path.display() + )), + ) + })) + .when(add_branch.is_some() || add_name.is_some() || add_force, |panel| { + panel.child( + div() + .px_2() + .pb_1() + .flex() + .flex_col() + .gap_0p5() + .when_some(add_branch.clone(), |details, branch| { + details.child( + div() + .text_xs() + .font_family(crate::font_preferences::EDITOR_MONOSPACE_FONT_FAMILY) + .text_color(theme.colors.text_muted) + .child(format!("Branch: {branch}")), + ) + }) + .when_some(add_name.clone(), |details, name| { + details.child( + div() + .text_xs() + .font_family(crate::font_preferences::EDITOR_MONOSPACE_FONT_FAMILY) + .text_color(theme.colors.text_muted) + .child(format!("Logical name: {name}")), + ) + }) + .when(add_force, |details| { + details.child( + div() + .text_xs() + .text_color(theme.colors.text_muted) + .child("Force: enabled"), + ) + }), + ) + }) + .child(div().border_t_1().border_color(theme.colors.border)) + .child( + div() + .px_2() + .py_1() + .flex() + .items_center() + .justify_between() + .child( + components::Button::new("submodule_trust_cancel", cancel_label) + .style(components::ButtonStyle::Outlined) + .on_click(theme, cx, move |this, _e, window, cx| { + this.store.dispatch(Msg::CancelSubmoduleTrustPrompt); + match operation.clone() { + SubmoduleTrustPromptOperation::Add { + url, + path, + branch, + name, + force, + } => { + let theme = this.theme; + let restored_branch = branch.unwrap_or_default(); + let restored_branch_for_input = restored_branch.clone(); + let restored_name = name.unwrap_or_default(); + let restored_name_for_input = restored_name.clone(); + this.submodule_url_input.update(cx, |input, cx| { + input.set_theme(theme, cx); + input.set_text(&url, cx); + cx.notify(); + }); + this.submodule_path_input.update(cx, |input, cx| { + input.set_theme(theme, cx); + input.set_text(path.display().to_string(), cx); + cx.notify(); + }); + this.submodule_branch_input.update(cx, move |input, cx| { + input.set_theme(theme, cx); + input.set_text(&restored_branch_for_input, cx); + cx.notify(); + }); + this.submodule_name_input.update(cx, move |input, cx| { + input.set_theme(theme, cx); + input.set_text(&restored_name_for_input, cx); + cx.notify(); + }); + this.submodule_add_advanced_expanded = + !restored_name.is_empty() || force; + this.submodule_force_enabled = force; + this.popover = Some(PopoverKind::submodule( + repo_id, + SubmodulePopoverKind::AddPrompt, + )); + let focus = this + .submodule_url_input + .read_with(cx, |input, _| input.focus_handle()); + window.focus(&focus, cx); + } + SubmoduleTrustPromptOperation::Update => { + this.popover = None; + this.popover_anchor = None; + } + } + cx.notify(); + }), + ) + .child( + components::Button::new("submodule_trust_confirm", confirm_label) + .style(components::ButtonStyle::Filled) + .on_click(theme, cx, |this, _e, _window, cx| { + this.store.dispatch(Msg::ConfirmSubmoduleTrustPrompt); + this.popover = None; + this.popover_anchor = None; + cx.notify(); + }), + ), + ) +} diff --git a/crates/gitcomet-ui-gpui/src/view/panes/sidebar.rs b/crates/gitcomet-ui-gpui/src/view/panes/sidebar.rs index 17822283..1bcd6778 100644 --- a/crates/gitcomet-ui-gpui/src/view/panes/sidebar.rs +++ b/crates/gitcomet-ui-gpui/src/view/panes/sidebar.rs @@ -443,10 +443,10 @@ mod tests { #[test] fn sidebar_notify_fingerprint_tracks_open_repo_workdirs() { let mut state = AppState { + repos: vec![repo_state(RepoId(1), "/tmp/repo")], active_repo: Some(RepoId(1)), ..AppState::default() }; - state.repos.push(repo_state(RepoId(1), "/tmp/repo")); let initial = SidebarNotifyFingerprint::from_state(&state); @@ -535,19 +535,23 @@ mod tests { #[test] fn sidebar_notify_fingerprint_ignores_repo_tab_order() { - let mut state_a = AppState { + let state_a = AppState { + repos: vec![ + repo_state(RepoId(1), "/tmp/repo"), + repo_state(RepoId(2), "/tmp/repo-wt"), + ], active_repo: Some(RepoId(1)), ..AppState::default() }; - state_a.repos.push(repo_state(RepoId(1), "/tmp/repo")); - state_a.repos.push(repo_state(RepoId(2), "/tmp/repo-wt")); - let mut state_b = AppState { + let state_b = AppState { + repos: vec![ + repo_state(RepoId(2), "/tmp/repo-wt"), + repo_state(RepoId(1), "/tmp/repo"), + ], active_repo: Some(RepoId(1)), ..AppState::default() }; - state_b.repos.push(repo_state(RepoId(2), "/tmp/repo-wt")); - state_b.repos.push(repo_state(RepoId(1), "/tmp/repo")); assert_eq!( SidebarNotifyFingerprint::from_state(&state_a), diff --git a/crates/gitcomet-ui-gpui/src/view/rows/benchmarks/diff_fixtures.rs b/crates/gitcomet-ui-gpui/src/view/rows/benchmarks/diff_fixtures.rs index 0d4253d9..b15ab493 100644 --- a/crates/gitcomet-ui-gpui/src/view/rows/benchmarks/diff_fixtures.rs +++ b/crates/gitcomet-ui-gpui/src/view/rows/benchmarks/diff_fixtures.rs @@ -1757,6 +1757,8 @@ pub struct RepoTabDragFixture { tab_count: usize, tab_width_px: f32, baseline: AppState, + #[cfg(test)] + pub hit_test_steps: Vec, } /// Sidecar metrics for repo tab drag benchmarks. @@ -1768,6 +1770,17 @@ pub struct RepoTabDragMetrics { pub noop_reorders: u64, } +#[cfg(test)] +pub struct RepoTabDragHitTestTarget { + pub repo_id: RepoId, +} + +#[cfg(test)] +pub struct RepoTabDragHitTestStep { + pub target: RepoTabDragHitTestTarget, + pub insert_before: Option, +} + impl RepoTabDragFixture { pub fn new(tab_count: usize) -> Self { let commits = build_synthetic_commits(10); @@ -1790,10 +1803,14 @@ impl RepoTabDragFixture { .collect(); let active = repos.first().map(|r| r.id); + #[cfg(test)] + let hit_test_steps = build_repo_tab_drag_hit_test_steps(&repos, tab_count, 120.0); Self { tab_count, tab_width_px: 120.0, baseline: bench_app_state(repos, active), + #[cfg(test)] + hit_test_steps, } } @@ -1926,6 +1943,41 @@ impl RepoTabDragFixture { } } +#[cfg(test)] +fn build_repo_tab_drag_hit_test_steps( + repos: &[RepoState], + tab_count: usize, + tab_width_px: f32, +) -> Vec { + let steps = tab_count * 3; + let total_bar_width = tab_count as f32 * tab_width_px; + let mut out = Vec::with_capacity(steps); + + for step in 0..steps { + let frac = (step as f32) / (steps.max(1) as f32); + let cursor_x = frac * total_bar_width; + let tab_ix = (cursor_x / tab_width_px) as usize; + let tab_ix = tab_ix.min(tab_count.saturating_sub(1)); + let target_repo_id = repos[tab_ix].id; + let tab_left = tab_ix as f32 * tab_width_px; + let tab_center = tab_left + tab_width_px / 2.0; + let insert_before = if cursor_x <= tab_center { + Some(target_repo_id) + } else { + repos.get(tab_ix + 1).map(|repo| repo.id) + }; + + out.push(RepoTabDragHitTestStep { + target: RepoTabDragHitTestTarget { + repo_id: target_repo_id, + }, + insert_before, + }); + } + + out +} + pub struct PatchDiffSearchQueryUpdateFixture { diff_rows: Vec, click_kinds: Vec, diff --git a/crates/gitcomet-ui-gpui/src/view/rows/benchmarks/runtime_fixtures.rs b/crates/gitcomet-ui-gpui/src/view/rows/benchmarks/runtime_fixtures.rs index 783f2ad0..48f40d32 100644 --- a/crates/gitcomet-ui-gpui/src/view/rows/benchmarks/runtime_fixtures.rs +++ b/crates/gitcomet-ui-gpui/src/view/rows/benchmarks/runtime_fixtures.rs @@ -714,26 +714,6 @@ fn idle_sample_steps(window: Duration, interval: Duration) -> usize { usize::try_from(steps.max(1)).unwrap_or(usize::MAX) } -#[cfg(target_os = "linux")] -#[cfg(any(test, feature = "benchmarks"))] -pub(crate) fn parse_first_u64_ascii_token(bytes: &[u8]) -> Option { - std::str::from_utf8(bytes) - .ok()? - .split_ascii_whitespace() - .next()? - .parse::() - .ok() -} - -#[cfg(target_os = "linux")] -#[cfg(any(test, feature = "benchmarks"))] -pub(crate) fn parse_vmrss_kib(bytes: &[u8]) -> Option { - std::str::from_utf8(bytes).ok()?.lines().find_map(|line| { - let value = line.strip_prefix("VmRSS:")?.split_whitespace().next()?; - value.parse::().ok() - }) -} - #[cfg(target_os = "linux")] #[cfg(any(test, feature = "benchmarks"))] fn current_cpu_runtime_ns() -> Option { @@ -760,6 +740,32 @@ fn current_rss_kib() -> Option { None } +#[cfg(target_os = "linux")] +#[cfg(any(test, feature = "benchmarks"))] +pub(crate) fn parse_first_u64_ascii_token(bytes: &[u8]) -> Option { + let start = bytes.iter().position(u8::is_ascii_digit)?; + let digits = &bytes[start..]; + let end = digits + .iter() + .position(|byte| !byte.is_ascii_digit()) + .unwrap_or(digits.len()); + std::str::from_utf8(&digits[..end]) + .ok()? + .parse::() + .ok() +} + +#[cfg(target_os = "linux")] +#[cfg(any(test, feature = "benchmarks"))] +pub(crate) fn parse_vmrss_kib(status: &[u8]) -> Option { + for line in status.split(|byte| *byte == b'\n') { + if let Some(rest) = line.strip_prefix(b"VmRSS:") { + return parse_first_u64_ascii_token(rest); + } + } + None +} + // --------------------------------------------------------------------------- // Clipboard — copy from diff, paste into commit message, selection range // --------------------------------------------------------------------------- diff --git a/crates/gitcomet-ui-gpui/src/view/rows/benchmarks/search_fixtures.rs b/crates/gitcomet-ui-gpui/src/view/rows/benchmarks/search_fixtures.rs index 775e28a4..dba42b92 100644 --- a/crates/gitcomet-ui-gpui/src/view/rows/benchmarks/search_fixtures.rs +++ b/crates/gitcomet-ui-gpui/src/view/rows/benchmarks/search_fixtures.rs @@ -366,12 +366,13 @@ impl CommitSearchFilterFixture { seen.len() } + /// Number of distinct lowercased message trigrams in the fixture. #[cfg(test)] pub fn distinct_message_trigrams(&self) -> usize { let mut seen = std::collections::HashSet::new(); for summary in &self.summaries_lower { for trigram in summary.as_bytes().windows(3) { - seen.insert(<[u8; 3]>::try_from(trigram).expect("3-byte trigram")); + seen.insert([trigram[0], trigram[1], trigram[2]]); } } seen.len() @@ -1184,6 +1185,26 @@ impl FileFuzzyFindFixture { pub fn total_files(&self) -> usize { self.total_files } + + #[cfg(test)] + pub fn run_find_without_ordered_pair_prefilter(&self, query: &str) -> u64 { + let Some(query) = AsciiCaseInsensitiveSubsequenceNeedle::new(query.trim()) else { + return 0; + }; + + let mut h = FxHasher::default(); + let mut matches_found = 0u64; + for (ix, path) in self.paths.iter().enumerate() { + if query.is_match(path.lowercase_bytes.as_ref()) { + ix.hash(&mut h); + path.len.hash(&mut h); + matches_found = matches_found.saturating_add(1); + } + } + matches_found.hash(&mut h); + self.total_files.hash(&mut h); + h.finish() + } } /// Build a deterministic corpus of `count` synthetic file paths with realistic diff --git a/crates/gitcomet-ui-gpui/src/view/state_apply.rs b/crates/gitcomet-ui-gpui/src/view/state_apply.rs index 5fbb40a4..16959e8f 100644 --- a/crates/gitcomet-ui-gpui/src/view/state_apply.rs +++ b/crates/gitcomet-ui-gpui/src/view/state_apply.rs @@ -11,6 +11,7 @@ impl GitCometView { let prev_had_repos = !self.state.repos.is_empty(); let prev_banner_error = self.state.banner_error.clone(); let prev_auth_prompt = self.state.auth_prompt.clone(); + let prev_submodule_trust_prompt = self.state.submodule_trust_prompt.clone(); let next_banner_error = next.banner_error.clone(); let mut follow_up_msgs = Vec::new(); @@ -129,8 +130,14 @@ impl GitCometView { } } + let next_submodule_add_progress = next + .repos + .iter() + .filter_map(|repo| repo.submodule_add_in_flight.clone()) + .collect::>(); self.toast_host.update(cx, |host, cx| { - host.sync_clone_progress(next.clone.as_ref(), cx) + host.sync_clone_progress(next.clone.as_ref(), cx); + host.sync_submodule_add_progress(&next_submodule_add_progress, cx); }); #[cfg(target_os = "macos")] @@ -158,6 +165,9 @@ impl GitCometView { if prev_auth_prompt != self.state.auth_prompt { self.auth_prompt_key = None; } + if prev_submodule_trust_prompt != self.state.submodule_trust_prompt { + self.pending_submodule_trust_prompt = self.state.submodule_trust_prompt.clone(); + } if prev_had_repos && self.state.repos.is_empty() { self.popover_host .update(cx, |host, cx| host.close_popover(cx)); diff --git a/crates/gitcomet-ui-gpui/src/view/tests.rs b/crates/gitcomet-ui-gpui/src/view/tests.rs index 40bf2257..2a5ded9e 100644 --- a/crates/gitcomet-ui-gpui/src/view/tests.rs +++ b/crates/gitcomet-ui-gpui/src/view/tests.rs @@ -2163,6 +2163,29 @@ fn generic_error_banner_is_hidden_when_auth_prompt_is_active() { assert!(!GitCometView::should_render_generic_error_banner(true)); } +#[test] +fn error_banner_overflow_hint_is_hidden_for_short_errors() { + assert!(!GitCometView::should_show_error_banner_overflow_hint( + "Submodule failed:\n\nfatal: branch not found" + )); +} + +#[test] +fn error_banner_overflow_hint_is_shown_for_long_command_failures() { + let error = [ + "Submodule failed:", + "", + " git submodule add --branch git-subtree /tmp/src comet2", + "", + " Cloning into '/tmp/comet2'...", + " done.", + " fatal: 'origin/git-subtree' is not a commit and a branch 'git-subtree' cannot be created from it", + " fatal: unable to checkout submodule 'comet2'", + ] + .join("\n"); + assert!(GitCometView::should_show_error_banner_overflow_hint(&error)); +} + #[test] fn auth_prompt_banner_colors_use_accent_palette() { let theme = AppTheme::gitcomet_light(); diff --git a/crates/gitcomet-ui-gpui/src/view/toast_host.rs b/crates/gitcomet-ui-gpui/src/view/toast_host.rs index 64dc9872..6c960f39 100644 --- a/crates/gitcomet-ui-gpui/src/view/toast_host.rs +++ b/crates/gitcomet-ui-gpui/src/view/toast_host.rs @@ -1,4 +1,5 @@ use super::*; +use gitcomet_state::model::SubmoduleAddProgressState; pub(super) struct ToastHost { theme: AppTheme, @@ -9,6 +10,7 @@ pub(super) struct ToastHost { clone_progress: Option, clone_progress_last_seq: u64, clone_progress_dest: Option>, + submodule_add_progress: Vec, } #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -146,6 +148,17 @@ fn apply_clone_progress_sync( } } +fn apply_submodule_add_progress_sync( + submodule_add_progress: &mut Vec, + next_submodule_add_progress: &[SubmoduleAddProgressState], +) -> bool { + if submodule_add_progress == next_submodule_add_progress { + return false; + } + *submodule_add_progress = next_submodule_add_progress.to_vec(); + true +} + impl ToastHost { pub(super) fn new( theme: AppTheme, @@ -160,6 +173,7 @@ impl ToastHost { clone_progress: None, clone_progress_last_seq: 0, clone_progress_dest: None, + submodule_add_progress: Vec::new(), } } @@ -320,19 +334,61 @@ impl ToastHost { } } - fn render_clone_progress_toast( - &self, - op: CloneOpState, + pub(super) fn sync_submodule_add_progress( + &mut self, + next_submodule_add_progress: &[SubmoduleAddProgressState], cx: &mut gpui::Context, - ) -> AnyElement { + ) { + if apply_submodule_add_progress_sync( + &mut self.submodule_add_progress, + next_submodule_add_progress, + ) { + cx.notify(); + } + } + + fn render_progress_shell(&self, content: impl IntoElement) -> AnyElement { let theme = self.theme; - let spinner_color = crate::view::clone_progress::clone_progress_color(theme, &op); let shell_bg = with_alpha( theme.colors.surface_bg_elevated, if theme.is_dark { 0.96 } else { 0.98 }, ); let shell_border = clone_progress_shell_border_color(theme); let shell_accent = clone_progress_shell_accent_color(theme); + + div() + .min_w(px(360.0)) + .max_w(px(900.0)) + .flex() + .gap(px(12.0)) + .bg(shell_bg) + .border_1() + .border_color(shell_border) + .rounded(px(theme.radii.panel)) + .overflow_hidden() + .shadow_sm() + .text_lg() + .text_color(theme.colors.text) + .child(div().w(px(5.0)).bg(shell_accent).flex_shrink_0()) + .child( + div() + .flex_1() + .min_w(px(0.0)) + .pl(px(16.0)) + .pr(px(16.0)) + .py(px(12.0)) + .child(content), + ) + .into_any_element() + } + + fn render_clone_progress_toast( + &self, + op: CloneOpState, + cx: &mut gpui::Context, + ) -> AnyElement { + let theme = self.theme; + let spinner_color = crate::view::clone_progress::clone_progress_color(theme, &op); let percent = op.progress.percent.min(100); let bar_track = with_alpha( theme.colors.text_muted, @@ -433,30 +489,53 @@ impl ToastHost { ) .child(div().pt_1().child(abort_button)); - div() - .min_w(px(360.0)) - .max_w(px(900.0)) - .flex() - .gap(px(12.0)) - .bg(shell_bg) - .border_1() - .border_color(shell_border) - .rounded(px(theme.radii.panel)) - .overflow_hidden() - .shadow_sm() - .text_lg() - .text_color(theme.colors.text) - .child(div().w(px(5.0)).bg(shell_accent).flex_shrink_0()) - .child( - div() - .flex_1() - .min_w(px(0.0)) - .pl(px(16.0)) - .pr(px(16.0)) - .py(px(12.0)) - .child(content), - ) - .into_any_element() + self.render_progress_shell(content) + } + + fn render_submodule_add_progress_toast( + &self, + ix: u64, + progress: &SubmoduleAddProgressState, + ) -> AnyElement { + let theme = self.theme; + let spinner_color = crate::view::clone_progress::clone_progress_loading_color(theme); + let content = div().w_full().flex().flex_col().gap_2().child( + div() + .w_full() + .flex() + .items_start() + .gap_2() + .child(svg_spinner( + ("submodule_add_progress_spinner", ix), + spinner_color, + px(16.0), + )) + .child( + div() + .flex_1() + .min_w(px(0.0)) + .flex_col() + .gap_0p5() + .child( + div() + .font_weight(FontWeight::BOLD) + .child("Adding submodule…"), + ) + .child( + div() + .text_sm() + .font_weight(FontWeight::SEMIBOLD) + .child(progress.path.display().to_string()), + ) + .child( + div() + .text_sm() + .text_color(theme.colors.text_muted) + .child(progress.url.clone()), + ), + ), + ); + self.render_progress_shell(content) } fn set_tooltip_text_if_changed( @@ -485,17 +564,28 @@ impl ToastHost { impl Render for ToastHost { fn render(&mut self, _window: &mut Window, cx: &mut gpui::Context) -> impl IntoElement { - if self.toasts.is_empty() && self.clone_progress.is_none() { + if self.toasts.is_empty() + && self.clone_progress.is_none() + && self.submodule_add_progress.is_empty() + { return div().into_any_element(); } let theme = self.theme; - let has_progress = self.clone_progress.is_some(); + let mut progress_toasts = Vec::new(); + if let Some(progress) = self.clone_progress.clone() { + progress_toasts.push(self.render_clone_progress_toast(progress, cx)); + } + progress_toasts.extend( + self.submodule_add_progress + .iter() + .enumerate() + .map(|(ix, progress)| { + self.render_submodule_add_progress_toast(ix as u64, progress) + }), + ); + let has_progress = !progress_toasts.is_empty(); let max_other = if has_progress { 2 } else { 3 }; - let progress_toast = self - .clone_progress - .clone() - .map(|progress| self.render_clone_progress_toast(progress, cx)); let mut displayed = self .toasts .iter() @@ -613,9 +703,7 @@ impl Render for ToastHost { .into_any_element() }) .collect::>(); - if let Some(progress_toast) = progress_toast { - children.push(progress_toast); - } + children.extend(progress_toasts); let root = div() .id("toast_layer") @@ -669,6 +757,13 @@ mod tests { } } + fn submodule_add_progress(url: &str, path: &str) -> SubmoduleAddProgressState { + SubmoduleAddProgressState { + url: url.to_string(), + path: PathBuf::from(path), + } + } + #[test] fn apply_clone_progress_sync_tracks_running_progress_and_deduplicates_same_seq() { let dest = Arc::new(PathBuf::from("/tmp/repo")); @@ -917,6 +1012,55 @@ mod tests { assert!(tracked_dest.is_none()); } + #[test] + fn apply_submodule_add_progress_sync_deduplicates_equal_snapshots() { + let mut progress = vec![submodule_add_progress( + "https://example.com/sub.git", + "mods/sub", + )]; + + assert!(!apply_submodule_add_progress_sync( + &mut progress, + &[submodule_add_progress( + "https://example.com/sub.git", + "mods/sub", + )], + )); + assert_eq!( + progress, + vec![submodule_add_progress( + "https://example.com/sub.git", + "mods/sub" + )] + ); + } + + #[test] + fn apply_submodule_add_progress_sync_replaces_and_clears_entries() { + let mut progress = vec![submodule_add_progress( + "https://example.com/one.git", + "mods/one", + )]; + + assert!(apply_submodule_add_progress_sync( + &mut progress, + &[ + submodule_add_progress("https://example.com/two.git", "mods/two"), + submodule_add_progress("https://example.com/three.git", "mods/three"), + ], + )); + assert_eq!( + progress, + vec![ + submodule_add_progress("https://example.com/two.git", "mods/two"), + submodule_add_progress("https://example.com/three.git", "mods/three"), + ] + ); + + assert!(apply_submodule_add_progress_sync(&mut progress, &[])); + assert!(progress.is_empty()); + } + #[test] fn clone_progress_shell_uses_subtle_accent_border_and_strip() { let dark = AppTheme::gitcomet_dark();