From 1d980196adfd79ae0936e681e8d98c57d9900785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= Date: Sat, 4 Apr 2026 17:12:44 +0000 Subject: [PATCH 01/51] doc: convert git-difftool manual page to synopsis style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * convert commands to synopsis style * use __ for arguments * fix conditional text to sentence limits Signed-off-by: Jean-Noël Avila Signed-off-by: Junio C Hamano --- Documentation/config/difftool.adoc | 24 ++++----- Documentation/config/mergetool.adoc | 8 +-- Documentation/git-difftool.adoc | 80 ++++++++++++++--------------- 3 files changed, 56 insertions(+), 56 deletions(-) diff --git a/Documentation/config/difftool.adoc b/Documentation/config/difftool.adoc index 4f7d40ce242b78..1b8d48381357aa 100644 --- a/Documentation/config/difftool.adoc +++ b/Documentation/config/difftool.adoc @@ -1,43 +1,43 @@ -diff.tool:: +`diff.tool`:: Controls which diff tool is used by linkgit:git-difftool[1]. This variable overrides the value configured in `merge.tool`. The list below shows the valid built-in values. Any other value is treated as a custom diff tool and requires - that a corresponding difftool..cmd variable is defined. + that a corresponding `difftool..cmd` variable is defined. -diff.guitool:: +`diff.guitool`:: Controls which diff tool is used by linkgit:git-difftool[1] when - the -g/--gui flag is specified. This variable overrides the value + the `-g`/`--gui` flag is specified. This variable overrides the value configured in `merge.guitool`. The list below shows the valid built-in values. Any other value is treated as a custom diff tool - and requires that a corresponding difftool..cmd variable + and requires that a corresponding `difftool..cmd` variable is defined. include::{build_dir}/mergetools-diff.adoc[] -difftool..cmd:: +`difftool..cmd`:: Specify the command to invoke the specified diff tool. The specified command is evaluated in shell with the following - variables available: 'LOCAL' is set to the name of the temporary - file containing the contents of the diff pre-image and 'REMOTE' + variables available: `LOCAL` is set to the name of the temporary + file containing the contents of the diff pre-image and `REMOTE` is set to the name of the temporary file containing the contents of the diff post-image. + See the `--tool=` option in linkgit:git-difftool[1] for more details. -difftool..path:: +`difftool..path`:: Override the path for the given tool. This is useful in case your tool is not in the PATH. -difftool.trustExitCode:: +`difftool.trustExitCode`:: Exit difftool if the invoked diff tool returns a non-zero exit status. + See the `--trust-exit-code` option in linkgit:git-difftool[1] for more details. -difftool.prompt:: +`difftool.prompt`:: Prompt before each invocation of the diff tool. -difftool.guiDefault:: +`difftool.guiDefault`:: Set `true` to use the `diff.guitool` by default (equivalent to specifying the `--gui` argument), or `auto` to select `diff.guitool` or `diff.tool` depending on the presence of a `DISPLAY` environment variable value. The diff --git a/Documentation/config/mergetool.adoc b/Documentation/config/mergetool.adoc index 7064f5a462cb56..7afdcad92b3934 100644 --- a/Documentation/config/mergetool.adoc +++ b/Documentation/config/mergetool.adoc @@ -52,13 +52,13 @@ if `merge.tool` is configured as __), Git will consult `mergetool..layout` to determine the tool's layout. If the variant-specific configuration is not available, `vimdiff` ' s is used as - fallback. If that too is not available, a default layout with 4 windows - will be used. To configure the layout, see the 'BACKEND SPECIFIC HINTS' + fallback. If that too is not available, a default layout with 4 windows + will be used. ifdef::git-mergetool[] - section. +To configure the layout, see the 'BACKEND SPECIFIC HINTS' section. endif::[] ifndef::git-mergetool[] - section in linkgit:git-mergetool[1]. +To configure the layout, see the 'BACKEND SPECIFIC HINTS' section in linkgit:git-mergetool[1]. endif::[] `mergetool.hideResolved`:: diff --git a/Documentation/git-difftool.adoc b/Documentation/git-difftool.adoc index 064bc683471f21..dd7cacf95e35df 100644 --- a/Documentation/git-difftool.adoc +++ b/Documentation/git-difftool.adoc @@ -7,64 +7,64 @@ git-difftool - Show changes using common diff tools SYNOPSIS -------- -[verse] -'git difftool' [] [ []] [--] [...] +[synopsis] +git difftool [] [ []] [--] [...] DESCRIPTION ----------- -'git difftool' is a Git command that allows you to compare and edit files -between revisions using common diff tools. 'git difftool' is a frontend -to 'git diff' and accepts the same options and arguments. See +`git difftool` is a Git command that allows you to compare and edit files +between revisions using common diff tools. `git difftool` is a frontend +to `git diff` and accepts the same options and arguments. See linkgit:git-diff[1]. OPTIONS ------- --d:: ---dir-diff:: +`-d`:: +`--dir-diff`:: Copy the modified files to a temporary location and perform a directory diff on them. This mode never prompts before launching the diff tool. --y:: ---no-prompt:: +`-y`:: +`--no-prompt`:: Do not prompt before launching a diff tool. ---prompt:: +`--prompt`:: Prompt before each invocation of the diff tool. This is the default behaviour; the option is provided to override any configuration settings. ---rotate-to=:: - Start showing the diff for the given path, +`--rotate-to=`:: + Start showing the diff for __, the paths before it will move to the end and output. ---skip-to=:: - Start showing the diff for the given path, skipping all +`--skip-to=`:: + Start showing the diff for __, skipping all the paths before it. --t :: ---tool=:: - Use the diff tool specified by . Valid values include +`-t `:: +`--tool=`:: + Use the diff tool specified by __. Valid values include emerge, kompare, meld, and vimdiff. Run `git difftool --tool-help` - for the list of valid settings. + for the list of valid __ settings. + -If a diff tool is not specified, 'git difftool' +If a diff tool is not specified, `git difftool` will use the configuration variable `diff.tool`. If the -configuration variable `diff.tool` is not set, 'git difftool' +configuration variable `diff.tool` is not set, `git difftool` will pick a suitable default. + You can explicitly provide a full path to the tool by setting the configuration variable `difftool..path`. For example, you can configure the absolute path to kdiff3 by setting -`difftool.kdiff3.path`. Otherwise, 'git difftool' assumes the +`difftool.kdiff3.path`. Otherwise, `git difftool` assumes the tool is available in PATH. + Instead of running one of the known diff tools, -'git difftool' can be customized to run an alternative program +`git difftool` can be customized to run an alternative program by specifying the command line to invoke in a configuration variable `difftool..cmd`. + -When 'git difftool' is invoked with this tool (either through the +When `git difftool` is invoked with this tool (either through the `-t` or `--tool` option or the `diff.tool` configuration variable) the configured command line will be invoked with the following variables available: `$LOCAL` is set to the name of the temporary @@ -74,30 +74,30 @@ of the diff post-image. `$MERGED` is the name of the file which is being compared. `$BASE` is provided for compatibility with custom merge tool commands and has the same value as `$MERGED`. ---tool-help:: +`--tool-help`:: Print a list of diff tools that may be used with `--tool`. ---symlinks:: ---no-symlinks:: - 'git difftool''s default behavior is to create symlinks to the +`--symlinks`:: +`--no-symlinks`:: + `git difftool`'s default behavior is to create symlinks to the working tree when run in `--dir-diff` mode and the right-hand side of the comparison yields the same content as the file in the working tree. + -Specifying `--no-symlinks` instructs 'git difftool' to create copies +Specifying `--no-symlinks` instructs `git difftool` to create copies instead. `--no-symlinks` is the default on Windows. --x :: ---extcmd=:: +`-x `:: +`--extcmd=`:: Specify a custom command for viewing diffs. - 'git-difftool' ignores the configured defaults and runs + `git-difftool` ignores the configured defaults and runs ` $LOCAL $REMOTE` when this option is specified. Additionally, `$BASE` is set in the environment. --g:: ---gui:: ---no-gui:: - When 'git-difftool' is invoked with the `-g` or `--gui` option +`-g`:: +`--gui`:: +`--no-gui`:: + When `git-difftool` is invoked with the `-g` or `--gui` option the default diff tool will be read from the configured `diff.guitool` variable instead of `diff.tool`. This may be selected automatically using the configuration variable @@ -106,20 +106,20 @@ instead. `--no-symlinks` is the default on Windows. fallback in the order of `merge.guitool`, `diff.tool`, `merge.tool` until a tool is found. ---trust-exit-code:: ---no-trust-exit-code:: +`--trust-exit-code`:: +`--no-trust-exit-code`:: Errors reported by the diff tool are ignored by default. - Use `--trust-exit-code` to make 'git-difftool' exit when an + Use `--trust-exit-code` to make `git-difftool` exit when an invoked diff tool returns a non-zero exit code. + -'git-difftool' will forward the exit code of the invoked tool when +`git-difftool` will forward the exit code of the invoked tool when `--trust-exit-code` is used. See linkgit:git-diff[1] for the full list of supported options. CONFIGURATION ------------- -'git difftool' falls back to 'git mergetool' config variables when the +`git difftool` falls back to `git mergetool` config variables when the difftool equivalents have not been defined. include::includes/cmd-config-section-rest.adoc[] From 5594be68eaa0fc9c87f7a50be09b85762415f070 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= Date: Sat, 4 Apr 2026 17:12:45 +0000 Subject: [PATCH 02/51] doc: convert git-range-diff manual page to synopsis style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * convert commands and options to synopsis style * use __ for arguments * small style fixes Signed-off-by: Jean-Noël Avila Signed-off-by: Junio C Hamano --- Documentation/git-range-diff.adoc | 50 +++++++++++++++---------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/Documentation/git-range-diff.adoc b/Documentation/git-range-diff.adoc index b5e85d37f1bee7..880557084533fb 100644 --- a/Documentation/git-range-diff.adoc +++ b/Documentation/git-range-diff.adoc @@ -7,8 +7,8 @@ git-range-diff - Compare two commit ranges (e.g. two versions of a branch) SYNOPSIS -------- -[verse] -'git range-diff' [--color=[]] [--no-color] [] +[synopsis] +git range-diff [--color=[]] [--no-color] [] [--no-dual-color] [--creation-factor=] [--left-only | --right-only] [--diff-merges=] [--remerge-diff] @@ -21,14 +21,14 @@ DESCRIPTION This command shows the differences between two versions of a patch series, or more generally, two commit ranges (ignoring merge commits). -In the presence of `` arguments, these commit ranges are limited +In the presence of __ arguments, these commit ranges are limited accordingly. To that end, it first finds pairs of commits from both commit ranges that correspond with each other. Two commits are said to correspond when the diff between their patches (i.e. the author information, the commit message and the commit diff) is reasonably small compared to the -patches' size. See ``Algorithm`` below for details. +patches' size. See 'Algorithm' below for details. Finally, the list of matching commits is shown in the order of the second commit range, with unmatched commits being inserted just after @@ -37,7 +37,7 @@ all of their ancestors have been shown. There are three ways to specify the commit ranges: - ` `: Either commit range can be of the form - `..`, `^!` or `^-`. See `SPECIFYING RANGES` + `..`, `^!` or `^-`. See 'SPECIFYING RANGES' in linkgit:gitrevisions[7] for more details. - `...`. This is equivalent to @@ -48,7 +48,7 @@ There are three ways to specify the commit ranges: OPTIONS ------- ---no-dual-color:: +`--no-dual-color`:: When the commit diffs differ, `git range-diff` recreates the original diffs' coloring, and adds outer -/+ diff markers with the *background* being red/green to make it easier to see e.g. @@ -56,33 +56,33 @@ OPTIONS + Additionally, the commit diff lines that are only present in the first commit range are shown "dimmed" (this can be overridden using the `color.diff.` -config setting where `` is one of `contextDimmed`, `oldDimmed` and +config setting where __ is one of `contextDimmed`, `oldDimmed` and `newDimmed`), and the commit diff lines that are only present in the second commit range are shown in bold (which can be overridden using the config -settings `color.diff.` with `` being one of `contextBold`, +settings `color.diff.` with __ being one of `contextBold`, `oldBold` or `newBold`). + This is known to `range-diff` as "dual coloring". Use `--no-dual-color` to revert to color all lines according to the outer diff markers (and completely ignore the inner diff when it comes to color). ---creation-factor=:: - Set the creation/deletion cost fudge factor to ``. +`--creation-factor=`:: + Set the creation/deletion cost fudge factor to __. Defaults to 60. Try a larger value if `git range-diff` erroneously considers a large change a total rewrite (deletion of one commit and addition of another), and a smaller one in the reverse case. - See the ``Algorithm`` section below for an explanation of why this is + See the 'Algorithm' section below for an explanation of why this is needed. ---left-only:: +`--left-only`:: Suppress commits that are missing from the first specified range - (or the "left range" when using the `...` format). + (or the "left range" when using the `...` form). ---right-only:: +`--right-only`:: Suppress commits that are missing from the second specified range - (or the "right range" when using the `...` format). + (or the "right range" when using the `...` form). ---diff-merges=:: +`--diff-merges=`:: Instead of ignoring merge commits, generate diffs for them using the corresponding `--diff-merges=` option of linkgit:git-log[1], and include them in the comparison. @@ -93,30 +93,30 @@ have produced. In other words, if a merge commit is the result of a non-conflicting `git merge`, the `remerge` mode will represent it with an empty diff. ---remerge-diff:: +`--remerge-diff`:: Convenience option, equivalent to `--diff-merges=remerge`. ---notes[=]:: ---no-notes:: +`--notes[=]`:: +`--no-notes`:: This flag is passed to the `git log` program (see linkgit:git-log[1]) that generates the patches. - :: +` `:: Compare the commits specified by the two ranges, where - `` is considered an older version of ``. + __ is considered an older version of __. -...:: +`...`:: Equivalent to passing `..` and `..`. - :: +` `:: Equivalent to passing `..` and `..`. - Note that `` does not need to be the exact branch point + Note that __ does not need to be the exact branch point of the branches. Example: after rebasing a branch `my-topic`, `git range-diff my-topic@{u} my-topic@{1} my-topic` would show the differences introduced by the rebase. `git range-diff` also accepts the regular diff options (see -linkgit:git-diff[1]), most notably the `--color=[]` and +linkgit:git-diff[1]), most notably the `--color[=]` and `--no-color` options. These options are used when generating the "diff between patches", i.e. to compare the author, commit message and diff of corresponding old/new commits. There is currently no means to tweak most of the From f4c1b8e3fe855355f3e4c84d3e1a50b9957bd240 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= Date: Sat, 4 Apr 2026 17:12:46 +0000 Subject: [PATCH 03/51] doc: convert git-shortlog manual page to synopsis style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * convert commands and options to synopsis style * use __ for arguments * small style fixes Signed-off-by: Jean-Noël Avila Signed-off-by: Junio C Hamano --- Documentation/git-shortlog.adoc | 60 ++++++++++++++++----------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/Documentation/git-shortlog.adoc b/Documentation/git-shortlog.adoc index a11b57c1cd7b2d..e067d39b3880a8 100644 --- a/Documentation/git-shortlog.adoc +++ b/Documentation/git-shortlog.adoc @@ -3,63 +3,63 @@ git-shortlog(1) NAME ---- -git-shortlog - Summarize 'git log' output +git-shortlog - Summarize `git log` output SYNOPSIS -------- -[verse] -'git shortlog' [] [] [[--] ...] -git log --pretty=short | 'git shortlog' [] +[synopsis] +git shortlog [] [] [[--] ...] +git log --pretty=short | git shortlog [] DESCRIPTION ----------- -Summarizes 'git log' output in a format suitable for inclusion +Summarizes `git log` output in a format suitable for inclusion in release announcements. Each commit will be grouped by author and title. Additionally, "[PATCH]" will be stripped from the commit description. If no revisions are passed on the command line and either standard input -is not a terminal or there is no current branch, 'git shortlog' will +is not a terminal or there is no current branch, `git shortlog` will output a summary of the log read from standard input, without reference to the current repository. OPTIONS ------- --n:: ---numbered:: +`-n`:: +`--numbered`:: Sort output according to the number of commits per author instead of author alphabetic order. --s:: ---summary:: +`-s`:: +`--summary`:: Suppress commit description and provide a commit count summary only. --e:: ---email:: +`-e`:: +`--email`:: Show the email address of each author. ---format[=]:: +`--format[=]`:: Instead of the commit subject, use some other information to - describe each commit. '' can be any string accepted - by the `--format` option of 'git log', such as '* [%h] %s'. - (See the "PRETTY FORMATS" section of linkgit:git-log[1].) + describe each commit. __ can be any string accepted + by the `--format` option of `git log`, such as '* [%h] %s'. + (See the 'PRETTY FORMATS' section of linkgit:git-log[1].) + Each pretty-printed commit will be rewrapped before it is shown. ---date=:: +`--date=`:: Show dates formatted according to the given date string. (See - the `--date` option in the "Commit Formatting" section of + the `--date` option in the 'Commit Formatting' section of linkgit:git-log[1]). Useful with `--group=format:`. ---group=:: - Group commits based on ``. If no `--group` option is - specified, the default is `author`. `` is one of: +`--group=`:: + Group commits based on __. If no `--group` option is + specified, the default is `author`. __ is one of: + -- - `author`, commits are grouped by author - `committer`, commits are grouped by committer (the same as `-c`) - - `trailer:`, the `` is interpreted as a case-insensitive + - `trailer:`, the __ is interpreted as a case-insensitive commit message trailer (see linkgit:git-interpret-trailers[1]). For example, if your project uses `Reviewed-by` trailers, you might want to see who has been reviewing with @@ -76,7 +76,7 @@ unless the `--email` option is specified. If the value cannot be parsed as an identity, it will be taken literally and completely. - `format:`, any string accepted by the `--format` option of - 'git log'. (See the "PRETTY FORMATS" section of + `git log`. (See the 'PRETTY FORMATS' section of linkgit:git-log[1].) -- + @@ -85,11 +85,11 @@ value (but again, only once per unique value in that commit). For example, `git shortlog --group=author --group=trailer:co-authored-by` counts both authors and co-authors. --c:: ---committer:: +`-c`:: +`--committer`:: This is an alias for `--group=committer`. --w[[,[,]]]:: +`-w[[,[,]]]`:: Linewrap the output by wrapping each line at `width`. The first line of each entry is indented by `indent1` spaces, and the second and subsequent lines are indented by `indent2` spaces. `width`, @@ -98,16 +98,16 @@ counts both authors and co-authors. If width is `0` (zero) then indent the lines of the output without wrapping them. -:: +``:: Show only commits in the specified revision range. When no - is specified, it defaults to `HEAD` (i.e. the + __ is specified, it defaults to `HEAD` (i.e. the whole history leading to the current commit). `origin..HEAD` specifies all the commits reachable from the current commit (i.e. `HEAD`), but not from `origin`. For a complete list of - ways to spell , see the "Specifying Ranges" + ways to spell __, see the 'Specifying Ranges' section of linkgit:gitrevisions[7]. -[--] ...:: +`[--] ...`:: Consider only commits that are enough to explain how the files that match the specified paths came to be. + From 80f4b802e964559c65b08641c07a8acb95d0617e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= Date: Sat, 4 Apr 2026 17:12:47 +0000 Subject: [PATCH 04/51] doc: convert git-describe manual page to synopsis style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * convert commands and options to synopsis style * use __ for arguments Signed-off-by: Jean-Noël Avila Signed-off-by: Junio C Hamano --- Documentation/git-describe.adoc | 96 ++++++++++++++++----------------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/Documentation/git-describe.adoc b/Documentation/git-describe.adoc index 08ff715709ccd1..b2cb1e47e46c67 100644 --- a/Documentation/git-describe.adoc +++ b/Documentation/git-describe.adoc @@ -7,10 +7,10 @@ git-describe - Give an object a human readable name based on an available ref SYNOPSIS -------- -[verse] -'git describe' [--all] [--tags] [--contains] [--abbrev=] [...] -'git describe' [--all] [--tags] [--contains] [--abbrev=] --dirty[=] -'git describe' +[synopsis] +git describe [--all] [--tags] [--contains] [--abbrev=] [...] +git describe [--all] [--tags] [--contains] [--abbrev=] --dirty[=] +git describe DESCRIPTION ----------- @@ -22,70 +22,70 @@ abbreviated object name of the most recent commit. The result is a "human-readable" object name which can also be used to identify the commit to other git commands. -By default (without --all or --tags) `git describe` only shows +By default (without `--all` or `--tags`) `git describe` only shows annotated tags. For more information about creating annotated tags -see the -a and -s options to linkgit:git-tag[1]. +see the `-a` and `-s` options to linkgit:git-tag[1]. If the given object refers to a blob, it will be described as `:`, such that the blob can be found -at `` in the ``, which itself describes the +at __ in the __, which itself describes the first commit in which this blob occurs in a reverse revision walk -from HEAD. +from `HEAD`. OPTIONS ------- -...:: - Commit-ish object names to describe. Defaults to HEAD if omitted. +`...`:: + Commit-ish object names to describe. Defaults to `HEAD` if omitted. ---dirty[=]:: ---broken[=]:: +`--dirty[=]`:: +`--broken[=]`:: Describe the state of the working tree. When the working - tree matches HEAD, the output is the same as "git describe - HEAD". If the working tree has local modification "-dirty" + tree matches `HEAD`, the output is the same as `git describe HEAD`. + If the working tree has local modification, `-dirty` is appended to it. If a repository is corrupt and Git cannot determine if there is local modification, Git will - error out, unless `--broken' is given, which appends - the suffix "-broken" instead. + error out, unless `--broken` is given, which appends + the suffix `-broken` instead. ---all:: +`--all`:: Instead of using only the annotated tags, use any ref found in `refs/` namespace. This option enables matching any known branch, remote-tracking branch, or lightweight tag. ---tags:: +`--tags`:: Instead of using only the annotated tags, use any tag found in `refs/tags` namespace. This option enables matching a lightweight (non-annotated) tag. ---contains:: +`--contains`:: Instead of finding the tag that predates the commit, find the tag that comes after the commit, and thus contains it. - Automatically implies --tags. + Automatically implies `--tags`. ---abbrev=:: +`--abbrev=`:: Instead of using the default number of hexadecimal digits (which will vary according to the number of objects in the repository with - a default of 7) of the abbreviated object name, use digits, or - as many digits as needed to form a unique object name. An of 0 + a default of 7) of the abbreviated object name, use __ digits, or + as many digits as needed to form a unique object name. An __ of 0 will suppress long format, only showing the closest tag. ---candidates=:: +`--candidates=`:: Instead of considering only the 10 most recent tags as candidates to describe the input commit-ish consider - up to candidates. Increasing above 10 will take + up to __ candidates. Increasing __ above 10 will take slightly longer but may produce a more accurate result. - An of 0 will cause only exact matches to be output. + An __ of 0 will cause only exact matches to be output. ---exact-match:: +`--exact-match`:: Only output exact matches (a tag directly references the - supplied commit). This is a synonym for --candidates=0. + supplied commit). This is a synonym for `--candidates=0`. ---debug:: +`--debug`:: Verbosely display information about the searching strategy being employed to standard error. The tag name will still be printed to standard out. ---long:: +`--long`:: Always output the long format (the tag, the number of commits and the abbreviated commit name) even when it matches a tag. This is useful when you want to see parts of the commit object name @@ -94,8 +94,8 @@ OPTIONS describe such a commit as v1.2-0-gdeadbee (0th commit since tag v1.2 that points at object deadbee....). ---match :: - Only consider tags matching the given `glob(7)` pattern, +`--match `:: + Only consider tags matching the given `glob`(7) pattern, excluding the "refs/tags/" prefix. If used with `--all`, it also considers local branches and remote-tracking references matching the pattern, excluding respectively "refs/heads/" and "refs/remotes/" @@ -104,22 +104,22 @@ OPTIONS matching any of the patterns will be considered. Use `--no-match` to clear and reset the list of patterns. ---exclude :: - Do not consider tags matching the given `glob(7)` pattern, excluding +`--exclude `:: + Do not consider tags matching the given `glob`(7) pattern, excluding the "refs/tags/" prefix. If used with `--all`, it also does not consider local branches and remote-tracking references matching the pattern, - excluding respectively "refs/heads/" and "refs/remotes/" prefix; + excluding respectively "`refs/heads/`" and "`refs/remotes/`" prefix; references of other types are never considered. If given multiple times, a list of patterns will be accumulated and tags matching any of the - patterns will be excluded. When combined with --match a tag will be - considered when it matches at least one --match pattern and does not - match any of the --exclude patterns. Use `--no-exclude` to clear and + patterns will be excluded. When combined with `--match` a tag will be + considered when it matches at least one `--match` pattern and does not + match any of the `--exclude` patterns. Use `--no-exclude` to clear and reset the list of patterns. ---always:: +`--always`:: Show uniquely abbreviated commit object as fallback. ---first-parent:: +`--first-parent`:: Follow only the first parent commit upon seeing a merge commit. This is useful when you wish to not match tags on branches merged in the history of the target commit. @@ -139,8 +139,8 @@ an abbreviated object name for the commit itself ("2414721") at the end. The number of additional commits is the number -of commits which would be displayed by "git log v1.0.4..parent". -The hash suffix is "-g" + an unambiguous abbreviation for the tip commit +of commits which would be displayed by `git log v1.0.4..parent`. +The hash suffix is "`-g`" + an unambiguous abbreviation for the tip commit of parent (which was `2414721b194453f058079d897d13c4e377f92dc6`). The length of the abbreviation scales as the repository grows, using the approximate number of objects in the repository and a bit of math @@ -149,12 +149,12 @@ The "g" prefix stands for "git" and is used to allow describing the version of a software depending on the SCM the software is managed with. This is useful in an environment where people may use different SCMs. -Doing a 'git describe' on a tag-name will just show the tag name: +Doing a `git describe` on a tag-name will just show the tag name: [torvalds@g5 git]$ git describe v1.0.4 v1.0.4 -With --all, the command can use branch heads as references, so +With `--all`, the command can use branch heads as references, so the output shows the reference path as well: [torvalds@g5 git]$ git describe --all --abbrev=4 v1.0.5^2 @@ -163,7 +163,7 @@ the output shows the reference path as well: [torvalds@g5 git]$ git describe --all --abbrev=4 HEAD^ heads/lt/describe-7-g975b -With --abbrev set to 0, the command can be used to find the +With `--abbrev` set to 0, the command can be used to find the closest tagname without any suffix: [torvalds@g5 git]$ git describe --abbrev=0 v1.0.5^2 @@ -179,13 +179,13 @@ be sufficient to disambiguate these commits. SEARCH STRATEGY --------------- -For each commit-ish supplied, 'git describe' will first look for +For each commit-ish supplied, `git describe` will first look for a tag which tags exactly that commit. Annotated tags will always be preferred over lightweight tags, and tags with newer dates will always be preferred over tags with older dates. If an exact match is found, its name will be output and searching will stop. -If an exact match was not found, 'git describe' will walk back +If an exact match was not found, `git describe` will walk back through the commit history to locate an ancestor commit which has been tagged. The ancestor's tag will be output along with an abbreviation of the input commit-ish's SHA-1. If `--first-parent` was @@ -203,7 +203,7 @@ BUGS Tree objects as well as tag objects not pointing at commits, cannot be described. When describing blobs, the lightweight tags pointing at blobs are ignored, -but the blob is still described as : despite the lightweight +but the blob is still described as `:` despite the lightweight tag being favorable. GIT From 521731213c905f0dfec6a55393f010d185492c85 Mon Sep 17 00:00:00 2001 From: David Lin Date: Mon, 6 Apr 2026 15:27:11 -0400 Subject: [PATCH 05/51] cache-tree: fix inverted object existence check in cache_tree_fully_valid The negation in front of the object existence check in cache_tree_fully_valid() was lost in 062b914c84 (treewide: convert users of `repo_has_object_file()` to `has_object()`, 2025-04-29), turning `!repo_has_object_file(...)` into `has_object(...)` instead of `!has_object(...)`. This makes cache_tree_fully_valid() always report the cache tree as invalid when objects exist (the common case), forcing callers like write_index_as_tree() to call cache_tree_update() on every invocation. An odb_has_object() check inside update_one() avoids a full tree rebuild, but the unnecessary call still pays the cost of opening an ODB transaction and, in partial clones, a promisor remote check. Restore the missing negation and add a test that verifies write-tree takes the cache-tree shortcut when the cache tree is valid. Helped-by: Derrick Stolee Signed-off-by: David Lin Signed-off-by: Junio C Hamano --- cache-tree.c | 2 +- t/t0090-cache-tree.sh | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/cache-tree.c b/cache-tree.c index 66ef2becbe01a4..366b1d7dcd8081 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -239,7 +239,7 @@ int cache_tree_fully_valid(struct cache_tree *it) if (!it) return 0; if (it->entry_count < 0 || - odb_has_object(the_repository->objects, &it->oid, + !odb_has_object(the_repository->objects, &it->oid, HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) return 0; for (i = 0; i < it->subtree_nr; i++) { diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh index d901588294668c..0964718d7f33f5 100755 --- a/t/t0090-cache-tree.sh +++ b/t/t0090-cache-tree.sh @@ -278,4 +278,12 @@ test_expect_success 'switching trees does not invalidate shared index' ' ) ' +test_expect_success 'cache-tree is used by write-tree when valid' ' + test_commit use-valid && + + # write-tree with a valid cache-tree should skip cache_tree_update + GIT_TRACE2_PERF="$(pwd)/trace.output" git write-tree && + test_grep ! region_enter.*cache_tree.*update trace.output +' + test_done From 8808e61fd3e953c3534633b8b5adc5b243dd696f Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:34 +0200 Subject: [PATCH 06/51] promisor-remote: try accepted remotes before others in get_direct() When a server advertises promisor remotes and the client accepts some of them, those remotes carry the server's intent: 'fetch missing objects preferably from here', and the client agrees with that for the remotes it accepts. However promisor_remote_get_direct() actually iterates over all promisor remotes in list order, which is the order they appear in the config files (except perhaps for the one appearing in the `extensions.partialClone` config variable which is tried last). This means an existing, but not accepted, promisor remote, could be tried before the accepted ones, which does not reflect the intent of the agreement between client and server. If the client doesn't care about what the server suggests, it should accept nothing and rely on its remotes as they are already configured. To better reflect the agreement between client and server, let's make promisor_remote_get_direct() try the accepted promisor remotes before the non-accepted ones. Concretely, let's extract a try_promisor_remotes() helper and call it twice from promisor_remote_get_direct(): - first with an `accepted_only=true` argument to try only the accepted remotes, - then with `accepted_only=false` to fall back to any remaining remote. Ensuring that accepted remotes are preferred will be even more important if in the future a mechanism is developed to allow the client to auto-configure remotes that the server advertises. This will in particular avoid fetching from the server (which is already configured as a promisor remote) before trying the auto-configured remotes, as these new remotes would likely appear at the end of the config file, and as the server might not appear in the `extensions.partialClone` config variable. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- Documentation/gitprotocol-v2.adoc | 4 ++ promisor-remote.c | 44 ++++++++++++----- t/t5710-promisor-remote-capability.sh | 69 +++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 13 deletions(-) diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc index f985cb4c474953..4fcb1a7bda1be7 100644 --- a/Documentation/gitprotocol-v2.adoc +++ b/Documentation/gitprotocol-v2.adoc @@ -848,6 +848,10 @@ advertised, it can reply with "promisor-remote=" where where `pr-name` is the urlencoded name of a promisor remote the server advertised and the client accepts. +The promisor remotes that the client accepted will be tried before the +other configured promisor remotes when the client attempts to fetch +missing objects. + Note that, everywhere in this document, the ';' and ',' characters MUST be encoded if they appear in `pr-name` or `field-value`. diff --git a/promisor-remote.c b/promisor-remote.c index 96fa215b06a924..7ce7d22f952e70 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -268,11 +268,35 @@ static int remove_fetched_oids(struct repository *repo, return remaining_nr; } +static int try_promisor_remotes(struct repository *repo, + struct object_id **remaining_oids, + int *remaining_nr, int *to_free, + bool accepted_only) +{ + struct promisor_remote *r = repo->promisor_remote_config->promisors; + + for (; r; r = r->next) { + if (accepted_only != r->accepted) + continue; + if (fetch_objects(repo, r->name, *remaining_oids, *remaining_nr) < 0) { + if (*remaining_nr == 1) + continue; + *remaining_nr = remove_fetched_oids(repo, remaining_oids, + *remaining_nr, *to_free); + if (*remaining_nr) { + *to_free = 1; + continue; + } + } + return 1; /* all fetched */ + } + return 0; +} + void promisor_remote_get_direct(struct repository *repo, const struct object_id *oids, int oid_nr) { - struct promisor_remote *r; struct object_id *remaining_oids = (struct object_id *)oids; int remaining_nr = oid_nr; int to_free = 0; @@ -283,19 +307,13 @@ void promisor_remote_get_direct(struct repository *repo, promisor_remote_init(repo); - for (r = repo->promisor_remote_config->promisors; r; r = r->next) { - if (fetch_objects(repo, r->name, remaining_oids, remaining_nr) < 0) { - if (remaining_nr == 1) - continue; - remaining_nr = remove_fetched_oids(repo, &remaining_oids, - remaining_nr, to_free); - if (remaining_nr) { - to_free = 1; - continue; - } - } + /* Try accepted remotes first (those the server told us to use) */ + if (try_promisor_remotes(repo, &remaining_oids, &remaining_nr, + &to_free, true)) + goto all_fetched; + if (try_promisor_remotes(repo, &remaining_oids, &remaining_nr, + &to_free, false)) goto all_fetched; - } for (i = 0; i < remaining_nr; i++) { if (is_promisor_object(repo, &remaining_oids[i])) diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh index 357822c01a7530..bf0eed9f109742 100755 --- a/t/t5710-promisor-remote-capability.sh +++ b/t/t5710-promisor-remote-capability.sh @@ -166,6 +166,75 @@ test_expect_success "init + fetch with promisor.advertise set to 'true'" ' check_missing_objects server 1 "$oid" ' +test_expect_success "clone with two promisors but only one advertised" ' + git -C server config promisor.advertise true && + test_when_finished "rm -rf client unused_lop" && + + # Create a promisor that will be configured but not be used + git init --bare unused_lop && + + # Clone from server to create a client + GIT_TRACE="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \ + -c remote.unused_lop.promisor=true \ + -c remote.unused_lop.fetch="+refs/heads/*:refs/remotes/unused_lop/*" \ + -c remote.unused_lop.url="file://$(pwd)/unused_lop" \ + -c remote.lop.promisor=true \ + -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ + -c remote.lop.url="file://$(pwd)/lop" \ + -c promisor.acceptfromserver=All \ + --no-local --filter="blob:limit=5k" server client && + + # Check that "unused_lop" appears before "lop" in the config + printf "remote.%s.promisor true\n" "unused_lop" "lop" "origin" >expect && + git -C client config get --all --show-names --regexp "^remote\..*\.promisor$" >actual && + test_cmp expect actual && + + # Check that "lop" was tried + test_grep " fetch lop " trace && + # Check that "unused_lop" was not contacted + # This means "lop", the accepted promisor, was tried first + test_grep ! " fetch unused_lop " trace && + + # Check that the largest object is still missing on the server + check_missing_objects server 1 "$oid" +' + +test_expect_success "init + fetch two promisors but only one advertised" ' + git -C server config promisor.advertise true && + test_when_finished "rm -rf client unused_lop" && + + # Create a promisor that will be configured but not be used + git init --bare unused_lop && + + mkdir client && + git -C client init && + git -C client config remote.unused_lop.promisor true && + git -C client config remote.unused_lop.fetch "+refs/heads/*:refs/remotes/unused_lop/*" && + git -C client config remote.unused_lop.url "file://$(pwd)/unused_lop" && + git -C client config remote.lop.promisor true && + git -C client config remote.lop.fetch "+refs/heads/*:refs/remotes/lop/*" && + git -C client config remote.lop.url "file://$(pwd)/lop" && + git -C client config remote.server.url "file://$(pwd)/server" && + git -C client config remote.server.fetch "+refs/heads/*:refs/remotes/server/*" && + git -C client config promisor.acceptfromserver All && + + # Check that "unused_lop" appears before "lop" in the config + printf "remote.%s.promisor true\n" "unused_lop" "lop" >expect && + git -C client config get --all --show-names --regexp "^remote\..*\.promisor$" >actual && + test_cmp expect actual && + + GIT_TRACE="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git -C client fetch --filter="blob:limit=5k" server && + + # Check that "lop" was tried + test_grep " fetch lop " trace && + # Check that "unused_lop" was not contacted + # This means "lop", the accepted promisor, was tried first + test_grep ! " fetch unused_lop " trace && + + # Check that the largest object is still missing on the server + check_missing_objects server 1 "$oid" +' + test_expect_success "clone with promisor.acceptfromserver set to 'KnownName'" ' git -C server config promisor.advertise true && test_when_finished "rm -rf client" && From 720b7c26c82ef212852897bedb0d38eee78cb531 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:35 +0200 Subject: [PATCH 07/51] promisor-remote: pass config entry to all_fields_match() directly The `in_list == 0` path of all_fields_match() looks up the remote in `config_info` by `advertised->name` repeatedly, even though every caller in should_accept_remote() has already performed this lookup and holds the result in `p`. To avoid this useless work, let's replace the `int in_list` parameter with a `struct promisor_info *config_entry` pointer: - When NULL (ACCEPT_ALL mode): scan the whole `config_info` list, as the old `in_list == 1` path did. - When non-NULL: match against that single config entry directly, avoiding the redundant string_list_lookup() call. This removes the hidden dependency on `advertised->name` inside all_fields_match(), which would be wrong if in the future auto-configured remotes are implemented, as the local config name may differ from the server's advertised name. While at it, let's also add a comment before all_fields_match() and match_field_against_config() to help understand how things work and help avoid similar issues. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index 7ce7d22f952e70..6c935f855af752 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -575,6 +575,12 @@ enum accept_promisor { ACCEPT_ALL }; +/* + * Check if a specific field and its advertised value match the local + * configuration of a given promisor remote. + * + * Returns 1 if they match, 0 otherwise. + */ static int match_field_against_config(const char *field, const char *value, struct promisor_info *config_info) { @@ -586,9 +592,18 @@ static int match_field_against_config(const char *field, const char *value, return 0; } +/* + * Check that the advertised fields match the local configuration. + * + * When 'config_entry' is NULL (ACCEPT_ALL mode), every checked field + * must match at least one remote in 'config_info'. + * + * When 'config_entry' points to a specific remote's config, the + * checked fields are compared against that single remote only. + */ static int all_fields_match(struct promisor_info *advertised, struct string_list *config_info, - int in_list) + struct promisor_info *config_entry) { struct string_list *fields = fields_checked(); struct string_list_item *item_checked; @@ -597,7 +612,6 @@ static int all_fields_match(struct promisor_info *advertised, int match = 0; const char *field = item_checked->string; const char *value = NULL; - struct string_list_item *item; if (!strcasecmp(field, promisor_field_filter)) value = advertised->filter; @@ -607,7 +621,11 @@ static int all_fields_match(struct promisor_info *advertised, if (!value) return 0; - if (in_list) { + if (config_entry) { + match = match_field_against_config(field, value, + config_entry); + } else { + struct string_list_item *item; for_each_string_list_item(item, config_info) { struct promisor_info *p = item->util; if (match_field_against_config(field, value, p)) { @@ -615,12 +633,6 @@ static int all_fields_match(struct promisor_info *advertised, break; } } - } else { - item = string_list_lookup(config_info, advertised->name); - if (item) { - struct promisor_info *p = item->util; - match = match_field_against_config(field, value, p); - } } if (!match) @@ -640,7 +652,7 @@ static int should_accept_remote(enum accept_promisor accept, const char *remote_url = advertised->url; if (accept == ACCEPT_ALL) - return all_fields_match(advertised, config_info, 1); + return all_fields_match(advertised, config_info, NULL); /* Get config info for that promisor remote */ item = string_list_lookup(config_info, remote_name); @@ -652,7 +664,7 @@ static int should_accept_remote(enum accept_promisor accept, p = item->util; if (accept == ACCEPT_KNOWN_NAME) - return all_fields_match(advertised, config_info, 0); + return all_fields_match(advertised, config_info, p); if (accept != ACCEPT_KNOWN_URL) BUG("Unhandled 'enum accept_promisor' value '%d'", accept); @@ -663,7 +675,7 @@ static int should_accept_remote(enum accept_promisor accept, } if (!strcmp(p->url, remote_url)) - return all_fields_match(advertised, config_info, 0); + return all_fields_match(advertised, config_info, p); warning(_("known remote named '%s' but with URL '%s' instead of '%s'"), remote_name, p->url, remote_url); From 4ed9283b36bc8652954578c3024a00b6e70f8960 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:36 +0200 Subject: [PATCH 08/51] promisor-remote: clarify that a remote is ignored In should_accept_remote() and parse_one_advertised_remote(), when a remote is ignored, we tell users why it is ignored in a warning, but we don't tell them that the remote is actually ignored. Let's clarify that, so users have a better idea of what's actually happening. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index 6c935f855af752..8e062ec16098ac 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -670,15 +670,16 @@ static int should_accept_remote(enum accept_promisor accept, BUG("Unhandled 'enum accept_promisor' value '%d'", accept); if (!remote_url || !*remote_url) { - warning(_("no or empty URL advertised for remote '%s'"), remote_name); + warning(_("no or empty URL advertised for remote '%s', " + "ignoring this remote"), remote_name); return 0; } if (!strcmp(p->url, remote_url)) return all_fields_match(advertised, config_info, p); - warning(_("known remote named '%s' but with URL '%s' instead of '%s'"), - remote_name, p->url, remote_url); + warning(_("known remote named '%s' but with URL '%s' instead of '%s', " + "ignoring this remote"), remote_name, p->url, remote_url); return 0; } @@ -722,8 +723,8 @@ static struct promisor_info *parse_one_advertised_remote(const char *remote_info string_list_clear(&elem_list, 0); if (!info->name || !info->url) { - warning(_("server advertised a promisor remote without a name or URL: %s"), - remote_info); + warning(_("server advertised a promisor remote without a name or URL: '%s', " + "ignoring this remote"), remote_info); promisor_info_free(info); return NULL; } From 3b4f0403d19738a26f0da58f4efc6f4e2473fcac Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:37 +0200 Subject: [PATCH 09/51] promisor-remote: reject empty name or URL in advertised remote In parse_one_advertised_remote(), we check for a NULL remote name and remote URL, but not for empty ones. An empty URL seems possible as url_percent_decode("") doesn't return NULL. In promisor_config_info_list(), we ignore remotes with empty URLs, so a Git server should not advertise remotes with empty URLs. It's possible that a buggy or malicious server would do it though. So let's tighten the check in parse_one_advertised_remote() to also reject empty strings at parse time. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/promisor-remote.c b/promisor-remote.c index 8e062ec16098ac..8322349ae8ba87 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -722,7 +722,7 @@ static struct promisor_info *parse_one_advertised_remote(const char *remote_info string_list_clear(&elem_list, 0); - if (!info->name || !info->url) { + if (!info->name || !*info->name || !info->url || !*info->url) { warning(_("server advertised a promisor remote without a name or URL: '%s', " "ignoring this remote"), remote_info); promisor_info_free(info); From 64f0f6b88aea33546afd1271862b486fafe7e9cc Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:38 +0200 Subject: [PATCH 10/51] promisor-remote: refactor should_accept_remote() control flow A previous commit made sure we now reject empty URLs early at parse time. This makes the existing warning() in case a remote URL is NULL or empty very unlikely to be useful. In future work, we also plan to add URL-based acceptance logic into should_accept_remote(). To adapt to previous changes and prepare for upcoming changes, let's restructure the control flow in should_accept_remote(). Concretely, let's: - Replace the warning() in case of an empty URL with a BUG(), as a previous commit made sure empty URLs are rejected early at parse time. - Move that modified empty-URL check to the very top of the function, so that every acceptance mode, instead of only ACCEPT_KNOWN_URL, is covered. - Invert the URL comparison: instead of returning on match and warning on mismatch, return early on mismatch and let the match case fall through. This opens a single exit path at the bottom of the function for future commits to extend. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index 8322349ae8ba87..5860a3d3f36e09 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -651,6 +651,11 @@ static int should_accept_remote(enum accept_promisor accept, const char *remote_name = advertised->name; const char *remote_url = advertised->url; + if (!remote_url || !*remote_url) + BUG("no or empty URL advertised for remote '%s'; " + "this remote should have been rejected earlier", + remote_name); + if (accept == ACCEPT_ALL) return all_fields_match(advertised, config_info, NULL); @@ -669,19 +674,14 @@ static int should_accept_remote(enum accept_promisor accept, if (accept != ACCEPT_KNOWN_URL) BUG("Unhandled 'enum accept_promisor' value '%d'", accept); - if (!remote_url || !*remote_url) { - warning(_("no or empty URL advertised for remote '%s', " - "ignoring this remote"), remote_name); + if (strcmp(p->url, remote_url)) { + warning(_("known remote named '%s' but with URL '%s' instead of '%s', " + "ignoring this remote"), + remote_name, p->url, remote_url); return 0; } - if (!strcmp(p->url, remote_url)) - return all_fields_match(advertised, config_info, p); - - warning(_("known remote named '%s' but with URL '%s' instead of '%s', " - "ignoring this remote"), remote_name, p->url, remote_url); - - return 0; + return all_fields_match(advertised, config_info, p); } static int skip_field_name_prefix(const char *elem, const char *field_name, const char **value) From 16a4372a3df7579429b7bc23e984bd797a4b7b8d Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:39 +0200 Subject: [PATCH 11/51] promisor-remote: refactor has_control_char() In a future commit we are going to check if some strings contain control characters, so let's refactor the logic to do that in a new has_control_char() helper function. It cleans up the code a bit anyway. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index 5860a3d3f36e09..d60518f19c053e 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -642,6 +642,14 @@ static int all_fields_match(struct promisor_info *advertised, return 1; } +static bool has_control_char(const char *s) +{ + for (const char *c = s; *c; c++) + if (iscntrl(*c)) + return true; + return false; +} + static int should_accept_remote(enum accept_promisor accept, struct promisor_info *advertised, struct string_list *config_info) @@ -772,18 +780,14 @@ static bool valid_filter(const char *filter, const char *remote_name) return !res; } -/* Check that a token doesn't contain any control character */ static bool valid_token(const char *token, const char *remote_name) { - const char *c = token; - - for (; *c; c++) - if (iscntrl(*c)) { - warning(_("invalid token '%s' for remote '%s' " - "will not be stored"), - token, remote_name); - return false; - } + if (has_control_char(token)) { + warning(_("invalid token '%s' for remote '%s' " + "will not be stored"), + token, remote_name); + return false; + } return true; } From 7557a562434804d27f1417fe94c4081e2ee7e68b Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:40 +0200 Subject: [PATCH 12/51] promisor-remote: refactor accept_from_server() In future commits, we are going to add more logic to filter_promisor_remote() which is already doing a lot of things. Let's alleviate that by moving the logic that checks and validates the value of the `promisor.acceptFromServer` config variable into its own accept_from_server() helper function. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index d60518f19c053e..8d80ef6040534c 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -862,20 +862,12 @@ static bool promisor_store_advertised_fields(struct promisor_info *advertised, return reload_config; } -static void filter_promisor_remote(struct repository *repo, - struct strvec *accepted, - const char *info) +static enum accept_promisor accept_from_server(struct repository *repo) { const char *accept_str; enum accept_promisor accept = ACCEPT_NONE; - struct string_list config_info = STRING_LIST_INIT_NODUP; - struct string_list remote_info = STRING_LIST_INIT_DUP; - struct store_info *store_info = NULL; - struct string_list_item *item; - bool reload_config = false; - struct string_list accepted_filters = STRING_LIST_INIT_DUP; - if (!repo_config_get_string_tmp(the_repository, "promisor.acceptfromserver", &accept_str)) { + if (!repo_config_get_string_tmp(repo, "promisor.acceptfromserver", &accept_str)) { if (!*accept_str || !strcasecmp("None", accept_str)) accept = ACCEPT_NONE; else if (!strcasecmp("KnownUrl", accept_str)) @@ -889,6 +881,21 @@ static void filter_promisor_remote(struct repository *repo, accept_str, "promisor.acceptfromserver"); } + return accept; +} + +static void filter_promisor_remote(struct repository *repo, + struct strvec *accepted, + const char *info) +{ + struct string_list config_info = STRING_LIST_INIT_NODUP; + struct string_list remote_info = STRING_LIST_INIT_DUP; + struct store_info *store_info = NULL; + struct string_list_item *item; + bool reload_config = false; + struct string_list accepted_filters = STRING_LIST_INIT_DUP; + enum accept_promisor accept = accept_from_server(repo); + if (accept == ACCEPT_NONE) return; From e0f80d8876960442dd2645215c4fe5e1b1d80fc3 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:41 +0200 Subject: [PATCH 13/51] promisor-remote: keep accepted promisor_info structs alive In filter_promisor_remote(), the instances of `struct promisor_info` for accepted remotes are dismantled into separate parallel data structures (the 'accepted' strvec for server names, and 'accepted_filters' for filter strings) and then immediately freed. Instead, let's keep these instances on an 'accepted_remotes' list. This way the post-loop phase can iterate a single list to build the protocol reply, apply advertised filters, and mark remotes as accepted, rather than iterating three separate structures. This refactoring also prepares for a future commit that will add a 'local_name' member to 'struct promisor_info'. Since struct instances stay alive, downstream code will be able to simply read both names from them rather than needing yet another parallel strvec. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 42 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index 8d80ef6040534c..74e65e9dd0de48 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -890,10 +890,10 @@ static void filter_promisor_remote(struct repository *repo, { struct string_list config_info = STRING_LIST_INIT_NODUP; struct string_list remote_info = STRING_LIST_INIT_DUP; + struct string_list accepted_remotes = STRING_LIST_INIT_NODUP; struct store_info *store_info = NULL; struct string_list_item *item; bool reload_config = false; - struct string_list accepted_filters = STRING_LIST_INIT_DUP; enum accept_promisor accept = accept_from_server(repo); if (accept == ACCEPT_NONE) @@ -922,17 +922,10 @@ static void filter_promisor_remote(struct repository *repo, if (promisor_store_advertised_fields(advertised, store_info)) reload_config = true; - strvec_push(accepted, advertised->name); - - /* Capture advertised filters for accepted remotes */ - if (advertised->filter) { - struct string_list_item *i; - i = string_list_append(&accepted_filters, advertised->name); - i->util = xstrdup(advertised->filter); - } + string_list_append(&accepted_remotes, advertised->name)->util = advertised; + } else { + promisor_info_free(advertised); } - - promisor_info_free(advertised); } promisor_info_list_clear(&config_info); @@ -942,24 +935,23 @@ static void filter_promisor_remote(struct repository *repo, if (reload_config) repo_promisor_remote_reinit(repo); - /* Apply accepted remote filters to the stable repo state */ - for_each_string_list_item(item, &accepted_filters) { - struct promisor_remote *r = repo_promisor_remote_find(repo, item->string); - if (r) { - free(r->advertised_filter); - r->advertised_filter = item->util; - item->util = NULL; - } - } + /* Apply accepted remotes to the stable repo state */ + for_each_string_list_item(item, &accepted_remotes) { + struct promisor_info *info = item->util; + struct promisor_remote *r = repo_promisor_remote_find(repo, info->name); - string_list_clear(&accepted_filters, 1); + strvec_push(accepted, info->name); - /* Mark the remotes as accepted in the repository state */ - for (size_t i = 0; i < accepted->nr; i++) { - struct promisor_remote *r = repo_promisor_remote_find(repo, accepted->v[i]); - if (r) + if (r) { r->accepted = 1; + if (info->filter) { + free(r->advertised_filter); + r->advertised_filter = xstrdup(info->filter); + } + } } + + promisor_info_list_clear(&accepted_remotes); } void promisor_remote_reply(const char *info, char **accepted_out) From d56e483b03bfe46340af5cdbcddec8858661d2e9 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:42 +0200 Subject: [PATCH 14/51] promisor-remote: remove the 'accepted' strvec In a previous commit, filter_promisor_remote() was refactored to keep accepted 'struct promisor_info' instances alive instead of dismantling them into separate parallel data structures. Let's go one step further and replace the 'struct strvec *accepted' argument passed to filter_promisor_remote() with a 'struct string_list *accepted_remotes' argument. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index 74e65e9dd0de48..38fa05054227f6 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -885,12 +885,11 @@ static enum accept_promisor accept_from_server(struct repository *repo) } static void filter_promisor_remote(struct repository *repo, - struct strvec *accepted, + struct string_list *accepted_remotes, const char *info) { struct string_list config_info = STRING_LIST_INIT_NODUP; struct string_list remote_info = STRING_LIST_INIT_DUP; - struct string_list accepted_remotes = STRING_LIST_INIT_NODUP; struct store_info *store_info = NULL; struct string_list_item *item; bool reload_config = false; @@ -922,7 +921,7 @@ static void filter_promisor_remote(struct repository *repo, if (promisor_store_advertised_fields(advertised, store_info)) reload_config = true; - string_list_append(&accepted_remotes, advertised->name)->util = advertised; + string_list_append(accepted_remotes, advertised->name)->util = advertised; } else { promisor_info_free(advertised); } @@ -936,12 +935,10 @@ static void filter_promisor_remote(struct repository *repo, repo_promisor_remote_reinit(repo); /* Apply accepted remotes to the stable repo state */ - for_each_string_list_item(item, &accepted_remotes) { + for_each_string_list_item(item, accepted_remotes) { struct promisor_info *info = item->util; struct promisor_remote *r = repo_promisor_remote_find(repo, info->name); - strvec_push(accepted, info->name); - if (r) { r->accepted = 1; if (info->filter) { @@ -950,23 +947,23 @@ static void filter_promisor_remote(struct repository *repo, } } } - - promisor_info_list_clear(&accepted_remotes); } void promisor_remote_reply(const char *info, char **accepted_out) { - struct strvec accepted = STRVEC_INIT; + struct string_list accepted_remotes = STRING_LIST_INIT_NODUP; - filter_promisor_remote(the_repository, &accepted, info); + filter_promisor_remote(the_repository, &accepted_remotes, info); if (accepted_out) { - if (accepted.nr) { + if (accepted_remotes.nr) { struct strbuf reply = STRBUF_INIT; - for (size_t i = 0; i < accepted.nr; i++) { - if (i) + struct string_list_item *item; + + for_each_string_list_item(item, &accepted_remotes) { + if (reply.len) strbuf_addch(&reply, ';'); - strbuf_addstr_urlencode(&reply, accepted.v[i], allow_unsanitized); + strbuf_addstr_urlencode(&reply, item->string, allow_unsanitized); } *accepted_out = strbuf_detach(&reply, NULL); } else { @@ -974,7 +971,7 @@ void promisor_remote_reply(const char *info, char **accepted_out) } } - strvec_clear(&accepted); + promisor_info_list_clear(&accepted_remotes); } void mark_promisor_remotes_as_accepted(struct repository *r, const char *remotes) From 8eb863597f630efe08f96ed12f8defbe5a5f0b1d Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:43 +0200 Subject: [PATCH 15/51] t5710: use proper file:// URIs for absolute paths In t5710, we frequently construct local file URIs using `file://$(pwd)`. On Unix-like systems, $(pwd) returns an absolute path starting with a slash (e.g., `/tmp/repo`), resulting in a valid 3-slash URI with an empty host (`file:///tmp/repo`). However, on Windows, $(pwd) returns a path starting with a drive letter (e.g., `D:/a/repo`). This results in a 2-slash URI (`file://D:/a/repo`). Standard URI parsers misinterpret this format, treating `D:` as the host rather than part of the absolute path. This is to be expected because RFC 8089 says that the `//` prefix with an empty local host must be followed by an absolute path starting with a slash. While this hasn't broken the existing tests (because the old `promisor.acceptFromServer` logic relies entirely on strict `strcmp()` without normalizing the URLs), it will break future commits that pass these URLs through `url_normalize()` or similar functions. To future-proof the tests and ensure cross-platform URI compliance, let's introduce a $TRASH_DIRECTORY_URL helper variable that explicitly guarantees a leading slash for the path component, ensuring valid 3-slash `file:///` URIs on all operating systems. While at it, let's also introduce $ENCODED_TRASH_DIRECTORY_URL to handle some common special characters in directory paths. To be extra safe, let's skip all the tests if there are uncommon special characters in the directory path. Then let's replace all instances of `file://$(pwd)` with $TRASH_DIRECTORY_URL across the test script, and let's simplify the `sendFields` and `checkFields` tests to use $ENCODED_TRASH_DIRECTORY_URL directly. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- t/t5710-promisor-remote-capability.sh | 79 +++++++++++++++++---------- 1 file changed, 51 insertions(+), 28 deletions(-) diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh index bf0eed9f109742..b404ad9f0a9e3d 100755 --- a/t/t5710-promisor-remote-capability.sh +++ b/t/t5710-promisor-remote-capability.sh @@ -76,6 +76,31 @@ copy_to_lop () { cp "$path" "$path2" } +# On Windows, `pwd` returns a path like 'D:/foo/bar'. Prepend '/' to turn +# it into '/D:/foo/bar', which is what git expects in file:// URLs on Windows. +# On Unix, the path already starts with '/', so this is a no-op. +pwd_path=$(pwd) +case "$pwd_path" in +[a-zA-Z]:*) pwd_path="/$pwd_path" ;; +esac + +# Allowed characters: alphanumeric, standard path/URI (_ . ~ / : -), +# and those percent-encoded below (% space = , ;) +rest=$(printf "%s" "$pwd_path" | tr -d 'a-zA-Z0-9_.~/:% =,;-') +if test -n "$rest" +then + skip_all="PWD contains unsupported special characters" + test_done +fi + +TRASH_DIRECTORY_URL="file://$pwd_path" + +encoded_path=$(printf "%s" "$pwd_path" | + sed -e 's/%/%25/g' -e 's/ /%20/g' -e 's/=/%3D/g' \ + -e 's/;/%3B/g' -e 's/,/%2C/g') + +ENCODED_TRASH_DIRECTORY_URL="file://$encoded_path" + test_expect_success "setup for testing promisor remote advertisement" ' # Create another bare repo called "lop" (for Large Object Promisor) git init --bare lop && @@ -88,7 +113,7 @@ test_expect_success "setup for testing promisor remote advertisement" ' initialize_server 1 "$oid" && # Configure lop as promisor remote for server - git -C server remote add lop "file://$(pwd)/lop" && + git -C server remote add lop "$TRASH_DIRECTORY_URL/lop" && git -C server config remote.lop.promisor true && git -C lop config uploadpack.allowFilter true && @@ -104,7 +129,7 @@ test_expect_success "clone with promisor.advertise set to 'true'" ' # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=All \ --no-local --filter="blob:limit=5k" server client && @@ -119,7 +144,7 @@ test_expect_success "clone with promisor.advertise set to 'false'" ' # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=All \ --no-local --filter="blob:limit=5k" server client && @@ -137,7 +162,7 @@ test_expect_success "clone with promisor.acceptfromserver set to 'None'" ' # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=None \ --no-local --filter="blob:limit=5k" server client && @@ -156,8 +181,8 @@ test_expect_success "init + fetch with promisor.advertise set to 'true'" ' git -C client init && git -C client config remote.lop.promisor true && git -C client config remote.lop.fetch "+refs/heads/*:refs/remotes/lop/*" && - git -C client config remote.lop.url "file://$(pwd)/lop" && - git -C client config remote.server.url "file://$(pwd)/server" && + git -C client config remote.lop.url "$TRASH_DIRECTORY_URL/lop" && + git -C client config remote.server.url "$TRASH_DIRECTORY_URL/server" && git -C client config remote.server.fetch "+refs/heads/*:refs/remotes/server/*" && git -C client config promisor.acceptfromserver All && GIT_NO_LAZY_FETCH=0 git -C client fetch --filter="blob:limit=5k" server && @@ -177,10 +202,10 @@ test_expect_success "clone with two promisors but only one advertised" ' GIT_TRACE="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \ -c remote.unused_lop.promisor=true \ -c remote.unused_lop.fetch="+refs/heads/*:refs/remotes/unused_lop/*" \ - -c remote.unused_lop.url="file://$(pwd)/unused_lop" \ + -c remote.unused_lop.url="$TRASH_DIRECTORY_URL/unused_lop" \ -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=All \ --no-local --filter="blob:limit=5k" server client && @@ -210,11 +235,11 @@ test_expect_success "init + fetch two promisors but only one advertised" ' git -C client init && git -C client config remote.unused_lop.promisor true && git -C client config remote.unused_lop.fetch "+refs/heads/*:refs/remotes/unused_lop/*" && - git -C client config remote.unused_lop.url "file://$(pwd)/unused_lop" && + git -C client config remote.unused_lop.url "$TRASH_DIRECTORY_URL/unused_lop" && git -C client config remote.lop.promisor true && git -C client config remote.lop.fetch "+refs/heads/*:refs/remotes/lop/*" && - git -C client config remote.lop.url "file://$(pwd)/lop" && - git -C client config remote.server.url "file://$(pwd)/server" && + git -C client config remote.lop.url "$TRASH_DIRECTORY_URL/lop" && + git -C client config remote.server.url "$TRASH_DIRECTORY_URL/server" && git -C client config remote.server.fetch "+refs/heads/*:refs/remotes/server/*" && git -C client config promisor.acceptfromserver All && @@ -242,7 +267,7 @@ test_expect_success "clone with promisor.acceptfromserver set to 'KnownName'" ' # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=KnownName \ --no-local --filter="blob:limit=5k" server client && @@ -257,7 +282,7 @@ test_expect_success "clone with 'KnownName' and different remote names" ' # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.serverTwo.promisor=true \ -c remote.serverTwo.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.serverTwo.url="file://$(pwd)/lop" \ + -c remote.serverTwo.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=KnownName \ --no-local --filter="blob:limit=5k" server client && @@ -294,7 +319,7 @@ test_expect_success "clone with promisor.acceptfromserver set to 'KnownUrl'" ' # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=KnownUrl \ --no-local --filter="blob:limit=5k" server client && @@ -311,7 +336,7 @@ test_expect_success "clone with 'KnownUrl' and different remote urls" ' # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/serverTwo" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/serverTwo" \ -c promisor.acceptfromserver=KnownUrl \ --no-local --filter="blob:limit=5k" server client && @@ -326,7 +351,7 @@ test_expect_success "clone with 'KnownUrl' and url not configured on the server" git -C server config promisor.advertise true && test_when_finished "rm -rf client" && - test_when_finished "git -C server config set remote.lop.url \"file://$(pwd)/lop\"" && + test_when_finished "git -C server config set remote.lop.url \"$TRASH_DIRECTORY_URL/lop\"" && git -C server config unset remote.lop.url && # Clone from server to create a client @@ -335,7 +360,7 @@ test_expect_success "clone with 'KnownUrl' and url not configured on the server" # missing, so the remote name will be used instead which will fail. test_must_fail env GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=KnownUrl \ --no-local --filter="blob:limit=5k" server client && @@ -347,7 +372,7 @@ test_expect_success "clone with 'KnownUrl' and empty url, so not advertised" ' git -C server config promisor.advertise true && test_when_finished "rm -rf client" && - test_when_finished "git -C server config set remote.lop.url \"file://$(pwd)/lop\"" && + test_when_finished "git -C server config set remote.lop.url \"$TRASH_DIRECTORY_URL/lop\"" && git -C server config set remote.lop.url "" && # Clone from server to create a client @@ -356,7 +381,7 @@ test_expect_success "clone with 'KnownUrl' and empty url, so not advertised" ' # so the remote name will be used instead which will fail. test_must_fail env GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=KnownUrl \ --no-local --filter="blob:limit=5k" server client && @@ -380,13 +405,12 @@ test_expect_success "clone with promisor.sendFields" ' GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \ -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=All \ --no-local --filter="blob:limit=5k" server client && # Check that fields are properly transmitted - ENCODED_URL=$(echo "file://$(pwd)/lop" | sed -e "s/ /%20/g") && - PR1="name=lop,url=$ENCODED_URL,partialCloneFilter=blob:none" && + PR1="name=lop,url=$ENCODED_TRASH_DIRECTORY_URL/lop,partialCloneFilter=blob:none" && PR2="name=otherLop,url=https://invalid.invalid,partialCloneFilter=blob:limit=10k,token=fooBar" && test_grep "clone< promisor-remote=$PR1;$PR2" trace && test_grep "clone> promisor-remote=lop;otherLop" trace && @@ -411,15 +435,14 @@ test_expect_success "clone with promisor.checkFields" ' GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \ -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c remote.lop.partialCloneFilter="blob:none" \ -c promisor.acceptfromserver=All \ -c promisor.checkFields=partialcloneFilter \ --no-local --filter="blob:limit=5k" server client && # Check that fields are properly transmitted - ENCODED_URL=$(echo "file://$(pwd)/lop" | sed -e "s/ /%20/g") && - PR1="name=lop,url=$ENCODED_URL,partialCloneFilter=blob:none" && + PR1="name=lop,url=$ENCODED_TRASH_DIRECTORY_URL/lop,partialCloneFilter=blob:none" && PR2="name=otherLop,url=https://invalid.invalid,partialCloneFilter=blob:limit=10k,token=fooBar" && test_grep "clone< promisor-remote=$PR1;$PR2" trace && test_grep "clone> promisor-remote=lop" trace && @@ -449,7 +472,7 @@ test_expect_success "clone with promisor.storeFields=partialCloneFilter" ' GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \ -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c remote.lop.token="fooYYY" \ -c remote.lop.partialCloneFilter="blob:none" \ -c promisor.acceptfromserver=All \ @@ -501,7 +524,7 @@ test_expect_success "clone and fetch with --filter=auto" ' GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \ -c remote.lop.promisor=true \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=All \ --no-local --filter=auto server client 2>err && @@ -558,7 +581,7 @@ test_expect_success "clone with promisor.advertise set to 'true' but don't delet # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=All \ --no-local --filter="blob:limit=5k" server client && From b886f0b5dc71030bc9dcf58376533cf8e1098e9a Mon Sep 17 00:00:00 2001 From: Shreyansh Paliwal Date: Sat, 4 Apr 2026 19:28:38 +0530 Subject: [PATCH 16/51] refs: add struct repository parameter in get_files_ref_lock_timeout_ms() get_files_ref_lock_timeout_ms() calls repo_config_get_int() using the_repository, as no repository instance is available in its scope. Add a struct repository parameter and use it instead of the_repository. Update all callers accordingly. In files-backend.c, lock_raw_ref() can obtain repository instance from the struct ref_transaction via transaction->ref_store->repo and pass it down. For create_reflock(), which is used as a callback, introduce a small wrapper struct to pass both struct lock_file and struct repository through the callback data. This reduces reliance on the_repository global, though the function still uses static variables and is not yet fully repository-scoped. This can be addressed in a follow-up change. Signed-off-by: Shreyansh Paliwal Acked-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.c | 4 ++-- refs/files-backend.c | 19 +++++++++++++------ refs/refs-internal.h | 2 +- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/refs.c b/refs.c index 5d1d28523d617f..4ab746a3cb555c 100644 --- a/refs.c +++ b/refs.c @@ -989,7 +989,7 @@ enum ref_worktree_type parse_worktree_ref(const char *maybe_worktree_ref, return REF_WORKTREE_SHARED; } -long get_files_ref_lock_timeout_ms(void) +long get_files_ref_lock_timeout_ms(struct repository *repo) { static int configured = 0; @@ -997,7 +997,7 @@ long get_files_ref_lock_timeout_ms(void) static int timeout_ms = 100; if (!configured) { - repo_config_get_int(the_repository, "core.filesreflocktimeout", &timeout_ms); + repo_config_get_int(repo, "core.filesreflocktimeout", &timeout_ms); configured = 1; } diff --git a/refs/files-backend.c b/refs/files-backend.c index 0537a72b2af9e0..10e4388d2ca01a 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -792,7 +792,7 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs, if (hold_lock_file_for_update_timeout( &lock->lk, ref_file.buf, LOCK_NO_DEREF, - get_files_ref_lock_timeout_ms()) < 0) { + get_files_ref_lock_timeout_ms(transaction->ref_store->repo)) < 0) { int myerr = errno; errno = 0; if (myerr == ENOENT && --attempts_remaining > 0) { @@ -1190,13 +1190,17 @@ static int remove_empty_directories(struct strbuf *path) return remove_dir_recursively(path, REMOVE_DIR_EMPTY_ONLY); } +struct create_reflock_cb { + struct lock_file *lk; + struct repository *repo; +}; + static int create_reflock(const char *path, void *cb) { - struct lock_file *lk = cb; - + struct create_reflock_cb *data = cb; return hold_lock_file_for_update_timeout( - lk, path, LOCK_NO_DEREF, - get_files_ref_lock_timeout_ms()) < 0 ? -1 : 0; + data->lk, path, LOCK_NO_DEREF, + get_files_ref_lock_timeout_ms(data->repo)) < 0 ? -1 : 0; } /* @@ -1208,6 +1212,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs, { struct strbuf ref_file = STRBUF_INIT; struct ref_lock *lock; + struct create_reflock_cb cb_data; files_assert_main_repository(refs, "lock_ref_oid_basic"); assert(err); @@ -1229,8 +1234,10 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs, lock->ref_name = xstrdup(refname); lock->count = 1; + cb_data.lk = &lock->lk; + cb_data.repo = refs->base.repo; - if (raceproof_create_file(ref_file.buf, create_reflock, &lock->lk)) { + if (raceproof_create_file(ref_file.buf, create_reflock, &cb_data)) { unable_to_lock_message(ref_file.buf, errno, err); goto error_return; } diff --git a/refs/refs-internal.h b/refs/refs-internal.h index d79e35fd269a6c..e4cfd9e19ee74f 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -43,7 +43,7 @@ struct ref_transaction; * Return the length of time to retry acquiring a loose reference lock * before giving up, in milliseconds: */ -long get_files_ref_lock_timeout_ms(void); +long get_files_ref_lock_timeout_ms(struct repository *repo); /* * Return true iff refname is minimally safe. "Safe" here means that From 9a03f165a41d708c672e18e69d43f69689981e7d Mon Sep 17 00:00:00 2001 From: Shreyansh Paliwal Date: Sat, 4 Apr 2026 19:28:39 +0530 Subject: [PATCH 17/51] refs: remove the_hash_algo global state refs.c uses the_hash_algo in multiple places, relying on global state for the object hash algorithm. Replace these uses with the appropriate repository-specific hash_algo. In transaction-related functions (ref_transaction_create, ref_transaction_delete, migrate_one_ref, and transaction_hook_feed_stdin), use transaction->ref_store->repo->hash_algo. In other cases, such as repo_get_submodule_ref_store(), use repo->hash_algo. Signed-off-by: Shreyansh Paliwal Acked-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/refs.c b/refs.c index 4ab746a3cb555c..d13ca9a37c63e3 100644 --- a/refs.c +++ b/refs.c @@ -1472,7 +1472,7 @@ int ref_transaction_create(struct ref_transaction *transaction, return 1; } return ref_transaction_update(transaction, refname, new_oid, - null_oid(the_hash_algo), new_target, NULL, flags, + null_oid(transaction->ref_store->repo->hash_algo), new_target, NULL, flags, msg, err); } @@ -1491,7 +1491,7 @@ int ref_transaction_delete(struct ref_transaction *transaction, if (old_target && !(flags & REF_NO_DEREF)) BUG("delete cannot operate on symrefs with deref mode"); return ref_transaction_update(transaction, refname, - null_oid(the_hash_algo), old_oid, + null_oid(transaction->ref_store->repo->hash_algo), old_oid, NULL, old_target, flags, msg, err); } @@ -2379,7 +2379,7 @@ struct ref_store *repo_get_submodule_ref_store(struct repository *repo, subrepo = xmalloc(sizeof(*subrepo)); if (repo_submodule_init(subrepo, repo, submodule, - null_oid(the_hash_algo))) { + null_oid(repo->hash_algo))) { free(subrepo); goto done; } @@ -2571,14 +2571,14 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_ strbuf_reset(buf); if (!(update->flags & REF_HAVE_OLD)) - strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo))); + strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo))); else if (update->old_target) strbuf_addf(buf, "ref:%s ", update->old_target); else strbuf_addf(buf, "%s ", oid_to_hex(&update->old_oid)); if (!(update->flags & REF_HAVE_NEW)) - strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo))); + strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo))); else if (update->new_target) strbuf_addf(buf, "ref:%s ", update->new_target); else @@ -3146,6 +3146,7 @@ struct migration_data { static int migrate_one_ref(const struct reference *ref, void *cb_data) { struct migration_data *data = cb_data; + const struct git_hash_algo *hash_algo = data->transaction->ref_store->repo->hash_algo; struct strbuf symref_target = STRBUF_INIT; int ret; @@ -3154,7 +3155,7 @@ static int migrate_one_ref(const struct reference *ref, void *cb_data) if (ret < 0) goto done; - ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(the_hash_algo), + ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(hash_algo), symref_target.buf, NULL, REF_SKIP_CREATE_REFLOG | REF_NO_DEREF, NULL, data->errbuf); if (ret < 0) From 57c590feb96b2298e0966bea3ce88c72fca37bbd Mon Sep 17 00:00:00 2001 From: Shreyansh Paliwal Date: Sat, 4 Apr 2026 19:28:40 +0530 Subject: [PATCH 18/51] refs/reftable-backend: drop uses of the_repository reftable_be_init() and reftable_be_create_on_disk() use the_repository even though a repository instance is already available, either directly or via struct ref_store. Replace these uses with the appropriate local repository instance (repo or ref_store->repo) to avoid relying on global state. Note that USE_THE_REPOSITORY_VARIABLE cannot be removed yet, as is_bare_repository() is still there in the file. Signed-off-by: Shreyansh Paliwal Acked-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs/reftable-backend.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index b124404663edf6..7c8a992fcb40b9 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -404,13 +404,13 @@ static struct ref_store *reftable_be_init(struct repository *repo, default: BUG("unknown hash algorithm %d", repo->hash_algo->format_id); } - refs->write_options.default_permissions = calc_shared_perm(the_repository, 0666 & ~mask); + refs->write_options.default_permissions = calc_shared_perm(repo, 0666 & ~mask); refs->write_options.disable_auto_compact = !git_env_bool("GIT_TEST_REFTABLE_AUTOCOMPACTION", 1); refs->write_options.lock_timeout_ms = 100; refs->write_options.fsync = reftable_be_fsync; - repo_config(the_repository, reftable_be_config, &refs->write_options); + repo_config(repo, reftable_be_config, &refs->write_options); /* * It is somewhat unfortunate that we have to mirror the default block @@ -492,7 +492,7 @@ static int reftable_be_create_on_disk(struct ref_store *ref_store, struct strbuf sb = STRBUF_INIT; strbuf_addf(&sb, "%s/reftable", refs->base.gitdir); - safe_create_dir(the_repository, sb.buf, 1); + safe_create_dir(ref_store->repo, sb.buf, 1); strbuf_reset(&sb); strbuf_release(&sb); From 6077dc8a427950392be15a5507908d5e87a721a6 Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Thu, 9 Apr 2026 22:44:31 +0000 Subject: [PATCH 19/51] docs: update version with default Rust support We missed the cut-off for Rust by default in 2.53, but we still can enable it by default for 2.54, so update our breaking changes document accordingly. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano --- Documentation/BreakingChanges.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc index f814450d2f65ac..510ed98b65d755 100644 --- a/Documentation/BreakingChanges.adoc +++ b/Documentation/BreakingChanges.adoc @@ -190,7 +190,7 @@ milestones for the introduction of Rust: 1. Initially, with Git 2.52, support for Rust will be auto-detected by Meson and disabled in our Makefile so that the project can sort out the initial infrastructure. -2. In Git 2.53, both build systems will default-enable support for Rust. +2. In Git 2.54, both build systems will default-enable support for Rust. Consequently, builds will break by default if Rust is not available on the build host. The use of Rust can still be explicitly disabled via build flags. From 40c789dfc250486e60b7d7cdba47d8423a754abf Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Thu, 9 Apr 2026 22:44:32 +0000 Subject: [PATCH 20/51] ci: install cargo on Alpine We'll make Rust the default in a future commit, so be sure to install Cargo (which will also install Rust) to prepare for that case. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano --- ci/install-dependencies.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index c55441d9df91fd..10c3530d1aacdd 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -29,7 +29,7 @@ alpine-*) apk add --update shadow sudo meson ninja-build gcc libc-dev curl-dev openssl-dev expat-dev gettext \ zlib-ng-dev pcre2-dev python3 musl-libintl perl-utils ncurses \ apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \ - bash cvs gnupg perl-cgi perl-dbd-sqlite perl-io-tty >/dev/null + bash cvs gnupg perl-cgi perl-dbd-sqlite perl-io-tty cargo >/dev/null ;; fedora-*|almalinux-*) case "$jobname" in From 30e6f7adf626af926a02897294363dbf5f3bbe65 Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Thu, 9 Apr 2026 22:44:33 +0000 Subject: [PATCH 21/51] Linux: link against libdl Older versions of Rust on Linux, such as that used in Debian 11 in our CI, require linking against libdl. Were we linking with Cargo, this would be included automatically, but since we're not, explicitly set it in the system-specific config. This library is part of libc, so linking against it if it happens to be unnecessary will add no dependencies to the resulting binary. In addition, it is provided by both glibc and musl, so it should be portable to almost all Linux systems. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano --- config.mak.uname | 1 + 1 file changed, 1 insertion(+) diff --git a/config.mak.uname b/config.mak.uname index ccb3f718812740..7aab56c590069e 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -63,6 +63,7 @@ ifeq ($(uname_S),Linux) PROCFS_EXECUTABLE_PATH = /proc/self/exe HAVE_PLATFORM_PROCINFO = YesPlease COMPAT_OBJS += compat/linux/procinfo.o + EXTLIBS += -ldl # centos7/rhel7 provides gcc 4.8.5 and zlib 1.2.7. ifneq ($(findstring .el7.,$(uname_R)),) BASIC_CFLAGS += -std=c99 From 32d5b905909e781786c4735e6bd71503b23e4fb1 Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Thu, 9 Apr 2026 22:44:34 +0000 Subject: [PATCH 22/51] Enable Rust by default Our breaking changes document says that we'll enable Rust by default in Git 2.54. Adjust the Makefile to switch the option from WITH_RUST to NO_RUST to enable it by default and update the help text accordingly. Similarly, for Meson, enable the option by default and do not automatically disable it if Cargo is missing, since the goal is to help users find where they are likely to have problems in the future. Update our CI tests to swap out the single Linux job with Rust to a single job without, both for Makefile and Meson. Similarly, update the Windows Makefile job to not use Rust, while the Meson job (which does not build with ci/lib.sh) will default to having it enabled. Move the check for Cargo in the Meson build because it is no longer needed in the main script. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano --- Makefile | 10 +++++----- ci/lib.sh | 3 +++ ci/run-build-and-tests.sh | 6 ++++-- meson.build | 3 +-- meson_options.txt | 2 +- src/meson.build | 1 + 6 files changed, 15 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index dbf00220541ce1..84b59959dedc5a 100644 --- a/Makefile +++ b/Makefile @@ -498,9 +498,9 @@ include shared.mak # # == Optional Rust support == # -# Define WITH_RUST if you want to include features and subsystems written in -# Rust into Git. For now, Rust is still an optional feature of the build -# process. With Git 3.0 though, Rust will always be enabled. +# Define NO_RUST if you want to disable features and subsystems written in Rust +# from being compiled into Git. For now, Rust is still an optional feature of +# the build process. With Git 3.0 though, Rust will always be enabled. # # Building Rust code requires Cargo. # @@ -1351,7 +1351,7 @@ LIB_OBJS += urlmatch.o LIB_OBJS += usage.o LIB_OBJS += userdiff.o LIB_OBJS += utf8.o -ifndef WITH_RUST +ifdef NO_RUST LIB_OBJS += varint.o endif LIB_OBJS += version.o @@ -1590,7 +1590,7 @@ endif ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(CFLAGS_APPEND) ALL_LDFLAGS = $(LDFLAGS) $(LDFLAGS_APPEND) -ifdef WITH_RUST +ifndef NO_RUST BASIC_CFLAGS += -DWITH_RUST GITLIBS += $(RUST_LIB) ifeq ($(uname_S),Windows) diff --git a/ci/lib.sh b/ci/lib.sh index 42a2b6a318b874..1cfc8c6efce09e 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -372,6 +372,9 @@ linux-asan-ubsan) osx-meson) MESONFLAGS="$MESONFLAGS -Dcredential_helpers=osxkeychain" ;; +windows-*) + export NO_RUST=UnfortunatelyYes + ;; esac MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}" diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh index 28cfe730ee5aed..e2d783d90b0181 100755 --- a/ci/run-build-and-tests.sh +++ b/ci/run-build-and-tests.sh @@ -8,11 +8,12 @@ export TEST_CONTRIB_TOO=yes case "$jobname" in +linux-musl-meson) + MESONFLAGS="$MESONFLAGS -Drust=disabled" + ;; fedora-breaking-changes-musl|linux-breaking-changes) export WITH_BREAKING_CHANGES=YesPlease - export WITH_RUST=YesPlease MESONFLAGS="$MESONFLAGS -Dbreaking_changes=true" - MESONFLAGS="$MESONFLAGS -Drust=enabled" ;; linux-TEST-vars) export OPENSSL_SHA1_UNSAFE=YesPlease @@ -30,6 +31,7 @@ linux-TEST-vars) export GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL=1 ;; linux-clang) + export NO_RUST=UnfortunatelyYes export GIT_TEST_DEFAULT_HASH=sha1 ;; linux-sha256) diff --git a/meson.build b/meson.build index 8309942d184847..deff129cf6d33a 100644 --- a/meson.build +++ b/meson.build @@ -1745,8 +1745,7 @@ version_def_h = custom_target( ) libgit_sources += version_def_h -cargo = find_program('cargo', dirs: program_path, native: true, required: get_option('rust')) -rust_option = get_option('rust').disable_auto_if(not cargo.found()) +rust_option = get_option('rust') if rust_option.allowed() subdir('src') libgit_c_args += '-DWITH_RUST' diff --git a/meson_options.txt b/meson_options.txt index 659cbb218f46e0..80a8025f20be6e 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -77,7 +77,7 @@ option('zlib_backend', type: 'combo', choices: ['auto', 'zlib', 'zlib-ng'], valu # Build tweaks. option('breaking_changes', type: 'boolean', value: false, description: 'Enable upcoming breaking changes.') -option('rust', type: 'feature', value: 'auto', +option('rust', type: 'feature', value: 'enabled', description: 'Enable building with Rust.') option('macos_use_homebrew_gettext', type: 'boolean', value: true, description: 'Use gettext from Homebrew instead of the slightly-broken system-provided one.') diff --git a/src/meson.build b/src/meson.build index 45739957b451c9..41a4b231e660c4 100644 --- a/src/meson.build +++ b/src/meson.build @@ -29,6 +29,7 @@ libgit_rs = custom_target('git_rs', ) libgit_dependencies += declare_dependency(link_with: libgit_rs) +cargo = find_program('cargo', dirs: program_path, native: true, required: get_option('rust')) if get_option('tests') test('rust', cargo, args: [ From 8d2ffcf4b4a3a55c56c57c8df617516c25d98380 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Fri, 10 Apr 2026 12:05:56 +0300 Subject: [PATCH 23/51] repository: fix repo_init() memleak due to missing _clear() There is an old pre-existing memory leak in repo_init() due to failing to call clear_repository_format() in the error case. It went undetected because a specific bug is required to trigger it: enable a v1 extension in a repository with format v0. Obviously this can only happen in a development environment, so it does not trigger in normal usage, however the memleak is real and needs fixing. Fix it by also calling clear_repository_format() in the error case. Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- repository.c | 1 + 1 file changed, 1 insertion(+) diff --git a/repository.c b/repository.c index 9e5537f53961ed..192d6dc9c477fa 100644 --- a/repository.c +++ b/repository.c @@ -323,6 +323,7 @@ int repo_init(struct repository *repo, return 0; error: + clear_repository_format(&format); repo_clear(repo); return -1; } From 1c9e5b3fa235e0da6f62359af36afea8e7617074 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Fri, 10 Apr 2026 12:05:57 +0300 Subject: [PATCH 24/51] config: add a repo_config_get_uint() helper Next commits add a 'hook.jobs' config option of type 'unsigned int', so add a helper to parse it since the API only supports int and ulong. An alternative is to make 'hook.jobs' an 'int' or parse it as an 'int' then cast it to unsigned, however it's better to use proper helpers for the type. Using 'ulong' is another option which already has helpers, but it's a bit excessive in size for just the jobs number. Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- config.c | 28 ++++++++++++++++++++++++++++ config.h | 13 +++++++++++++ parse.c | 9 +++++++++ parse.h | 1 + 4 files changed, 51 insertions(+) diff --git a/config.c b/config.c index 156f2a24fa0027..a1b92fe083cf43 100644 --- a/config.c +++ b/config.c @@ -1212,6 +1212,15 @@ int git_config_int(const char *name, const char *value, return ret; } +unsigned int git_config_uint(const char *name, const char *value, + const struct key_value_info *kvi) +{ + unsigned int ret; + if (!git_parse_uint(value, &ret)) + die_bad_number(name, value, kvi); + return ret; +} + int64_t git_config_int64(const char *name, const char *value, const struct key_value_info *kvi) { @@ -1907,6 +1916,18 @@ int git_configset_get_int(struct config_set *set, const char *key, int *dest) return 1; } +int git_configset_get_uint(struct config_set *set, const char *key, unsigned int *dest) +{ + const char *value; + struct key_value_info kvi; + + if (!git_configset_get_value(set, key, &value, &kvi)) { + *dest = git_config_uint(key, value, &kvi); + return 0; + } else + return 1; +} + int git_configset_get_ulong(struct config_set *set, const char *key, unsigned long *dest) { const char *value; @@ -2356,6 +2377,13 @@ int repo_config_get_int(struct repository *repo, return git_configset_get_int(repo->config, key, dest); } +int repo_config_get_uint(struct repository *repo, + const char *key, unsigned int *dest) +{ + git_config_check_init(repo); + return git_configset_get_uint(repo->config, key, dest); +} + int repo_config_get_ulong(struct repository *repo, const char *key, unsigned long *dest) { diff --git a/config.h b/config.h index ba426a960af9f4..bf47fb3afc61bf 100644 --- a/config.h +++ b/config.h @@ -267,6 +267,12 @@ int git_config_int(const char *, const char *, const struct key_value_info *); int64_t git_config_int64(const char *, const char *, const struct key_value_info *); +/** + * Identical to `git_config_int`, but for unsigned ints. + */ +unsigned int git_config_uint(const char *, const char *, + const struct key_value_info *); + /** * Identical to `git_config_int`, but for unsigned longs. */ @@ -560,6 +566,7 @@ int git_configset_get_value(struct config_set *cs, const char *key, int git_configset_get_string(struct config_set *cs, const char *key, char **dest); int git_configset_get_int(struct config_set *cs, const char *key, int *dest); +int git_configset_get_uint(struct config_set *cs, const char *key, unsigned int *dest); int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned long *dest); int git_configset_get_bool(struct config_set *cs, const char *key, int *dest); int git_configset_get_bool_or_int(struct config_set *cs, const char *key, int *is_bool, int *dest); @@ -650,6 +657,12 @@ int repo_config_get_string_tmp(struct repository *r, */ int repo_config_get_int(struct repository *r, const char *key, int *dest); +/** + * Similar to `repo_config_get_int` but for unsigned ints. + */ +int repo_config_get_uint(struct repository *r, + const char *key, unsigned int *dest); + /** * Similar to `repo_config_get_int` but for unsigned longs. */ diff --git a/parse.c b/parse.c index 48313571aab129..d77f28046a0916 100644 --- a/parse.c +++ b/parse.c @@ -107,6 +107,15 @@ int git_parse_int64(const char *value, int64_t *ret) return 1; } +int git_parse_uint(const char *value, unsigned int *ret) +{ + uintmax_t tmp; + if (!git_parse_unsigned(value, &tmp, maximum_unsigned_value_of_type(unsigned int))) + return 0; + *ret = tmp; + return 1; +} + int git_parse_ulong(const char *value, unsigned long *ret) { uintmax_t tmp; diff --git a/parse.h b/parse.h index ea32de9a91fbfb..a6dd37c4cba273 100644 --- a/parse.h +++ b/parse.h @@ -5,6 +5,7 @@ int git_parse_signed(const char *value, intmax_t *ret, intmax_t max); int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max); int git_parse_ssize_t(const char *, ssize_t *); int git_parse_ulong(const char *, unsigned long *); +int git_parse_uint(const char *value, unsigned int *ret); int git_parse_int(const char *value, int *ret); int git_parse_int64(const char *value, int64_t *ret); int git_parse_double(const char *value, double *ret); From b9a4c9ad247a09602e0e6d0eccec6a43857f62da Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Fri, 10 Apr 2026 12:05:58 +0300 Subject: [PATCH 25/51] hook: parse the hook.jobs config The hook.jobs config is a global way to set hook parallelization for all hooks, in the sense that it is not per-event nor per-hook. Finer-grained configs will be added in later commits which can override it, for e.g. via a per-event type job options. Next commits will also add to this item's documentation. Parse hook.jobs config key in hook_config_lookup_all() and store its value in hook_all_config_cb.jobs, then transfer it into r->jobs after the config pass completes. This is mostly plumbing and the cached value is not yet used. Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/hook.adoc | 4 ++++ hook.c | 23 +++++++++++++++++++++-- repository.h | 3 +++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index 9e78f264396ca5..b7847f9338c65f 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -22,3 +22,7 @@ hook..enabled:: configuration. This is particularly useful when a hook is defined in a system or global config file and needs to be disabled for a specific repository. See linkgit:git-hook[1]. + +hook.jobs:: + Specifies how many hooks can be run simultaneously during parallelized + hook execution. If unspecified, defaults to 1 (serial execution). diff --git a/hook.c b/hook.c index cc23276d27f035..b8cce00e578d3c 100644 --- a/hook.c +++ b/hook.c @@ -123,11 +123,13 @@ struct hook_config_cache_entry { * commands: friendly-name to command map. * event_hooks: event-name to list of friendly-names map. * disabled_hooks: set of friendly-names with hook..enabled = false. + * jobs: value of the global hook.jobs key. Defaults to 0 if unset (stored in r->hook_jobs). */ struct hook_all_config_cb { struct strmap commands; struct strmap event_hooks; struct string_list disabled_hooks; + unsigned int jobs; }; /* repo_config() callback that collects all hook.* configuration in one pass. */ @@ -143,6 +145,20 @@ static int hook_config_lookup_all(const char *key, const char *value, if (parse_config_key(key, "hook", &name, &name_len, &subkey)) return 0; + /* Handle plain hook. entries that have no hook name component. */ + if (!name) { + if (!strcmp(subkey, "jobs") && value) { + unsigned int v; + if (!git_parse_uint(value, &v)) + warning(_("hook.jobs must be a positive integer, ignoring: '%s'"), value); + else if (!v) + warning(_("hook.jobs must be positive, ignoring: 0")); + else + data->jobs = v; + } + return 0; + } + if (!value) return config_error_nonbool(key); @@ -240,7 +256,7 @@ void hook_cache_clear(struct strmap *cache) /* Populate `cache` with the complete hook configuration */ static void build_hook_config_map(struct repository *r, struct strmap *cache) { - struct hook_all_config_cb cb_data; + struct hook_all_config_cb cb_data = { 0 }; struct hashmap_iter iter; struct strmap_entry *e; @@ -248,7 +264,7 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) strmap_init(&cb_data.event_hooks); string_list_init_dup(&cb_data.disabled_hooks); - /* Parse all configs in one run. */ + /* Parse all configs in one run, capturing hook.* including hook.jobs. */ repo_config(r, hook_config_lookup_all, &cb_data); /* Construct the cache from parsed configs. */ @@ -292,6 +308,9 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) strmap_put(cache, e->key, hooks); } + if (r) + r->hook_jobs = cb_data.jobs; + strmap_clear(&cb_data.commands, 1); string_list_clear(&cb_data.disabled_hooks, 0); strmap_for_each_entry(&cb_data.event_hooks, &iter, e) { diff --git a/repository.h b/repository.h index 078059a6e02b10..58e46853d089bf 100644 --- a/repository.h +++ b/repository.h @@ -172,6 +172,9 @@ struct repository { */ struct strmap *hook_config_cache; + /* Cached value of hook.jobs config (0 if unset, defaults to serial). */ + unsigned int hook_jobs; + /* Configurations related to promisor remotes. */ char *repository_format_partial_clone; struct promisor_remote_config *promisor_remote_config; From 680e69f60d2b3838bb98938dbd3e8881bdfde7d6 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Fri, 10 Apr 2026 12:05:59 +0300 Subject: [PATCH 26/51] hook: allow parallel hook execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hooks always run in sequential order due to the hardcoded jobs == 1 passed to run_process_parallel(). Remove that hardcoding to allow users to run hooks in parallel (opt-in). Users need to decide which hooks to run in parallel, by specifying "parallel = true" in the config, because Git cannot know if their specific hooks are safe to run or not in parallel (for e.g. two hooks might write to the same file or call the same program). Some hooks are unsafe to run in parallel by design: these will marked in the next commit using RUN_HOOKS_OPT_INIT_FORCE_SERIAL. The hook.jobs config specifies the default number of jobs applied to all hooks which have parallelism enabled. Signed-off-by: Emily Shaffer Helped-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/hook.adoc | 13 +++ hook.c | 79 ++++++++++++++++-- hook.h | 25 ++++++ t/t1800-hook.sh | 142 +++++++++++++++++++++++++++++++++ 4 files changed, 253 insertions(+), 6 deletions(-) diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index b7847f9338c65f..21800db648dca5 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -23,6 +23,19 @@ hook..enabled:: in a system or global config file and needs to be disabled for a specific repository. See linkgit:git-hook[1]. +hook..parallel:: + Whether the hook `hook.` may run in parallel with other hooks + for the same event. Defaults to `false`. Set to `true` only when the + hook script is safe to run concurrently with other hooks for the same + event. If any hook for an event does not have this set to `true`, + all hooks for that event run sequentially regardless of `hook.jobs`. + Only configured (named) hooks need to declare this. Traditional hooks + found in the hooks directory do not need to, and run in parallel when + the effective job count is greater than 1. See linkgit:git-hook[1]. + hook.jobs:: Specifies how many hooks can be run simultaneously during parallelized hook execution. If unspecified, defaults to 1 (serial execution). ++ +This setting has no effect unless all configured hooks for the event have +`hook..parallel` set to `true`. diff --git a/hook.c b/hook.c index b8cce00e578d3c..85c0de5e47b426 100644 --- a/hook.c +++ b/hook.c @@ -116,6 +116,7 @@ struct hook_config_cache_entry { char *command; enum config_scope scope; bool disabled; + bool parallel; }; /* @@ -123,12 +124,14 @@ struct hook_config_cache_entry { * commands: friendly-name to command map. * event_hooks: event-name to list of friendly-names map. * disabled_hooks: set of friendly-names with hook..enabled = false. + * parallel_hooks: friendly-name to parallel flag. * jobs: value of the global hook.jobs key. Defaults to 0 if unset (stored in r->hook_jobs). */ struct hook_all_config_cb { struct strmap commands; struct strmap event_hooks; struct string_list disabled_hooks; + struct strmap parallel_hooks; unsigned int jobs; }; @@ -219,6 +222,15 @@ static int hook_config_lookup_all(const char *key, const char *value, default: break; /* ignore unrecognised values */ } + } else if (!strcmp(subkey, "parallel")) { + int v = git_parse_maybe_bool(value); + if (v >= 0) + strmap_put(&data->parallel_hooks, hook_name, + (void *)(uintptr_t)v); + else + warning(_("hook.%s.parallel must be a boolean," + " ignoring: '%s'"), + hook_name, value); } free(hook_name); @@ -263,6 +275,7 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) strmap_init(&cb_data.commands); strmap_init(&cb_data.event_hooks); string_list_init_dup(&cb_data.disabled_hooks); + strmap_init(&cb_data.parallel_hooks); /* Parse all configs in one run, capturing hook.* including hook.jobs. */ repo_config(r, hook_config_lookup_all, &cb_data); @@ -282,6 +295,7 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) struct hook_config_cache_entry *entry; char *command; + bool is_par = !!strmap_get(&cb_data.parallel_hooks, hname); bool is_disabled = !!unsorted_string_list_lookup( &cb_data.disabled_hooks, hname); @@ -302,6 +316,7 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) entry->command = xstrdup_or_null(command); entry->scope = scope; entry->disabled = is_disabled; + entry->parallel = is_par; string_list_append(hooks, hname)->util = entry; } @@ -312,6 +327,7 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) r->hook_jobs = cb_data.jobs; strmap_clear(&cb_data.commands, 1); + strmap_clear(&cb_data.parallel_hooks, 0); /* values are uintptr_t, not heap ptrs */ string_list_clear(&cb_data.disabled_hooks, 0); strmap_for_each_entry(&cb_data.event_hooks, &iter, e) { string_list_clear(e->value, 0); @@ -389,6 +405,7 @@ static void list_hooks_add_configured(struct repository *r, entry->command ? xstrdup(entry->command) : NULL; hook->u.configured.scope = entry->scope; hook->u.configured.disabled = entry->disabled; + hook->parallel = entry->parallel; string_list_append(list, friendly_name)->util = hook; } @@ -538,21 +555,75 @@ static void run_hooks_opt_clear(struct run_hooks_opt *options) strvec_clear(&options->args); } +/* Determine how many jobs to use for hook execution. */ +static unsigned int get_hook_jobs(struct repository *r, + struct run_hooks_opt *options, + struct string_list *hook_list) +{ + /* + * Hooks needing separate output streams must run sequentially. + * Next commit will allow parallelizing these as well. + */ + if (!options->stdout_to_stderr) + return 1; + + /* + * An explicit job count overrides everything else: this covers both + * FORCE_SERIAL callers (for hooks that must never run in parallel) + * and the -j flag from the CLI. The CLI override is intentional: users + * may want to serialize hooks declared parallel or to parallelize more + * aggressively than the default. + */ + if (options->jobs) + return options->jobs; + + /* + * Use hook.jobs from the already-parsed config cache (in-repo), or + * fallback to a direct config lookup (out-of-repo). + * Default to 1 (serial execution) on failure. + */ + options->jobs = 1; + if (r) { + if (r->gitdir && r->hook_config_cache && r->hook_jobs) + options->jobs = r->hook_jobs; + else + repo_config_get_uint(r, "hook.jobs", &options->jobs); + } + + /* + * Cap to serial any configured hook not marked as parallel = true. + * This enforces the parallel = false default, even for "traditional" + * hooks from the hookdir which cannot be marked parallel = true. + */ + for (size_t i = 0; i < hook_list->nr; i++) { + struct hook *h = hook_list->items[i].util; + if (h->kind == HOOK_CONFIGURED && !h->parallel) { + options->jobs = 1; + break; + } + } + + return options->jobs; +} + int run_hooks_opt(struct repository *r, const char *hook_name, struct run_hooks_opt *options) { + struct string_list *hook_list = list_hooks(r, hook_name, options); struct hook_cb_data cb_data = { .rc = 0, .hook_name = hook_name, + .hook_command_list = hook_list, .options = options, }; int ret = 0; + unsigned int jobs = get_hook_jobs(r, options, hook_list); const struct run_process_parallel_opts opts = { .tr2_category = "hook", .tr2_label = hook_name, - .processes = options->jobs, - .ungroup = options->jobs == 1, + .processes = jobs, + .ungroup = jobs == 1, .get_next_task = pick_next_hook, .start_failure = notify_start_failure, @@ -568,9 +639,6 @@ int run_hooks_opt(struct repository *r, const char *hook_name, if (options->path_to_stdin && options->feed_pipe) BUG("options path_to_stdin and feed_pipe are mutually exclusive"); - if (!options->jobs) - BUG("run_hooks_opt must be called with options.jobs >= 1"); - /* * Ensure cb_data copy and free functions are either provided together, * or neither one is provided. @@ -581,7 +649,6 @@ int run_hooks_opt(struct repository *r, const char *hook_name, if (options->invoked_hook) *options->invoked_hook = 0; - cb_data.hook_command_list = list_hooks(r, hook_name, options); if (!cb_data.hook_command_list->nr) { if (options->error_if_missing) ret = error("cannot find a hook named %s", hook_name); diff --git a/hook.h b/hook.h index 5c5628dd1f822c..ba7056f8723b73 100644 --- a/hook.h +++ b/hook.h @@ -35,6 +35,13 @@ struct hook { } configured; } u; + /** + * Whether this hook may run in parallel with other hooks for the same + * event. Only useful for configured (named) hooks. Traditional hooks + * always default to 0 (serial). Set via `hook..parallel = true`. + */ + bool parallel; + /** * Opaque data pointer used to keep internal state across callback calls. * @@ -72,6 +79,8 @@ struct run_hooks_opt { * * If > 1, output will be buffered and de-interleaved (ungroup=0). * If == 1, output will be real-time (ungroup=1). + * If == 0, the 'hook.jobs' config is used or, if the config is unset, + * defaults to 1 (serial execution). */ unsigned int jobs; @@ -152,7 +161,23 @@ struct run_hooks_opt { hook_data_free_fn feed_pipe_cb_data_free; }; +/** + * Default initializer for hooks. Parallelism is opt-in: .jobs = 0 defers to + * the 'hook.jobs' config, falling back to serial (1) if unset. + */ #define RUN_HOOKS_OPT_INIT { \ + .env = STRVEC_INIT, \ + .args = STRVEC_INIT, \ + .stdout_to_stderr = 1, \ + .jobs = 0, \ +} + +/** + * Initializer for hooks that must always run sequentially regardless of + * 'hook.jobs'. Use this when git knows the hook cannot safely be parallelized + * .jobs = 1 is non-overridable. + */ +#define RUN_HOOKS_OPT_INIT_FORCE_SERIAL { \ .env = STRVEC_INIT, \ .args = STRVEC_INIT, \ .stdout_to_stderr = 1, \ diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 33decc66c0ea8d..a3011a01ca2908 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -21,6 +21,57 @@ setup_hookdir () { test_when_finished rm -rf .git/hooks } +# write_sentinel_hook [sentinel] +# +# Writes a hook that marks itself as started, sleeps for a few seconds, then +# marks itself done. The sleep must be long enough that sentinel_detector can +# observe .started before .done appears when both hooks +# run concurrently in parallel mode. +write_sentinel_hook () { + sentinel="${2:-sentinel}" + write_script "$1" <<-EOF + touch ${sentinel}.started && + sleep 2 && + touch ${sentinel}.done + EOF +} + +# sentinel_detector +# +# Returns a shell command string suitable for use as hook..command. +# The detector must be registered after the sentinel: +# 1. In serial mode, the sentinel has completed (and .done exists) +# before the detector starts. +# 2. In parallel mode, both run concurrently so .done has not appeared +# yet and the detector just sees .started. +# +# At start, poll until .started exists to absorb startup jitter, then +# write to : +# 1. 'serial' if .done exists (sentinel finished before we started), +# 2. 'parallel' if only .started exists (sentinel still running), +# 3. 'timeout' if .started never appeared. +# +# The command ends with ':' so when git appends "$@" for hooks that receive +# positional arguments (e.g. pre-push), the result ': "$@"' is valid shell +# rather than a syntax error 'fi "$@"'. +sentinel_detector () { + cat <<-EOF + i=0 + while ! test -f ${1}.started && test \$i -lt 10; do + sleep 1 + i=\$((i+1)) + done + if test -f ${1}.done; then + echo serial >${2} + elif test -f ${1}.started; then + echo parallel >${2} + else + echo timeout >${2} + fi + : + EOF +} + test_expect_success 'git hook usage' ' test_expect_code 129 git hook && test_expect_code 129 git hook run && @@ -658,4 +709,95 @@ test_expect_success 'server push-to-checkout hook expects stdout redirected to s check_stdout_merged_to_stderr push-to-checkout ' +test_expect_success 'hook.jobs=1 config runs hooks in series' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + + # Use two configured hooks so the execution order is deterministic: + # hook-1 (sentinel) is listed before hook-2 (detector), so hook-1 + # always runs first even in serial mode. + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + + test_config hook.jobs 1 && + + git hook run --allow-unknown-hook-name test-hook >out 2>err && + echo serial >expect && + test_cmp expect hook.order +' + +test_expect_success 'hook.jobs=2 config runs hooks in parallel' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_when_finished "rm -rf .git/hooks" && + + mkdir -p .git/hooks && + write_sentinel_hook .git/hooks/test-hook && + + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + test_config hook.hook-2.parallel true && + + test_config hook.jobs 2 && + + git hook run --allow-unknown-hook-name test-hook >out 2>err && + echo parallel >expect && + test_cmp expect hook.order +' + +test_expect_success 'hook..parallel=true enables parallel execution' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + test_config hook.hook-1.parallel true && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + test_config hook.hook-2.parallel true && + + test_config hook.jobs 2 && + + git hook run --allow-unknown-hook-name test-hook >out 2>err && + echo parallel >expect && + test_cmp expect hook.order +' + +test_expect_success 'hook..parallel=false (default) forces serial execution' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + + test_config hook.jobs 2 && + + git hook run --allow-unknown-hook-name test-hook >out 2>err && + echo serial >expect && + test_cmp expect hook.order +' + +test_expect_success 'one non-parallel hook forces the whole event to run serially' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + test_config hook.hook-1.parallel true && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + # hook-2 has no parallel=true: should force serial for all + + test_config hook.jobs 2 && + + git hook run --allow-unknown-hook-name test-hook >out 2>err && + echo serial >expect && + test_cmp expect hook.order +' + test_done From f776b77f0032fb342d567156626ef3fe9586443f Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Fri, 10 Apr 2026 12:06:00 +0300 Subject: [PATCH 27/51] hook: allow pre-push parallel execution pre-push is the only hook that keeps stdout and stderr separate (for backwards compatibility with git-lfs and potentially other users). This prevents parallelizing it because run-command needs stdout_to_stderr=1 to buffer and de-interleave parallel outputs. Since we now default to jobs=1, backwards compatibility is maintained without needing any extension or extra config: when no parallelism is requested, pre-push behaves exactly as before. When the user explicitly opts into parallelism via hook.jobs > 1, hook..jobs > 1, or -jN, they accept the changed output behavior. Document this and let get_hook_jobs() set stdout_to_stderr=1 automatically when jobs > 1, removing the need for any extension infrastructure. Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/hook.adoc | 4 ++++ hook.c | 24 ++++++++++++++++-------- hook.h | 6 ++++-- t/t1800-hook.sh | 32 ++++++++++++++++++++++++++++++++ transport.c | 6 ++++-- 5 files changed, 60 insertions(+), 12 deletions(-) diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index 21800db648dca5..94c7a9808e29ef 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -39,3 +39,7 @@ hook.jobs:: + This setting has no effect unless all configured hooks for the event have `hook..parallel` set to `true`. ++ +For `pre-push` hooks, which normally keep stdout and stderr separate, +setting this to a value greater than 1 (or passing `-j`) will merge stdout +into stderr to allow correct de-interleaving of parallel output. diff --git a/hook.c b/hook.c index 85c0de5e47b426..25762b6c8d18f9 100644 --- a/hook.c +++ b/hook.c @@ -555,18 +555,24 @@ static void run_hooks_opt_clear(struct run_hooks_opt *options) strvec_clear(&options->args); } +/* + * When running in parallel, stdout must be merged into stderr so + * run-command can buffer and de-interleave outputs correctly. This + * applies even to hooks like pre-push that normally keep stdout and + * stderr separate: the user has opted into parallelism, so the output + * stream behavior changes accordingly. + */ +static void merge_output_if_parallel(struct run_hooks_opt *options) +{ + if (options->jobs > 1) + options->stdout_to_stderr = 1; +} + /* Determine how many jobs to use for hook execution. */ static unsigned int get_hook_jobs(struct repository *r, struct run_hooks_opt *options, struct string_list *hook_list) { - /* - * Hooks needing separate output streams must run sequentially. - * Next commit will allow parallelizing these as well. - */ - if (!options->stdout_to_stderr) - return 1; - /* * An explicit job count overrides everything else: this covers both * FORCE_SERIAL callers (for hooks that must never run in parallel) @@ -575,7 +581,7 @@ static unsigned int get_hook_jobs(struct repository *r, * aggressively than the default. */ if (options->jobs) - return options->jobs; + goto cleanup; /* * Use hook.jobs from the already-parsed config cache (in-repo), or @@ -603,6 +609,8 @@ static unsigned int get_hook_jobs(struct repository *r, } } +cleanup: + merge_output_if_parallel(options); return options->jobs; } diff --git a/hook.h b/hook.h index ba7056f8723b73..01db4226a60306 100644 --- a/hook.h +++ b/hook.h @@ -106,8 +106,10 @@ struct run_hooks_opt { * Send the hook's stdout to stderr. * * This is the default behavior for all hooks except pre-push, - * which has separate stdout and stderr streams for backwards - * compatibility reasons. + * which keeps stdout and stderr separate for backwards compatibility. + * When parallel execution is requested (jobs > 1), get_hook_jobs() + * overrides this to 1 for all hooks so run-command can de-interleave + * their outputs correctly. */ unsigned int stdout_to_stderr:1; diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index a3011a01ca2908..4a978aff5e0c1e 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -800,4 +800,36 @@ test_expect_success 'one non-parallel hook forces the whole event to run seriall test_cmp expect hook.order ' +test_expect_success 'client hooks: pre-push parallel execution merges stdout to stderr' ' + test_when_finished "rm -rf remote-par stdout.actual stderr.actual" && + git init --bare remote-par && + git remote add origin-par remote-par && + test_commit par-commit && + mkdir -p .git/hooks && + setup_hooks pre-push && + test_config hook.jobs 2 && + git push origin-par HEAD:main >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr pre-push +' + +test_expect_success 'client hooks: pre-push runs in parallel when hook.jobs > 1' ' + test_when_finished "rm -rf repo-parallel remote-parallel" && + git init --bare remote-parallel && + git init repo-parallel && + git -C repo-parallel remote add origin ../remote-parallel && + test_commit -C repo-parallel A && + + write_sentinel_hook repo-parallel/.git/hooks/pre-push && + git -C repo-parallel config hook.hook-2.event pre-push && + git -C repo-parallel config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + git -C repo-parallel config hook.hook-2.parallel true && + + git -C repo-parallel config hook.jobs 2 && + + git -C repo-parallel push origin HEAD >out 2>err && + echo parallel >expect && + test_cmp expect repo-parallel/hook.order +' + test_done diff --git a/transport.c b/transport.c index e53936d87b641f..9406ec4f2d682a 100644 --- a/transport.c +++ b/transport.c @@ -1391,8 +1391,10 @@ static int run_pre_push_hook(struct transport *transport, opt.feed_pipe_cb_data_free = pre_push_hook_data_free; /* - * pre-push hooks expect stdout & stderr to be separate, so don't merge - * them to keep backwards compatibility with existing hooks. + * pre-push hooks keep stdout and stderr separate by default for + * backwards compatibility. When the user opts into parallel execution + * via hook.jobs > 1 or -j, get_hook_jobs() will set stdout_to_stderr=1 + * automatically so run-command can de-interleave the outputs. */ opt.stdout_to_stderr = 0; From ae25764e50f38b6625e11c3a7d7de290a0075b9c Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Fri, 10 Apr 2026 12:06:01 +0300 Subject: [PATCH 28/51] hook: mark non-parallelizable hooks Several hooks are known to be inherently non-parallelizable, so initialize them with RUN_HOOKS_OPT_INIT_FORCE_SERIAL. This pins jobs=1 and overrides any hook.jobs or runtime -j flags. These hooks are: applypatch-msg, pre-commit, prepare-commit-msg, commit-msg, post-commit, post-checkout, and push-to-checkout. Signed-off-by: Emily Shaffer Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/hook.adoc | 14 ++++++++++++++ builtin/am.c | 8 +++++--- builtin/checkout.c | 19 +++++++++++++------ builtin/clone.c | 6 ++++-- builtin/receive-pack.c | 3 ++- builtin/worktree.c | 2 +- commit.c | 2 +- t/t1800-hook.sh | 16 ++++++++++++++++ 8 files changed, 56 insertions(+), 14 deletions(-) diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index 94c7a9808e29ef..6f60775c28a902 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -36,6 +36,20 @@ hook..parallel:: hook.jobs:: Specifies how many hooks can be run simultaneously during parallelized hook execution. If unspecified, defaults to 1 (serial execution). + Some hooks always run sequentially regardless of this setting because + they operate on shared data and cannot safely be parallelized: ++ +-- +`applypatch-msg`;; +`prepare-commit-msg`;; +`commit-msg`;; + Receive a commit message file and may rewrite it in place. +`pre-commit`;; +`post-checkout`;; +`push-to-checkout`;; +`post-commit`;; + Access the working tree, index, or repository state. +-- + This setting has no effect unless all configured hooks for the event have `hook..parallel` set to `true`. diff --git a/builtin/am.c b/builtin/am.c index fe6e087eee9ff5..e9623b8307793f 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -490,9 +490,11 @@ static int run_applypatch_msg_hook(struct am_state *state) assert(state->msg); - if (!state->no_verify) - ret = run_hooks_l(the_repository, "applypatch-msg", - am_path(state, "final-commit"), NULL); + if (!state->no_verify) { + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_FORCE_SERIAL; + strvec_push(&opt.args, am_path(state, "final-commit")); + ret = run_hooks_opt(the_repository, "applypatch-msg", &opt); + } if (!ret) { FREE_AND_NULL(state->msg); diff --git a/builtin/checkout.c b/builtin/checkout.c index e031e6188613a6..ac0186a33e559a 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -31,6 +31,7 @@ #include "resolve-undo.h" #include "revision.h" #include "setup.h" +#include "strvec.h" #include "submodule.h" #include "symlinks.h" #include "trace2.h" @@ -123,13 +124,19 @@ static void branch_info_release(struct branch_info *info) static int post_checkout_hook(struct commit *old_commit, struct commit *new_commit, int changed) { - return run_hooks_l(the_repository, "post-checkout", - oid_to_hex(old_commit ? &old_commit->object.oid : null_oid(the_hash_algo)), - oid_to_hex(new_commit ? &new_commit->object.oid : null_oid(the_hash_algo)), - changed ? "1" : "0", NULL); - /* "new_commit" can be NULL when checking out from the index before - a commit exists. */ + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_FORCE_SERIAL; + /* + * "new_commit" can be NULL when checking out from the index before + * a commit exists. + */ + strvec_pushl(&opt.args, + oid_to_hex(old_commit ? &old_commit->object.oid : null_oid(the_hash_algo)), + oid_to_hex(new_commit ? &new_commit->object.oid : null_oid(the_hash_algo)), + changed ? "1" : "0", + NULL); + + return run_hooks_opt(the_repository, "post-checkout", &opt); } static int update_some(const struct object_id *oid, struct strbuf *base, diff --git a/builtin/clone.c b/builtin/clone.c index fba3c9c508bc06..d23b0cafcfec30 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -647,6 +647,7 @@ static int checkout(int submodule_progress, struct tree *tree; struct tree_desc t; int err = 0; + struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT_FORCE_SERIAL; if (option_no_checkout) return 0; @@ -697,8 +698,9 @@ static int checkout(int submodule_progress, if (write_locked_index(the_repository->index, &lock_file, COMMIT_LOCK)) die(_("unable to write new index file")); - err |= run_hooks_l(the_repository, "post-checkout", oid_to_hex(null_oid(the_hash_algo)), - oid_to_hex(&oid), "1", NULL); + strvec_pushl(&hook_opt.args, oid_to_hex(null_oid(the_hash_algo)), + oid_to_hex(&oid), "1", NULL); + err |= run_hooks_opt(the_repository, "post-checkout", &hook_opt); if (!err && (option_recurse_submodules.nr > 0)) { struct child_process cmd = CHILD_PROCESS_INIT; diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index dada55884a0b06..6da60f640ce3af 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1455,7 +1455,8 @@ static const char *push_to_checkout(unsigned char *hash, struct strvec *env, const char *work_tree) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_FORCE_SERIAL; + opt.invoked_hook = invoked_hook; strvec_pushf(env, "GIT_WORK_TREE=%s", absolute_path(work_tree)); diff --git a/builtin/worktree.c b/builtin/worktree.c index 4fd6f7575f9f76..d21c43fde38b5e 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -609,7 +609,7 @@ static int add_worktree(const char *path, const char *refname, * is_junk is cleared, but do return appropriate code when hook fails. */ if (!ret && opts->checkout && !opts->orphan) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_FORCE_SERIAL; strvec_pushl(&opt.env, "GIT_DIR", "GIT_WORK_TREE", NULL); strvec_pushl(&opt.args, diff --git a/commit.c b/commit.c index 80d8d078757dbc..4385ae4329e921 100644 --- a/commit.c +++ b/commit.c @@ -1970,7 +1970,7 @@ size_t ignored_log_message_bytes(const char *buf, size_t len) int run_commit_hook(int editor_is_used, const char *index_file, int *invoked_hook, const char *name, ...) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_FORCE_SERIAL; va_list args; const char *arg; diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 4a978aff5e0c1e..63fa25bca23c51 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -832,4 +832,20 @@ test_expect_success 'client hooks: pre-push runs in parallel when hook.jobs > 1' test_cmp expect repo-parallel/hook.order ' +test_expect_success 'hook.jobs=2 is ignored for force-serial hooks (pre-commit)' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event pre-commit && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + test_config hook.hook-1.parallel true && + test_config hook.hook-2.event pre-commit && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + test_config hook.hook-2.parallel true && + test_config hook.jobs 2 && + git commit --allow-empty -m "test: verify force-serial on pre-commit" && + echo serial >expect && + test_cmp expect hook.order +' + test_done From 091d2dbeb452b2c8223c622b54e96ebd273b5a78 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Fri, 10 Apr 2026 12:06:02 +0300 Subject: [PATCH 29/51] hook: add -j/--jobs option to git hook run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expose the parallel job count as a command-line flag so callers can request parallelism without relying only on the hook.jobs config. Add tests covering serial/parallel execution and TTY behaviour under -j1 vs -jN. Signed-off-by: Emily Shaffer Helped-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/git-hook.adoc | 23 +++++- builtin/hook.c | 5 +- hook.c | 17 +++++ t/t1800-hook.sh | 135 ++++++++++++++++++++++++++++++++++-- 4 files changed, 170 insertions(+), 10 deletions(-) diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc index 318c637bd8eba5..46ea52db55f268 100644 --- a/Documentation/git-hook.adoc +++ b/Documentation/git-hook.adoc @@ -8,7 +8,8 @@ git-hook - Run git hooks SYNOPSIS -------- [verse] -'git hook' run [--allow-unknown-hook-name] [--ignore-missing] [--to-stdin=] [-- ] +'git hook' run [--allow-unknown-hook-name] [--ignore-missing] [--to-stdin=] [(-j|--jobs) ] + [-- ] 'git hook' list [--allow-unknown-hook-name] [-z] [--show-scope] DESCRIPTION @@ -147,6 +148,23 @@ OPTIONS mirroring the output style of `git config --show-scope`. Traditional hooks from the hookdir are unaffected. +-j:: +--jobs:: + Only valid for `run`. ++ +Specify how many hooks to run simultaneously. If this flag is not specified, +the value of the `hook.jobs` config is used, see linkgit:git-config[1]. If +neither is specified, defaults to 1 (serial execution). ++ +When greater than 1, it overrides the per-hook `hook..parallel` +setting, allowing all hooks for the event to run concurrently, even if they +are not individually marked as parallel. ++ +Some hooks always run sequentially regardless of this flag or the +`hook.jobs` config, because git knows they cannot safely run in parallel: +`applypatch-msg`, `pre-commit`, `prepare-commit-msg`, `commit-msg`, +`post-commit`, `post-checkout`, and `push-to-checkout`. + WRAPPERS -------- @@ -169,7 +187,8 @@ running: git hook run --allow-unknown-hook-name mywrapper-start-tests \ # providing something to stdin --stdin some-tempfile-123 \ - # execute hooks in serial + # execute multiple hooks in parallel + --jobs 3 \ # plus some arguments of your own... -- \ --testname bar \ diff --git a/builtin/hook.c b/builtin/hook.c index c0585587e5e4fa..bea0668b475931 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -8,7 +8,8 @@ #include "parse-options.h" #define BUILTIN_HOOK_RUN_USAGE \ - N_("git hook run [--allow-unknown-hook-name] [--ignore-missing] [--to-stdin=] [-- ]") + N_("git hook run [--allow-unknown-hook-name] [--ignore-missing] [--to-stdin=] [(-j|--jobs) ]\n" \ + " [-- ]") #define BUILTIN_HOOK_LIST_USAGE \ N_("git hook list [--allow-unknown-hook-name] [-z] [--show-scope] ") @@ -132,6 +133,8 @@ static int run(int argc, const char **argv, const char *prefix, N_("silently ignore missing requested ")), OPT_STRING(0, "to-stdin", &opt.path_to_stdin, N_("path"), N_("file to read into hooks' stdin")), + OPT_UNSIGNED('j', "jobs", &opt.jobs, + N_("run up to hooks simultaneously")), OPT_END(), }; int ret; diff --git a/hook.c b/hook.c index 25762b6c8d18f9..c0b71322cf2ef6 100644 --- a/hook.c +++ b/hook.c @@ -568,6 +568,22 @@ static void merge_output_if_parallel(struct run_hooks_opt *options) options->stdout_to_stderr = 1; } +static void warn_non_parallel_hooks_override(unsigned int jobs, + struct string_list *hook_list) +{ + /* Don't warn for hooks running sequentially. */ + if (jobs == 1) + return; + + for (size_t i = 0; i < hook_list->nr; i++) { + struct hook *h = hook_list->items[i].util; + if (h->kind == HOOK_CONFIGURED && !h->parallel) + warning(_("hook '%s' is not marked as parallel=true, " + "running in parallel anyway due to -j%u"), + h->u.configured.friendly_name, jobs); + } +} + /* Determine how many jobs to use for hook execution. */ static unsigned int get_hook_jobs(struct repository *r, struct run_hooks_opt *options, @@ -611,6 +627,7 @@ static unsigned int get_hook_jobs(struct repository *r, cleanup: merge_output_if_parallel(options); + warn_non_parallel_hooks_override(options->jobs, hook_list); return options->jobs; } diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 63fa25bca23c51..aa37a5181a0e0e 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -268,10 +268,20 @@ test_expect_success 'git -c core.hooksPath= hook run' ' ' test_hook_tty () { - cat >expect <<-\EOF - STDOUT TTY - STDERR TTY - EOF + expect_tty=$1 + shift + + if test "$expect_tty" != "no_tty"; then + cat >expect <<-\EOF + STDOUT TTY + STDERR TTY + EOF + else + cat >expect <<-\EOF + STDOUT NO TTY + STDERR NO TTY + EOF + fi test_when_finished "rm -rf repo" && git init repo && @@ -289,12 +299,21 @@ test_hook_tty () { test_cmp expect repo/actual } -test_expect_success TTY 'git hook run: stdout and stderr are connected to a TTY' ' - test_hook_tty hook run pre-commit +test_expect_success TTY 'git hook run -j1: stdout and stderr are connected to a TTY' ' + # hooks running sequentially (-j1) are always connected to the tty for + # optimum real-time performance. + test_hook_tty tty hook run -j1 pre-commit +' + +test_expect_success TTY 'git hook run -jN: stdout and stderr are not connected to a TTY' ' + # Hooks are not connected to the tty when run in parallel, instead they + # output to a pipe through which run-command collects and de-interlaces + # their outputs, which then gets passed either to the tty or a sideband. + test_hook_tty no_tty hook run -j2 pre-commit ' test_expect_success TTY 'git commit: stdout and stderr are connected to a TTY' ' - test_hook_tty commit -m"B.new" + test_hook_tty tty commit -m"B.new" ' test_expect_success 'git hook list orders by config order' ' @@ -709,6 +728,108 @@ test_expect_success 'server push-to-checkout hook expects stdout redirected to s check_stdout_merged_to_stderr push-to-checkout ' +test_expect_success 'parallel hook output is not interleaved' ' + test_when_finished "rm -rf .git/hooks" && + + write_script .git/hooks/test-hook <<-EOF && + echo "Hook 1 Start" + sleep 1 + echo "Hook 1 End" + EOF + + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "echo \"Hook 2 Start\"; sleep 2; echo \"Hook 2 End\"" && + test_config hook.hook-2.parallel true && + test_config hook.hook-3.event test-hook && + test_config hook.hook-3.command \ + "echo \"Hook 3 Start\"; sleep 3; echo \"Hook 3 End\"" && + test_config hook.hook-3.parallel true && + + git hook run --allow-unknown-hook-name -j3 test-hook >out 2>err.parallel && + + # Verify Hook 1 output is grouped + sed -n "/Hook 1 Start/,/Hook 1 End/p" err.parallel >hook1_out && + test_line_count = 2 hook1_out && + + # Verify Hook 2 output is grouped + sed -n "/Hook 2 Start/,/Hook 2 End/p" err.parallel >hook2_out && + test_line_count = 2 hook2_out && + + # Verify Hook 3 output is grouped + sed -n "/Hook 3 Start/,/Hook 3 End/p" err.parallel >hook3_out && + test_line_count = 2 hook3_out +' + +test_expect_success 'git hook run -j1 runs hooks in series' ' + test_when_finished "rm -rf .git/hooks" && + + test_config hook.series-1.event "test-hook" && + test_config hook.series-1.command "echo 1" --add && + test_config hook.series-2.event "test-hook" && + test_config hook.series-2.command "echo 2" --add && + + mkdir -p .git/hooks && + write_script .git/hooks/test-hook <<-EOF && + echo 3 + EOF + + cat >expected <<-\EOF && + 1 + 2 + 3 + EOF + + git hook run --allow-unknown-hook-name -j1 test-hook 2>actual && + test_cmp expected actual +' + +test_expect_success 'git hook run -j2 runs hooks in parallel' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_when_finished "rm -rf .git/hooks" && + + mkdir -p .git/hooks && + write_sentinel_hook .git/hooks/test-hook && + + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + test_config hook.hook-2.parallel true && + + git hook run --allow-unknown-hook-name -j2 test-hook >out 2>err && + echo parallel >expect && + test_cmp expect hook.order +' + +test_expect_success 'git hook run -j2 overrides parallel=false' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + # hook-1 intentionally has no parallel=true + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + # hook-2 also has no parallel=true + + # -j2 overrides parallel=false; hooks run in parallel with a warning. + git hook run --allow-unknown-hook-name -j2 test-hook >out 2>err && + echo parallel >expect && + test_cmp expect hook.order +' + +test_expect_success 'git hook run -j2 warns for hooks not marked parallel=true' ' + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command "true" && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command "true" && + # neither hook has parallel=true + + git hook run --allow-unknown-hook-name -j2 test-hook >out 2>err && + grep "hook .hook-1. is not marked as parallel=true" err && + grep "hook .hook-2. is not marked as parallel=true" err +' + test_expect_success 'hook.jobs=1 config runs hooks in series' ' test_when_finished "rm -f sentinel.started sentinel.done hook.order" && From 084a55b3adf33f70c84091d5957b8bede9b01174 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Fri, 10 Apr 2026 12:06:03 +0300 Subject: [PATCH 30/51] hook: add per-event jobs config Add a hook..jobs count config that allows users to override the global hook.jobs setting for specific hook events. This allows finer-grained control over parallelism on a per-event basis. For example, to run `post-receive` hooks with up to 4 parallel jobs while keeping other events at their global default: [hook] post-receive.jobs = 4 Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/hook.adoc | 19 +++++++++++ hook.c | 46 +++++++++++++++++++++++--- repository.c | 1 + repository.h | 3 ++ t/t1800-hook.sh | 59 ++++++++++++++++++++++++++++++++++ 5 files changed, 123 insertions(+), 5 deletions(-) diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index 6f60775c28a902..d4fa29d936d6e2 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -33,9 +33,28 @@ hook..parallel:: found in the hooks directory do not need to, and run in parallel when the effective job count is greater than 1. See linkgit:git-hook[1]. +hook..jobs:: + Specifies how many hooks can be run simultaneously for the `` + hook event (e.g. `hook.post-receive.jobs = 4`). Overrides `hook.jobs` + for this specific event. The same parallelism restrictions apply: this + setting has no effect unless all configured hooks for the event have + `hook..parallel` set to `true`. Must be a positive int, + zero is rejected with a warning. See linkgit:git-hook[1]. ++ +Note on naming: although this key resembles `hook..*` +(a per-hook setting), `` must be the event name, not a hook +friendly name. The key component is stored literally and looked up by +event name at runtime with no translation between the two namespaces. +A key like `hook.my-hook.jobs` is stored under `"my-hook"` but the +lookup at runtime uses the event name (e.g. `"post-receive"`), so +`hook.my-hook.jobs` is silently ignored even when `my-hook` is +registered for that event. Use `hook.post-receive.jobs` or any other +valid event name when setting `hook..jobs`. + hook.jobs:: Specifies how many hooks can be run simultaneously during parallelized hook execution. If unspecified, defaults to 1 (serial execution). + Can be overridden on a per-event basis with `hook..jobs`. Some hooks always run sequentially regardless of this setting because they operate on shared data and cannot safely be parallelized: + diff --git a/hook.c b/hook.c index c0b71322cf2ef6..d98b01156366a4 100644 --- a/hook.c +++ b/hook.c @@ -125,6 +125,7 @@ struct hook_config_cache_entry { * event_hooks: event-name to list of friendly-names map. * disabled_hooks: set of friendly-names with hook..enabled = false. * parallel_hooks: friendly-name to parallel flag. + * event_jobs: event-name to per-event jobs count (stored as uintptr_t, NULL == unset). * jobs: value of the global hook.jobs key. Defaults to 0 if unset (stored in r->hook_jobs). */ struct hook_all_config_cb { @@ -132,6 +133,7 @@ struct hook_all_config_cb { struct strmap event_hooks; struct string_list disabled_hooks; struct strmap parallel_hooks; + struct strmap event_jobs; unsigned int jobs; }; @@ -231,6 +233,18 @@ static int hook_config_lookup_all(const char *key, const char *value, warning(_("hook.%s.parallel must be a boolean," " ignoring: '%s'"), hook_name, value); + } else if (!strcmp(subkey, "jobs")) { + unsigned int v; + if (!git_parse_uint(value, &v)) + warning(_("hook.%s.jobs must be a positive integer," + " ignoring: '%s'"), + hook_name, value); + else if (!v) + warning(_("hook.%s.jobs must be positive," + " ignoring: 0"), hook_name); + else + strmap_put(&data->event_jobs, hook_name, + (void *)(uintptr_t)v); } free(hook_name); @@ -276,6 +290,7 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) strmap_init(&cb_data.event_hooks); string_list_init_dup(&cb_data.disabled_hooks); strmap_init(&cb_data.parallel_hooks); + strmap_init(&cb_data.event_jobs); /* Parse all configs in one run, capturing hook.* including hook.jobs. */ repo_config(r, hook_config_lookup_all, &cb_data); @@ -323,8 +338,10 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) strmap_put(cache, e->key, hooks); } - if (r) + if (r) { r->hook_jobs = cb_data.jobs; + r->event_jobs = cb_data.event_jobs; + } strmap_clear(&cb_data.commands, 1); strmap_clear(&cb_data.parallel_hooks, 0); /* values are uintptr_t, not heap ptrs */ @@ -587,6 +604,7 @@ static void warn_non_parallel_hooks_override(unsigned int jobs, /* Determine how many jobs to use for hook execution. */ static unsigned int get_hook_jobs(struct repository *r, struct run_hooks_opt *options, + const char *hook_name, struct string_list *hook_list) { /* @@ -606,16 +624,34 @@ static unsigned int get_hook_jobs(struct repository *r, */ options->jobs = 1; if (r) { - if (r->gitdir && r->hook_config_cache && r->hook_jobs) - options->jobs = r->hook_jobs; - else + if (r->gitdir && r->hook_config_cache) { + void *event_jobs; + + if (r->hook_jobs) + options->jobs = r->hook_jobs; + + event_jobs = strmap_get(&r->event_jobs, hook_name); + if (event_jobs) + options->jobs = (unsigned int)(uintptr_t)event_jobs; + } else { + unsigned int event_jobs; + char *key; + repo_config_get_uint(r, "hook.jobs", &options->jobs); + + key = xstrfmt("hook.%s.jobs", hook_name); + if (!repo_config_get_uint(r, key, &event_jobs) && event_jobs) + options->jobs = event_jobs; + free(key); + } } /* * Cap to serial any configured hook not marked as parallel = true. * This enforces the parallel = false default, even for "traditional" * hooks from the hookdir which cannot be marked parallel = true. + * The same restriction applies whether jobs came from hook.jobs or + * hook..jobs. */ for (size_t i = 0; i < hook_list->nr; i++) { struct hook *h = hook_list->items[i].util; @@ -642,7 +678,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name, .options = options, }; int ret = 0; - unsigned int jobs = get_hook_jobs(r, options, hook_list); + unsigned int jobs = get_hook_jobs(r, options, hook_name, hook_list); const struct run_process_parallel_opts opts = { .tr2_category = "hook", .tr2_label = hook_name, diff --git a/repository.c b/repository.c index 192d6dc9c477fa..4030db4460714d 100644 --- a/repository.c +++ b/repository.c @@ -426,6 +426,7 @@ void repo_clear(struct repository *repo) hook_cache_clear(repo->hook_config_cache); FREE_AND_NULL(repo->hook_config_cache); } + strmap_clear(&repo->event_jobs, 0); /* values are uintptr_t, not heap ptrs */ if (repo->promisor_remote_config) { promisor_remote_clear(repo->promisor_remote_config); diff --git a/repository.h b/repository.h index 58e46853d089bf..6b67ec02e2984c 100644 --- a/repository.h +++ b/repository.h @@ -175,6 +175,9 @@ struct repository { /* Cached value of hook.jobs config (0 if unset, defaults to serial). */ unsigned int hook_jobs; + /* Cached map of event-name -> jobs count (as uintptr_t) from hook..jobs. */ + struct strmap event_jobs; + /* Configurations related to promisor remotes. */ char *repository_format_partial_clone; struct promisor_remote_config *promisor_remote_config; diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index aa37a5181a0e0e..24a3c92b6deb80 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -969,4 +969,63 @@ test_expect_success 'hook.jobs=2 is ignored for force-serial hooks (pre-commit)' test_cmp expect hook.order ' +test_expect_success 'hook..jobs overrides hook.jobs for that event' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + test_config hook.hook-1.parallel true && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + test_config hook.hook-2.parallel true && + + # Global hook.jobs=1 (serial), but per-event override allows parallel. + test_config hook.jobs 1 && + test_config hook.test-hook.jobs 2 && + + git hook run --allow-unknown-hook-name test-hook >out 2>err && + echo parallel >expect && + test_cmp expect hook.order +' + +test_expect_success 'hook..jobs=1 forces serial even when hook.jobs>1' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + test_config hook.hook-1.parallel true && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + test_config hook.hook-2.parallel true && + + # Global hook.jobs=4 allows parallel, but per-event override forces serial. + test_config hook.jobs 4 && + test_config hook.test-hook.jobs 1 && + + git hook run --allow-unknown-hook-name test-hook >out 2>err && + echo serial >expect && + test_cmp expect hook.order +' + +test_expect_success 'hook..jobs still requires hook..parallel=true' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + # hook-1 intentionally has no parallel=true + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + # hook-2 also has no parallel=true + + # Per-event jobs=2 but no hook has parallel=true: must still run serially. + test_config hook.test-hook.jobs 2 && + + git hook run --allow-unknown-hook-name test-hook >out 2>err && + echo serial >expect && + test_cmp expect hook.order +' + test_done From 5e57b209ff21bf1087dd8539c458737c89b03150 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Fri, 10 Apr 2026 12:06:04 +0300 Subject: [PATCH 31/51] hook: warn when hook..jobs is set Issue a warning when the user confuses the hook process and event namespaces by setting hook..jobs. Detect this by checking whether the name carrying .jobs also has .command, .event, or .parallel configured. Extract is_friendly_name() as a helper for this check, to be reused by future per-event config handling. Suggested-by: Junio C Hamano Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- hook.c | 40 ++++++++++++++++++++++++++++++++++++++++ t/t1800-hook.sh | 30 ++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/hook.c b/hook.c index d98b01156366a4..0493993bbe6738 100644 --- a/hook.c +++ b/hook.c @@ -279,6 +279,44 @@ void hook_cache_clear(struct strmap *cache) strmap_clear(cache, 0); } +/* + * Return true if `name` is a hook friendly-name, i.e. it has at least one of + * .command, .event, or .parallel configured. These are the reliable clues + * that distinguish a friendly-name from an event name. Note: .enabled is + * deliberately excluded because it can appear under both namespaces. + */ +static int is_friendly_name(struct hook_all_config_cb *cb, const char *name) +{ + struct hashmap_iter iter; + struct strmap_entry *e; + + if (strmap_get(&cb->commands, name) || strmap_get(&cb->parallel_hooks, name)) + return 1; + + strmap_for_each_entry(&cb->event_hooks, &iter, e) { + if (unsorted_string_list_lookup(e->value, name)) + return 1; + } + + return 0; +} + +/* Warn if any name in event_jobs is also a hook friendly-name. */ +static void warn_jobs_on_friendly_names(struct hook_all_config_cb *cb_data) +{ + struct hashmap_iter iter; + struct strmap_entry *e; + + strmap_for_each_entry(&cb_data->event_jobs, &iter, e) { + if (is_friendly_name(cb_data, e->key)) + warning(_("hook.%s.jobs is set but '%s' looks like a " + "hook friendly-name, not an event name; " + "hook..jobs uses the event name " + "(e.g. hook.post-receive.jobs), so this " + "setting will be ignored"), e->key, e->key); + } +} + /* Populate `cache` with the complete hook configuration */ static void build_hook_config_map(struct repository *r, struct strmap *cache) { @@ -295,6 +333,8 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) /* Parse all configs in one run, capturing hook.* including hook.jobs. */ repo_config(r, hook_config_lookup_all, &cb_data); + warn_jobs_on_friendly_names(&cb_data); + /* Construct the cache from parsed configs. */ strmap_for_each_entry(&cb_data.event_hooks, &iter, e) { struct string_list *hook_names = e->value; diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 24a3c92b6deb80..89fedc48ff497f 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -1028,4 +1028,34 @@ test_expect_success 'hook..jobs still requires hook..parallel=true' test_cmp expect hook.order ' +test_expect_success 'hook..jobs warns when name has .command' ' + test_config hook.my-hook.command "true" && + test_config hook.my-hook.jobs 2 && + git hook run --allow-unknown-hook-name --ignore-missing test-hook >out 2>err && + test_grep "hook.my-hook.jobs.*friendly-name" err +' + +test_expect_success 'hook..jobs warns when name has .event' ' + test_config hook.my-hook.event test-hook && + test_config hook.my-hook.command "true" && + test_config hook.my-hook.jobs 2 && + git hook run --allow-unknown-hook-name --ignore-missing test-hook >out 2>err && + test_grep "hook.my-hook.jobs.*friendly-name" err +' + +test_expect_success 'hook..jobs warns when name has .parallel' ' + test_config hook.my-hook.event test-hook && + test_config hook.my-hook.command "true" && + test_config hook.my-hook.parallel true && + test_config hook.my-hook.jobs 2 && + git hook run --allow-unknown-hook-name --ignore-missing test-hook >out 2>err && + test_grep "hook.my-hook.jobs.*friendly-name" err +' + +test_expect_success 'hook..jobs does not warn for a real event name' ' + test_config hook.test-hook.jobs 2 && + git hook run --allow-unknown-hook-name --ignore-missing test-hook >out 2>err && + test_grep ! "friendly-name" err +' + test_done From 2eb541e8f2a9b0dd923279421c741d0a0c00420d Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Fri, 10 Apr 2026 12:06:05 +0300 Subject: [PATCH 32/51] hook: move is_known_hook() to hook.c for wider use Move is_known_hook() from builtin/hook.c (static) into hook.c and export it via hook.h so it can be reused. Make it return bool and the iterator `h` for clarity (iterate hooks). Both meson.build and the Makefile are updated to reflect that the header is now used by libgit, not the builtin sources. The next commit will use this to reject hook friendly-names that collide with known event names. Co-authored-by: Patrick Steinhardt Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Makefile | 2 +- builtin/hook.c | 10 ---------- hook.c | 10 ++++++++++ hook.h | 6 ++++++ meson.build | 24 ++++++++++++------------ 5 files changed, 29 insertions(+), 23 deletions(-) diff --git a/Makefile b/Makefile index 5d22394c2ec1a6..c4e83823e4a547 100644 --- a/Makefile +++ b/Makefile @@ -2675,7 +2675,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS) help.sp help.s help.o: command-list.h builtin/bugreport.sp builtin/bugreport.s builtin/bugreport.o: hook-list.h -builtin/hook.sp builtin/hook.s builtin/hook.o: hook-list.h +hook.sp hook.s hook.o: hook-list.h builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \ diff --git a/builtin/hook.c b/builtin/hook.c index bea0668b475931..1839412dca3edc 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -4,7 +4,6 @@ #include "environment.h" #include "gettext.h" #include "hook.h" -#include "hook-list.h" #include "parse-options.h" #define BUILTIN_HOOK_RUN_USAGE \ @@ -13,15 +12,6 @@ #define BUILTIN_HOOK_LIST_USAGE \ N_("git hook list [--allow-unknown-hook-name] [-z] [--show-scope] ") -static int is_known_hook(const char *name) -{ - const char **p; - for (p = hook_name_list; *p; p++) - if (!strcmp(*p, name)) - return 1; - return 0; -} - static const char * const builtin_hook_usage[] = { BUILTIN_HOOK_RUN_USAGE, BUILTIN_HOOK_LIST_USAGE, diff --git a/hook.c b/hook.c index 0493993bbe6738..19076f8f2baba6 100644 --- a/hook.c +++ b/hook.c @@ -5,6 +5,7 @@ #include "environment.h" #include "gettext.h" #include "hook.h" +#include "hook-list.h" #include "parse.h" #include "path.h" #include "run-command.h" @@ -12,6 +13,15 @@ #include "strbuf.h" #include "strmap.h" +bool is_known_hook(const char *name) +{ + const char **h; + for (h = hook_name_list; *h; h++) + if (!strcmp(*h, name)) + return true; + return false; +} + const char *find_hook(struct repository *r, const char *name) { static struct strbuf path = STRBUF_INIT; diff --git a/hook.h b/hook.h index 01db4226a60306..5a93f56618e123 100644 --- a/hook.h +++ b/hook.h @@ -234,6 +234,12 @@ void hook_free(void *p, const char *str); */ void hook_cache_clear(struct strmap *cache); +/** + * Returns true if `name` is a recognized hook event name + * (e.g. "pre-commit", "post-receive"). + */ +bool is_known_hook(const char *name); + /** * Returns the path to the hook file, or NULL if the hook is missing * or disabled. Note that this points to static storage that will be diff --git a/meson.build b/meson.build index 8309942d184847..f438d5545dafb7 100644 --- a/meson.build +++ b/meson.build @@ -563,6 +563,18 @@ libgit_sources += custom_target( env: script_environment, ) +libgit_sources += custom_target( + input: 'Documentation/githooks.adoc', + output: 'hook-list.h', + command: [ + shell, + meson.current_source_dir() + '/tools/generate-hooklist.sh', + meson.current_source_dir(), + '@OUTPUT@', + ], + env: script_environment, +) + builtin_sources = [ 'builtin/add.c', 'builtin/am.c', @@ -739,18 +751,6 @@ builtin_sources += custom_target( env: script_environment, ) -builtin_sources += custom_target( - input: 'Documentation/githooks.adoc', - output: 'hook-list.h', - command: [ - shell, - meson.current_source_dir() + '/tools/generate-hooklist.sh', - meson.current_source_dir(), - '@OUTPUT@', - ], - env: script_environment, -) - # This contains the variables for GIT-BUILD-OPTIONS, which we use to propagate # build options to our tests. build_options_config = configuration_data() From dcfb5af67e7d7156c4d1ede66de18088c990356c Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Fri, 10 Apr 2026 12:06:06 +0300 Subject: [PATCH 33/51] hook: add hook..enabled switch Add a hook..enabled config key that disables all hooks for a given event, when set to false, acting as a high-level switch above the existing per-hook hook..enabled. Event-disabled hooks are shown in "git hook list" with an "event-disabled" tab-separated prefix before the name: $ git hook list test-hook event-disabled hook-1 event-disabled hook-2 With --show-scope: $ git hook list --show-scope test-hook local event-disabled hook-1 When a hook is both per-hook disabled and event-disabled, only "event-disabled" is shown: the event-level switch is the more relevant piece of information, and the per-hook "disabled" status will surface once the event is re-enabled. Using an event name as a friendly-name (e.g. hook..enabled) can cause ambiguity, so a fatal error is issued when using a known event name and a warning is issued for unknown event name, since a collision cannot be detected with certainty for unknown events. Suggested-by: Patrick Steinhardt Suggested-by: Junio C Hamano Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/hook.adoc | 20 ++++++++ builtin/hook.c | 20 +++++--- hook.c | 47 +++++++++++++++++-- hook.h | 1 + repository.c | 1 + repository.h | 4 ++ t/t1800-hook.sh | 83 ++++++++++++++++++++++++++++++++++ 7 files changed, 165 insertions(+), 11 deletions(-) diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index d4fa29d936d6e2..e0db3afa194080 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -15,6 +15,12 @@ hook..event:: events, specify the key more than once. An empty value resets the list of events, clearing any previously defined events for `hook.`. See linkgit:git-hook[1]. ++ +The `` must not be the same as a known hook event name +(e.g. do not use `hook.pre-commit.event`). Using a known event name as +a friendly-name is a fatal error because it creates an ambiguity with +`hook..enabled` and `hook..jobs`. For unknown event names, +a warning is issued when `` matches the event value. hook..enabled:: Whether the hook `hook.` is enabled. Defaults to `true`. @@ -33,6 +39,20 @@ hook..parallel:: found in the hooks directory do not need to, and run in parallel when the effective job count is greater than 1. See linkgit:git-hook[1]. +hook..enabled:: + Switch to enable or disable all hooks for the `` hook event. + When set to `false`, no hooks fire for that event, regardless of any + per-hook `hook..enabled` settings. Defaults to `true`. + See linkgit:git-hook[1]. ++ +Note on naming: `` must be the event name (e.g. `pre-commit`), +not a hook friendly-name. Since using a known event name as a +friendly-name is disallowed (see `hook..event` above), +there is no ambiguity between event-level and per-hook `.enabled` +settings for known events. For unknown events, if a friendly-name +matches the event name despite the warning, `.enabled` is treated +as per-hook only. + hook..jobs:: Specifies how many hooks can be run simultaneously for the `` hook event (e.g. `hook.post-receive.jobs = 4`). Overrides `hook.jobs` diff --git a/builtin/hook.c b/builtin/hook.c index 1839412dca3edc..8e47e22e2a1e5f 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -87,14 +87,22 @@ static int list(int argc, const char **argv, const char *prefix, const char *name = h->u.configured.friendly_name; const char *scope = show_scope ? config_scope_name(h->u.configured.scope) : NULL; + /* + * Show the most relevant disable reason. Event-level + * takes precedence: if the whole event is off, that + * is what the user needs to know. The per-hook + * "disabled" surfaces once the event is re-enabled. + */ + const char *disability = + h->u.configured.event_disabled ? "event-disabled\t" : + h->u.configured.disabled ? "disabled\t" : + ""; if (scope) - printf("%s\t%s%s%c", scope, - h->u.configured.disabled ? "disabled\t" : "", - name, line_terminator); + printf("%s\t%s%s%c", scope, disability, name, + line_terminator); else - printf("%s%s%c", - h->u.configured.disabled ? "disabled\t" : "", - name, line_terminator); + printf("%s%s%c", disability, name, + line_terminator); break; } default: diff --git a/hook.c b/hook.c index 19076f8f2baba6..bc990d4ed4d754 100644 --- a/hook.c +++ b/hook.c @@ -133,7 +133,9 @@ struct hook_config_cache_entry { * Callback struct to collect all hook.* keys in a single config pass. * commands: friendly-name to command map. * event_hooks: event-name to list of friendly-names map. - * disabled_hooks: set of friendly-names with hook..enabled = false. + * disabled_hooks: set of all names with hook..enabled = false; after + * parsing, names that are not friendly-names become event-level + * disables stored in r->disabled_events. This collects all. * parallel_hooks: friendly-name to parallel flag. * event_jobs: event-name to per-event jobs count (stored as uintptr_t, NULL == unset). * jobs: value of the global hook.jobs key. Defaults to 0 if unset (stored in r->hook_jobs). @@ -189,8 +191,21 @@ static int hook_config_lookup_all(const char *key, const char *value, strmap_for_each_entry(&data->event_hooks, &iter, e) unsorted_string_list_remove(e->value, hook_name, 0); } else { - struct string_list *hooks = - strmap_get(&data->event_hooks, value); + struct string_list *hooks; + + if (is_known_hook(hook_name)) + die(_("hook friendly-name '%s' collides with " + "a known event name; please choose a " + "different friendly-name"), + hook_name); + + if (!strcmp(hook_name, value)) + warning(_("hook friendly-name '%s' is the " + "same as its event; this may cause " + "ambiguity with hook.%s.enabled"), + hook_name, hook_name); + + hooks = strmap_get(&data->event_hooks, value); if (!hooks) { CALLOC_ARRAY(hooks, 1); @@ -345,6 +360,22 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) warn_jobs_on_friendly_names(&cb_data); + /* + * Populate disabled_events: names in disabled_hooks that are not + * friendly-names are event-level switches (hook..enabled = false). + * Names that are friendly-names are already handled per-hook via the + * hook_config_cache_entry.disabled flag below. + */ + if (r) { + string_list_clear(&r->disabled_events, 0); + string_list_init_dup(&r->disabled_events); + for (size_t i = 0; i < cb_data.disabled_hooks.nr; i++) { + const char *n = cb_data.disabled_hooks.items[i].string; + if (!is_friendly_name(&cb_data, n)) + string_list_append(&r->disabled_events, n); + } + } + /* Construct the cache from parsed configs. */ strmap_for_each_entry(&cb_data.event_hooks, &iter, e) { struct string_list *hook_names = e->value; @@ -446,6 +477,8 @@ static void list_hooks_add_configured(struct repository *r, { struct strmap *cache = get_hook_config_cache(r); struct string_list *configured_hooks = strmap_get(cache, hookname); + bool event_is_disabled = r ? !!unsorted_string_list_lookup(&r->disabled_events, + hookname) : 0; /* Iterate through configured hooks and initialize internal states */ for (size_t i = 0; configured_hooks && i < configured_hooks->nr; i++) { @@ -472,6 +505,7 @@ static void list_hooks_add_configured(struct repository *r, entry->command ? xstrdup(entry->command) : NULL; hook->u.configured.scope = entry->scope; hook->u.configured.disabled = entry->disabled; + hook->u.configured.event_disabled = event_is_disabled; hook->parallel = entry->parallel; string_list_append(list, friendly_name)->util = hook; @@ -484,6 +518,8 @@ static void list_hooks_add_configured(struct repository *r, if (!r || !r->gitdir) { hook_cache_clear(cache); free(cache); + if (r) + string_list_clear(&r->disabled_events, 0); } } @@ -515,7 +551,7 @@ int hook_exists(struct repository *r, const char *name) for (size_t i = 0; i < hooks->nr; i++) { struct hook *h = hooks->items[i].util; if (h->kind == HOOK_TRADITIONAL || - !h->u.configured.disabled) { + (!h->u.configured.disabled && !h->u.configured.event_disabled)) { exists = 1; break; } @@ -538,7 +574,8 @@ static int pick_next_hook(struct child_process *cp, if (hook_cb->hook_to_run_index >= hook_list->nr) return 0; h = hook_list->items[hook_cb->hook_to_run_index++].util; - } while (h->kind == HOOK_CONFIGURED && h->u.configured.disabled); + } while (h->kind == HOOK_CONFIGURED && + (h->u.configured.disabled || h->u.configured.event_disabled)); cp->no_stdin = 1; strvec_pushv(&cp->env, hook_cb->options->env.v); diff --git a/hook.h b/hook.h index 5a93f56618e123..b4372b636ff4de 100644 --- a/hook.h +++ b/hook.h @@ -32,6 +32,7 @@ struct hook { const char *command; enum config_scope scope; bool disabled; + bool event_disabled; } configured; } u; diff --git a/repository.c b/repository.c index 4030db4460714d..db57b8308b94e7 100644 --- a/repository.c +++ b/repository.c @@ -427,6 +427,7 @@ void repo_clear(struct repository *repo) FREE_AND_NULL(repo->hook_config_cache); } strmap_clear(&repo->event_jobs, 0); /* values are uintptr_t, not heap ptrs */ + string_list_clear(&repo->disabled_events, 0); if (repo->promisor_remote_config) { promisor_remote_clear(repo->promisor_remote_config); diff --git a/repository.h b/repository.h index 6b67ec02e2984c..4969d8b8ebed60 100644 --- a/repository.h +++ b/repository.h @@ -2,6 +2,7 @@ #define REPOSITORY_H #include "strmap.h" +#include "string-list.h" #include "repo-settings.h" #include "environment.h" @@ -178,6 +179,9 @@ struct repository { /* Cached map of event-name -> jobs count (as uintptr_t) from hook..jobs. */ struct strmap event_jobs; + /* Cached list of event names with hook..enabled = false. */ + struct string_list disabled_events; + /* Configurations related to promisor remotes. */ char *repository_format_partial_clone; struct promisor_remote_config *promisor_remote_config; diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 89fedc48ff497f..c4ff25f6b088ea 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -1058,4 +1058,87 @@ test_expect_success 'hook..jobs does not warn for a real event name' ' test_grep ! "friendly-name" err ' +test_expect_success 'hook..enabled=false skips all hooks for event' ' + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command "echo ran" && + test_config hook.test-hook.enabled false && + git hook run --allow-unknown-hook-name test-hook >out 2>err && + test_must_be_empty out +' + +test_expect_success 'hook..enabled=true does not suppress hooks' ' + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command "echo ran" && + test_config hook.test-hook.enabled true && + git hook run --allow-unknown-hook-name test-hook >out 2>err && + test_grep "ran" err +' + +test_expect_success 'hook..enabled=false does not affect other events' ' + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command "echo ran" && + test_config hook.other-event.enabled false && + git hook run --allow-unknown-hook-name test-hook >out 2>err && + test_grep "ran" err +' + +test_expect_success 'hook..enabled=false still disables that hook' ' + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command "echo hook-1" && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command "echo hook-2" && + test_config hook.hook-1.enabled false && + git hook run --allow-unknown-hook-name test-hook >out 2>err && + test_grep ! "hook-1" err && + test_grep "hook-2" err +' + +test_expect_success 'git hook list shows event-disabled hooks as event-disabled' ' + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command "echo ran" && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command "echo ran" && + test_config hook.test-hook.enabled false && + git hook list --allow-unknown-hook-name test-hook >actual && + test_grep "^event-disabled hook-1$" actual && + test_grep "^event-disabled hook-2$" actual +' + +test_expect_success 'git hook list shows scope with event-disabled' ' + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command "echo ran" && + test_config hook.test-hook.enabled false && + git hook list --allow-unknown-hook-name --show-scope test-hook >actual && + test_grep "^local event-disabled hook-1$" actual +' + +test_expect_success 'git hook list still shows hooks when event is disabled' ' + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command "echo ran" && + test_config hook.test-hook.enabled false && + git hook list --allow-unknown-hook-name test-hook >actual && + test_grep "event-disabled" actual +' + +test_expect_success 'friendly-name matching known event name is rejected' ' + test_config hook.pre-commit.event pre-commit && + test_config hook.pre-commit.command "echo oops" && + test_must_fail git hook run pre-commit 2>err && + test_grep "collides with a known event name" err +' + +test_expect_success 'friendly-name matching known event name is rejected even for different event' ' + test_config hook.pre-commit.event post-commit && + test_config hook.pre-commit.command "echo oops" && + test_must_fail git hook run post-commit 2>err && + test_grep "collides with a known event name" err +' + +test_expect_success 'friendly-name matching unknown event warns' ' + test_config hook.test-hook.event test-hook && + test_config hook.test-hook.command "echo ran" && + git hook run --allow-unknown-hook-name test-hook >out 2>err && + test_grep "same as its event" err +' + test_done From 495b7d54dc006556548e2fd3ca15c4f533917329 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Fri, 10 Apr 2026 12:06:07 +0300 Subject: [PATCH 34/51] hook: allow hook.jobs=-1 to use all available CPU cores Allow -1 as a value for hook.jobs, hook..jobs, and the -j CLI flag to mean "use as many jobs as there are CPU cores", matching the convention used by fetch.parallel and other Git subsystems. The value is resolved to online_cpus() at parse time so the rest of the code always works with a positive resolved count. Other non-positive values (0, -2, etc) are rejected with a warning (config) or die (CLI). Suggested-by: Patrick Steinhardt Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/hook.adoc | 4 ++- builtin/hook.c | 15 +++++++-- hook.c | 60 ++++++++++++++++++++++++---------- t/t1800-hook.sh | 49 +++++++++++++++++++++++++++ 4 files changed, 108 insertions(+), 20 deletions(-) diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index e0db3afa194080..a9dc0063c12102 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -58,7 +58,8 @@ hook..jobs:: hook event (e.g. `hook.post-receive.jobs = 4`). Overrides `hook.jobs` for this specific event. The same parallelism restrictions apply: this setting has no effect unless all configured hooks for the event have - `hook..parallel` set to `true`. Must be a positive int, + `hook..parallel` set to `true`. Set to `-1` to use the + number of available CPU cores. Must be a positive integer or `-1`; zero is rejected with a warning. See linkgit:git-hook[1]. + Note on naming: although this key resembles `hook..*` @@ -74,6 +75,7 @@ valid event name when setting `hook..jobs`. hook.jobs:: Specifies how many hooks can be run simultaneously during parallelized hook execution. If unspecified, defaults to 1 (serial execution). + Set to `-1` to use the number of available CPU cores. Can be overridden on a per-event basis with `hook..jobs`. Some hooks always run sequentially regardless of this setting because they operate on shared data and cannot safely be parallelized: diff --git a/builtin/hook.c b/builtin/hook.c index 8e47e22e2a1e5f..cceeb3586e5daf 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -5,6 +5,7 @@ #include "gettext.h" #include "hook.h" #include "parse-options.h" +#include "thread-utils.h" #define BUILTIN_HOOK_RUN_USAGE \ N_("git hook run [--allow-unknown-hook-name] [--ignore-missing] [--to-stdin=] [(-j|--jobs) ]\n" \ @@ -123,6 +124,7 @@ static int run(int argc, const char **argv, const char *prefix, struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; int ignore_missing = 0; int allow_unknown = 0; + int jobs = 0; const char *hook_name; struct option run_options[] = { OPT_BOOL(0, "allow-unknown-hook-name", &allow_unknown, @@ -131,8 +133,8 @@ static int run(int argc, const char **argv, const char *prefix, N_("silently ignore missing requested ")), OPT_STRING(0, "to-stdin", &opt.path_to_stdin, N_("path"), N_("file to read into hooks' stdin")), - OPT_UNSIGNED('j', "jobs", &opt.jobs, - N_("run up to hooks simultaneously")), + OPT_INTEGER('j', "jobs", &jobs, + N_("run up to hooks simultaneously (-1 for CPU count)")), OPT_END(), }; int ret; @@ -141,6 +143,15 @@ static int run(int argc, const char **argv, const char *prefix, builtin_hook_run_usage, PARSE_OPT_KEEP_DASHDASH); + if (jobs == -1) + opt.jobs = online_cpus(); + else if (jobs < 0) + die(_("invalid value for -j: %d" + " (use -1 for CPU count or a" + " positive integer)"), jobs); + else + opt.jobs = jobs; + if (!argc) goto usage; diff --git a/hook.c b/hook.c index bc990d4ed4d754..d10eef4763c679 100644 --- a/hook.c +++ b/hook.c @@ -12,6 +12,7 @@ #include "setup.h" #include "strbuf.h" #include "strmap.h" +#include "thread-utils.h" bool is_known_hook(const char *name) { @@ -165,13 +166,17 @@ static int hook_config_lookup_all(const char *key, const char *value, /* Handle plain hook. entries that have no hook name component. */ if (!name) { if (!strcmp(subkey, "jobs") && value) { - unsigned int v; - if (!git_parse_uint(value, &v)) - warning(_("hook.jobs must be a positive integer, ignoring: '%s'"), value); - else if (!v) - warning(_("hook.jobs must be positive, ignoring: 0")); - else + int v; + if (!git_parse_int(value, &v)) + warning(_("hook.jobs must be an integer, ignoring: '%s'"), value); + else if (v == -1) + data->jobs = online_cpus(); + else if (v > 0) data->jobs = v; + else + warning(_("hook.jobs must be a positive integer" + " or -1, ignoring: '%s'"), + value); } return 0; } @@ -259,17 +264,21 @@ static int hook_config_lookup_all(const char *key, const char *value, " ignoring: '%s'"), hook_name, value); } else if (!strcmp(subkey, "jobs")) { - unsigned int v; - if (!git_parse_uint(value, &v)) - warning(_("hook.%s.jobs must be a positive integer," + int v; + if (!git_parse_int(value, &v)) + warning(_("hook.%s.jobs must be an integer," " ignoring: '%s'"), hook_name, value); - else if (!v) - warning(_("hook.%s.jobs must be positive," - " ignoring: 0"), hook_name); - else + else if (v == -1) + strmap_put(&data->event_jobs, hook_name, + (void *)(uintptr_t)online_cpus()); + else if (v > 0) strmap_put(&data->event_jobs, hook_name, (void *)(uintptr_t)v); + else + warning(_("hook.%s.jobs must be a positive" + " integer or -1, ignoring: '%s'"), + hook_name, value); } free(hook_name); @@ -688,6 +697,25 @@ static void warn_non_parallel_hooks_override(unsigned int jobs, } } +/* Resolve a hook.jobs config key, handling -1 as online_cpus(). */ +static void resolve_hook_config_jobs(struct repository *r, + const char *key, + unsigned int *jobs) +{ + int v; + + if (repo_config_get_int(r, key, &v)) + return; + + if (v == -1) + *jobs = online_cpus(); + else if (v > 0) + *jobs = v; + else + warning(_("%s must be a positive integer or -1," + " ignoring: %d"), key, v); +} + /* Determine how many jobs to use for hook execution. */ static unsigned int get_hook_jobs(struct repository *r, struct run_hooks_opt *options, @@ -721,14 +749,12 @@ static unsigned int get_hook_jobs(struct repository *r, if (event_jobs) options->jobs = (unsigned int)(uintptr_t)event_jobs; } else { - unsigned int event_jobs; char *key; - repo_config_get_uint(r, "hook.jobs", &options->jobs); + resolve_hook_config_jobs(r, "hook.jobs", &options->jobs); key = xstrfmt("hook.%s.jobs", hook_name); - if (!repo_config_get_uint(r, key, &event_jobs) && event_jobs) - options->jobs = event_jobs; + resolve_hook_config_jobs(r, key, &options->jobs); free(key); } } diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index c4ff25f6b088ea..41b2b2c7460066 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -1058,6 +1058,55 @@ test_expect_success 'hook..jobs does not warn for a real event name' ' test_grep ! "friendly-name" err ' +test_expect_success 'hook.jobs=-1 resolves to online_cpus()' ' + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command "true" && + test_config hook.hook-1.parallel true && + + test_config hook.jobs -1 && + + cpus=$(test-tool online-cpus) && + GIT_TRACE2_EVENT="$(pwd)/trace.txt" \ + git hook run --allow-unknown-hook-name test-hook >out 2>err && + grep "\"region_enter\".*\"hook\".*\"test-hook\".*\"max:$cpus\"" trace.txt +' + +test_expect_success 'hook..jobs=-1 resolves to online_cpus()' ' + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command "true" && + test_config hook.hook-1.parallel true && + + test_config hook.test-hook.jobs -1 && + + cpus=$(test-tool online-cpus) && + GIT_TRACE2_EVENT="$(pwd)/trace.txt" \ + git hook run --allow-unknown-hook-name test-hook >out 2>err && + grep "\"region_enter\".*\"hook\".*\"test-hook\".*\"max:$cpus\"" trace.txt +' + +test_expect_success 'git hook run -j-1 resolves to online_cpus()' ' + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command "true" && + test_config hook.hook-1.parallel true && + + cpus=$(test-tool online-cpus) && + GIT_TRACE2_EVENT="$(pwd)/trace.txt" \ + git hook run --allow-unknown-hook-name -j-1 test-hook >out 2>err && + grep "\"region_enter\".*\"hook\".*\"test-hook\".*\"max:$cpus\"" trace.txt +' + +test_expect_success 'hook.jobs rejects values less than -1' ' + test_config hook.jobs -2 && + git hook run --allow-unknown-hook-name --ignore-missing test-hook >out 2>err && + test_grep "hook.jobs must be a positive integer or -1" err +' + +test_expect_success 'hook..jobs rejects values less than -1' ' + test_config hook.test-hook.jobs -5 && + git hook run --allow-unknown-hook-name --ignore-missing test-hook >out 2>err && + test_grep "hook.test-hook.jobs must be a positive integer or -1" err +' + test_expect_success 'hook..enabled=false skips all hooks for event' ' test_config hook.hook-1.event test-hook && test_config hook.hook-1.command "echo ran" && From 75b7cb5e14f03965cf87a976356bcbdcfb4edbad Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 10 Apr 2026 12:06:08 +0300 Subject: [PATCH 35/51] t1800: test SIGPIPE with parallel hooks We recently fixed a bug in commit 2226ffaacd (run_processes_parallel(): fix order of sigpipe handling, 2026-04-08) where a hook that caused us to get SIGPIPE would accidentally trigger the run_processes_parallel() cleanup handler killing the child processes. For a single hook, this meant killing the already-exited hook. This case was triggered by our tests, but was only a problem on some platforms. But if you have multiple hooks running in parallel, this causes a problem everywhere, since one hook failing to read its input would take down all hooks. Now that we have parallel hook support, we can add a test for this case. It should pass already, due to the existing fix. Signed-off-by: Jeff King Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- t/t1800-hook.sh | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 41b2b2c7460066..0132e772e472e2 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -1190,4 +1190,42 @@ test_expect_success 'friendly-name matching unknown event warns' ' test_grep "same as its event" err ' +test_expect_success 'hooks in parallel that do not read input' ' + # Add this to our $PATH to avoid having to write the whole trash + # directory into our config options, which would require quoting. + mkdir bin && + PATH=$PWD/bin:$PATH && + + write_script bin/hook-fast <<-\EOF && + # This hook does not read its input, so the parent process + # may see SIGPIPE if it is not ignored. It should happen + # relatively quickly. + exit 0 + EOF + + write_script bin/hook-slow <<-\EOF && + # This hook is slow, so we expect it to still be running + # when the other hook has exited (and the parent has a pipe error + # writing to it). + # + # So we want to be slow enough that we expect this to happen, but not + # so slow that the test takes forever. 1 second is probably enough + # in practice (and if it is occasionally not on a loaded system, we + # will err on the side of having the test pass). + sleep 1 + exit 0 + EOF + + git init --bare parallel.git && + git -C parallel.git config hook.fast.command "hook-fast" && + git -C parallel.git config hook.fast.event pre-receive && + git -C parallel.git config hook.fast.parallel true && + git -C parallel.git config hook.slow.command "hook-slow" && + git -C parallel.git config hook.slow.event pre-receive && + git -C parallel.git config hook.slow.parallel true && + git -C parallel.git config hook.jobs 2 && + + git push ./parallel.git "+refs/heads/*:refs/heads/*" +' + test_done From 955c88fbc5ac916f8dababa458a963ebbeba9b41 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Wed, 15 Apr 2026 02:27:42 +0000 Subject: [PATCH 36/51] userdiff: tighten word-diff test case of the scheme driver The scheme driver separates identifiers only at parentheses of all sorts and whitespace, except that vertical bars act as brackets that enclose an identifier. The test case attempts to demonstrate the vertical bars with a change from 'some-text' to '|a greeting|'. However, this misses the goal because the same word coloring would be applied if '|a greeting|' were parsed as two words. Have an identifier between vertical bars with a space in both the pre- and the post-image and change only one side of the space to show that the single word exists between the vertical bars. Also add cases that change parentheses of all kinds in a sequence of parentheses to show that they are their own word each. Signed-off-by: Johannes Sixt Signed-off-by: Scott L. Burson Signed-off-by: Junio C Hamano --- t/t4034/scheme/expect | 5 +++-- t/t4034/scheme/post | 1 + t/t4034/scheme/pre | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/t/t4034/scheme/expect b/t/t4034/scheme/expect index 496cd5de8c9af3..138abe9f56b38f 100644 --- a/t/t4034/scheme/expect +++ b/t/t4034/scheme/expect @@ -2,10 +2,11 @@ index 74b6605..63b6ac4 100644 --- a/pre +++ b/post -@@ -1,6 +1,6 @@ +@@ -1,7 +1,7 @@ (define (myfunc a bmy-func first second) ; This is a really(moderately) cool function. (this\placethat\place (+ 3 4)) - (define some-text|a greeting| "hello") + (define |the greeting||a greeting| "hello") + ({}(([](func-n)[])){}) (let ((c (+ a badd1 first))) (format "one more than the total is %d" (add1+ c second)))) diff --git a/t/t4034/scheme/post b/t/t4034/scheme/post index 63b6ac4f8754d8..0e3bab101da03e 100644 --- a/t/t4034/scheme/post +++ b/t/t4034/scheme/post @@ -2,5 +2,6 @@ ; This is a (moderately) cool function. (that\place (+ 3 4)) (define |a greeting| "hello") + ({(([(func-n)]))}) (let ((c (add1 first))) (format "one more than the total is %d" (+ c second)))) diff --git a/t/t4034/scheme/pre b/t/t4034/scheme/pre index 74b66053574b67..03d77c7c430e07 100644 --- a/t/t4034/scheme/pre +++ b/t/t4034/scheme/pre @@ -1,6 +1,7 @@ (define (myfunc a b) ; This is a really cool function. (this\place (+ 3 4)) - (define some-text "hello") + (define |the greeting| "hello") + ({}(([](func-n)[])){}) (let ((c (+ a b))) (format "one more than the total is %d" (add1 c)))) From b79f7a3ad3ffde16b2cbc2457561669f4833f861 Mon Sep 17 00:00:00 2001 From: "Scott L. Burson" Date: Wed, 15 Apr 2026 02:27:43 +0000 Subject: [PATCH 37/51] userdiff: extend Scheme support to cover other Lisp dialects Common Lisp has top-level forms, such as 'defun' and 'defmacro', that are not matched by the current Scheme pattern. Also, it is more common in CL, when defining user macros intended as top-level forms, to prefix their names with "def" instead of "define"; such forms are also not matched. And some top-level forms don't even begin with "def". On the other hand, it is an established formatting convention in the Lisp community that only top-level forms start at the left margin. So matching any unindented line starting with an open parenthesis is an acceptable heuristic; false positives will be rare. However, there are also cases where notionally top-level forms are grouped together within some containing form. At least in the Common Lisp community, it is conventional to indent these by two spaces, or sometimes one. But matching just an open parenthesis indented by two spaces would be too broad; so the pattern added by this commit requires an indented form to start with "(def". It is believed that this strikes a good balance between potential false positives and false negatives. Signed-off-by: Scott L. Burson Acked-by: Johannes Sixt Signed-off-by: Junio C Hamano --- Documentation/gitattributes.adoc | 3 ++- t/t4018/scheme-lisp-defun-a | 4 ++++ t/t4018/scheme-lisp-defun-b | 4 ++++ t/t4018/scheme-lisp-eval-when | 4 ++++ t/t4018/{scheme-module => scheme-module-a} | 0 t/t4018/scheme-module-b | 6 ++++++ t/t4034/scheme/expect | 2 +- t/t4034/scheme/post | 2 +- t/t4034/scheme/pre | 2 +- userdiff.c | 22 ++++++++++++++++------ 10 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 t/t4018/scheme-lisp-defun-a create mode 100644 t/t4018/scheme-lisp-defun-b create mode 100644 t/t4018/scheme-lisp-eval-when rename t/t4018/{scheme-module => scheme-module-a} (100%) create mode 100644 t/t4018/scheme-module-b diff --git a/Documentation/gitattributes.adoc b/Documentation/gitattributes.adoc index f20041a323d174..bd76167a45eb71 100644 --- a/Documentation/gitattributes.adoc +++ b/Documentation/gitattributes.adoc @@ -911,7 +911,8 @@ patterns are available: - `rust` suitable for source code in the Rust language. -- `scheme` suitable for source code in the Scheme language. +- `scheme` suitable for source code in most Lisp dialects, + including Scheme, Emacs Lisp, Common Lisp, and Clojure. - `tex` suitable for source code for LaTeX documents. diff --git a/t/t4018/scheme-lisp-defun-a b/t/t4018/scheme-lisp-defun-a new file mode 100644 index 00000000000000..c3c750f76d7b07 --- /dev/null +++ b/t/t4018/scheme-lisp-defun-a @@ -0,0 +1,4 @@ +(defun some-func (x y z) RIGHT + (let ((a x) + (b y)) + (ChangeMe a b))) diff --git a/t/t4018/scheme-lisp-defun-b b/t/t4018/scheme-lisp-defun-b new file mode 100644 index 00000000000000..21be305968bf6b --- /dev/null +++ b/t/t4018/scheme-lisp-defun-b @@ -0,0 +1,4 @@ +(macrolet ((foo (x) `(bar ,x))) + (defun mumble (x) ; RIGHT + (when (> x 0) + (foo x)))) ; ChangeMe diff --git a/t/t4018/scheme-lisp-eval-when b/t/t4018/scheme-lisp-eval-when new file mode 100644 index 00000000000000..5d941d7e0edda2 --- /dev/null +++ b/t/t4018/scheme-lisp-eval-when @@ -0,0 +1,4 @@ +(eval-when (:compile-toplevel :load-toplevel :execute) ; RIGHT + (set-macro-character #\? + (lambda (stream char) + `(make-pattern-variable ,(read stream))))) ; ChangeMe diff --git a/t/t4018/scheme-module b/t/t4018/scheme-module-a similarity index 100% rename from t/t4018/scheme-module rename to t/t4018/scheme-module-a diff --git a/t/t4018/scheme-module-b b/t/t4018/scheme-module-b new file mode 100644 index 00000000000000..77bc0c5eff4775 --- /dev/null +++ b/t/t4018/scheme-module-b @@ -0,0 +1,6 @@ +(module A + (export with-display-exception) + (extern (display-exception display-exception)) + (def (with-display-exception thunk) RIGHT + (with-catch (lambda (e) (display-exception e (current-error-port)) e) + thunk ChangeMe))) diff --git a/t/t4034/scheme/expect b/t/t4034/scheme/expect index 138abe9f56b38f..fb7f2616fea547 100644 --- a/t/t4034/scheme/expect +++ b/t/t4034/scheme/expect @@ -6,7 +6,7 @@ (define (myfunc a bmy-func first second) ; This is a really(moderately) cool function. (this\placethat\place (+ 3 4)) - (define |the greeting||a greeting| "hello") + (define |the \| \greeting||a \greeting| |hello there|) ({}(([](func-n)[])){}) (let ((c (+ a badd1 first))) (format "one more than the total is %d" (add1+ c second)))) diff --git a/t/t4034/scheme/post b/t/t4034/scheme/post index 0e3bab101da03e..450cc234f75aea 100644 --- a/t/t4034/scheme/post +++ b/t/t4034/scheme/post @@ -1,7 +1,7 @@ (define (my-func first second) ; This is a (moderately) cool function. (that\place (+ 3 4)) - (define |a greeting| "hello") + (define |a \greeting| |hello there|) ({(([(func-n)]))}) (let ((c (add1 first))) (format "one more than the total is %d" (+ c second)))) diff --git a/t/t4034/scheme/pre b/t/t4034/scheme/pre index 03d77c7c430e07..e16ee7584946e4 100644 --- a/t/t4034/scheme/pre +++ b/t/t4034/scheme/pre @@ -1,7 +1,7 @@ (define (myfunc a b) ; This is a really cool function. (this\place (+ 3 4)) - (define |the greeting| "hello") + (define |the \| \greeting| |hello there|) ({}(([](func-n)[])){}) (let ((c (+ a b))) (format "one more than the total is %d" (add1 c)))) diff --git a/userdiff.c b/userdiff.c index fe710a68bfdfa6..b5412e6bc3ecd3 100644 --- a/userdiff.c +++ b/userdiff.c @@ -344,14 +344,24 @@ PATTERNS("rust", "|[0-9][0-9_a-fA-Fiosuxz]*(\\.([0-9]*[eE][+-]?)?[0-9_fF]*)?" "|[-+*\\/<>%&^|=!:]=|<<=?|>>=?|&&|\\|\\||->|=>|\\.{2}=|\\.{3}|::"), PATTERNS("scheme", - "^[\t ]*(\\(((define|def(struct|syntax|class|method|rules|record|proto|alias)?)[-*/ \t]|(library|module|struct|class)[*+ \t]).*)$", /* - * R7RS valid identifiers include any sequence enclosed - * within vertical lines having no backslashes + * An unindented opening parenthesis identifies a top-level + * expression in all Lisp dialects. */ - "\\|([^\\\\]*)\\|" - /* All other words should be delimited by spaces or parentheses */ - "|([^][)(}{[ \t])+"), + "^(\\(.*)$\n" + /* For Scheme: a possibly indented left paren followed by a keyword. */ + "^[\t ]*(\\(((define|def(struct|syntax|class|method|rules|record|proto|alias)?)[-*/ \t]|(library|module|struct|class)[*+ \t]).*)$\n" + /* + * For all Lisp dialects: a slightly indented line starting with "(def". + */ + "^ ?(\\([Dd][Ee][Ff].*)$", + /* + * The union of R7RS and Common Lisp symbol syntax: allows arbitrary + * strings between vertical bars, including any escaped characters. + */ + "\\|([^|\\\\]|\\\\.)*\\|" + /* All other words should be delimited by spaces or parentheses. */ + "|([^][)(}{ \t])+"), PATTERNS("tex", "^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$", "\\\\[a-zA-Z@]+|\\\\.|([a-zA-Z0-9]|[^\x01-\x7f])+"), { .name = "default", .binary = -1 }, From b96490241e342fe1aecbd3c4f40de6998d2a3eaa Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 10 Apr 2026 11:10:48 -0700 Subject: [PATCH 38/51] CodingGuidelines: st_mtimespec vs st_mtim vs st_mtime Most unfortunately macOS does not support st_[amc]tim for timestamps down to nanosecond resolution as POSIX systems. Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index b8670751f5c705..b9a29e39f2eec4 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -681,6 +681,12 @@ For C programs: char *dogs[] = ...; walk_all_dogs(dogs); + - For file timestamps, do not use "st_mtim" (and other timestamp + members in "struct stat") unconditionally; not everybody is POSIX + (grep for USE_ST_TIMESPEC). If you only need a timestamp in whole + second resolution, "st_mtime" should work fine everywhere. + + For Perl programs: - Most of the C guidelines above apply. From 0cb6316b08463d9ec491e6db5ce53c3a65c048a8 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:14 +0200 Subject: [PATCH 39/51] t: prepare `test_match_signal ()` calls for `set -e` We have a couple of calls to `test_match_signal ()` where we execute a Git command and expect it to die with a specific signal. These calls will essentially execute the process in a subshell via `foo; echo $?`, but as we expect `foo` to fail this will cause the overall subshell to fail once we `set -e`. Fix this issue by using `foo && echo 0 || echo $?` instead. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t0005-signals.sh | 4 ++-- t/t3600-rm.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t0005-signals.sh b/t/t0005-signals.sh index afba0fc3fc673e..84319cf169e571 100755 --- a/t/t0005-signals.sh +++ b/t/t0005-signals.sh @@ -42,12 +42,12 @@ test_expect_success 'create blob' ' ' test_expect_success !MINGW 'a constipated git dies with SIGPIPE' ' - OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) && + OUT=$( ((large_git && echo 0 1>&3 || echo $? 1>&3) | :) 3>&1 ) && test_match_signal 13 "$OUT" ' test_expect_success !MINGW 'a constipated git dies with SIGPIPE even if parent ignores it' ' - OUT=$( ((trap "" PIPE && large_git; echo $? 1>&3) | :) 3>&1 ) && + OUT=$( ((trap "" PIPE && large_git && echo 0 1>&3 || echo $? 1>&3) | :) 3>&1 ) && test_match_signal 13 "$OUT" ' diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 1f16e6b52285bc..a371ea690ef79d 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -260,7 +260,7 @@ test_expect_success 'choking "git rm" should not let it die with cruft (induce S test_expect_success !MINGW 'choking "git rm" should not let it die with cruft (induce and check SIGPIPE)' ' choke_git_rm_setup && - OUT=$( ((trap "" PIPE && git rm -n "some-file-*"; echo $? 1>&3) | :) 3>&1 ) && + OUT=$( ((trap "" PIPE && git rm -n "some-file-*" && echo 0 1>&3 || echo $? 1>&3) | :) 3>&1 ) && test_match_signal 13 "$OUT" && test_path_is_missing .git/index.lock ' From 990fd368ac7fcb56f69a27ba341cac10f04e285b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:15 +0200 Subject: [PATCH 40/51] t: prepare `test_must_fail ()` for `set -e` The helper function `test_must_fail ()` executes a specific Git command that may or may not fail in a specific way. This is done by executing the command in question and then comparing its exit code against a set of conditions. This works, but once we run our test suite with `set -e` we may bail out of `test_must_fail ()` early in case the command actually fails, even though we expect it to fail. Prepare for this change by handling the failed case with `||`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/test-lib-functions.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index f3af10fb7e0205..5fd5494ef1030c 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -1195,8 +1195,9 @@ test_must_fail () { echo >&7 "test_must_fail: only 'git' is allowed: $*" return 1 fi - "$@" 2>&7 - exit_code=$? + + exit_code=0; "$@" 2>&7 || exit_code=$? + if test $exit_code -eq 0 && ! list_contains "$_test_ok" success then echo >&4 "test_must_fail: command succeeded: $*" From f43d01ab90be2711927c8d265b3fb21d2213d2ab Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:16 +0200 Subject: [PATCH 41/51] t: prepare `stop_git_daemon ()` for `set -e` We have a couple of calls to `stop_git_daemon ()` outside of specific test cases that will kill a backgrounded git-daemon(1) process and expect the process with a specific error code. While these function calls do end up killing git-daemon(1), the error handling we have in those contexts is basically ineffective. So while we expect the process to exit with a specific error code, we will just continue with any error in case it doesn't. This will change once we enable `set -e` in a subsequent commit. There's two issues though that will make this _always_ fail: - Our call to `wait` is expected to fail, but because it's not part of a condition it will cause us to bail out immediately with `set -e`. - We try to kill git-daemon(1) a second time via the pidfile. We can generally expect that this is the same PID though as we had in the "GIT_DAEMON_PID" environment variable, and thus it's more likely than not that we have already killed it, and the call to kill will fail. Prepare for this change by handling the failure of `wait` with `||` and by silencing failures of the second call to `kill`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/lib-git-daemon.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh index e62569222b55aa..d172aa51f0c386 100644 --- a/t/lib-git-daemon.sh +++ b/t/lib-git-daemon.sh @@ -85,14 +85,16 @@ stop_git_daemon() { # kill git-daemon child of git say >&3 "Stopping git daemon ..." + kill "$GIT_DAEMON_PID" - wait "$GIT_DAEMON_PID" >&3 2>&4 - ret=$? + ret=0; wait "$GIT_DAEMON_PID" >&3 2>&4 || ret=$? + if ! test_match_signal 15 $ret then error "git daemon exited with status: $ret" fi - kill "$(cat "$GIT_DAEMON_PIDFILE")" 2>/dev/null + + kill "$(cat "$GIT_DAEMON_PIDFILE")" 2>/dev/null || : GIT_DAEMON_PID= rm -f git_daemon_output "$GIT_DAEMON_PIDFILE" } From 9c64add20b09cc47c191c2bc7b2503b02d290385 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:17 +0200 Subject: [PATCH 42/51] t: prepare `git config --unset` calls for `set -e` We have a couple of calls to `git config --unset` that ultimately end up as no-ops as the configuration variables aren't set (anymore) in the first place. These calls are mostly intended to recover unconditionally from tests that may have executed only partially, but they'll ultimately fail during a normal test run. This hasn't been a problem until now as we aren't running tests with `set -e`. This is about to change though, so let's silence the case where we cannot unset the config keys. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t4032-diff-inter-hunk-context.sh | 2 +- t/t7508-status.sh | 4 ++-- t/t9138-git-svn-authors-prog.sh | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t4032-diff-inter-hunk-context.sh b/t/t4032-diff-inter-hunk-context.sh index bada0cbd32f764..c98eb6abb2e3e5 100755 --- a/t/t4032-diff-inter-hunk-context.sh +++ b/t/t4032-diff-inter-hunk-context.sh @@ -17,7 +17,7 @@ f() { t() { use_config= - git config --unset diff.interHunkContext + git config --unset diff.interHunkContext || : case $# in 4) hunks=$4; cmd="diff -U$3";; diff --git a/t/t7508-status.sh b/t/t7508-status.sh index a5e21bf8bffb45..1167b835a4635c 100755 --- a/t/t7508-status.sh +++ b/t/t7508-status.sh @@ -773,8 +773,8 @@ test_expect_success TTY 'status --porcelain ignores color.status' ' ' # recover unconditionally from color tests -git config --unset color.status -git config --unset color.ui +git config --unset color.status || : +git config --unset color.ui || : test_expect_success 'status --porcelain respects -b' ' diff --git a/t/t9138-git-svn-authors-prog.sh b/t/t9138-git-svn-authors-prog.sh index 784ec7fc2d6e4d..5bb38cb23a64e9 100755 --- a/t/t9138-git-svn-authors-prog.sh +++ b/t/t9138-git-svn-authors-prog.sh @@ -68,8 +68,8 @@ test_expect_success 'authors-file overrode authors-prog' ' ) ' -git --git-dir=x/.git config --unset svn.authorsfile -git --git-dir=x/.git config --unset svn.authorsprog +git --git-dir=x/.git config --unset svn.authorsfile || : +git --git-dir=x/.git config --unset svn.authorsprog || : test_expect_success 'authors-prog imported user without email' ' svn mkdir -m gg --username gg-hermit "$svnrepo"/gg && From 0c6600cdc751e8c018e6baddc0edce070c7c3f55 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:18 +0200 Subject: [PATCH 43/51] t: prepare conditional test execution for `set -e` We have some test in our test suite where we use the pattern of `test ... && test_expect_succeess` to conditionally execute a test. The problem is that when we decide to not execute the test, we'll indeed skip the test, but the overall statement will also be unsuccessful. This will become a problem once we enable `set -e`. Prepare for this future by turning this into a proper conditional, which is also a bit easier to read overall. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t4032-diff-inter-hunk-context.sh | 12 +++++++----- t/t7450-bad-git-dotfiles.sh | 24 +++++++++++++----------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/t/t4032-diff-inter-hunk-context.sh b/t/t4032-diff-inter-hunk-context.sh index c98eb6abb2e3e5..2d216fb70f982b 100755 --- a/t/t4032-diff-inter-hunk-context.sh +++ b/t/t4032-diff-inter-hunk-context.sh @@ -40,11 +40,13 @@ t() { test $(git $cmd $file | grep '^@@ ' | wc -l) = $hunks " - test -f $expected && - test_expect_success "$label: check output" " - git $cmd $file | grep -v '^index ' >actual && - test_cmp $expected actual - " + if test -f $expected + then + test_expect_success "$label: check output" " + git $cmd $file | grep -v '^index ' >actual && + test_cmp $expected actual + " + fi } cat <expected.f1.0.1 || exit 1 diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh index f512eed278c46b..8cc86522b27d9b 100755 --- a/t/t7450-bad-git-dotfiles.sh +++ b/t/t7450-bad-git-dotfiles.sh @@ -220,17 +220,19 @@ check_dotx_symlink () { ) ' - test -n "$refuse_index" && - test_expect_success "refuse to load symlinked $name into index ($type)" ' - test_must_fail \ - git -C $dir \ - -c core.protectntfs \ - -c core.protecthfs \ - read-tree $tree 2>err && - grep "invalid path.*$name" err && - git -C $dir ls-files -s >out && - test_must_be_empty out - ' + if test -n "$refuse_index" + then + test_expect_success "refuse to load symlinked $name into index ($type)" ' + test_must_fail \ + git -C $dir \ + -c core.protectntfs \ + -c core.protecthfs \ + read-tree $tree 2>err && + grep "invalid path.*$name" err && + git -C $dir ls-files -s >out && + test_must_be_empty out + ' + fi } check_dotx_symlink gitmodules vanilla .gitmodules From 5f0d596fe4a7a4ea8752d3e5115190906c1bea4a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:19 +0200 Subject: [PATCH 44/51] t: prepare execution of potentially failing commands for `set -e` Several of our tests verify whether a certain binary can be executed, potentially skipping tests in case we cannot, for example because the binary doesn't exist. In those cases we often run the binary outside of any conditionally. This will start to fail once we enable `set -e`, as that will cause us to bail out the test immediately. Improve these tests by executing them inside of a conditional instead. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/lib-git-svn.sh | 7 +++---- t/lib-httpd.sh | 3 +-- t/t3901-i18n-patch.sh | 3 ++- t/t5000-tar-tree.sh | 4 ++-- t/t7422-submodule-output.sh | 2 +- t/t9200-git-cvsexportcommit.sh | 3 +-- t/t9400-git-cvsserver-server.sh | 5 +++-- t/t9401-git-cvsserver-crlf.sh | 4 ++-- t/t9402-git-cvsserver-refs.sh | 4 ++-- t/test-lib-functions.sh | 3 +-- t/test-lib.sh | 10 ++++++---- 11 files changed, 24 insertions(+), 24 deletions(-) diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh index 2fde2353fd3835..52843f667de0d6 100644 --- a/t/lib-git-svn.sh +++ b/t/lib-git-svn.sh @@ -15,8 +15,7 @@ GIT_SVN_DIR=$GIT_DIR/svn/refs/remotes/git-svn SVN_TREE=$GIT_SVN_DIR/svn-tree test_set_port SVNSERVE_PORT -svn >/dev/null 2>&1 -if test $? -ne 1 +if ! svn help >/dev/null 2>&1 then skip_all='skipping git svn tests, svn not found' test_done @@ -27,13 +26,13 @@ export svnrepo svnconf=$PWD/svnconf export svnconf +x=0 perl -w -e " use SVN::Core; use SVN::Repos; \$SVN::Core::VERSION gt '1.1.0' or exit(42); system(qw/svnadmin create --fs-type fsfs/, \$ENV{svnrepo}) == 0 or exit(41); -" >&3 2>&4 -x=$? +" >&3 2>&4 || x=$? if test $x -ne 0 then if test $x -eq 42; then diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index 4c76e813e396bf..fc646447d5c038 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -235,11 +235,10 @@ start_httpd() { test_atexit stop_httpd - "$LIB_HTTPD_PATH" -d "$HTTPD_ROOT_PATH" \ + if ! "$LIB_HTTPD_PATH" -d "$HTTPD_ROOT_PATH" \ -f "$TEST_PATH/apache.conf" $HTTPD_PARA \ -c "Listen 127.0.0.1:$LIB_HTTPD_PORT" -k start \ >&3 2>&4 - if test $? -ne 0 then cat "$HTTPD_ROOT_PATH"/error.log >&4 2>/dev/null test_skip_or_die GIT_TEST_HTTPD "web server setup failed" diff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh index f03601b49a9397..ef7d7e1edc2aee 100755 --- a/t/t3901-i18n-patch.sh +++ b/t/t3901-i18n-patch.sh @@ -28,7 +28,8 @@ check_encoding () { 8859) grep "^encoding ISO8859-1" ;; *) - grep "^encoding ISO8859-1"; test "$?" != 0 ;; + ret=0; grep "^encoding ISO8859-1" || ret=$? + test "$ret" != 0 ;; esac || return 1 j=$i i=$(($i+1)) diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh index 5465054f1779f0..a8c28533dc73f8 100755 --- a/t/t5000-tar-tree.sh +++ b/t/t5000-tar-tree.sh @@ -503,8 +503,8 @@ test_expect_success LONG_IS_64BIT 'set up repository with huge blob' ' # would generate the whole 64GB). test_expect_success LONG_IS_64BIT 'generate tar with huge size' ' { - git archive HEAD - echo $? >exit-code + { ret=0 && git archive HEAD || ret=$?; } && + echo "$ret" >exit-code } | test_copy_bytes 4096 >huge.tar && echo 141 >expect && test_cmp expect exit-code diff --git a/t/t7422-submodule-output.sh b/t/t7422-submodule-output.sh index aea1ddf117e8ee..852136fdfd3ec8 100755 --- a/t/t7422-submodule-output.sh +++ b/t/t7422-submodule-output.sh @@ -198,7 +198,7 @@ test_expect_success !MINGW 'git submodule status --recursive propagates SIGPIPE' ( cd repo && GIT_ALLOW_PROTOCOL=file git submodule add "$(pwd)"/../submodule && - { git submodule status --recursive 2>err; echo $?>status; } | + { { ret=0 && git submodule status --recursive 2>err || ret=$?; } && echo $ret >status; } | grep -q recursive-submodule-path-1 && test_must_be_empty err && test_match_signal 13 "$(cat status)" diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh index 14cbe9652779bc..581cf3d28fc05b 100755 --- a/t/t9200-git-cvsexportcommit.sh +++ b/t/t9200-git-cvsexportcommit.sh @@ -11,8 +11,7 @@ if ! test_have_prereq PERL; then test_done fi -cvs >/dev/null 2>&1 -if test $? -ne 1 +if ! cvs version >/dev/null 2>&1 then skip_all='skipping git cvsexportcommit tests, cvs not found' test_done diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh index e499c7f955125e..4b45398bab2295 100755 --- a/t/t9400-git-cvsserver-server.sh +++ b/t/t9400-git-cvsserver-server.sh @@ -17,12 +17,13 @@ if ! test_have_prereq PERL; then skip_all='skipping git cvsserver tests, perl not available' test_done fi -cvs >/dev/null 2>&1 -if test $? -ne 1 + +if ! cvs version >/dev/null 2>&1 then skip_all='skipping git-cvsserver tests, cvs not found' test_done fi + perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || { skip_all='skipping git-cvsserver tests, Perl SQLite interface unavailable' test_done diff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh index a34805acdc25cc..6b4cbb165131e0 100755 --- a/t/t9401-git-cvsserver-crlf.sh +++ b/t/t9401-git-cvsserver-crlf.sh @@ -60,12 +60,12 @@ check_status_options() { return $stat } -cvs >/dev/null 2>&1 -if test $? -ne 1 +if ! cvs version >/dev/null 2>&1 then skip_all='skipping git-cvsserver tests, cvs not found' test_done fi + if ! test_have_prereq PERL then skip_all='skipping git-cvsserver tests, perl not available' diff --git a/t/t9402-git-cvsserver-refs.sh b/t/t9402-git-cvsserver-refs.sh index 2ee41f9443eefb..65f2ceedecb2e7 100755 --- a/t/t9402-git-cvsserver-refs.sh +++ b/t/t9402-git-cvsserver-refs.sh @@ -68,12 +68,12 @@ check_diff() { ######### -cvs >/dev/null 2>&1 -if test $? -ne 1 +if ! cvs version >/dev/null 2>&1 then skip_all='skipping git-cvsserver tests, cvs not found' test_done fi + if ! test_have_prereq PERL then skip_all='skipping git-cvsserver tests, perl not available' diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 5fd5494ef1030c..879ee1ee597715 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -1248,8 +1248,7 @@ test_might_fail () { test_expect_code () { want_code=$1 shift - "$@" 2>&7 - exit_code=$? + exit_code=0; "$@" 2>&7 || exit_code=$? if test $exit_code = $want_code then return 0 diff --git a/t/test-lib.sh b/t/test-lib.sh index 70fd3e9bafb800..de7d9e7b925049 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -143,8 +143,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME ################################################################ # It appears that people try to run tests without building... GIT_BINARY="${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X" -"$GIT_BINARY" >/dev/null -if test $? != 1 + +if ! "$GIT_BINARY" version >/dev/null then if test -n "$GIT_TEST_INSTALLED" then @@ -454,8 +454,10 @@ then # from any previous runs. >"$GIT_TEST_TEE_OUTPUT_FILE" - (GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} "$0" "$@" 2>&1; - echo $? >"$TEST_RESULTS_BASE.exit") | tee -a "$GIT_TEST_TEE_OUTPUT_FILE" + ( + ret=0 && GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} "$0" "$@" 2>&1 || ret=$? + echo "$ret" >"$TEST_RESULTS_BASE.exit" + ) | tee -a "$GIT_TEST_TEE_OUTPUT_FILE" test "$(cat "$TEST_RESULTS_BASE.exit")" = 0 exit fi From c1e29bcfa88fb8997379c3c7c888961b728e581d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:20 +0200 Subject: [PATCH 45/51] t: prepare `test_when_finished ()`/`test_atexit()` for `set -e` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both `test_when_finished ()` and `test_atexit ()` build up a chain of cleanup commands by prepending each new command to the existing cleanup string. To preserve the exit code of the test body across cleanup execution, we append the following logic: } && (exit "$eval_ret"); eval_ret=$?; ... The intent of this is to run the cleanup block and then unconditionally restore `eval_ret`. The original behaviour of this is is: +------------------+---------+------------------------------------+ |test body │ cleanup │ old behaviour │ +------------------+---------+------------------------------------+ │pass (eval_ret=0) | pass │ && taken -> (exit 0) -> eval_ret=0 | +------------------+---------+------------------------------------+ │pass (eval_ret=0) | fail │ && not taken -> eval_ret=$? | +------------------+---------+------------------------------------+ │fail (eval_ret=1) | pass │ && taken -> (exit 1) -> eval_ret=1 | +------------------+---------+------------------------------------+ │fail (eval_ret=1) | fail | && not taken -> eval_ret=$? | +------------------+---------+------------------------------------+ This logic will start to fail once we enable `set -e`. When `$eval_ret` is non-zero, the subshell we create will fail, and with `set -e` we'll thus bail out without evaluating the logic after the semicolon. Fix this issue by instead using `|| eval_ret=\$?; ...`. Besides being a bit simpler, it also retains the original behaviour: +------------------+---------+------------------------------------+ |test body │ cleanup │ old behaviour │ +------------------+---------+------------------------------------+ │pass (eval_ret=0) | pass │ || not taken -> eval_ret unchanged | +------------------+---------+------------------------------------+ │pass (eval_ret=0) | fail │ || taken -> eval_ret=$? | +------------------+---------+------------------------------------+ │fail (eval_ret=1) | pass │ || not taken -> eval_ret unchanged | +------------------+---------+------------------------------------+ │fail (eval_ret=1) | fail | || taken -> eval_ret=$? | +------------------+---------+------------------------------------+ Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/test-lib-functions.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 879ee1ee597715..502bb0ddcbca31 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -1512,7 +1512,7 @@ test_when_finished () { test "${BASH_SUBSHELL-0}" = 0 || BUG "test_when_finished does nothing in a subshell" test_cleanup="{ $* - } && (exit \"\$eval_ret\"); eval_ret=\$?; $test_cleanup" + } || eval_ret=\$?; $test_cleanup" } # This function can be used to schedule some commands to be run @@ -1540,7 +1540,7 @@ test_atexit () { test "${BASH_SUBSHELL-0}" = 0 || BUG "test_atexit does nothing in a subshell" test_atexit_cleanup="{ $* - } && (exit \"\$eval_ret\"); eval_ret=\$?; $test_atexit_cleanup" + } || eval_ret=\$?; $test_atexit_cleanup" } # Deprecated wrapper for "git init", use "git init" directly instead From b94900a0cf20e213038102a4cbf072dec5b1bb18 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:21 +0200 Subject: [PATCH 46/51] t0008: silence error in subshell when using `grep -v` In t0008 we use `grep -v` in a subshell, but expect that this command will sometimes not match anything. This would cause grep(1) to return an error code, but given that we don't run with `set -e` we swallow this error. We're about to enable `set -e`. Prepare for this by ignoring any errors. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t0008-ignores.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh index e716b5cdfa1bee..d77a179bddcbed 100755 --- a/t/t0008-ignores.sh +++ b/t/t0008-ignores.sh @@ -122,8 +122,8 @@ test_expect_success_multiple () { fi testname="$1" expect_all="$2" code="$3" - expect_verbose=$( echo "$expect_all" | grep -v '^:: ' ) - expect=$( echo "$expect_verbose" | sed -e 's/.* //' ) + expect_verbose=$(echo "$expect_all" | grep -v '^:: ' || :) + expect=$(echo "$expect_verbose" | sed -e 's/.* //') test_expect_success $prereq "$testname${no_index_opt:+ with $no_index_opt}" ' expect "$expect" && From 4f917bbf718047dc1b3c4346a9c47c84a52a810b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:22 +0200 Subject: [PATCH 47/51] t1301: don't fail in case setfacl(1) doesn't exist or fails In t1301 we're trying to remove any potentially-existing default ACLs that might exist on the transh directory by executing setfacl(1). According to 8ed0a740dd (t1301-shared-repo.sh: don't let a default ACL interfere with the test, 2008-10-16), this is done because we play around with permissions and umasks in this test suite. The setfacl(1) binary may not exist on some systems though, even though tests ultimately still pass. This doesn't matter currently, but will cause the test to fail once we start running with `set -e`. Silence such failures by ignoring failures here. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t1301-shared-repo.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh index 630a47af21e655..0e0d07a1a1e7d8 100755 --- a/t/t1301-shared-repo.sh +++ b/t/t1301-shared-repo.sh @@ -12,7 +12,7 @@ TEST_CREATE_REPO_NO_TEMPLATE=1 . ./test-lib.sh # Remove a default ACL from the test dir if possible. -setfacl -k . 2>/dev/null +setfacl -k . 2>/dev/null || : # User must have read permissions to the repo -> failure on --shared=0400 test_expect_success 'shared = 0400 (faulty permission u-w)' ' From 090af9957c14915461f302f422d2c147b3e9175b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:23 +0200 Subject: [PATCH 48/51] t6002: fix use of `expr` with `set -e` In `test_bisection_diff ()` we use `expr` to perform some math. This command has some gotchas though in that it will only return success when the result is neither null nor zero. In some of our cases though it actually _is_ zero, and that will cause the expressions to fail once we enable `set -e`. Prepare for this change by instead using `$(( ))`, which doesn't have the same issue. While at it, modernize the function a tiny bit. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t6002-rev-list-bisect.sh | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh index daa009c9a1b4b6..f2de40b5ed8f14 100755 --- a/t/t6002-rev-list-bisect.sh +++ b/t/t6002-rev-list-bisect.sh @@ -27,13 +27,16 @@ test_bisection_diff() # Test if bisection size is close to half of list size within # tolerance. # - _bisect_err=$(expr $_list_size - $_bisection_size \* 2) - test "$_bisect_err" -lt 0 && _bisect_err=$(expr 0 - $_bisect_err) - _bisect_err=$(expr $_bisect_err / 2) ; # floor - - test_expect_success \ - "bisection diff $_bisect_option $_head $* <= $_max_diff" \ - 'test $_bisect_err -le $_max_diff' + _bisect_err=$(($_list_size - $_bisection_size * 2)) + if test "$_bisect_err" -lt 0 + then + _bisect_err=$((0 - $_bisect_err)) + fi + _bisect_err=$(($_bisect_err / 2)) ; # floor + + test_expect_success "bisection diff $_bisect_option $_head $* <= $_max_diff" ' + test $_bisect_err -le $_max_diff + ' } date >path0 From 1ecf6538263cf81c151f2e720cd73fde3d50a07e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:24 +0200 Subject: [PATCH 49/51] t9902: fix use of `read` with `set -e` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In t9902 we're using the `read` builtin to read some values into a variable. This is done by using `-d ""`, which cause us to read until the end of the heredoc. As the read is terminated by EOF, the command will end up returning a non-zero error code. This hasn't been an issue until now as we didn't run with `set -e`, but that'll change in a subsequent commit. Prepare for this change by not using read at all, as we can simply store the multi-line value directly. Suggested-by: SZEDER Gábor Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t9902-completion.sh | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 2f9a597ec7f493..28f61f08fb4cec 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -590,12 +590,10 @@ test_expect_success '__gitcomp - doesnt fail because of invalid variable name' ' __gitcomp "$invalid_variable_name" ' -read -r -d "" refs <<-\EOF -main +refs='main maint next -seen -EOF +seen' test_expect_success '__gitcomp_nl - trailing space' ' test_gitcomp_nl "m" "$refs" <<-EOF From ffe8005b9d8e0cf1b5d5d796ca4da76fbe219011 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:25 +0200 Subject: [PATCH 50/51] t: detect errors outside of test cases We have recently merged a patch series that had a simple misspelling of `test_expect_success`. Instead of making our tests fail though, this typo went completely undetected and all of our tests passed, which is of course unfortunate. This is a more general issue with our test suite: all commands that run outside of a specific test case can fail, and if we don't explicitly check for such failure then this failure will be silently ignored. Improve the status quo by enabling the errexit option so that any such unchecked failures will cause us to abort immediately. Note that for now, we only enable this option for Bash 5 and newer. This is because other shells have wildly different behaviour, and older versions of Bash (especially on macOS) are buggy. The list of enabled shells may be extended going forward. Helped-by: Jeff King Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- ci/run-build-and-tests.sh | 6 ++++++ t/test-lib.sh | 25 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh index 28cfe730ee5aed..de08a08d59fc8f 100755 --- a/ci/run-build-and-tests.sh +++ b/ci/run-build-and-tests.sh @@ -7,6 +7,12 @@ export TEST_CONTRIB_TOO=yes +case "$jobname" in +almalinux-*|debian-*|fedora-*|linux-*) + export GIT_TEST_USE_SET_E=yes + ;; +esac + case "$jobname" in fedora-breaking-changes-musl|linux-breaking-changes) export WITH_BREAKING_CHANGES=YesPlease diff --git a/t/test-lib.sh b/t/test-lib.sh index de7d9e7b925049..cded7bd693e6f6 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -15,6 +15,31 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see https://www.gnu.org/licenses/ . +# Enable the use of errexit so that any unexpected failures will cause us to +# abort tests, even when outside of a specific test case. +# +# Note that we only enable this on Bash 5 and newer, or when explicitly +# requested by the user via `GIT_TEST_USE_SET_E=true`. This ib secause `set -e` +# has wildly different behaviour across shells. The list of default-enabled +# shells may be extended going forward. +if test -z "$GIT_TEST_USE_SET_E" && test "${BASH_VERSINFO:=0}" -ge 5 +then + GIT_TEST_USE_SET_E=true +fi + +# We cannot use `test-tool env-helper` here, as it's not yet available. +case "${GIT_TEST_USE_SET_E:-false}" in +1|on|true|yes) + set -e + ;; +0|off|false|no) + ;; +*) + echo "GIT_TEST_USE_SET_E requires a boolean" >&2 + exit 1 + ;; +esac + # Test the binaries we have just built. The tests are kept in # t/ subdirectory and are run in 'trash directory' subdirectory. if test -z "$TEST_DIRECTORY" From 8a101334b374889938403824af956cd92e47b84d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 11 May 2026 10:04:56 +0900 Subject: [PATCH 51/51] Start 2.55 cycle Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.55.0.adoc | 43 ++++++++++++++++++++++++++++++ RelNotes | 2 +- 2 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 Documentation/RelNotes/2.55.0.adoc diff --git a/Documentation/RelNotes/2.55.0.adoc b/Documentation/RelNotes/2.55.0.adoc new file mode 100644 index 00000000000000..3bee5b9dd85e06 --- /dev/null +++ b/Documentation/RelNotes/2.55.0.adoc @@ -0,0 +1,43 @@ +Git v2.55 Release Notes +======================= + +UI, Workflows & Features +------------------------ + + * Hook scripts defined via the configuration system can now be + configured to run in parallel. + + * The userdiff driver for the Scheme language has been extended to + cover other Lisp dialects. + + +Performance, Internal Implementation, Development Support etc. +-------------------------------------------------------------- + + * Promisor remote handling has been refactored and fixed in + preparation for auto-configuration of advertised remotes. + + * Rust support is enabled by default (but still allows opting out) in + some future version of Git. + + +Fixes since v2.54 +----------------- + + * Code clean-up to use the right instance of a repository instance in + calls inside refs subsystem. + (merge 57c590feb9 sp/refs-reduce-the-repository later to maint). + + * The check that implements the logic to see if an in-core cache-tree + is fully ready to write out a tree object was broken, which has + been corrected. + (merge 521731213c dl/cache-tree-fully-valid-fix later to maint). + + * The test suite harness and many individual test scripts have been + updated to work correctly when 'set -e' is in effect, which helps + detect misspelled test commands. + (merge ffe8005b9d ps/test-set-e-clean later to maint). + + * Other code cleanup, docfix, build fix, etc. + (merge 80f4b802e9 ja/doc-difftool-synopsis-style later to maint). + (merge b96490241e jc/doc-timestamps-in-stat later to maint). diff --git a/RelNotes b/RelNotes index 84ef7387adb35c..159e44a9490cc3 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.54.0.adoc \ No newline at end of file +Documentation/RelNotes/2.55.0.adoc \ No newline at end of file