diff --git a/src/command/diff.rs b/src/command/diff.rs index b7914267d..76371e355 100644 --- a/src/command/diff.rs +++ b/src/command/diff.rs @@ -1058,7 +1058,7 @@ async fn resolve_positional_revisions(args: &mut DiffArgs) -> Result<(), DiffErr let dashdash = !args.after_dashdash.is_empty() || bare_dashdash_in_diff_argv(); // Post-`--` tokens are pathspecs verbatim (no existence check, matching // `git diff -- nosuch` → empty diff). Fold them in up front. - let trailing_paths: Vec = args.after_dashdash.drain(..).collect(); + let trailing_paths = std::mem::take(&mut args.after_dashdash); if args.old.is_some() || args.new.is_some() { args.pathspec.extend(trailing_paths); diff --git a/src/command/for_each_ref.rs b/src/command/for_each_ref.rs index 4226e8d58..58eb389fc 100644 --- a/src/command/for_each_ref.rs +++ b/src/command/for_each_ref.rs @@ -2347,10 +2347,9 @@ fn short_refname(refname: &str) -> String { fn refname_strip_atom(token: &str, refname: &str) -> Option { let (from_left, num) = if let Some(n) = token.strip_prefix("refname:lstrip=") { (true, n) - } else if let Some(n) = token.strip_prefix("refname:rstrip=") { - (false, n) } else { - return None; + let n = token.strip_prefix("refname:rstrip=")?; + (false, n) }; Some(strip_ref_components(refname, from_left, num.parse().ok()?)) } diff --git a/src/command/merge.rs b/src/command/merge.rs index d2be1b8aa..7332bccd4 100644 --- a/src/command/merge.rs +++ b/src/command/merge.rs @@ -23,7 +23,7 @@ use git_internal::{ use serde::{Deserialize, Serialize}; use super::{ - get_target_commit, load_object, log, reset, + get_target_commit, load_object, reset, restore::{self, RestoreArgs}, save_object, status, switch, }; @@ -35,6 +35,7 @@ use crate::{ config::ConfigKv, db::get_db_conn_instance, head::Head, + merge_base, reflog::{ReflogAction, ReflogContext, with_reflog}, tree_plumbing, }, @@ -958,7 +959,6 @@ async fn run_merge_for_pull_inner( })?; let lca = lca_commit(¤t_commit, &target_commit) - .await .map_err(|error| PullMergeError::History(error.to_string()))?; let lca = lca.ok_or(PullMergeError::UnrelatedHistories)?; @@ -1545,32 +1545,20 @@ async fn resolve_merge_target(target_ref: &str) -> Result Result, CliError> { - let lhs_reachable = log::get_reachable_commits(lhs.id.to_string(), None).await?; - let rhs_reachable = log::get_reachable_commits(rhs.id.to_string(), None).await?; - - // Commit `eq` is based on tree_id, so we shouldn't use it here - - for commit in lhs_reachable.iter() { - if commit.id == rhs.id { - return Ok(Some(commit.to_owned())); - } - } - - for commit in rhs_reachable.iter() { - if commit.id == lhs.id { - return Ok(Some(commit.to_owned())); - } - } +fn lca_commit(lhs: &Commit, rhs: &Commit) -> Result, CliError> { + let Some(base_id) = merge_base::merge_base(&lhs.id, &rhs.id).map_err(|error| { + CliError::fatal(format!("failed to compute merge base: {error}")) + .with_stable_code(StableErrorCode::RepoCorrupt) + })? + else { + return Ok(None); + }; - for lhs_parent in lhs_reachable.iter() { - for rhs_parent in rhs_reachable.iter() { - if lhs_parent.id == rhs_parent.id { - return Ok(Some(lhs_parent.to_owned())); - } - } - } - Ok(None) + let base = load_object::(&base_id).map_err(|error| { + CliError::fatal(format!("failed to load merge base {base_id}: {error}")) + .with_stable_code(StableErrorCode::RepoCorrupt) + })?; + Ok(Some(base)) } async fn apply_fast_forward_merge( diff --git a/src/command/mod.rs b/src/command/mod.rs index 022865afb..1fcece89c 100644 --- a/src/command/mod.rs +++ b/src/command/mod.rs @@ -133,19 +133,24 @@ pub mod switch; pub mod web_assets; pub mod write_tree; -use std::{io, io::Write, path::Path}; +use std::{ + fs::File, + io::{self, Read, Write}, + path::Path, +}; use git_internal::{ errors::GitError, hash::ObjectHash, internal::object::{ObjectTrait, blob::Blob}, + utils::HashAlgorithm, }; use rpassword::read_password; use crate::{ internal::protocol::https_client::BasicAuth, utils, - utils::{client_storage::ClientStorage, error::emit_warning, object_ext::BlobExt, util}, + utils::{client_storage::ClientStorage, error::emit_warning, util}, }; // impl load for all objects @@ -253,13 +258,35 @@ pub fn ask_basic_auth() -> BasicAuth { /// Calculate the hash of a file blob /// - for `lfs` file: calculate hash of the pointer data pub fn calc_file_blob_hash(path: impl AsRef) -> io::Result { - let blob = if utils::lfs::is_lfs_tracked(&path) { + if utils::lfs::is_lfs_tracked(&path) { let (pointer, _) = utils::lfs::generate_pointer_file(&path); - Blob::from_content(&pointer) - } else { - Blob::from_file(&path) - }; - Ok(blob.id) + return Ok(Blob::from_content(&pointer).id); + } + + stream_file_blob_hash(path) +} + +fn stream_file_blob_hash(path: impl AsRef) -> io::Result { + let path = path.as_ref(); + let file = File::open(path)?; + let len = file.metadata()?.len(); + let mut reader = io::BufReader::new(file); + let mut hasher = HashAlgorithm::new(); + + hasher.update(b"blob "); + hasher.update(len.to_string().as_bytes()); + hasher.update(b"\0"); + + let mut buffer = [0_u8; 64 * 1024]; + loop { + let read = reader.read(&mut buffer)?; + if read == 0 { + break; + } + hasher.update(&buffer[..read]); + } + + ObjectHash::from_bytes(&hasher.finalize()).map_err(io::Error::other) } /// Get the commit hash from branch name or commit hash, support remote branch diff --git a/src/command/restore.rs b/src/command/restore.rs index 7b07a6129..737c3db47 100644 --- a/src/command/restore.rs +++ b/src/command/restore.rs @@ -1,7 +1,7 @@ //! Implements restore flows to reset files or entire trees from commits or the index, respecting pathspecs and staged vs worktree targets. use std::{ - collections::{HashMap, HashSet}, + collections::{BTreeSet, HashMap, HashSet}, fs, io, path::{Path, PathBuf}, }; @@ -34,9 +34,7 @@ use crate::{ lfs, object_ext::{BlobExt, CommitExt, TreeExt}, output::{OutputConfig, emit_json_data}, - path, - path_ext::PathExt, - util, + path, util, }, }; @@ -594,61 +592,34 @@ async fn restore_worktree_tracked( overlay: bool, ) -> Result<(Vec, Vec), RestoreError> { let target_map = preprocess_blobs(target_blobs); - // This set holds source paths that are MISSING from the worktree — they must - // be (re)created in BOTH modes. Overlay only suppresses *removal* of paths - // present in the worktree but absent from the source (gated below); it still - // creates/updates everything the source provides. - let deleted_files = get_worktree_deleted_files_in_filters(filter, &target_map); - - for path in filter { - if !path.exists() - && !target_map - .iter() - .any(|(p, _)| util::is_sub_path(p.workdir_to_absolute(), path)) - { - return Err(pathspec_not_matched(path)); - } - } - - let mut file_paths = - util::integrate_pathspec(filter).map_err(|_| RestoreError::ReadWorktree)?; - file_paths.extend(deleted_files); - // `integrate_pathspec` inserts a *deleted* directory pathspec verbatim (a - // missing path is not `is_dir()`), so a bare directory placeholder can land - // in `file_paths`. Restoring its children recreates the directory, after - // which hashing the bare entry as a blob would panic. Keep only entries that - // name a real source blob or an existing worktree file; the directory's - // actual files arrive via the `deleted_files` discovery set above. - file_paths.retain(|p| target_map.contains_key(p) || util::workdir_to_absolute(p).is_file()); - let index = Index::load(path::index()).map_err(|_| RestoreError::ReadIndex)?; + let file_paths = collect_restore_worktree_paths(filter, &target_map, &index)?; let mut restored = Vec::new(); let mut deleted = Vec::new(); for path_wd in &file_paths { let path_abs = util::workdir_to_absolute(path_wd); + let path_wd_str = path_to_utf8_typed(path_wd)?; + let tracked = index.tracked(path_wd_str, 0); if !path_abs.exists() { if let Some(target) = target_map.get(path_wd) { restore_to_file_typed(&target.hash, path_wd).await?; apply_worktree_target_mode(&path_abs, target.mode)?; restored.push(path_wd.display().to_string()); - } else { + } else if !tracked { return Err(pathspec_not_matched(path_wd)); } - } else { - let path_wd_str = path_to_utf8_typed(path_wd)?; + } else if let Some(target) = target_map.get(path_wd) { let hash = calc_file_blob_hash(&path_abs).map_err(|_| RestoreError::ReadObject)?; - if let Some(target) = target_map.get(path_wd) { - if hash != target.hash { - restore_to_file_typed(&target.hash, path_wd).await?; - restored.push(path_wd.display().to_string()); - } - apply_worktree_target_mode(&path_abs, target.mode)?; - } else if !overlay && index.tracked(path_wd_str, 0) { - fs::remove_file(&path_abs).map_err(|_| RestoreError::WriteWorktree)?; - util::clear_empty_dir(&path_abs); - deleted.push(path_wd.display().to_string()); + if hash != target.hash { + restore_to_file_typed(&target.hash, path_wd).await?; + restored.push(path_wd.display().to_string()); } + apply_worktree_target_mode(&path_abs, target.mode)?; + } else if !overlay && tracked { + fs::remove_file(&path_abs).map_err(|_| RestoreError::WriteWorktree)?; + util::clear_empty_dir(&path_abs); + deleted.push(path_wd.display().to_string()); } } @@ -980,6 +951,36 @@ fn legacy_targets(blobs: &[(PathBuf, ObjectHash)]) -> Vec<(PathBuf, RestoreTarge .collect() } +fn collect_restore_worktree_paths( + filter: &[PathBuf], + target_map: &HashMap, + index: &Index, +) -> Result, RestoreError> { + let tracked_paths = index.tracked_files(); + for path in filter { + if path.exists() { + continue; + } + + let matches_target = target_map + .keys() + .any(|p| util::is_sub_path(util::workdir_to_absolute(p), path)); + let matches_index = tracked_paths + .iter() + .any(|p| util::is_sub_path(util::workdir_to_absolute(p), path)); + if !matches_target && !matches_index { + return Err(pathspec_not_matched(path)); + } + } + + let filter_vec = filter.to_vec(); + let target_paths = target_map.keys().cloned().collect::>(); + let mut paths = BTreeSet::new(); + paths.extend(util::filter_to_fit_paths(&target_paths, &filter_vec)); + paths.extend(util::filter_to_fit_paths(&tracked_paths, &filter_vec)); + Ok(paths.into_iter().collect()) +} + fn tree_item_mode_to_index_mode(mode: TreeItemMode) -> Option { match mode { TreeItemMode::Blob => Some(0o100644), @@ -1296,20 +1297,6 @@ pub async fn restore_to_file(hash: &ObjectHash, path: &PathBuf) -> io::Result<() Ok(()) } -fn get_worktree_deleted_files_in_filters( - filters: &[PathBuf], - target_blobs: &HashMap, -) -> HashSet { - target_blobs - .iter() - .filter(|(path, _)| { - let path = util::workdir_to_absolute(path); - !path.exists() && path.sub_of_paths(filters) - }) - .map(|(path, _)| path.clone()) - .collect() -} - // ── Legacy worktree/index restore (kept for execute_checked) ───────── pub async fn restore_worktree( @@ -1318,53 +1305,31 @@ pub async fn restore_worktree( ) -> io::Result<()> { let target_blobs = legacy_targets(target_blobs); let target_blobs = preprocess_blobs(&target_blobs); - let deleted_files = get_worktree_deleted_files_in_filters(filter, &target_blobs); - - { - for path in filter { - if !path.exists() - && !target_blobs - .iter() - .any(|(p, _)| util::is_sub_path(p.workdir_to_absolute(), path)) - { - return Err(io::Error::other(format!( - "pathspec '{}' did not match any files", - path.display() - ))); - } - } - } - - let mut file_paths = util::integrate_pathspec(filter)?; - file_paths.extend(deleted_files); - // Drop bare deleted-directory placeholders so recreating their children does - // not leave a directory entry that would later be hashed as a blob (panic). - file_paths.retain(|p| target_blobs.contains_key(p) || util::workdir_to_absolute(p).is_file()); - let index = Index::load(path::index()).map_err(|e| io::Error::other(e.to_string()))?; + let file_paths = collect_restore_worktree_paths(filter, &target_blobs, &index) + .map_err(|error| io::Error::other(error.to_string()))?; for path_wd in &file_paths { let path_abs = util::workdir_to_absolute(path_wd); + let path_wd_str = path_to_utf8(path_wd)?; + let tracked = index.tracked(path_wd_str, 0); if !path_abs.exists() { if let Some(target) = target_blobs.get(path_wd) { restore_to_file(&target.hash, path_wd).await?; - } else { + } else if !tracked { return Err(io::Error::other(format!( "pathspec '{}' did not match any files", path_wd.display() ))); } - } else { - let path_wd_str = path_to_utf8(path_wd)?; + } else if let Some(target) = target_blobs.get(path_wd) { let hash = calc_file_blob_hash(&path_abs).map_err(|e| io::Error::other(e.to_string()))?; - if target_blobs.contains_key(path_wd) { - if hash != target_blobs[path_wd].hash { - restore_to_file(&target_blobs[path_wd].hash, path_wd).await?; - } - } else if index.tracked(path_wd_str, 0) { - fs::remove_file(&path_abs)?; - util::clear_empty_dir(&path_abs); + if hash != target.hash { + restore_to_file(&target.hash, path_wd).await?; } + } else if tracked { + fs::remove_file(&path_abs)?; + util::clear_empty_dir(&path_abs); } } Ok(()) @@ -1376,45 +1341,26 @@ async fn restore_worktree_legacy_typed( ) -> Result<(), RestoreError> { let target_blobs = legacy_targets(target_blobs); let target_blobs = preprocess_blobs(&target_blobs); - let deleted_files = get_worktree_deleted_files_in_filters(filter, &target_blobs); - - for path in filter { - if !path.exists() - && !target_blobs - .iter() - .any(|(p, _)| util::is_sub_path(p.workdir_to_absolute(), path)) - { - return Err(pathspec_not_matched(path)); - } - } - - let mut file_paths = - util::integrate_pathspec(filter).map_err(|_| RestoreError::ReadWorktree)?; - file_paths.extend(deleted_files); - // Drop bare deleted-directory placeholders so recreating their children does - // not leave a directory entry that would later be hashed as a blob (panic). - file_paths.retain(|p| target_blobs.contains_key(p) || util::workdir_to_absolute(p).is_file()); - let index = Index::load(path::index()).map_err(|_| RestoreError::ReadIndex)?; + let file_paths = collect_restore_worktree_paths(filter, &target_blobs, &index)?; for path_wd in &file_paths { let path_abs = util::workdir_to_absolute(path_wd); + let path_wd_str = path_to_utf8_typed(path_wd)?; + let tracked = index.tracked(path_wd_str, 0); if !path_abs.exists() { if let Some(target) = target_blobs.get(path_wd) { restore_to_file_typed(&target.hash, path_wd).await?; - } else { + } else if !tracked { return Err(pathspec_not_matched(path_wd)); } - } else { - let path_wd_str = path_to_utf8_typed(path_wd)?; + } else if let Some(target) = target_blobs.get(path_wd) { let hash = calc_file_blob_hash(&path_abs).map_err(|_| RestoreError::ReadObject)?; - if target_blobs.contains_key(path_wd) { - if hash != target_blobs[path_wd].hash { - restore_to_file_typed(&target_blobs[path_wd].hash, path_wd).await?; - } - } else if index.tracked(path_wd_str, 0) { - fs::remove_file(&path_abs).map_err(|_| RestoreError::WriteWorktree)?; - util::clear_empty_dir(&path_abs); + if hash != target.hash { + restore_to_file_typed(&target.hash, path_wd).await?; } + } else if tracked { + fs::remove_file(&path_abs).map_err(|_| RestoreError::WriteWorktree)?; + util::clear_empty_dir(&path_abs); } } diff --git a/src/command/verify_pack_index_common.rs b/src/command/verify_pack_index_common.rs index f028265f2..a1ad84e62 100644 --- a/src/command/verify_pack_index_common.rs +++ b/src/command/verify_pack_index_common.rs @@ -9,15 +9,12 @@ pub(super) fn parse_fanout(bytes: &[u8], offset: usize) -> Result<[u32; 256], St } let mut fanout = [0u32; 256]; - for (slot, chunk) in fanout - .iter_mut() - .zip(bytes[offset..offset + FANOUT_LEN].chunks_exact(4)) - { - *slot = u32::from_be_bytes( - chunk - .try_into() - .map_err(|_| "truncated fanout entry".to_string())?, - ); + let (chunks, remainder) = bytes[offset..offset + FANOUT_LEN].as_chunks::<4>(); + if !remainder.is_empty() { + return Err("truncated fanout entry".to_string()); + } + for (slot, chunk) in fanout.iter_mut().zip(chunks) { + *slot = u32::from_be_bytes(*chunk); } Ok(fanout) } diff --git a/src/command/verify_pack_index_v2.rs b/src/command/verify_pack_index_v2.rs index ba000b073..1d4c7a0d7 100644 --- a/src/command/verify_pack_index_v2.rs +++ b/src/command/verify_pack_index_v2.rs @@ -77,12 +77,13 @@ fn idx_v2_layout_matches_hash_kind(bytes: &[u8], object_count: usize, kind: Hash let offset_table = &bytes[cursor..offsets_end]; cursor = offsets_end; - let large_count = offset_table - .chunks_exact(4) - .filter(|raw| { - let offset = u32::from_be_bytes([raw[0], raw[1], raw[2], raw[3]]); - offset & 0x8000_0000 != 0 - }) + let (offset_chunks, offset_remainder) = offset_table.as_chunks::<4>(); + if !offset_remainder.is_empty() { + return false; + } + let large_count = offset_chunks + .iter() + .filter(|raw| u32::from_be_bytes(**raw) & 0x8000_0000 != 0) .count(); let Some(trailer_start) = cursor.checked_add(large_count.saturating_mul(8)) else { return false; @@ -138,9 +139,13 @@ pub(super) fn parse_idx_v2(bytes: &[u8]) -> Result { let offset_table = &bytes[cursor..offsets_end]; cursor = offsets_end; - let large_count = offset_table - .chunks_exact(4) - .filter(|raw| u32::from_be_bytes([raw[0], raw[1], raw[2], raw[3]]) & 0x8000_0000 != 0) + let (offset_chunks, offset_remainder) = offset_table.as_chunks::<4>(); + if !offset_remainder.is_empty() { + return Err("pack index v2 offset table is truncated".to_string()); + } + let large_count = offset_chunks + .iter() + .filter(|raw| u32::from_be_bytes(**raw) & 0x8000_0000 != 0) .count(); let large_offsets_end = cursor + large_count * 8; let trailer_start = large_offsets_end; @@ -153,12 +158,13 @@ pub(super) fn parse_idx_v2(bytes: &[u8]) -> Result { let index_hash_len = remaining - hash_len; let mut large_offsets = Vec::with_capacity(large_count); - for chunk in bytes[cursor..large_offsets_end].chunks_exact(8) { - large_offsets.push(u64::from_be_bytes( - chunk - .try_into() - .map_err(|_| "truncated v2 large offset".to_string())?, - )); + let (large_offset_chunks, large_offset_remainder) = + bytes[cursor..large_offsets_end].as_chunks::<8>(); + if !large_offset_remainder.is_empty() { + return Err("truncated v2 large offset".to_string()); + } + for chunk in large_offset_chunks { + large_offsets.push(u64::from_be_bytes(*chunk)); } let mut entries = Vec::with_capacity(object_count); diff --git a/src/internal/tui/markdown_render.rs b/src/internal/tui/markdown_render.rs index 1fcf6c7e9..94f2d687a 100644 --- a/src/internal/tui/markdown_render.rs +++ b/src/internal/tui/markdown_render.rs @@ -418,7 +418,7 @@ impl Renderer { } let wrapped = wrap_segments( - self.current_segments.drain(..).collect(), + std::mem::take(&mut self.current_segments), self.width, &prefixes, ); diff --git a/tests/command/index_pack_test.rs b/tests/command/index_pack_test.rs index 15f6c5c0f..e22904029 100644 --- a/tests/command/index_pack_test.rs +++ b/tests/command/index_pack_test.rs @@ -331,9 +331,11 @@ fn parse_idx_v2(bytes: &[u8], kind: HashKind) -> ParsedIdxV2 { let offsets = &bytes[cursor..offsets_end]; cursor = offsets_end; - let large_count = offsets - .chunks_exact(4) - .filter(|raw| u32::from_be_bytes((*raw).try_into().unwrap()) & 0x8000_0000 != 0) + let (offset_chunks, offset_remainder) = offsets.as_chunks::<4>(); + assert!(offset_remainder.is_empty(), "idx v2 offsets are truncated"); + let large_count = offset_chunks + .iter() + .filter(|raw| u32::from_be_bytes(**raw) & 0x8000_0000 != 0) .count(); let large_offsets_end = cursor + large_count * 8; assert!( @@ -341,8 +343,14 @@ fn parse_idx_v2(bytes: &[u8], kind: HashKind) -> ParsedIdxV2 { "idx v2 large offsets or trailer are truncated" ); let mut large_offsets = Vec::with_capacity(large_count); - for chunk in bytes[cursor..large_offsets_end].chunks_exact(8) { - large_offsets.push(u64::from_be_bytes(chunk.try_into().unwrap())); + let (large_offset_chunks, large_offset_remainder) = + bytes[cursor..large_offsets_end].as_chunks::<8>(); + assert!( + large_offset_remainder.is_empty(), + "idx v2 large offsets are truncated" + ); + for chunk in large_offset_chunks { + large_offsets.push(u64::from_be_bytes(*chunk)); } cursor = large_offsets_end; diff --git a/tests/command/pull_test.rs b/tests/command/pull_test.rs index b599d9f2d..a6a41d626 100644 --- a/tests/command/pull_test.rs +++ b/tests/command/pull_test.rs @@ -243,6 +243,60 @@ async fn test_pull_ff_only_fast_forward_updates_head_from_tracking_remote() { ); } +#[cfg(unix)] +#[tokio::test] +#[serial] +async fn test_pull_fast_forward_skips_untracked_artifacts_during_restore() { + use std::os::unix::fs::PermissionsExt; + + let (_temp_root, remote_dir, work_dir, branch) = create_remote_fixture(); + + let local_repo = tempdir().expect("failed to create local repo"); + init_repo_via_cli(local_repo.path()); + configure_identity_via_cli(local_repo.path()); + configure_pull_tracking(local_repo.path(), &remote_dir, &branch); + + let first_pull = run_libra_command(&["pull"], local_repo.path()); + assert_cli_success(&first_pull, "initial pull"); + + fs::write(local_repo.path().join(".libraignore"), "target/\n") + .expect("failed to write ignore file"); + let artifact_dir = local_repo.path().join("target/deep"); + fs::create_dir_all(&artifact_dir).expect("failed to create artifact dir"); + fs::write(artifact_dir.join("artifact.bin"), b"untracked build output") + .expect("failed to write artifact"); + fs::set_permissions(&artifact_dir, fs::Permissions::from_mode(0o000)) + .expect("failed to lock artifact dir"); + + let new_head = push_remote_commit( + &work_dir, + &branch, + "remote.txt", + "remote change\n", + "remote update", + ); + let output = run_libra_command(&["pull", "--ff-only"], local_repo.path()); + + fs::set_permissions(&artifact_dir, fs::Permissions::from_mode(0o700)) + .expect("failed to unlock artifact dir"); + + assert_cli_success(&output, "pull should not scan untracked build artifacts"); + + let _guard = ChangeDirGuard::new(local_repo.path()); + let head = Head::current_commit() + .await + .expect("pull should update HEAD to the fetched commit"); + assert_eq!(head.to_string(), new_head); + assert!( + local_repo.path().join("remote.txt").exists(), + "pull should restore tracked files from the fetched commit" + ); + assert!( + artifact_dir.join("artifact.bin").exists(), + "pull should leave untracked artifacts alone" + ); +} + #[tokio::test] #[serial] async fn test_pull_diverged_remote_creates_three_way_merge() {