From 48a92521630cf77759a814fd7c47ebfd4cbff005 Mon Sep 17 00:00:00 2001 From: Miao Date: Mon, 4 Aug 2025 15:55:07 +0200 Subject: [PATCH] Fix large file check to use staged_get instead of intent_add (#332) --- .../check_added_large_files.rs | 4 +- tests/builtin_hooks.rs | 105 ++++++++++++++++++ tests/common/mod.rs | 35 ++++++ 3 files changed, 142 insertions(+), 2 deletions(-) diff --git a/src/builtin/pre_commit_hooks/check_added_large_files.rs b/src/builtin/pre_commit_hooks/check_added_large_files.rs index 739bfb5b..2d9dbdb7 100644 --- a/src/builtin/pre_commit_hooks/check_added_large_files.rs +++ b/src/builtin/pre_commit_hooks/check_added_large_files.rs @@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet}; use clap::Parser; use futures::StreamExt; -use crate::git::{intent_to_add_files, lfs_files}; +use crate::git::{get_staged_files, lfs_files}; use crate::hook::Hook; use crate::run::CONCURRENCY; @@ -39,7 +39,7 @@ pub(crate) async fn check_added_large_files( let filter = if args.enforce_all { FileFilter::NoFilter } else { - let add_files: HashSet<_> = intent_to_add_files().await?.into_iter().collect(); + let add_files: HashSet<_> = get_staged_files().await?.into_iter().collect(); FileFilter::Files(add_files) }; diff --git a/tests/builtin_hooks.rs b/tests/builtin_hooks.rs index d7f9bc26..c70515c5 100644 --- a/tests/builtin_hooks.rs +++ b/tests/builtin_hooks.rs @@ -98,3 +98,108 @@ fn end_of_file_fixer_hook() -> Result<()> { Ok(()) } + +#[test] +fn check_added_large_files_hook() -> Result<()> { + let context = TestContext::new(); + context.init_project(); + context.configure_git_author(); + + // Create an initial commit + let cwd = context.work_dir(); + cwd.child("README.md").write_str("Initial commit")?; + context.git_add("."); + context.git_commit("Initial commit"); + + context.write_pre_commit_config(indoc::indoc! {r" + repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v5.0.0 + hooks: + - id: check-added-large-files + args: ['1'] + "}); + + // Create test files + cwd.child("small_file.txt").write_str("Hello World\n")?; + let large_file = cwd.child("large_file.txt"); + large_file.write_binary(&[0; 2048])?; // 2KB file + + context.git_add("."); + + // First run: hook should fail because of the large file + cmd_snapshot!(context.filters(), context.run(), @r#" + success: false + exit_code: 1 + ----- stdout ----- + check for added large files..............................................Failed + - hook id: check-added-large-files + - exit code: 1 + large_file.txt (2 KB) exceeds 1 KB + + ----- stderr ----- + "#); + + // Commit the files + context.git_add("."); + context.git_commit("Add large file"); + + // Create a new unstaged large file + let unstaged_large_file = cwd.child("unstaged_large_file.txt"); + unstaged_large_file.write_binary(&[0; 2048])?; // 2KB file + context.git_add("unstaged_large_file.txt"); + + context.write_pre_commit_config(indoc::indoc! {r" + repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v5.0.0 + hooks: + - id: check-added-large-files + args: ['1', '--enforce-all'] + "}); + + // Second run: the hook should check all files even if not staged + cmd_snapshot!(context.filters(), context.run().arg("--all-files"), @r#" + success: false + exit_code: 1 + ----- stdout ----- + check for added large files..............................................Failed + - hook id: check-added-large-files + - exit code: 1 + unstaged_large_file.txt (2 KB) exceeds 1 KB + large_file.txt (2 KB) exceeds 1 KB + + ----- stderr ----- + "#); + + context.git_rm("unstaged_large_file.txt"); + context.git_clean(); + + // Test git-lfs integration + context.write_pre_commit_config(indoc::indoc! {r" + repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v5.0.0 + hooks: + - id: check-added-large-files + args: ['1'] + "}); + cwd.child(".gitattributes") + .write_str("*.dat filter=lfs diff=lfs merge=lfs -text")?; + context.git_add(".gitattributes"); + let lfs_file = cwd.child("lfs_file.dat"); + lfs_file.write_binary(&[0; 2048])?; // 2KB file + context.git_add("lfs_file.dat"); + + // Third run: hook should pass because the large file is tracked by git-lfs + cmd_snapshot!(context.filters(), context.run(), @r#" + success: true + exit_code: 0 + ----- stdout ----- + check for added large files..............................................Passed + + ----- stderr ----- + "#); + + Ok(()) +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 2373d16e..35e1776a 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -239,6 +239,41 @@ impl TestContext { .success(); } + /// Run `git reset`. + pub fn git_reset(&self, target: &str) { + Command::new("git") + .arg("reset") + .arg(target) + .current_dir(&self.temp_dir) + .assert() + .success(); + } + + /// Run `git rm`. + pub fn git_rm(&self, path: &str) { + Command::new("git") + .arg("rm") + .arg("--cached") + .arg(path) + .current_dir(&self.temp_dir) + .assert() + .success(); + let file_path = self.temp_dir.child(path); + if file_path.exists() { + fs_err::remove_file(file_path).unwrap(); + } + } + + /// Run `git clean`. + pub fn git_clean(&self) { + Command::new("git") + .arg("clean") + .arg("-fdx") + .current_dir(&self.temp_dir) + .assert() + .success(); + } + /// Write a `.pre-commit-config.yaml` file in the temporary directory. pub fn write_pre_commit_config(&self, content: &str) { self.temp_dir