Skip to content

Commit 1171596

Browse files
committed
Allow pass_filenames to accept a positive integer
Setting `pass_filenames: n` limits each hook invocation to at most `n` filenames, with multiple parallel invocations for larger file sets. This mirrors the `-n` flag of `xargs` and is useful for tools that can only process a limited number of files per invocation (typically, if a limit applies, it is 1). The existing boolean behaviour is preserved: `true` passes all filenames (default) and `false` passes none. So this is a backwards compatible change. Resolves: #1471 Signed-off-by: JP-Ellis <josh@jpellis.me>
1 parent f7b0c00 commit 1171596

File tree

12 files changed

+173
-30
lines changed

12 files changed

+173
-30
lines changed

crates/prek/src/cli/run/run.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use crate::cli::reporter::{HookInitReporter, HookInstallReporter, HookRunReporte
2121
use crate::cli::run::keeper::WorkTreeKeeper;
2222
use crate::cli::run::{CollectOptions, FileFilter, Selectors, collect_files};
2323
use crate::cli::{ExitStatus, RunExtraArgs};
24-
use crate::config::{Language, Stage};
24+
use crate::config::{Language, PassFilenames, Stage};
2525
use crate::fs::CWD;
2626
use crate::git::GIT_ROOT;
2727
use crate::hook::{Hook, InstallInfo, InstalledHook, Repo};
@@ -1019,11 +1019,12 @@ async fn run_hook(
10191019
}
10201020
let start = std::time::Instant::now();
10211021

1022-
let filenames = if hook.pass_filenames {
1023-
shuffle(&mut filenames);
1024-
filenames
1025-
} else {
1026-
vec![]
1022+
let filenames = match hook.pass_filenames {
1023+
PassFilenames::All | PassFilenames::Limited(_) => {
1024+
shuffle(&mut filenames);
1025+
filenames
1026+
}
1027+
PassFilenames::None => vec![],
10271028
};
10281029

10291030
let (exit_status, hook_output) = if dry_run {

crates/prek/src/config.rs

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,68 @@ impl<'de> Deserialize<'de> for Stages {
329329
}
330330
}
331331

332+
/// Controls whether filenames are appended to a hook's command line.
333+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
334+
pub(crate) enum PassFilenames {
335+
/// Pass all matching filenames (default). Corresponds to `pass_filenames:
336+
/// true`.
337+
All,
338+
/// Pass no filenames. Corresponds to `pass_filenames: false`.
339+
None,
340+
/// Pass at most `n` filenames per invocation. Corresponds to
341+
/// `pass_filenames: n`.
342+
Limited(std::num::NonZeroUsize),
343+
}
344+
345+
impl<'de> Deserialize<'de> for PassFilenames {
346+
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
347+
where
348+
D: Deserializer<'de>,
349+
{
350+
struct PassFilenamesVisitor;
351+
352+
impl serde::de::Visitor<'_> for PassFilenamesVisitor {
353+
type Value = PassFilenames;
354+
355+
fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
356+
f.write_str("a boolean or a positive integer")
357+
}
358+
359+
fn visit_bool<E: DeError>(self, v: bool) -> Result<PassFilenames, E> {
360+
Ok(if v {
361+
PassFilenames::All
362+
} else {
363+
PassFilenames::None
364+
})
365+
}
366+
367+
fn visit_u64<E: DeError>(self, v: u64) -> Result<PassFilenames, E> {
368+
let n = usize::try_from(v)
369+
.ok()
370+
.and_then(std::num::NonZeroUsize::new)
371+
.ok_or_else(|| {
372+
E::custom(
373+
"pass_filenames must be a positive integer; use `false` to pass no filenames",
374+
)
375+
})?;
376+
Ok(PassFilenames::Limited(n))
377+
}
378+
379+
fn visit_i64<E: DeError>(self, v: i64) -> Result<PassFilenames, E> {
380+
if v <= 0 {
381+
return Err(E::custom(
382+
"pass_filenames must be a positive integer; use `false` to pass no filenames",
383+
));
384+
}
385+
#[allow(clippy::cast_sign_loss)]
386+
self.visit_u64(v as u64)
387+
}
388+
}
389+
390+
deserializer.deserialize_any(PassFilenamesVisitor)
391+
}
392+
}
393+
332394
/// Common hook options.
333395
#[derive(Debug, Clone, Default, Deserialize)]
334396
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
@@ -363,7 +425,7 @@ pub(crate) struct HookOptions {
363425
pub fail_fast: Option<bool>,
364426
/// Append filenames that would be checked to the hook entry as arguments.
365427
/// Default is true.
366-
pub pass_filenames: Option<bool>,
428+
pub pass_filenames: Option<PassFilenames>,
367429
/// A description of the hook. For metadata only.
368430
pub description: Option<String>,
369431
/// Run the hook on a specific version of the language.

crates/prek/src/hook.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use tracing::trace;
1616

1717
use crate::config::{
1818
self, BuiltinHook, Config, FilePattern, HookOptions, Language, LocalHook, ManifestHook,
19-
MetaHook, RemoteHook, Stages, read_manifest,
19+
MetaHook, PassFilenames, RemoteHook, Stages, read_manifest,
2020
};
2121
use crate::languages::version::LanguageRequest;
2222
use crate::languages::{extract_metadata, resolve_command};
@@ -324,7 +324,7 @@ impl HookBuilder {
324324
let exclude_types = options.exclude_types.unwrap_or_default();
325325
let always_run = options.always_run.unwrap_or(false);
326326
let fail_fast = options.fail_fast.unwrap_or(false);
327-
let pass_filenames = options.pass_filenames.unwrap_or(true);
327+
let pass_filenames = options.pass_filenames.unwrap_or(PassFilenames::All);
328328
let require_serial = options.require_serial.unwrap_or(false);
329329
let verbose = options.verbose.unwrap_or(false);
330330
let stages = options.stages.unwrap_or_default();
@@ -456,7 +456,7 @@ pub(crate) struct Hook {
456456
pub env: FxHashMap<String, String>,
457457
pub always_run: bool,
458458
pub fail_fast: bool,
459-
pub pass_filenames: bool,
459+
pub pass_filenames: PassFilenames,
460460
pub description: Option<String>,
461461
pub language_request: LanguageRequest,
462462
pub log_file: Option<String>,
@@ -838,7 +838,7 @@ mod tests {
838838
use prek_identify::tags;
839839
use rustc_hash::FxHashMap;
840840

841-
use crate::config::{HookOptions, Language, RemoteHook};
841+
use crate::config::{HookOptions, Language, PassFilenames, RemoteHook};
842842
use crate::hook::HookSpec;
843843
use crate::languages::version::LanguageRequest;
844844
use crate::workspace::Project;
@@ -899,7 +899,7 @@ mod tests {
899899
args: Some(vec!["--flag".to_string()]),
900900
env: Some(override_env),
901901
always_run: Some(true),
902-
pass_filenames: Some(false),
902+
pass_filenames: Some(PassFilenames::None),
903903
verbose: Some(true),
904904
description: Some("desc".to_string()),
905905
..Default::default()
@@ -974,7 +974,7 @@ mod tests {
974974
},
975975
always_run: true,
976976
fail_fast: false,
977-
pass_filenames: false,
977+
pass_filenames: None,
978978
description: Some(
979979
"desc",
980980
),

crates/prek/src/hooks/builtin_hooks/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use anyhow::Result;
55
use prek_identify::tags;
66

77
use crate::cli::reporter::HookRunReporter;
8-
use crate::config::{BuiltinHook, HookOptions, Stage};
8+
use crate::config::{BuiltinHook, HookOptions, PassFilenames, Stage};
99
use crate::hook::Hook;
1010
use crate::hooks::pre_commit_hooks;
1111
use crate::store::Store;
@@ -264,7 +264,7 @@ impl BuiltinHook {
264264
entry: "no-commit-to-branch".to_string(),
265265
priority: None,
266266
options: HookOptions {
267-
pass_filenames: Some(false),
267+
pass_filenames: Some(PassFilenames::None),
268268
always_run: Some(true),
269269
..Default::default()
270270
},

crates/prek/src/run.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use prek_consts::env_vars::EnvVars;
99
use rustc_hash::FxHashMap;
1010
use tracing::trace;
1111

12+
use crate::config::PassFilenames;
1213
use crate::hook::Hook;
1314

1415
pub(crate) static USE_COLOR: LazyLock<bool> =
@@ -143,7 +144,10 @@ impl<'a> Partitions<'a> {
143144
filenames: &'a [&'a Path],
144145
concurrency: usize,
145146
) -> anyhow::Result<Self> {
146-
let max_per_batch = max(4, filenames.len().div_ceil(concurrency));
147+
let max_per_batch = match hook.pass_filenames {
148+
PassFilenames::Limited(n) => n.get(),
149+
_ => max(4, filenames.len().div_ceil(concurrency)),
150+
};
147151
let mut arg_max = platform_max_cli_length();
148152

149153
let cmd = Path::new(&entry[0]);

crates/prek/src/schema.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::config::{
2-
BuiltinHook, BuiltinRepo, FilePattern, LocalRepo, MetaHook, MetaRepo, RemoteHook, RemoteRepo,
3-
Repo, Stage, Stages,
2+
BuiltinHook, BuiltinRepo, FilePattern, LocalRepo, MetaHook, MetaRepo, PassFilenames,
3+
RemoteHook, RemoteRepo, Repo, Stage, Stages,
44
};
55
use std::borrow::Cow;
66

@@ -178,6 +178,25 @@ impl schemars::JsonSchema for FilePattern {
178178
}
179179
}
180180

181+
impl schemars::JsonSchema for PassFilenames {
182+
fn schema_name() -> std::borrow::Cow<'static, str> {
183+
std::borrow::Cow::Borrowed("PassFilenames")
184+
}
185+
186+
fn json_schema(_gen: &mut schemars::generate::SchemaGenerator) -> schemars::Schema {
187+
schemars::json_schema!({
188+
"description": "Whether to pass filenames to the hook. \
189+
`true` passes all matching filenames (default), \
190+
`false` passes none, and \
191+
a positive integer limits each invocation to at most that many filenames.",
192+
"oneOf": [
193+
{"type": "boolean"},
194+
{"type": "integer", "exclusiveMinimum": 0}
195+
]
196+
})
197+
}
198+
}
199+
181200
fn predefined_hook_schema(
182201
schema_gen: &mut schemars::SchemaGenerator,
183202
description: &str,

crates/prek/src/snapshots/prek__config__tests__read_config_with_nested_merge_keys.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ Config {
2727
always_run: None,
2828
fail_fast: None,
2929
pass_filenames: Some(
30-
false,
30+
None,
3131
),
3232
description: None,
3333
language_version: None,

crates/prek/src/snapshots/prek__config__tests__read_manifest.snap

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ Manifest {
3030
always_run: None,
3131
fail_fast: None,
3232
pass_filenames: Some(
33-
false,
33+
None,
3434
),
3535
description: Some(
3636
"Automatically run 'uv pip compile' on your requirements",
@@ -72,7 +72,7 @@ Manifest {
7272
always_run: None,
7373
fail_fast: None,
7474
pass_filenames: Some(
75-
false,
75+
None,
7676
),
7777
description: Some(
7878
"Automatically run 'uv lock' on your project dependencies",
@@ -117,7 +117,7 @@ Manifest {
117117
always_run: None,
118118
fail_fast: None,
119119
pass_filenames: Some(
120-
false,
120+
None,
121121
),
122122
description: Some(
123123
"Automatically run 'uv export' on your project dependencies",

crates/prek/src/snapshots/prek__config__tests__read_yaml_config.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ Config {
109109
always_run: None,
110110
fail_fast: None,
111111
pass_filenames: Some(
112-
false,
112+
None,
113113
),
114114
description: None,
115115
language_version: None,
@@ -156,7 +156,7 @@ Config {
156156
always_run: None,
157157
fail_fast: None,
158158
pass_filenames: Some(
159-
false,
159+
None,
160160
),
161161
description: None,
162162
language_version: None,

crates/prek/tests/run.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3105,3 +3105,44 @@ fn run_with_tree_object_as_ref() -> Result<()> {
31053105

31063106
Ok(())
31073107
}
3108+
3109+
/// `pass_filenames: n` limits each invocation to at most n files.
3110+
/// With n=1, each matched file gets its own invocation.
3111+
#[test]
3112+
fn pass_filenames_integer_limits_batch_size() -> Result<()> {
3113+
let context = TestContext::new();
3114+
context.init_project();
3115+
3116+
let cwd = context.work_dir();
3117+
// Use a script that errors if it receives more than one filename argument.
3118+
context.write_pre_commit_config(indoc::indoc! {r#"
3119+
repos:
3120+
- repo: local
3121+
hooks:
3122+
- id: one-at-a-time
3123+
name: one at a time
3124+
entry: python -c "import sys; args = sys.argv[1:]; sys.exit(0 if len(args) <= 1 else 1)"
3125+
language: system
3126+
pass_filenames: 1
3127+
require_serial: true
3128+
verbose: true
3129+
"#});
3130+
3131+
cwd.child("a.txt").write_str("a")?;
3132+
cwd.child("b.txt").write_str("b")?;
3133+
cwd.child("c.txt").write_str("c")?;
3134+
context.git_add(".");
3135+
3136+
cmd_snapshot!(context.filters(), context.run().arg("--all-files"), @r"
3137+
success: true
3138+
exit_code: 0
3139+
----- stdout -----
3140+
one at a time............................................................Passed
3141+
- hook id: one-at-a-time
3142+
- duration: [TIME]
3143+
3144+
----- stderr -----
3145+
");
3146+
3147+
Ok(())
3148+
}

0 commit comments

Comments
 (0)