* [PATCH v1 06/11] git gui: GIT_DIR / GIT_WORK_TREE make any discovery error fatal
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
In-Reply-To: <20260514143322.865587-1-mlevedahl@gmail.com>
git accepts any combination of GIT_DIR and GIT_WORK_TREE to override the
normal repository and worktree discovery process. git-gui should accept
any such valid configuration, but overriding the discovery process means
the user has assured that the combination of current directory, GIT_DIR,
and GIT_WORK_TREE will lead to the correct repository and worktree. As
such, an error found during discovery where either or both of GIT_DIR
and GIT_WORK_TREE are set is a fatal error, no further exploration
should be tried.
Provide a common proc to support displaying an error message and exiting
if GIT_DIR or GIT_WORK_TREE are in the environment.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/git-gui.sh b/git-gui.sh
index c2cf5f1..2e2ddc0 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1122,6 +1122,24 @@ unset argv0dir
##
## repository setup
+proc is_gitvars_error {err} {
+ set havevars 0
+ set GIT_DIR {}
+ set GIT_WORK_TREE {}
+ catch {set GIT_DIR $::env(GIT_DIR); set havevars 1}
+ catch {set GIT_WORK_TREE $::env(GIT_WORK_TREE) ; set havevars 1}
+
+ if {$havevars} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Invalid configuration:"] \
+ "\n" "GIT_DIR: " $GIT_DIR \
+ "\n" "GIT_WORK_TREE: " $GIT_WORK_TREE \
+ "\n\n$err"]
+ return 1
+ }
+ return 0
+}
+
proc set_gitdir_vars {} {
global _gitdir _gitworktree env
if {$_gitdir ne {}} {
--
2.54.0.99.14
^ permalink raw reply related
* [PATCH v1 03/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
In-Reply-To: <20260514143322.865587-1-mlevedahl@gmail.com>
git-gui unconditionally exports GIT_DIR and GIT_WORK_TREE to the
environment, and furthmore unconditionally unsets these in many places.
But, GIT_WORK_TREE should be set only if it is not {} as the empty
value, really meaning no work-tree is found, causes git to throw fatal
errors (git-gui gets the error from branch --show-current). Fixing this
is required to allow blame and browser to operate from a repository
without a worktree.
Establish a pair of functions to remove GIT_DIR and GIT_WORK_TREE from
the environment, avoiding any error if they do not exist. Also, add a
function to export these, but export GIT_WORK_TREE only if not empty.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index a951fcd..387cad6 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1122,6 +1122,22 @@ unset argv0dir
##
## repository setup
+proc set_gitdir_vars {} {
+ global _gitdir _gitworktree env
+ if {$_gitdir ne {}} {
+ set env(GIT_DIR) $_gitdir
+ }
+ if {$_gitworktree ne {}} {
+ set env(GIT_WORK_TREE) $_gitworktree
+ }
+}
+
+proc unset_gitdir_vars {} {
+ global env
+ catch {unset env(GIT_DIR)}
+ catch {unset env(GIT_WORK_TREE)}
+}
+
set picked 0
if {[catch {
set _gitdir $env(GIT_DIR)
@@ -1207,8 +1223,8 @@ if {[lindex $_reponame end] eq {.git}} {
set _reponame [lindex $_reponame end]
}
-set env(GIT_DIR) $_gitdir
-set env(GIT_WORK_TREE) $_gitworktree
+# Export the final paths
+set_gitdir_vars
######################################################################
##
@@ -2050,13 +2066,11 @@ proc do_gitk {revs {is_submodule false}} {
# TODO we could make life easier (start up faster?) for gitk
# by setting these to the appropriate values to allow gitk
# to skip the heuristics to find their proper value
- unset env(GIT_DIR)
- unset env(GIT_WORK_TREE)
+ unset_gitdir_vars
}
safe_exec_bg [concat $cmd $revs "--" "--"]
- set env(GIT_DIR) $_gitdir
- set env(GIT_WORK_TREE) $_gitworktree
+ set_gitdir_vars
cd $pwd
if {[info exists main_status]} {
@@ -2084,16 +2098,14 @@ proc do_git_gui {} {
# see note in do_gitk about unsetting these vars when
# running tools in a submodule
- unset env(GIT_DIR)
- unset env(GIT_WORK_TREE)
+ unset_gitdir_vars
set pwd [pwd]
cd $current_diff_path
safe_exec_bg [concat $exe gui]
- set env(GIT_DIR) $_gitdir
- set env(GIT_WORK_TREE) $_gitworktree
+ set_gitdir_vars
cd $pwd
set status_operation [$::main_status \
--
2.54.0.99.14
^ permalink raw reply related
* [PATCH v1 05/11] git-gui: use --absolute-git-dir
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
In-Reply-To: <20260514143322.865587-1-mlevedahl@gmail.com>
git-gui uses git rev-parse --git-dir to get the pathname of the
discovered git repository. The returned value can be relative, and is
'.' if the current directory is the top of the repository directory
itself. git-gui has code to change '.' to [pwd] in this case so that
subsequent logic runs.
But, git rev-parse supports --absolute-git-dir from fac60b8925
("rev-parse: add option for absolute or relative path formatting",
2020-12-13), and included in git 2.31. git-gui requires git >= 2.36, so
this more useful form is always available. Use --absolute-git-dir to
always get an absolute path, avoiding the need for other checks.
---
git-gui.sh | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 0b73c35..c2cf5f1 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1156,7 +1156,7 @@ if {[catch {
&& [catch {
# beware that from the .git dir this sets _gitdir to .
# and _prefix to the empty string
- set _gitdir [git rev-parse --git-dir]
+ set _gitdir [git rev-parse --absolute-git-dir]
set _prefix [git rev-parse --show-prefix]
} err]} {
pick_repo
@@ -1173,18 +1173,12 @@ if {$hashalgorithm eq "sha1"} {
exit 1
}
-# we expand the _gitdir when it's just a single dot (i.e. when we're being
-# run from the .git dir itself) lest the routines to find the worktree
-# get confused
-if {$_gitdir eq "."} {
- set _gitdir [pwd]
-}
-
if {![file isdirectory $_gitdir]} {
catch {wm withdraw .}
error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
exit 1
}
+
# _gitdir exists, so try loading the config
load_config 0
apply_config
--
2.54.0.99.14
^ permalink raw reply related
* [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
In-Reply-To: <20260514143322.865587-1-mlevedahl@gmail.com>
git-gui includes a 'repository picker', which allows creating a new
repository + worktree, or selecting a worktree from a recent list.
git-gui runs the picker when a valid git repository is not found. All of
the code for this is embedded in the discovery process block, making the
latter more difficult to read, and also making things more difficult if
we want to have an explicit 'pick' subcommand to force this to run.
Let's move this invocation and supporting code to a separate proc,
aiding in subsequent refactoring. Assure GIT_DIR and GIT_WORK_TREE are
unset, configuration is loaded, ant that _gitdir is correctly set
afterwards. As this is invoked before worktree discovery, later code
will set that anyway so need not be included here.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 387cad6..0b73c35 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1139,6 +1139,16 @@ proc unset_gitdir_vars {} {
}
set picked 0
+proc pick_repo {} {
+ unset_gitdir_vars
+ load_config 1
+ apply_config
+ choose_repository::pick
+ set _gitdir [git rev-parse --absolute-git-dir]
+ set _prefix {}
+ set picked 1
+}
+
if {[catch {
set _gitdir $env(GIT_DIR)
set _prefix {}
@@ -1149,13 +1159,7 @@ if {[catch {
set _gitdir [git rev-parse --git-dir]
set _prefix [git rev-parse --show-prefix]
} err]} {
- load_config 1
- apply_config
- choose_repository::pick
- if {![file isdirectory $_gitdir]} {
- exit 1
- }
- set picked 1
+ pick_repo
}
# Use object format as hash algorithm (either "sha1" or "sha256")
--
2.54.0.99.14
^ permalink raw reply related
* [PATCH v1 02/11] git-gui: refactor browser / blame argument parsing
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
In-Reply-To: <20260514143322.865587-1-mlevedahl@gmail.com>
git-gui has subcommands blame and browser, both of which accept a
pathname, possibly preceded by a commit-ish item to specify a revision.
Also, blame can take a first argument that gives a line number to focus.
The command line parser for the above is more complex than needed, and
cannot work without a worktree as the pathname objects are checked
against the current worktree for existence. This also precludes naming a
directory or file that does not exist on the currently checked out
branch.
So, replace this with a simpler parser that looks at argument number and
number of arguments to know what value to expect. The blame and browser
backends already have error checking with diagnostic information, so
defer most error checking to those. Also, allow a line-number selection
to be given and silently ignored for the browser, further simplifying
this code.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 66 +++++++++++++-----------------------------------------
1 file changed, 16 insertions(+), 50 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 6048f92..a951fcd 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2986,51 +2986,34 @@ blame {
set head {}
set path {}
set jump_spec {}
- set is_path 0
+ set nargs [llength $argv]
+ if {$nargs < 1} {
+ usage
+ }
+ set argn 0
foreach a $argv {
- set p [file join $_prefix $a]
+ set argn [expr {$argn + 1}]
- if {$is_path || [file exists $p]} {
- if {$path ne {}} usage
- set path [normalize_relpath $p]
- break
- } elseif {$a eq {--}} {
- if {$path ne {}} {
- if {$head ne {}} usage
- set head $path
- set path {}
+ if {$argn < $nargs} {
+ # revision or line number
+ if {[regexp {^--line=(\d+)$} $a a lnum]} {
+ set jump_spec [list $lnum]
+ } else {
+ set head $a
}
- set is_path 1
- } elseif {[regexp {^--line=(\d+)$} $a a lnum]} {
- if {$jump_spec ne {} || $head ne {}} usage
- set jump_spec [list $lnum]
- } elseif {$head eq {}} {
- if {$head ne {}} usage
- set head $a
- set is_path 1
- } else {
- usage
- }
- }
- unset is_path
-
- if {$head ne {} && $path eq {}} {
- if {[string index $head 0] eq {/}} {
- set path [normalize_relpath $head]
- set head {}
} else {
- set path [normalize_relpath $_prefix$head]
- set head {}
+ set path [normalize_relpath $a]
}
}
if {$head eq {}} {
load_current_branch
+ set head $current_branch
} else {
if {[regexp [string map "@@ [expr $hashlength - 1]" {^[0-9a-f]{1,@@}$}] $head]} {
if {[catch {
- set head [git rev-parse --verify $head]
- } err]} {
+ set head [git rev-parse --verify $head]
+ } err]} {
if {[tk windowingsystem] eq "win32"} {
tk_messageBox -icon error -title [mc Error] -message $err
} else {
@@ -3046,26 +3029,9 @@ blame {
switch -- $subcommand {
browser {
if {$jump_spec ne {}} usage
- if {$head eq {}} {
- if {$path ne {} && [file isdirectory $path]} {
- set head $current_branch
- } else {
- set head $path
- set path {}
- }
- }
browser::new $head $path
}
blame {
- if {$head eq {} && ![file exists $path]} {
- catch {wm withdraw .}
- tk_messageBox \
- -icon error \
- -type ok \
- -title [mc "git-gui: fatal error"] \
- -message [mc "fatal: cannot stat path %s: No such file or directory" $path]
- exit 1
- }
blame::new $head $path $jump_spec
}
}
--
2.54.0.99.14
^ permalink raw reply related
* [PATCH v1 01/11] git-gui: allow specifying path '.' to the browser
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
In-Reply-To: <20260514143322.865587-1-mlevedahl@gmail.com>
Invoking "git-gui browser rev ." should show the file browser for the
commitish rev, starting at the root directory. This errors out in
normalize_relpath because the '.' is removed, yielding an empty list as
argument to [file join ...]. Fix this.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/git-gui.sh b/git-gui.sh
index 23fe76e..6048f92 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2965,7 +2965,11 @@ proc normalize_relpath {path} {
}
lappend elements $item
}
- return [eval file join $elements]
+ if {$elements ne {}} {
+ return [eval file join $elements]
+ } else {
+ return {}
+ }
}
# -- Not a normal commit type invocation? Do that instead!
--
2.54.0.99.14
^ permalink raw reply related
* [PATCH v1 00/11] Improve git gui operation without a worktree
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
In-Reply-To: <50df7f28-c63c-4762-b542-b888ea3604c0@gmail.com>
git gui has a number of inter-related problems that result in problems
during startup from anything but a checked out worktree pointing at a
valid git repository. Some of the symptoms are:
- blame / browser subcommands, and launching gitk, are intended to be
useful without a worktree, but fail to work.
- unlike git, git-gui is supposed to use the parent directory as a
worktree if started from the .git subdirectory in the very common
single worktree + embedded git repository format. This does not
work.
- git-gui includes a repository picker allowing a user select a worktree
from a list and/or start a new repo+worktree: this dialog appears at
unexpected times, masking useful error feedback on configuration
problems.
This patch series addresses the above issues, substantially rewriting
the blame / browser command line process, the initial repository and
worktree discovery processes, and using git rev-parse when possible to
handle repository / worktree discovery including any specification of
GIT_DIR or GIT_WORK_TREE to reduce the future likelihood of conflict
with command line git. This also allows explicit user control to avoid
the repository picker masking a configuration error.
Note: I question why git-gui ever exports GIT_WORK_TREE. If it is not
empty, that is the current directory when startup is complete and any
git command will use the current directory as the worktree. If empty,
there is no worktree and the current directory should be (and after this
series, is) at the toplevel of the gitdir: again, there is nothing to
communicate to another process. If a process being launched needs a
different worktree, that should be the startup directory given to the
process without changing git-gui's current directory.
Mark Levedahl (11):
git-gui: allow specifying path '.' to the browser
git-gui: refactor browser / blame argument parsing
git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
git-gui: put choose_repository::pick in a proc
git-gui: use --absolute-git-dir
git gui: GIT_DIR / GIT_WORK_TREE make any discovery error fatal
git-gui: use rev-parse exclusively to find a repository
git-gui: simplify [is_bare] to report if a worktree is known
git-gui: support using repository parent dir as a worktree
git-gui: improve worktree discovery
git-gui: add gui and pick as explicit subcommands
git-gui.sh | 276 ++++++++++++++++++++++++++---------------------------
1 file changed, 135 insertions(+), 141 deletions(-)
--
2.54.0.99.14
^ permalink raw reply
* Re: [PATCH v7 0/3] git-gui: robustify startup and fix environment handling
From: Mark Levedahl @ 2026-05-14 14:28 UTC (permalink / raw)
To: Shroom Moo, git; +Cc: Johannes Sixt, Aina Boot
In-Reply-To: <tencent_66A1C2CDB9D5B764A5B4468D3F11845A2A09@qq.com>
On 5/9/26 9:37 AM, Shroom Moo wrote:
> Shroom Moo (3):
> git-gui: restructure repository startup
> git-gui: disable gitk visualization when no worktree available
> git-gui: handle GIT_DIR and GIT_WORK_TREE early
>
>
After careful consideration, I find starting off by fixing what is broken in git-gui about
using a bare-repository, and letting git core handle GIT_DIR and GIT_WORK_TREE, leads to a
much more complete and different solution. A patch series (attempting to) do so will follow.
Mark
^ permalink raw reply
* Re: I object the social engineering from Master to Main
From: Chandra @ 2026-05-14 13:42 UTC (permalink / raw)
To: A D, git@vger.kernel.org
In-Reply-To: <CANULcizX5J5zE+QgY9TOvqpjrcJM3uFOOAWRJzOW2rBweg2WNA@mail.gmail.com>
I am strongly in favor of changing master to main. As a person currently in a country where 14 million of my people are enslaved, who also had experiences living in a factory as a child, with many friends who are descendents or have their own experiences being trafficked, this is one of those things that causes unnecessary cognitive ergonomic friction. It's a bandwidth tax for anyone not privileged enough to know only freedom.
As an engineering tool, creating pointless cognitive bandwidth sapping is counterproductive.
This is not just about history, this is about the present, where even in Florida there are 700k people who were trafficked in the last year. This is not just about the sensitivities of the privileged, but also about preserving the cognitive bandwidth of those without it.
An engineering tool should not cause emotional recoil just to use. Accessible and ergonomic design standards ought to be upheld to support seamless usage for all who use a tool.
Thank you,
Chandra Kethi-Reddy
@archonphronesis:matrix.org
Sent from Proton Mail for iOS.
-------- Original Message --------
On Thursday, 05/14/26 at 19:00 A D <diop.alpha@gmail.com> wrote:
It is not your role, as a tool, to try to engineer society through
warnings or changes of names. Your role is to deliver solid, reliable
versioning, that's it! Leave the rest to spheres where it can actually
be done without weaponizing your position or your audience.
I happen to be black African myself, and I never had any problem with
the term "Master", and its history for literal decades. The term has
been used in Computer Engineering longer than some of you have been
alive, and I see no reason to change, or to nudge people toward
changing it, just because it offends the sensibilities of some of you,
who I very much doubt are even black in the first place.
"Master" is a term with a history, and it's cool, if some of you are
offended by it, change your repos, but leave ours alone and keep your
warnings to yourself.
Cordially,
Saliou Alpha Diop
^ permalink raw reply
* I object the social engineering from Master to Main
From: A D @ 2026-05-14 13:30 UTC (permalink / raw)
To: git
It is not your role, as a tool, to try to engineer society through
warnings or changes of names. Your role is to deliver solid, reliable
versioning, that's it! Leave the rest to spheres where it can actually
be done without weaponizing your position or your audience.
I happen to be black African myself, and I never had any problem with
the term "Master", and its history for literal decades. The term has
been used in Computer Engineering longer than some of you have been
alive, and I see no reason to change, or to nudge people toward
changing it, just because it offends the sensibilities of some of you,
who I very much doubt are even black in the first place.
"Master" is a term with a history, and it's cool, if some of you are
offended by it, change your repos, but leave ours alone and keep your
warnings to yourself.
Cordially,
Saliou Alpha Diop
^ permalink raw reply
* [PATCH v4 8/8] send-pack: pass negotiation config in push
From: Derrick Stolee via GitGitGadget @ 2026-05-14 12:41 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v4.git.1778762495.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
When push.negotiate is enabled, 'git push' spawns a child 'git fetch
--negotiate-only' process to find common commits. Pass
--negotiation-include and --negotiation-restrict options from the
'remote.<name>.negotiationInclude' and
'remote.<name>.negotiationRestrict' config keys to this child process.
When negotiationRestrict is configured, it replaces the default
behavior of using all remote refs as negotiation tips. This allows
the user to control which local refs are used for push negotiation.
When negotiationInclude is configured, the specified ref patterns
are passed as --negotiation-include to ensure their tips are always
sent as 'have' lines during push negotiation.
This change also updates the use of --negotiation-tip into
--negotiation-restrict now that the new synonym exists.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/config/remote.adoc | 6 ++++++
send-pack.c | 37 ++++++++++++++++++++++++++------
send-pack.h | 2 ++
t/t5516-fetch-push.sh | 30 ++++++++++++++++++++++++++
transport.c | 2 ++
5 files changed, 70 insertions(+), 7 deletions(-)
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 9ae20e4379..460b4e7952 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -122,6 +122,9 @@ command-line option. If `--negotiation-restrict` (or its synonym
`--negotiation-tip`) is specified on the command line, then the config
values are not used.
+
+These values also influence negotiation during `git push` if
+`push.negotiate` is enabled.
++
Blank values signal to ignore all previous values, allowing a reset of
the list from broader config scenarios.
@@ -149,6 +152,9 @@ unconditionally on top of those heuristically selected commits. This
option is also used during push negotiation when `push.negotiate` is
enabled.
+
+These values also influence negotiation during `git push` if
+`push.negotiate` is enabled.
++
Blank values signal to ignore all previous values, allowing a reset of
the list from broader config scenarios.
diff --git a/send-pack.c b/send-pack.c
index 3d5d36ba3b..d18e030ce8 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -433,28 +433,48 @@ static void reject_invalid_nonce(const char *nonce, int len)
static void get_commons_through_negotiation(struct repository *r,
const char *url,
+ const struct string_list *negotiation_include,
+ const struct string_list *negotiation_restrict,
const struct ref *remote_refs,
struct oid_array *commons)
{
struct child_process child = CHILD_PROCESS_INIT;
const struct ref *ref;
int len = r->hash_algo->hexsz + 1; /* hash + NL */
- int nr_negotiation_tip = 0;
+ int nr_negotiation = 0;
child.git_cmd = 1;
child.no_stdin = 1;
child.out = -1;
strvec_pushl(&child.args, "fetch", "--negotiate-only", NULL);
- for (ref = remote_refs; ref; ref = ref->next) {
- if (!is_null_oid(&ref->new_oid)) {
+
+ if (negotiation_restrict && negotiation_restrict->nr) {
+ struct string_list_item *item;
+ for_each_string_list_item(item, negotiation_restrict)
strvec_pushf(&child.args, "--negotiation-restrict=%s",
- oid_to_hex(&ref->new_oid));
- nr_negotiation_tip++;
+ item->string);
+ nr_negotiation = negotiation_restrict->nr;
+ } else {
+ for (ref = remote_refs; ref; ref = ref->next) {
+ if (!is_null_oid(&ref->new_oid)) {
+ strvec_pushf(&child.args, "--negotiation-restrict=%s",
+ oid_to_hex(&ref->new_oid));
+ nr_negotiation++;
+ }
}
}
+
+ if (negotiation_include && negotiation_include->nr) {
+ struct string_list_item *item;
+ for_each_string_list_item(item, negotiation_include)
+ strvec_pushf(&child.args, "--negotiation-include=%s",
+ item->string);
+ nr_negotiation += negotiation_include->nr;
+ }
+
strvec_push(&child.args, url);
- if (!nr_negotiation_tip) {
+ if (!nr_negotiation) {
child_process_clear(&child);
return;
}
@@ -528,7 +548,10 @@ int send_pack(struct repository *r,
repo_config_get_bool(r, "push.negotiate", &push_negotiate);
if (push_negotiate) {
trace2_region_enter("send_pack", "push_negotiate", r);
- get_commons_through_negotiation(r, args->url, remote_refs, &commons);
+ get_commons_through_negotiation(r, args->url,
+ args->negotiation_include,
+ args->negotiation_restrict,
+ remote_refs, &commons);
trace2_region_leave("send_pack", "push_negotiate", r);
}
diff --git a/send-pack.h b/send-pack.h
index c5ded2d200..13850c98bb 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -18,6 +18,8 @@ struct repository;
struct send_pack_args {
const char *url;
+ const struct string_list *negotiation_include;
+ const struct string_list *negotiation_restrict;
unsigned verbose:1,
quiet:1,
porcelain:1,
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index ac8447f21e..177cbc6c75 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -254,6 +254,36 @@ test_expect_success 'push with negotiation does not attempt to fetch submodules'
! grep "Fetching submodule" err
'
+test_expect_success 'push with negotiation and remote.<name>.negotiationInclude' '
+ test_when_finished rm -rf negotiation_include &&
+ mk_empty negotiation_include &&
+ git push negotiation_include $the_first_commit:refs/remotes/origin/first_commit &&
+ test_commit -C negotiation_include unrelated_commit &&
+ git -C negotiation_include config receive.hideRefs refs/remotes/origin/first_commit &&
+ test_when_finished "rm event" &&
+ GIT_TRACE2_EVENT="$(pwd)/event" \
+ git -c protocol.version=2 -c push.negotiate=1 \
+ -c remote.negotiation_include.negotiationInclude=refs/heads/main \
+ push negotiation_include refs/heads/main:refs/remotes/origin/main &&
+ test_grep \"key\":\"total_rounds\" event &&
+ grep_wrote 2 event # 1 commit, 1 tree
+'
+
+test_expect_success 'push with negotiation and remote.<name>.negotiationRestrict' '
+ test_when_finished rm -rf negotiation_restrict &&
+ mk_empty negotiation_restrict &&
+ git push negotiation_restrict $the_first_commit:refs/remotes/origin/first_commit &&
+ test_commit -C negotiation_restrict unrelated_commit &&
+ git -C negotiation_restrict config receive.hideRefs refs/remotes/origin/first_commit &&
+ test_when_finished "rm event" &&
+ GIT_TRACE2_EVENT="$(pwd)/event" \
+ git -c protocol.version=2 -c push.negotiate=1 \
+ -c remote.negotiation_restrict.negotiationRestrict=refs/heads/main \
+ push negotiation_restrict refs/heads/main:refs/remotes/origin/main &&
+ test_grep \"key\":\"total_rounds\" event &&
+ grep_wrote 2 event # 1 commit, 1 tree
+'
+
test_expect_success 'push without wildcard' '
mk_empty testrepo &&
diff --git a/transport.c b/transport.c
index fa54928966..a2d8958cb8 100644
--- a/transport.c
+++ b/transport.c
@@ -921,6 +921,8 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
args.atomic = !!(flags & TRANSPORT_PUSH_ATOMIC);
args.push_options = transport->push_options;
args.url = transport->url;
+ args.negotiation_include = &transport->remote->negotiation_include;
+ args.negotiation_restrict = &transport->remote->negotiation_restrict;
if (flags & TRANSPORT_PUSH_CERT_ALWAYS)
args.push_cert = SEND_PACK_PUSH_CERT_ALWAYS;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 7/8] remote: add remote.*.negotiationInclude config
From: Derrick Stolee via GitGitGadget @ 2026-05-14 12:41 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v4.git.1778762495.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
Add a new 'remote.<name>.negotiationInclude' multi-valued config option that
provides default values for --negotiation-include when no
--negotiation-include arguments are specified over the command line. This
is a mirror of how 'remote.<name>.negotiationRestrict' specifies defaults
for the --negotiation-restrict arguments.
Each value is either an exact ref name or a glob pattern whose tips should
always be sent as 'have' lines during negotiation. The config values are
resolved through the same resolve_negotiation_include() codepath as the CLI
options.
This option is additive with the normal negotiation process: the negotiation
algorithm still runs and advertises its own selected commits, but the refs
matching the config are sent unconditionally on top of those heuristically
selected commits.
Similar to the negotiationRestrict config, an empty value resets the value
list to allow ignoring earlier config values, such as those that might be
set in system or global config.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/config/remote.adoc | 27 ++++++++++++++++++
Documentation/fetch-options.adoc | 4 +++
builtin/fetch.c | 11 +++++++
remote.c | 5 ++++
remote.h | 1 +
t/t5510-fetch.sh | 49 ++++++++++++++++++++++++++++++++
6 files changed, 97 insertions(+)
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 4dcf81fbce..9ae20e4379 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -125,6 +125,33 @@ values are not used.
Blank values signal to ignore all previous values, allowing a reset of
the list from broader config scenarios.
+remote.<name>.negotiationInclude::
+ When negotiating with this remote during `git fetch`, the client
+ advertises a list of commits that exist locally. In repos with
+ many references, this list of "haves" can be truncated. Depending
+ on data shape, dropping certain references may be expensive. This
+ multi-valued config option specifies references, commit hashes,
+ or ref pattern globs whose tips should always be sent as "have"
+ commits during fetch negotiation with this remote.
++
+Each value is either an exact ref name (e.g. `refs/heads/release`), a
+commit hash, or a glob pattern (e.g. `refs/heads/release/*`). The
+pattern syntax is the same as for `--negotiation-include`.
++
+These config values are used as defaults for the `--negotiation-include`
+command-line option. If `--negotiation-include` is specified on the
+command line, then the config values are not used.
++
+This option is additive with the normal negotiation process: the
+negotiation algorithm still runs and advertises its own selected commits,
+but the refs matching `remote.<name>.negotiationInclude` are sent
+unconditionally on top of those heuristically selected commits. This
+option is also used during push negotiation when `push.negotiate` is
+enabled.
++
+Blank values signal to ignore all previous values, allowing a reset of
+the list from broader config scenarios.
+
remote.<name>.followRemoteHEAD::
How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
when fetching using the configured refspecs of a remote.
diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index 7b897a7202..8074004377 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -91,6 +91,10 @@ The pattern syntax is the same as for `--negotiation-restrict`.
If `--negotiation-restrict` is used, the have set is first restricted by
that option and then increased to include the tips specified by
`--negotiation-include`.
++
+If this option is not specified on the command line, then any
+`remote.<name>.negotiationInclude` config values for the current remote
+are used instead.
`--negotiate-only`::
Do not fetch anything from the server, and instead print the
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 6b456b3689..2308cab377 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1630,6 +1630,17 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
else
warning(_("ignoring %s because the protocol does not support it"),
"--negotiation-include");
+ } else if (remote->negotiation_include.nr) {
+ if (transport->smart_options) {
+ add_negotiation_tips(&remote->negotiation_include,
+ &transport->smart_options->negotiation_include_tips);
+ } else {
+ struct strbuf config_name = STRBUF_INIT;
+ strbuf_addf(&config_name, "remote.%s.negotiationInclude", remote->name);
+ warning(_("ignoring %s because the protocol does not support it"),
+ config_name.buf);
+ strbuf_release(&config_name);
+ }
}
return transport;
}
diff --git a/remote.c b/remote.c
index 620086e16e..6fb5758820 100644
--- a/remote.c
+++ b/remote.c
@@ -153,6 +153,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
refspec_init_fetch(&ret->fetch);
string_list_init_dup(&ret->server_options);
string_list_init_dup(&ret->negotiation_restrict);
+ string_list_init_dup(&ret->negotiation_include);
ALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,
remote_state->remotes_alloc);
@@ -181,6 +182,7 @@ static void remote_clear(struct remote *remote)
FREE_AND_NULL(remote->http_proxy_authmethod);
string_list_clear(&remote->server_options, 0);
string_list_clear(&remote->negotiation_restrict, 0);
+ string_list_clear(&remote->negotiation_include, 0);
}
static void add_merge(struct branch *branch, const char *name)
@@ -567,6 +569,9 @@ static int handle_config(const char *key, const char *value,
} else if (!strcmp(subkey, "negotiationrestrict")) {
return parse_transport_option(key, value,
&remote->negotiation_restrict);
+ } else if (!strcmp(subkey, "negotiationinclude")) {
+ return parse_transport_option(key, value,
+ &remote->negotiation_include);
} else if (!strcmp(subkey, "followremotehead")) {
const char *no_warn_branch;
if (!strcmp(value, "never"))
diff --git a/remote.h b/remote.h
index e6ec37c393..d8809b6991 100644
--- a/remote.h
+++ b/remote.h
@@ -118,6 +118,7 @@ struct remote {
struct string_list server_options;
struct string_list negotiation_restrict;
+ struct string_list negotiation_include;
enum follow_remote_head_settings follow_remote_head;
const char *no_warn_branch;
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index bc2e2af959..33f61ac12a 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1587,6 +1587,55 @@ test_expect_success '--negotiation-include avoids duplicates with negotiator' '
test_line_count = 1 matches
'
+test_expect_success 'remote.<name>.negotiationInclude used as default for --negotiation-include' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ # test the reset of the list on an empty value
+ git -C client config --add remote.origin.negotiationInclude refs/tags/alpha_1 &&
+ git -C client config --add remote.origin.negotiationInclude "" &&
+ git -C client config --add remote.origin.negotiationInclude refs/tags/beta_1 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=beta_2 \
+ origin alpha_s beta_s &&
+
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ test_grep ! "fetch> have $ALPHA_1" trace &&
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace
+'
+
+test_expect_success 'remote.<name>.negotiationInclude works with glob patterns' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ git -C client config --add remote.origin.negotiationInclude "refs/tags/beta_*" &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ origin alpha_s beta_s &&
+
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace &&
+ BETA_2=$(git -C client rev-parse beta_2) &&
+ test_grep "fetch> have $BETA_2" trace
+'
+
+test_expect_success 'CLI --negotiation-include overrides remote.<name>.negotiationInclude' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ git -C client config --add remote.origin.negotiationInclude refs/tags/beta_2 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/beta_1 \
+ origin alpha_s beta_s &&
+
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace &&
+ BETA_2=$(git -C client rev-parse beta_2) &&
+ test_grep ! "fetch> have $BETA_2" trace
+'
+
test_expect_success '--negotiation-include avoids duplicates with v0' '
test_when_finished rm -f trace &&
setup_negotiation_tip server server 0 &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 6/8] fetch: add --negotiation-include option for negotiation
From: Derrick Stolee via GitGitGadget @ 2026-05-14 12:41 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v4.git.1778762495.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
Add a new --negotiation-include option to 'git fetch', which ensures
that certain ref tips are always sent as 'have' lines during fetch
negotiation, regardless of what the negotiation algorithm selects.
This is useful when the repository has a large number of references, so
the normal negotiation algorithm truncates the list. This is especially
important in repositories with long parallel commit histories. For
example, a repo could have a 'dev' branch for development and a
'release' branch for released versions. If the 'dev' branch isn't
selected for negotiation, then it's not a big deal because there are
many in-progress development branches with a shared history. However, if
'release' is not selected for negotiation, then the server may think
that this is the first time the client has asked for that reference,
causing a full download of its parallel commit history (and any extra
data that may be unique to that branch). This is based on a real example
where certain fetches would grow to 60+ GB when a release branch
updated.
This option is a complement to --negotiation-restrict, which reduces the
negotiation ref set to a specific list. In the earlier example, using
--negotiation-restrict to focus the negotiation to 'dev' and 'release'
would avoid those problematic downloads, but would still not allow
advertising potentially-relevant user branches. In this way, the
'include' version solves the problem I mention while allowing
negotiation to pick other references opportunistically. The two options
can also be combined to allow the best of both worlds.
The argument may be an exact ref name or a glob pattern. Non-existent
refs are silently ignored. This behavior is also updated in the ref matching
logic for the related --negotiation-restrict option to match.
The implementation outputs the requested objects as haves before the
negotiator performs its own algorithm to choose the next haves. Use the new
have_sent() interface to signal these have commits were sent before engaging
with the negotiator's next() iterator.
Also add --negotiation-include to 'git pull' passthrough options.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/fetch-options.adoc | 19 +++++++
builtin/fetch.c | 32 ++++++++---
builtin/pull.c | 3 ++
fetch-pack.c | 81 +++++++++++++++++++++++++---
fetch-pack.h | 6 ++-
t/t5510-fetch.sh | 91 ++++++++++++++++++++++++++++++++
transport.c | 8 ++-
transport.h | 5 +-
8 files changed, 227 insertions(+), 18 deletions(-)
diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index d39cecb446..7b897a7202 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -73,6 +73,25 @@ See also the `fetch.negotiationAlgorithm` and `push.negotiate`
configuration variables documented in linkgit:git-config[1], and the
`--negotiate-only` option below.
+`--negotiation-include=(<commit>|<glob>)`::
+ Ensure that the commits at the given tips are always sent as "have"
+ lines during fetch negotiation, regardless of what the negotiation
+ algorithm selects. This is useful to guarantee that common
+ history reachable from specific refs is always considered, even
+ when `--negotiation-restrict` restricts the set of tips or when
+ the negotiation algorithm would otherwise skip them.
++
+This option may be specified more than once; if so, each commit is sent
+unconditionally.
++
+The argument may be an exact ref name (e.g. `refs/heads/release`), an
+object hash, or a glob pattern (e.g. `refs/heads/release/{asterisk}`).
+The pattern syntax is the same as for `--negotiation-restrict`.
++
+If `--negotiation-restrict` is used, the have set is first restricted by
+that option and then increased to include the tips specified by
+`--negotiation-include`.
+
`--negotiate-only`::
Do not fetch anything from the server, and instead print the
ancestors of the provided `--negotiation-restrict=` arguments,
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a957739f37..6b456b3689 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -99,6 +99,7 @@ static struct transport *gsecondary;
static struct refspec refmap = REFSPEC_INIT_FETCH;
static struct string_list server_options = STRING_LIST_INIT_DUP;
static struct string_list negotiation_restrict = STRING_LIST_INIT_NODUP;
+static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;
struct fetch_config {
enum display_format display_format;
@@ -1534,23 +1535,28 @@ static int add_oid(const struct reference *ref, void *cb_data)
return 0;
}
-static void add_negotiation_restrict_tips(struct git_transport_options *smart_options)
+static void add_negotiation_tips(struct string_list *input_list,
+ struct oid_array **output_list)
{
struct oid_array *oids = xcalloc(1, sizeof(*oids));
int i;
- for (i = 0; i < negotiation_restrict.nr; i++) {
- const char *s = negotiation_restrict.items[i].string;
+ for (i = 0; i < input_list->nr; i++) {
+ const char *s = input_list->items[i].string;
struct refs_for_each_ref_options opts = {
.pattern = s,
};
int old_nr;
if (!has_glob_specials(s)) {
struct object_id oid;
+
+ /* Ignore missing reference. */
if (repo_get_oid(the_repository, s, &oid))
- die(_("%s is not a valid object"), s);
+ continue;
+ /* Fail on missing object pointed by ref. */
if (!odb_has_object(the_repository->objects, &oid, 0))
die(_("the object %s does not exist"), s);
+
oid_array_append(oids, &oid);
continue;
}
@@ -1561,7 +1567,7 @@ static void add_negotiation_restrict_tips(struct git_transport_options *smart_op
warning(_("ignoring %s=%s because it does not match any refs"),
"--negotiation-restrict", s);
}
- smart_options->negotiation_restrict_tips = oids;
+ *output_list = oids;
}
static struct transport *prepare_transport(struct remote *remote, int deepen,
@@ -1597,7 +1603,8 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
}
if (negotiation_restrict.nr) {
if (transport->smart_options)
- add_negotiation_restrict_tips(transport->smart_options);
+ add_negotiation_tips(&negotiation_restrict,
+ &transport->smart_options->negotiation_restrict_tips);
else
warning(_("ignoring %s because the protocol does not support it"),
"--negotiation-restrict");
@@ -1606,7 +1613,8 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
for_each_string_list_item(item, &remote->negotiation_restrict)
string_list_append(&negotiation_restrict, item->string);
if (transport->smart_options)
- add_negotiation_restrict_tips(transport->smart_options);
+ add_negotiation_tips(&negotiation_restrict,
+ &transport->smart_options->negotiation_restrict_tips);
else {
struct strbuf config_name = STRBUF_INIT;
strbuf_addf(&config_name, "remote.%s.negotiationRestrict", remote->name);
@@ -1615,6 +1623,14 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
strbuf_release(&config_name);
}
}
+ if (negotiation_include.nr) {
+ if (transport->smart_options)
+ add_negotiation_tips(&negotiation_include,
+ &transport->smart_options->negotiation_include_tips);
+ else
+ warning(_("ignoring %s because the protocol does not support it"),
+ "--negotiation-include");
+ }
return transport;
}
@@ -2582,6 +2598,8 @@ int cmd_fetch(int argc,
OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_restrict, N_("revision"),
N_("report that we have only objects reachable from this object")),
OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
+ OPT_STRING_LIST(0, "negotiation-include", &negotiation_include, N_("revision"),
+ N_("ensure this ref is always sent as a negotiation have")),
OPT_BOOL(0, "negotiate-only", &negotiate_only,
N_("do not fetch a packfile; instead, print ancestors of negotiation tips")),
OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
diff --git a/builtin/pull.c b/builtin/pull.c
index cc6ce485fc..d49b09114a 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1000,6 +1000,9 @@ int cmd_pull(int argc,
N_("report that we have only objects reachable from this object"),
0),
OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
+ OPT_PASSTHRU_ARGV(0, "negotiation-include", &opt_fetch, N_("revision"),
+ N_("ensure this ref is always sent as a negotiation have"),
+ 0),
OPT_BOOL(0, "show-forced-updates", &opt_show_forced_updates,
N_("check for forced-updates on all updated branches")),
OPT_PASSTHRU(0, "set-upstream", &set_upstream, NULL,
diff --git a/fetch-pack.c b/fetch-pack.c
index baf239adf9..96071434b8 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -25,6 +25,7 @@
#include "oidset.h"
#include "packfile.h"
#include "odb.h"
+#include "object-name.h"
#include "path.h"
#include "connected.h"
#include "fetch-negotiator.h"
@@ -332,6 +333,21 @@ static void send_filter(struct fetch_pack_args *args,
}
}
+static void add_oids_to_set(const struct oid_array *array,
+ struct oidset *set)
+{
+ if (!array)
+ return;
+
+ for (size_t i = 0; i < array->nr; i++) {
+ struct object_id *oid = &array->oid[i];
+ if (!odb_has_object(the_repository->objects, oid, 0))
+ die(_("the object %s does not exist"), oid_to_hex(oid));
+
+ oidset_insert(set, oid);
+ }
+}
+
static int find_common(struct fetch_negotiator *negotiator,
struct fetch_pack_args *args,
int fd[2], struct object_id *result_oid,
@@ -347,6 +363,7 @@ static int find_common(struct fetch_negotiator *negotiator,
struct strbuf req_buf = STRBUF_INIT;
size_t state_len = 0;
struct packet_reader reader;
+ struct oidset negotiation_include_oids = OIDSET_INIT;
if (args->stateless_rpc && multi_ack == 1)
die(_("the option '%s' requires '%s'"), "--stateless-rpc", "multi_ack_detailed");
@@ -474,6 +491,27 @@ static int find_common(struct fetch_negotiator *negotiator,
trace2_region_enter("fetch-pack", "negotiation_v0_v1", the_repository);
flushes = 0;
retval = -1;
+
+ /* Send unconditional haves from --negotiation-include */
+ add_oids_to_set(args->negotiation_include_tips,
+ &negotiation_include_oids);
+ if (oidset_size(&negotiation_include_oids)) {
+ struct oidset_iter iter;
+ oidset_iter_init(&negotiation_include_oids, &iter);
+
+ while ((oid = oidset_iter_next(&iter))) {
+ struct commit *commit;
+ packet_buf_write(&req_buf, "have %s\n",
+ oid_to_hex(oid));
+ print_verbose(args, "have %s", oid_to_hex(oid));
+ count++;
+
+ commit = lookup_commit(the_repository, oid);
+ if (commit)
+ negotiator->have_sent(negotiator, commit);
+ }
+ }
+
while ((oid = negotiator->next(negotiator))) {
packet_buf_write(&req_buf, "have %s\n", oid_to_hex(oid));
print_verbose(args, "have %s", oid_to_hex(oid));
@@ -584,6 +622,7 @@ done:
flushes++;
}
strbuf_release(&req_buf);
+ oidset_clear(&negotiation_include_oids);
if (!got_ready || !no_done)
consume_shallow_list(args, &reader);
@@ -1305,11 +1344,27 @@ static void add_common(struct strbuf *req_buf, struct oidset *common)
static int add_haves(struct fetch_negotiator *negotiator,
struct strbuf *req_buf,
- int *haves_to_send)
+ int *haves_to_send,
+ struct oidset *negotiation_include_oids)
{
int haves_added = 0;
const struct object_id *oid;
+ /* Send unconditional haves from --negotiation-include */
+ if (negotiation_include_oids) {
+ struct oidset_iter iter;
+ oidset_iter_init(negotiation_include_oids, &iter);
+
+ while ((oid = oidset_iter_next(&iter))) {
+ struct commit *commit = lookup_commit(the_repository, oid);
+ if (commit) {
+ packet_buf_write(req_buf, "have %s\n",
+ oid_to_hex(oid));
+ negotiator->have_sent(negotiator, commit);
+ }
+ }
+ }
+
while ((oid = negotiator->next(negotiator))) {
packet_buf_write(req_buf, "have %s\n", oid_to_hex(oid));
if (++haves_added >= *haves_to_send)
@@ -1358,7 +1413,8 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
struct fetch_pack_args *args,
const struct ref *wants, struct oidset *common,
int *haves_to_send, int *in_vain,
- int sideband_all, int seen_ack)
+ int sideband_all, int seen_ack,
+ struct oidset *negotiation_include_oids)
{
int haves_added;
int done_sent = 0;
@@ -1413,7 +1469,8 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
/* Add all of the common commits we've found in previous rounds */
add_common(&req_buf, common);
- haves_added = add_haves(negotiator, &req_buf, haves_to_send);
+ haves_added = add_haves(negotiator, &req_buf, haves_to_send,
+ negotiation_include_oids);
*in_vain += haves_added;
trace2_data_intmax("negotiation_v2", the_repository, "haves_added", haves_added);
trace2_data_intmax("negotiation_v2", the_repository, "in_vain", *in_vain);
@@ -1657,6 +1714,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
struct ref *ref = copy_ref_list(orig_ref);
enum fetch_state state = FETCH_CHECK_LOCAL;
struct oidset common = OIDSET_INIT;
+ struct oidset negotiation_include_oids = OIDSET_INIT;
struct packet_reader reader;
int in_vain = 0, negotiation_started = 0;
int negotiation_round = 0;
@@ -1729,6 +1787,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
state = FETCH_SEND_REQUEST;
mark_tips(negotiator, args->negotiation_restrict_tips);
+ add_oids_to_set(args->negotiation_include_tips,
+ &negotiation_include_oids);
for_each_cached_alternate(negotiator,
insert_one_alternate_object);
break;
@@ -1747,7 +1807,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
&common,
&haves_to_send, &in_vain,
reader.use_sideband,
- seen_ack)) {
+ seen_ack,
+ &negotiation_include_oids)) {
trace2_region_leave_printf("negotiation_v2", "round",
the_repository, "%d",
negotiation_round);
@@ -1883,6 +1944,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
negotiator->release(negotiator);
oidset_clear(&common);
+ oidset_clear(&negotiation_include_oids);
return ref;
}
@@ -2181,12 +2243,14 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
const struct string_list *server_options,
int stateless_rpc,
int fd[],
- struct oidset *acked_commits)
+ struct oidset *acked_commits,
+ const struct oid_array *negotiation_include_tips)
{
struct fetch_negotiator negotiator;
struct packet_reader reader;
struct object_array nt_object_array = OBJECT_ARRAY_INIT;
struct strbuf req_buf = STRBUF_INIT;
+ struct oidset negotiation_include_oids = OIDSET_INIT;
int haves_to_send = INITIAL_FLUSH;
int in_vain = 0;
int seen_ack = 0;
@@ -2197,6 +2261,9 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
fetch_negotiator_init(the_repository, &negotiator);
mark_tips(&negotiator, negotiation_restrict_tips);
+ add_oids_to_set(negotiation_include_tips,
+ &negotiation_include_oids);
+
packet_reader_init(&reader, fd[0], NULL, 0,
PACKET_READ_CHOMP_NEWLINE |
PACKET_READ_DIE_ON_ERR_PACKET);
@@ -2221,7 +2288,8 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
packet_buf_write(&req_buf, "wait-for-done");
- haves_added = add_haves(&negotiator, &req_buf, &haves_to_send);
+ haves_added = add_haves(&negotiator, &req_buf, &haves_to_send,
+ &negotiation_include_oids);
in_vain += haves_added;
if (!haves_added || (seen_ack && in_vain >= MAX_IN_VAIN))
last_iteration = 1;
@@ -2273,6 +2341,7 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
clear_common_flag(acked_commits);
object_array_clear(&nt_object_array);
+ oidset_clear(&negotiation_include_oids);
negotiator.release(&negotiator);
strbuf_release(&req_buf);
}
diff --git a/fetch-pack.h b/fetch-pack.h
index 6c70c942c2..6d0dec7f41 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -19,9 +19,10 @@ struct fetch_pack_args {
/*
* If not NULL, during packfile negotiation, fetch-pack will send "have"
- * lines only with these tips and their ancestors.
+ * lines for all _include_ tips and then a subset of the _restrict_ tips.
*/
const struct oid_array *negotiation_restrict_tips;
+ const struct oid_array *negotiation_include_tips;
unsigned deepen_relative:1;
unsigned quiet:1;
@@ -93,7 +94,8 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
const struct string_list *server_options,
int stateless_rpc,
int fd[],
- struct oidset *acked_commits);
+ struct oidset *acked_commits,
+ const struct oid_array *negotiation_include_tips);
/*
* Print an appropriate error message for each sought ref that wasn't
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index eff3ce8e2d..bc2e2af959 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1460,6 +1460,16 @@ EOF
test_cmp fatal-expect fatal-actual
'
+test_expect_success '--negotiation-tip ignores missing refs and invalid hashes' '
+ setup_negotiation_tip server server 0 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-tip=alpha_1 --negotiation-tip=beta_1 \
+ --negotiation-tip=no-such-ref \
+ --negotiation-tip=invalid-hash \
+ origin alpha_s beta_s &&
+ check_negotiation_tip
+'
+
test_expect_success '--negotiation-restrict limits "have" lines sent' '
setup_negotiation_tip server server 0 &&
GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
@@ -1511,6 +1521,87 @@ test_expect_success 'CLI --negotiation-restrict overrides remote config' '
test_grep ! "fetch> have $BETA_1" trace
'
+test_expect_success '--negotiation-include includes configured refs as haves' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/beta_1 \
+ origin alpha_s beta_s &&
+
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ test_grep "fetch> have $ALPHA_1" trace &&
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace
+'
+
+test_expect_success '--negotiation-include works with glob patterns' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include="refs/tags/beta_*" \
+ origin alpha_s beta_s &&
+
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace &&
+ BETA_2=$(git -C client rev-parse beta_2) &&
+ test_grep "fetch> have $BETA_2" trace
+'
+
+test_expect_success '--negotiation-include is additive with negotiation' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-include=refs/tags/beta_1 \
+ origin alpha_s beta_s &&
+
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace
+'
+
+test_expect_success '--negotiation-include ignores non-existent refs silently' '
+ setup_negotiation_tip server server 0 &&
+
+ git -C client fetch --quiet \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/nonexistent \
+ origin alpha_s beta_s 2>err &&
+ test_must_be_empty err
+'
+
+test_expect_success '--negotiation-include avoids duplicates with negotiator' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/alpha_1 \
+ origin alpha_s beta_s &&
+
+ test_grep "fetch> have $ALPHA_1" trace >matches &&
+ test_line_count = 1 matches
+'
+
+test_expect_success '--negotiation-include avoids duplicates with v0' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client \
+ -c protocol.version=0 fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/alpha_1 \
+ origin alpha_s beta_s &&
+
+ test_grep "fetch> have $ALPHA_1" trace >matches &&
+ test_line_count = 1 matches
+'
+
test_expect_success SYMLINKS 'clone does not get confused by a D/F conflict' '
git init df-conflict &&
(
diff --git a/transport.c b/transport.c
index a3051f6733..fa54928966 100644
--- a/transport.c
+++ b/transport.c
@@ -464,6 +464,7 @@ static int fetch_refs_via_pack(struct transport *transport,
args.stateless_rpc = transport->stateless_rpc;
args.server_options = transport->server_options;
args.negotiation_restrict_tips = data->options.negotiation_restrict_tips;
+ args.negotiation_include_tips = data->options.negotiation_include_tips;
args.reject_shallow_remote = transport->smart_options->reject_shallow;
if (!data->finished_handshake) {
@@ -495,7 +496,8 @@ static int fetch_refs_via_pack(struct transport *transport,
transport->server_options,
transport->stateless_rpc,
data->fd,
- data->options.acked_commits);
+ data->options.acked_commits,
+ data->options.negotiation_include_tips);
ret = 0;
}
goto cleanup;
@@ -983,6 +985,10 @@ static int disconnect_git(struct transport *transport)
oid_array_clear(data->options.negotiation_restrict_tips);
free(data->options.negotiation_restrict_tips);
}
+ if (data->options.negotiation_include_tips) {
+ oid_array_clear(data->options.negotiation_include_tips);
+ free(data->options.negotiation_include_tips);
+ }
list_objects_filter_release(&data->options.filter_options);
oid_array_clear(&data->extra_have);
oid_array_clear(&data->shallow);
diff --git a/transport.h b/transport.h
index cdeb33c16f..97d905ecc0 100644
--- a/transport.h
+++ b/transport.h
@@ -40,13 +40,14 @@ struct git_transport_options {
/*
* This is only used during fetch. See the documentation of
- * negotiation_restrict_tips in struct fetch_pack_args.
+ * these member names in struct fetch_pack_args.
*
- * This field is only supported by transports that support connect or
+ * These fields are only supported by transports that support connect or
* stateless_connect. Set this field directly instead of using
* transport_set_option().
*/
struct oid_array *negotiation_restrict_tips;
+ struct oid_array *negotiation_include_tips;
/*
* If allocated, whenever transport_fetch_refs() is called, add known
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 5/8] negotiator: add have_sent() interface
From: Derrick Stolee via GitGitGadget @ 2026-05-14 12:41 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v4.git.1778762495.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
In a future change, we will introduce a capability to choose specific commit
OIDs as 'have's in fetch negotiation, with the ability to have the
negotiator choose more 'have's to increase coverage beyond that required
core set. The negotiator works to avoid emitting 'have's that can reach each
other, but that logic is hidden beneath the negotiator's iterator function
pointer ('next'). We need a way to communicate to the negotiator that we
have picked a 'have' so it could incorporate that into its logic.
Add a have_sent() method to the fetch_negotiator interface. This is the
signal that allows the negotiator to track the commit as already shown and
can perform the proper bookkeeping to avoid emitting those objects or
anything they can reach.
For our non-trivial negotiators, it is sufficient to mark these commits as
common, so the implementation is quite simple. This logic will be exercised
in the next change.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
fetch-negotiator.h | 9 +++++++++
negotiator/default.c | 8 ++++++++
negotiator/noop.c | 7 +++++++
negotiator/skipping.c | 8 ++++++++
4 files changed, 32 insertions(+)
diff --git a/fetch-negotiator.h b/fetch-negotiator.h
index e348905a1f..6ca422a064 100644
--- a/fetch-negotiator.h
+++ b/fetch-negotiator.h
@@ -47,6 +47,15 @@ struct fetch_negotiator {
*/
int (*ack)(struct fetch_negotiator *, struct commit *);
+ /*
+ * Inform the negotiator that this commit has already been sent as
+ * a "have" line outside of the negotiator's control. The negotiator
+ * should avoid outputting it from next() and may use it to optimize
+ * further negotiation (e.g., by treating it and its ancestors as
+ * common).
+ */
+ void (*have_sent)(struct fetch_negotiator *, struct commit *);
+
void (*release)(struct fetch_negotiator *);
/* internal use */
diff --git a/negotiator/default.c b/negotiator/default.c
index 116dedcf83..05ab616f39 100644
--- a/negotiator/default.c
+++ b/negotiator/default.c
@@ -175,6 +175,13 @@ static int ack(struct fetch_negotiator *n, struct commit *c)
return known_to_be_common;
}
+static void have_sent(struct fetch_negotiator *n, struct commit *c)
+{
+ if (repo_parse_commit(the_repository, c))
+ return;
+ mark_common(n->data, c, 0, 0);
+}
+
static void release(struct fetch_negotiator *n)
{
clear_prio_queue(&((struct negotiation_state *)n->data)->rev_list);
@@ -188,6 +195,7 @@ void default_negotiator_init(struct fetch_negotiator *negotiator)
negotiator->add_tip = add_tip;
negotiator->next = next;
negotiator->ack = ack;
+ negotiator->have_sent = have_sent;
negotiator->release = release;
negotiator->data = CALLOC_ARRAY(ns, 1);
ns->rev_list.compare = compare_commits_by_commit_date;
diff --git a/negotiator/noop.c b/negotiator/noop.c
index 65e3c20008..edf1b456f3 100644
--- a/negotiator/noop.c
+++ b/negotiator/noop.c
@@ -29,6 +29,12 @@ static int ack(struct fetch_negotiator *n UNUSED, struct commit *c UNUSED)
return 0;
}
+static void have_sent(struct fetch_negotiator *n UNUSED,
+ struct commit *c UNUSED)
+{
+ /* nothing to do */
+}
+
static void release(struct fetch_negotiator *n UNUSED)
{
/* nothing to release */
@@ -40,6 +46,7 @@ void noop_negotiator_init(struct fetch_negotiator *negotiator)
negotiator->add_tip = add_tip;
negotiator->next = next;
negotiator->ack = ack;
+ negotiator->have_sent = have_sent;
negotiator->release = release;
negotiator->data = NULL;
}
diff --git a/negotiator/skipping.c b/negotiator/skipping.c
index 0a272130fb..69472c58e1 100644
--- a/negotiator/skipping.c
+++ b/negotiator/skipping.c
@@ -243,6 +243,13 @@ static int ack(struct fetch_negotiator *n, struct commit *c)
return known_to_be_common;
}
+static void have_sent(struct fetch_negotiator *n, struct commit *c)
+{
+ if (repo_parse_commit(the_repository, c))
+ return;
+ mark_common(n->data, c);
+}
+
static void release(struct fetch_negotiator *n)
{
struct data *data = n->data;
@@ -259,6 +266,7 @@ void skipping_negotiator_init(struct fetch_negotiator *negotiator)
negotiator->add_tip = add_tip;
negotiator->next = next;
negotiator->ack = ack;
+ negotiator->have_sent = have_sent;
negotiator->release = release;
negotiator->data = CALLOC_ARRAY(data, 1);
data->rev_list.compare = compare;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 4/8] remote: add remote.*.negotiationRestrict config
From: Derrick Stolee via GitGitGadget @ 2026-05-14 12:41 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v4.git.1778762495.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
In a previous change, the --negotiation-restrict command-line option of 'git
fetch' was added as a synonym of --negotiation-tip. Both of these options
restrict the set of 'haves' the client can send as part of negotiation.
This was previously not available via a configuration option. Add a new
'remote.<name>.negotiationRestrict' multi-valued config option that updates
'git fetch <name>' to use these restrictions by default.
If the user provides even one --negotiation-restrict argument, then the
config is ignored.
An empty value resets the value list to allow ignoring earlier config
values, such as those that might be set in system or global config.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/config/remote.adoc | 18 ++++++++++++++++++
builtin/fetch.c | 28 +++++++++++++++++++++-------
remote.c | 5 +++++
remote.h | 1 +
t/t5510-fetch.sh | 26 ++++++++++++++++++++++++++
5 files changed, 71 insertions(+), 7 deletions(-)
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 91e46f66f5..4dcf81fbce 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -107,6 +107,24 @@ priority configuration file (e.g. `.git/config` in a repository) to clear
the values inherited from a lower priority configuration files (e.g.
`$HOME/.gitconfig`).
+remote.<name>.negotiationRestrict::
+ When negotiating with this remote during `git fetch`, restrict the
+ commits advertised as "have" lines to only those reachable from refs
+ matching the given patterns. This multi-valued config option behaves
+ like `--negotiation-restrict` on the command line.
++
+Each value is either an exact ref name (e.g. `refs/heads/release`) or a
+glob pattern (e.g. `refs/heads/release/*`). The pattern syntax is the
+same as for `--negotiation-restrict`.
++
+These config values are used as defaults for the `--negotiation-restrict`
+command-line option. If `--negotiation-restrict` (or its synonym
+`--negotiation-tip`) is specified on the command line, then the config
+values are not used.
++
+Blank values signal to ignore all previous values, allowing a reset of
+the list from broader config scenarios.
+
remote.<name>.followRemoteHEAD::
How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
when fetching using the configured refspecs of a remote.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 2ba0051d52..a957739f37 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1601,6 +1601,19 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
else
warning(_("ignoring %s because the protocol does not support it"),
"--negotiation-restrict");
+ } else if (remote->negotiation_restrict.nr) {
+ struct string_list_item *item;
+ for_each_string_list_item(item, &remote->negotiation_restrict)
+ string_list_append(&negotiation_restrict, item->string);
+ if (transport->smart_options)
+ add_negotiation_restrict_tips(transport->smart_options);
+ else {
+ struct strbuf config_name = STRBUF_INIT;
+ strbuf_addf(&config_name, "remote.%s.negotiationRestrict", remote->name);
+ warning(_("ignoring %s because the protocol does not support it"),
+ config_name.buf);
+ strbuf_release(&config_name);
+ }
}
return transport;
}
@@ -2658,10 +2671,6 @@ int cmd_fetch(int argc,
config.display_format = DISPLAY_FORMAT_PORCELAIN;
}
- if (negotiate_only && !negotiation_restrict.nr)
- die(_("%s needs one or more %s"), "--negotiate-only",
- "--negotiation-restrict=*");
-
if (deepen_relative) {
if (deepen_relative < 0)
die(_("negative depth in --deepen is not supported"));
@@ -2749,14 +2758,19 @@ int cmd_fetch(int argc,
if (!remote)
die(_("must supply remote when using --negotiate-only"));
gtransport = prepare_transport(remote, 1, &filter_options);
- if (gtransport->smart_options) {
- gtransport->smart_options->acked_commits = &acked_commits;
- } else {
+
+ if (!gtransport->smart_options) {
warning(_("protocol does not support --negotiate-only, exiting"));
result = 1;
trace2_region_leave("fetch", "negotiate-only", the_repository);
goto cleanup;
}
+ if (!gtransport->smart_options->negotiation_restrict_tips)
+ die(_("%s needs one or more %s"), "--negotiate-only",
+ "--negotiation-restrict=*");
+
+ gtransport->smart_options->acked_commits = &acked_commits;
+
if (server_options.nr)
gtransport->server_options = &server_options;
result = transport_fetch_refs(gtransport, NULL);
diff --git a/remote.c b/remote.c
index 7ca2a6501b..620086e16e 100644
--- a/remote.c
+++ b/remote.c
@@ -152,6 +152,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
refspec_init_push(&ret->push);
refspec_init_fetch(&ret->fetch);
string_list_init_dup(&ret->server_options);
+ string_list_init_dup(&ret->negotiation_restrict);
ALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,
remote_state->remotes_alloc);
@@ -179,6 +180,7 @@ static void remote_clear(struct remote *remote)
FREE_AND_NULL(remote->http_proxy);
FREE_AND_NULL(remote->http_proxy_authmethod);
string_list_clear(&remote->server_options, 0);
+ string_list_clear(&remote->negotiation_restrict, 0);
}
static void add_merge(struct branch *branch, const char *name)
@@ -562,6 +564,9 @@ static int handle_config(const char *key, const char *value,
} else if (!strcmp(subkey, "serveroption")) {
return parse_transport_option(key, value,
&remote->server_options);
+ } else if (!strcmp(subkey, "negotiationrestrict")) {
+ return parse_transport_option(key, value,
+ &remote->negotiation_restrict);
} else if (!strcmp(subkey, "followremotehead")) {
const char *no_warn_branch;
if (!strcmp(value, "never"))
diff --git a/remote.h b/remote.h
index fc052945ee..e6ec37c393 100644
--- a/remote.h
+++ b/remote.h
@@ -117,6 +117,7 @@ struct remote {
char *http_proxy_authmethod;
struct string_list server_options;
+ struct string_list negotiation_restrict;
enum follow_remote_head_settings follow_remote_head;
const char *no_warn_branch;
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index dc3ce56d84..eff3ce8e2d 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1485,6 +1485,32 @@ test_expect_success '--negotiation-restrict and --negotiation-tip can be mixed'
check_negotiation_tip
'
+test_expect_success 'remote.<name>.negotiationRestrict used as default' '
+ setup_negotiation_tip server server 0 &&
+
+ # test the reset of the list on an empty value
+ git -C client config --add remote.origin.negotiationRestrict alpha_2 &&
+ git -C client config --add remote.origin.negotiationRestrict "" &&
+ git -C client config --add remote.origin.negotiationRestrict alpha_1 &&
+ git -C client config --add remote.origin.negotiationRestrict beta_1 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ origin alpha_s beta_s &&
+ check_negotiation_tip
+'
+
+test_expect_success 'CLI --negotiation-restrict overrides remote config' '
+ setup_negotiation_tip server server 0 &&
+ git -C client config --add remote.origin.negotiationRestrict alpha_1 &&
+ git -C client config --add remote.origin.negotiationRestrict beta_1 &&
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ origin alpha_s beta_s &&
+ test_grep "fetch> have $ALPHA_1" trace &&
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep ! "fetch> have $BETA_1" trace
+'
+
test_expect_success SYMLINKS 'clone does not get confused by a D/F conflict' '
git init df-conflict &&
(
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 3/8] transport: rename negotiation_tips
From: Derrick Stolee via GitGitGadget @ 2026-05-14 12:41 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v4.git.1778762495.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
The previous change added the --negotiation-restrict synonym for the
--negotiation-tip option for 'git fetch'. In anticipation of adding a new
option that behaves similarly but with distinct changes to its behavior,
rename the internal representation of this data from 'negotiation_tips' to
'negotiation_restrict_tips'.
The 'tips' part is kept because this is an oid_array in the transport layer.
This requires the builtin to handle parsing refs into collections of oids so
the transport layer can handle this cleaner form of the data.
Also update the string_list used to store the inputs from command-line
options.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
builtin/fetch.c | 18 +++++++++---------
fetch-pack.c | 18 +++++++++---------
fetch-pack.h | 4 ++--
transport-helper.c | 2 +-
transport.c | 10 +++++-----
transport.h | 4 ++--
6 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index fc950fe35b..2ba0051d52 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -98,7 +98,7 @@ static struct transport *gtransport;
static struct transport *gsecondary;
static struct refspec refmap = REFSPEC_INIT_FETCH;
static struct string_list server_options = STRING_LIST_INIT_DUP;
-static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;
+static struct string_list negotiation_restrict = STRING_LIST_INIT_NODUP;
struct fetch_config {
enum display_format display_format;
@@ -1534,13 +1534,13 @@ static int add_oid(const struct reference *ref, void *cb_data)
return 0;
}
-static void add_negotiation_tips(struct git_transport_options *smart_options)
+static void add_negotiation_restrict_tips(struct git_transport_options *smart_options)
{
struct oid_array *oids = xcalloc(1, sizeof(*oids));
int i;
- for (i = 0; i < negotiation_tip.nr; i++) {
- const char *s = negotiation_tip.items[i].string;
+ for (i = 0; i < negotiation_restrict.nr; i++) {
+ const char *s = negotiation_restrict.items[i].string;
struct refs_for_each_ref_options opts = {
.pattern = s,
};
@@ -1561,7 +1561,7 @@ static void add_negotiation_tips(struct git_transport_options *smart_options)
warning(_("ignoring %s=%s because it does not match any refs"),
"--negotiation-restrict", s);
}
- smart_options->negotiation_tips = oids;
+ smart_options->negotiation_restrict_tips = oids;
}
static struct transport *prepare_transport(struct remote *remote, int deepen,
@@ -1595,9 +1595,9 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER, spec);
set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
}
- if (negotiation_tip.nr) {
+ if (negotiation_restrict.nr) {
if (transport->smart_options)
- add_negotiation_tips(transport->smart_options);
+ add_negotiation_restrict_tips(transport->smart_options);
else
warning(_("ignoring %s because the protocol does not support it"),
"--negotiation-restrict");
@@ -2566,7 +2566,7 @@ int cmd_fetch(int argc,
N_("specify fetch refmap"), PARSE_OPT_NONEG, parse_refmap_arg),
OPT_STRING_LIST('o', "server-option", &server_options, N_("server-specific"), N_("option to transmit")),
OPT_IPVERSION(&family),
- OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, N_("revision"),
+ OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_restrict, N_("revision"),
N_("report that we have only objects reachable from this object")),
OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
OPT_BOOL(0, "negotiate-only", &negotiate_only,
@@ -2658,7 +2658,7 @@ int cmd_fetch(int argc,
config.display_format = DISPLAY_FORMAT_PORCELAIN;
}
- if (negotiate_only && !negotiation_tip.nr)
+ if (negotiate_only && !negotiation_restrict.nr)
die(_("%s needs one or more %s"), "--negotiate-only",
"--negotiation-restrict=*");
diff --git a/fetch-pack.c b/fetch-pack.c
index 6ecd468ef7..baf239adf9 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -291,21 +291,21 @@ static int next_flush(int stateless_rpc, int count)
}
static void mark_tips(struct fetch_negotiator *negotiator,
- const struct oid_array *negotiation_tips)
+ const struct oid_array *negotiation_restrict_tips)
{
struct refs_for_each_ref_options opts = {
.flags = REFS_FOR_EACH_INCLUDE_BROKEN,
};
int i;
- if (!negotiation_tips) {
+ if (!negotiation_restrict_tips) {
refs_for_each_ref_ext(get_main_ref_store(the_repository),
rev_list_insert_ref_oid, negotiator, &opts);
return;
}
- for (i = 0; i < negotiation_tips->nr; i++)
- rev_list_insert_ref(negotiator, &negotiation_tips->oid[i]);
+ for (i = 0; i < negotiation_restrict_tips->nr; i++)
+ rev_list_insert_ref(negotiator, &negotiation_restrict_tips->oid[i]);
return;
}
@@ -355,7 +355,7 @@ static int find_common(struct fetch_negotiator *negotiator,
PACKET_READ_CHOMP_NEWLINE |
PACKET_READ_DIE_ON_ERR_PACKET);
- mark_tips(negotiator, args->negotiation_tips);
+ mark_tips(negotiator, args->negotiation_restrict_tips);
for_each_cached_alternate(negotiator, insert_one_alternate_object);
fetching = 0;
@@ -1728,7 +1728,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
else
state = FETCH_SEND_REQUEST;
- mark_tips(negotiator, args->negotiation_tips);
+ mark_tips(negotiator, args->negotiation_restrict_tips);
for_each_cached_alternate(negotiator,
insert_one_alternate_object);
break;
@@ -2177,7 +2177,7 @@ static void clear_common_flag(struct oidset *s)
}
}
-void negotiate_using_fetch(const struct oid_array *negotiation_tips,
+void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
const struct string_list *server_options,
int stateless_rpc,
int fd[],
@@ -2195,13 +2195,13 @@ void negotiate_using_fetch(const struct oid_array *negotiation_tips,
timestamp_t min_generation = GENERATION_NUMBER_INFINITY;
fetch_negotiator_init(the_repository, &negotiator);
- mark_tips(&negotiator, negotiation_tips);
+ mark_tips(&negotiator, negotiation_restrict_tips);
packet_reader_init(&reader, fd[0], NULL, 0,
PACKET_READ_CHOMP_NEWLINE |
PACKET_READ_DIE_ON_ERR_PACKET);
- oid_array_for_each((struct oid_array *) negotiation_tips,
+ oid_array_for_each((struct oid_array *) negotiation_restrict_tips,
add_to_object_array,
&nt_object_array);
diff --git a/fetch-pack.h b/fetch-pack.h
index 9d3470366f..6c70c942c2 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -21,7 +21,7 @@ struct fetch_pack_args {
* If not NULL, during packfile negotiation, fetch-pack will send "have"
* lines only with these tips and their ancestors.
*/
- const struct oid_array *negotiation_tips;
+ const struct oid_array *negotiation_restrict_tips;
unsigned deepen_relative:1;
unsigned quiet:1;
@@ -89,7 +89,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
* In the capability advertisement that has happened prior to invoking this
* function, the "wait-for-done" capability must be present.
*/
-void negotiate_using_fetch(const struct oid_array *negotiation_tips,
+void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
const struct string_list *server_options,
int stateless_rpc,
int fd[],
diff --git a/transport-helper.c b/transport-helper.c
index dd78d40668..f4388da766 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -754,7 +754,7 @@ static int fetch_refs(struct transport *transport,
set_helper_option(transport, "filter", spec);
}
- if (data->transport_options.negotiation_tips)
+ if (data->transport_options.negotiation_restrict_tips)
warning(_("ignoring %s because the protocol does not support it."),
"--negotiation-restrict");
diff --git a/transport.c b/transport.c
index 107f4fa5dc..a3051f6733 100644
--- a/transport.c
+++ b/transport.c
@@ -463,7 +463,7 @@ static int fetch_refs_via_pack(struct transport *transport,
args.refetch = data->options.refetch;
args.stateless_rpc = transport->stateless_rpc;
args.server_options = transport->server_options;
- args.negotiation_tips = data->options.negotiation_tips;
+ args.negotiation_restrict_tips = data->options.negotiation_restrict_tips;
args.reject_shallow_remote = transport->smart_options->reject_shallow;
if (!data->finished_handshake) {
@@ -491,7 +491,7 @@ static int fetch_refs_via_pack(struct transport *transport,
warning(_("server does not support wait-for-done"));
ret = -1;
} else {
- negotiate_using_fetch(data->options.negotiation_tips,
+ negotiate_using_fetch(data->options.negotiation_restrict_tips,
transport->server_options,
transport->stateless_rpc,
data->fd,
@@ -979,9 +979,9 @@ static int disconnect_git(struct transport *transport)
finish_connect(data->conn);
}
- if (data->options.negotiation_tips) {
- oid_array_clear(data->options.negotiation_tips);
- free(data->options.negotiation_tips);
+ if (data->options.negotiation_restrict_tips) {
+ oid_array_clear(data->options.negotiation_restrict_tips);
+ free(data->options.negotiation_restrict_tips);
}
list_objects_filter_release(&data->options.filter_options);
oid_array_clear(&data->extra_have);
diff --git a/transport.h b/transport.h
index 892f19454a..cdeb33c16f 100644
--- a/transport.h
+++ b/transport.h
@@ -40,13 +40,13 @@ struct git_transport_options {
/*
* This is only used during fetch. See the documentation of
- * negotiation_tips in struct fetch_pack_args.
+ * negotiation_restrict_tips in struct fetch_pack_args.
*
* This field is only supported by transports that support connect or
* stateless_connect. Set this field directly instead of using
* transport_set_option().
*/
- struct oid_array *negotiation_tips;
+ struct oid_array *negotiation_restrict_tips;
/*
* If allocated, whenever transport_fetch_refs() is called, add known
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 2/8] fetch: add --negotiation-restrict option
From: Derrick Stolee via GitGitGadget @ 2026-05-14 12:41 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v4.git.1778762495.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
The --negotiation-tip option to 'git fetch' and 'git pull' allows users
to specify that they want to focus negotiation on a small set of
references. This is a _restriction_ on the negotiation set, helping to
focus the negotiation when the ref count is high. However, it doesn't
allow for the ability to opportunistically select references beyond that
list.
This subtle detail that this is a 'maximum set' and not a 'minimum set'
is not immediately clear from the option name. This makes it more
complicated to add a new option that provides the complementary behavior
of a minimum set.
For now, create a new synonym option, --negotiation-restrict, that
behaves identically to --negotiation-tip. Update the documentation to
make it clear that this new name is the preferred option, but we keep
the old name for compatibility. Mark --negotiation-tip as an alias of the
new, preferred option.
Update a few warning messages with the new option, but also make them
translatable with the option name inserted by formatting. At least one
of these messages will be reused later for a new option.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/config/fetch.adoc | 2 +-
Documentation/fetch-options.adoc | 6 +++++-
builtin/fetch.c | 13 ++++++++-----
builtin/pull.c | 3 ++-
send-pack.c | 2 +-
t/t5510-fetch.sh | 25 +++++++++++++++++++++++++
t/t5702-protocol-v2.sh | 4 ++--
transport-helper.c | 3 ++-
8 files changed, 46 insertions(+), 12 deletions(-)
diff --git a/Documentation/config/fetch.adoc b/Documentation/config/fetch.adoc
index cd40db0cad..04ac90912d 100644
--- a/Documentation/config/fetch.adoc
+++ b/Documentation/config/fetch.adoc
@@ -76,7 +76,7 @@
default is `skipping`. Unknown values will cause `git fetch` to
error out.
+
-See also the `--negotiate-only` and `--negotiation-tip` options to
+See also the `--negotiate-only` and `--negotiation-restrict` options to
linkgit:git-fetch[1].
`fetch.showForcedUpdates`::
diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index 81a9d7f9bb..d39cecb446 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -49,6 +49,7 @@ the current repository has the same history as the source repository.
`.git/shallow`. This option updates `.git/shallow` and accepts such
refs.
+`--negotiation-restrict=(<commit>|<glob>)`::
`--negotiation-tip=(<commit>|<glob>)`::
By default, Git will report, to the server, commits reachable
from all local refs to find common commits in an attempt to
@@ -58,6 +59,9 @@ the current repository has the same history as the source repository.
local ref is likely to have commits in common with the
upstream ref being fetched.
+
+`--negotiation-restrict` is the preferred name for this option;
+`--negotiation-tip` is accepted as a synonym.
++
This option may be specified more than once; if so, Git will report
commits reachable from any of the given commits.
+
@@ -71,7 +75,7 @@ configuration variables documented in linkgit:git-config[1], and the
`--negotiate-only`::
Do not fetch anything from the server, and instead print the
- ancestors of the provided `--negotiation-tip=` arguments,
+ ancestors of the provided `--negotiation-restrict=` arguments,
which we have in common with the server.
+
This is incompatible with `--recurse-submodules=(yes|on-demand)`.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 4795b2a13c..fc950fe35b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1558,8 +1558,8 @@ static void add_negotiation_tips(struct git_transport_options *smart_options)
refs_for_each_ref_ext(get_main_ref_store(the_repository),
add_oid, oids, &opts);
if (old_nr == oids->nr)
- warning("ignoring --negotiation-tip=%s because it does not match any refs",
- s);
+ warning(_("ignoring %s=%s because it does not match any refs"),
+ "--negotiation-restrict", s);
}
smart_options->negotiation_tips = oids;
}
@@ -1599,7 +1599,8 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
if (transport->smart_options)
add_negotiation_tips(transport->smart_options);
else
- warning("ignoring --negotiation-tip because the protocol does not support it");
+ warning(_("ignoring %s because the protocol does not support it"),
+ "--negotiation-restrict");
}
return transport;
}
@@ -2565,8 +2566,9 @@ int cmd_fetch(int argc,
N_("specify fetch refmap"), PARSE_OPT_NONEG, parse_refmap_arg),
OPT_STRING_LIST('o', "server-option", &server_options, N_("server-specific"), N_("option to transmit")),
OPT_IPVERSION(&family),
- OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"),
+ OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, N_("revision"),
N_("report that we have only objects reachable from this object")),
+ OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
OPT_BOOL(0, "negotiate-only", &negotiate_only,
N_("do not fetch a packfile; instead, print ancestors of negotiation tips")),
OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
@@ -2657,7 +2659,8 @@ int cmd_fetch(int argc,
}
if (negotiate_only && !negotiation_tip.nr)
- die(_("--negotiate-only needs one or more --negotiation-tip=*"));
+ die(_("%s needs one or more %s"), "--negotiate-only",
+ "--negotiation-restrict=*");
if (deepen_relative) {
if (deepen_relative < 0)
diff --git a/builtin/pull.c b/builtin/pull.c
index 7e67fdce97..cc6ce485fc 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -996,9 +996,10 @@ int cmd_pull(int argc,
OPT_PASSTHRU('6', "ipv6", &opt_ipv6, NULL,
N_("use IPv6 addresses only"),
PARSE_OPT_NOARG),
- OPT_PASSTHRU_ARGV(0, "negotiation-tip", &opt_fetch, N_("revision"),
+ OPT_PASSTHRU_ARGV(0, "negotiation-restrict", &opt_fetch, N_("revision"),
N_("report that we have only objects reachable from this object"),
0),
+ OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
OPT_BOOL(0, "show-forced-updates", &opt_show_forced_updates,
N_("check for forced-updates on all updated branches")),
OPT_PASSTHRU(0, "set-upstream", &set_upstream, NULL,
diff --git a/send-pack.c b/send-pack.c
index 67d6987b1c..3d5d36ba3b 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -447,7 +447,7 @@ static void get_commons_through_negotiation(struct repository *r,
strvec_pushl(&child.args, "fetch", "--negotiate-only", NULL);
for (ref = remote_refs; ref; ref = ref->next) {
if (!is_null_oid(&ref->new_oid)) {
- strvec_pushf(&child.args, "--negotiation-tip=%s",
+ strvec_pushf(&child.args, "--negotiation-restrict=%s",
oid_to_hex(&ref->new_oid));
nr_negotiation_tip++;
}
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 5dcb4b51a4..dc3ce56d84 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1460,6 +1460,31 @@ EOF
test_cmp fatal-expect fatal-actual
'
+test_expect_success '--negotiation-restrict limits "have" lines sent' '
+ setup_negotiation_tip server server 0 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 --negotiation-restrict=beta_1 \
+ origin alpha_s beta_s &&
+ check_negotiation_tip
+'
+
+test_expect_success '--negotiation-restrict understands globs' '
+ setup_negotiation_tip server server 0 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=*_1 \
+ origin alpha_s beta_s &&
+ check_negotiation_tip
+'
+
+test_expect_success '--negotiation-restrict and --negotiation-tip can be mixed' '
+ setup_negotiation_tip server server 0 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-tip=beta_1 \
+ origin alpha_s beta_s &&
+ check_negotiation_tip
+'
+
test_expect_success SYMLINKS 'clone does not get confused by a D/F conflict' '
git init df-conflict &&
(
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index f826ac46a5..9f6cf4142d 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -869,14 +869,14 @@ setup_negotiate_only () {
test_commit -C client three
}
-test_expect_success 'usage: --negotiate-only without --negotiation-tip' '
+test_expect_success 'usage: --negotiate-only without --negotiation-restrict' '
SERVER="server" &&
URI="file://$(pwd)/server" &&
setup_negotiate_only "$SERVER" "$URI" &&
cat >err.expect <<-\EOF &&
- fatal: --negotiate-only needs one or more --negotiation-tip=*
+ fatal: --negotiate-only needs one or more --negotiation-restrict=*
EOF
test_must_fail git -c protocol.version=2 -C client fetch \
diff --git a/transport-helper.c b/transport-helper.c
index 4d95d84f9e..dd78d40668 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -755,7 +755,8 @@ static int fetch_refs(struct transport *transport,
}
if (data->transport_options.negotiation_tips)
- warning("Ignoring --negotiation-tip because the protocol does not support it.");
+ warning(_("ignoring %s because the protocol does not support it."),
+ "--negotiation-restrict");
if (data->fetch)
return fetch_with_fetch(transport, nr_heads, to_fetch);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 1/8] t5516: fix test order flakiness
From: Derrick Stolee via GitGitGadget @ 2026-05-14 12:41 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v4.git.1778762495.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
The 'fetch follows tags by default' test sorts using 'sort -k 4', but
for-each-ref output only has 3 columns. This relies on sort treating records
with fewer fields as having an empty fourth field, which may produce
unstable results depending on locale. This appears to be an accident added
in 3f763ddf28 (fetch: set remote/HEAD if it does not exist, 2024-11-22).
Use 'sort -k 3' to match the actual number of columns in the output.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
t/t5516-fetch-push.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 29e2f17608..ac8447f21e 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1349,7 +1349,7 @@ test_expect_success 'fetch follows tags by default' '
git for-each-ref >tmp1 &&
sed -n "p; s|refs/heads/main$|refs/remotes/origin/main|p" tmp1 |
sed -n "p; s|refs/heads/main$|refs/remotes/origin/HEAD|p" |
- sort -k 4 >../expect
+ sort -k 3 >../expect
) &&
test_when_finished "rm -rf dst" &&
git init dst &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 0/8] fetch: rework negotiation tip options
From: Derrick Stolee via GitGitGadget @ 2026-05-14 12:41 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee
In-Reply-To: <pull.2085.v3.git.1776871546.gitgitgadget@gmail.com>
Fetch negotiation aims to find enough information from haves and wants such
that the server can be reasonably confident that it will send all necessary
objects and not too many "extra" objects that the client already has.
However, this can break down if there are too many references, since Git
truncates the list of haves based on a few factors (a 256 count limit or the
server sending an ACK at the right time).
We already have the --negotiation-tip feature to focus the set of references
that are used in negotiation, but I feel like this is designed backwards.
I'd rather that we have a way to say "this is an important set of refs, but
feel free to add more refs if needed" than "only use these refs for
negotiation".
Here's an example that demonstrates the problem. In an internal monorepo,
developers work off of the 'main' branch so there are thousands of user
branches that each add a few commits different from the 'main' branch.
However, there is also a long-lived 'release' branch. This branch has a
first-parent history that is parallel to 'main' and each of those commits is
a merge whose second parent is a commit from 'main' that had a successful CI
run. There are additional changes in the 'release' branch merge commits that
add some changelog data, so there is a nontrivial set of novel blob content
in that branch and not just a different set of commits.
The problem we had was that our georeplication system was regularly fetching
from the origin and trying to get all data from all reachable branches. When
the 'release' branch updated, the client would run out of haves before
advertising its copy of the 'release' branch, but it would still list the
new 'release' tip as a want. The server would then think that the client had
never fetched that branch before and would send all of the changelog data
from the whole history of the repo. (This led to a lot of downstream
problems; we mitigated by setting a refspec that stopped fetching the
'release' branch, but this is not ideal.)
What I'd like is a mechanism to say "always advertise the client's version
of 'main' and 'release' but also opportunistically include some user
branches".
Based on my understanding, the '--negotiation-tip' option is close but not
quite what I want. I could have the client only advertise 'release' and
'main' and never advertise any user branches. But then we'd download all
content from each user branch every time it updates. Perhaps this would
happen even with opportunistic inclusion of more haves, but I'd like to
explore this area more.
There's also an issue that the '--negotiation-tip' feature doesn't seem to
have a config key that enables it without CLI arguments. This is something
that we could consider independently.
This patch series adds a new '--negotiation-include' option that does what I
want: it makes sure that these references are included as 'have's during
negotiation. In order to help clarify the difference between this and
'--negotiation-tip', I first create a synonym called
'--negotiation-restrict'.
Both of these options get 'remote.*.negotiation(Include|Restrict)' config
options that enable their behavior by default.
During development, I had briefly considered only using config values, but
that required some strange changes to care about the remote name in the
transport layer. This was most different in the 'git push' integration. When
I discovered the '--negotiation-tip' feature during the process, that gave
me a clear pattern to follow with the addition of a config on top.
Updates in v2
=============
This version is a near-complete rewrite based on feedback around the names
of the previous option and config. The --negotiation-restrict option is new
and the ability to set it via config is also new.
I did try to be more careful around translatable error messages, too.
Updates in v3
=============
* --negotiation-tip is now an alias of --negotiation-restrict.
* More translatable strings use %s to isolate non-translatable options from
translatable words.
* The string_list named negotiation_tip is now renamed to
negotiation_restrict.
* The config options now allow an empty value to reset the list.
* The --negotiation-require option is now called --negotiation-include.
* Similarly, the config option is renamed and all code references.
* The included haves now mark their commits as COMMON so commits that they
can reach are not included in the negotiation walk if they are reached
from the restricted commits.
* The ref iterators are more careful about failing on bad references (ref
exists but object doesn't) and ignoring missing references (perhaps
config is erroneous?).
* When sending tips during push negotiation, use the --negotiation-restrict
option instead of -tip.
Updates in v4
=============
Thanks, Matthew, for the detailed review! There are some big changes in this
version.
* Expanded commit message to cite the commit that introduced the bug
(3f763ddf28).
* Renamed --negotiation-tip to --negotiation-restrict throughout docs/code
(including send-pack.c, transport-helper.c, builtin/pull.c). Added
OPT_ALIAS in git-pull.
* Switched config parsing to use parse_transport_option() helper. Removed
git push from docs (not implemented yet). Restructured --negotiate-only
validation flow.
* NEW Patch 5: Added have_sent() interface to negotiators, so included
haves can be de-duplicated properly by the negotiation algorithm.
* Replaced COMMON flag hack with negotiator->have_sent() calls. Moved
ref-pattern resolution into builtin/fetch.c (add_negotiation_tips()) so
fetch-pack receives pre-resolved oid_array instead of string_list. Added
test for --negotiation-tip ignoring missing refs. Added
duplicate-avoidance test for v0. Accepts commit hashes in addition to ref
names/globs.
* Use parse_transport_option() for config. Updated docs to mention commit
hashes. Removed git push from config docs. Fixed test to use correct
restrict/include combinations.
* In the last patch, add doc notes that remote config values also apply
during git push with push.negotiate, now that they are integrated by that
change.
Thanks, -Stolee
Derrick Stolee (8):
t5516: fix test order flakiness
fetch: add --negotiation-restrict option
transport: rename negotiation_tips
remote: add remote.*.negotiationRestrict config
negotiator: add have_sent() interface
fetch: add --negotiation-include option for negotiation
remote: add remote.*.negotiationInclude config
send-pack: pass negotiation config in push
Documentation/config/fetch.adoc | 2 +-
Documentation/config/remote.adoc | 51 +++++++++
Documentation/fetch-options.adoc | 29 ++++-
builtin/fetch.c | 82 ++++++++++---
builtin/pull.c | 6 +-
fetch-negotiator.h | 9 ++
fetch-pack.c | 99 +++++++++++++---
fetch-pack.h | 10 +-
negotiator/default.c | 8 ++
negotiator/noop.c | 7 ++
negotiator/skipping.c | 8 ++
remote.c | 10 ++
remote.h | 2 +
send-pack.c | 39 +++++--
send-pack.h | 2 +
t/t5510-fetch.sh | 191 +++++++++++++++++++++++++++++++
t/t5516-fetch-push.sh | 32 +++++-
t/t5702-protocol-v2.sh | 4 +-
transport-helper.c | 5 +-
transport.c | 20 +++-
transport.h | 7 +-
21 files changed, 561 insertions(+), 62 deletions(-)
base-commit: 6e8d538aab8fe4dd07ba9fb87b5c7edcfa5706ad
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2085%2Fderrickstolee%2Fmust-have-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2085/derrickstolee/must-have-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/2085
Range-diff vs v3:
1: 466c56abe0 ! 1: 7409a479d6 t5516: fix test order flakiness
@@ Commit message
t5516: fix test order flakiness
The 'fetch follows tags by default' test sorts using 'sort -k 4', but
- for-each-ref output only has 3 columns. This relies on sort treating
- records with fewer fields as having an empty fourth field, which may
- produce unstable results depending on locale. Use 'sort -k 3' to match
- the actual number of columns in the output.
+ for-each-ref output only has 3 columns. This relies on sort treating records
+ with fewer fields as having an empty fourth field, which may produce
+ unstable results depending on locale. This appears to be an accident added
+ in 3f763ddf28 (fetch: set remote/HEAD if it does not exist, 2024-11-22).
+
+ Use 'sort -k 3' to match the actual number of columns in the output.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
2: fe875399a8 ! 2: 7836a2d6a5 fetch: add --negotiation-restrict option
@@ Commit message
Signed-off-by: Derrick Stolee <stolee@gmail.com>
+ ## Documentation/config/fetch.adoc ##
+@@
+ default is `skipping`. Unknown values will cause `git fetch` to
+ error out.
+ +
+-See also the `--negotiate-only` and `--negotiation-tip` options to
++See also the `--negotiate-only` and `--negotiation-restrict` options to
+ linkgit:git-fetch[1].
+
+ `fetch.showForcedUpdates`::
+
## Documentation/fetch-options.adoc ##
@@ Documentation/fetch-options.adoc: the current repository has the same history as the source repository.
`.git/shallow`. This option updates `.git/shallow` and accepts such
@@ Documentation/fetch-options.adoc: the current repository has the same history as
This option may be specified more than once; if so, Git will report
commits reachable from any of the given commits.
+
+@@ Documentation/fetch-options.adoc: configuration variables documented in linkgit:git-config[1], and the
+
+ `--negotiate-only`::
+ Do not fetch anything from the server, and instead print the
+- ancestors of the provided `--negotiation-tip=` arguments,
++ ancestors of the provided `--negotiation-restrict=` arguments,
+ which we have in common with the server.
+ +
+ This is incompatible with `--recurse-submodules=(yes|on-demand)`.
## builtin/fetch.c ##
@@ builtin/fetch.c: static void add_negotiation_tips(struct git_transport_options *smart_options)
@@ builtin/fetch.c: int cmd_fetch(int argc,
## builtin/pull.c ##
@@ builtin/pull.c: int cmd_pull(int argc,
- OPT_PASSTHRU_ARGV(0, "negotiation-tip", &opt_fetch, N_("revision"),
+ OPT_PASSTHRU('6', "ipv6", &opt_ipv6, NULL,
+ N_("use IPv6 addresses only"),
+ PARSE_OPT_NOARG),
+- OPT_PASSTHRU_ARGV(0, "negotiation-tip", &opt_fetch, N_("revision"),
++ OPT_PASSTHRU_ARGV(0, "negotiation-restrict", &opt_fetch, N_("revision"),
N_("report that we have only objects reachable from this object"),
0),
-+ OPT_PASSTHRU_ARGV(0, "negotiation-restrict", &opt_fetch, N_("revision"),
-+ N_("report that we have only objects reachable from this object"),
-+ 0),
++ OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
OPT_BOOL(0, "show-forced-updates", &opt_show_forced_updates,
N_("check for forced-updates on all updated branches")),
OPT_PASSTHRU(0, "set-upstream", &set_upstream, NULL,
+ ## send-pack.c ##
+@@ send-pack.c: static void get_commons_through_negotiation(struct repository *r,
+ strvec_pushl(&child.args, "fetch", "--negotiate-only", NULL);
+ for (ref = remote_refs; ref; ref = ref->next) {
+ if (!is_null_oid(&ref->new_oid)) {
+- strvec_pushf(&child.args, "--negotiation-tip=%s",
++ strvec_pushf(&child.args, "--negotiation-restrict=%s",
+ oid_to_hex(&ref->new_oid));
+ nr_negotiation_tip++;
+ }
+
## t/t5510-fetch.sh ##
@@ t/t5510-fetch.sh: EOF
test_cmp fatal-expect fatal-actual
@@ t/t5702-protocol-v2.sh: setup_negotiate_only () {
EOF
test_must_fail git -c protocol.version=2 -C client fetch \
+
+ ## transport-helper.c ##
+@@ transport-helper.c: static int fetch_refs(struct transport *transport,
+ }
+
+ if (data->transport_options.negotiation_tips)
+- warning("Ignoring --negotiation-tip because the protocol does not support it.");
++ warning(_("ignoring %s because the protocol does not support it."),
++ "--negotiation-restrict");
+
+ if (data->fetch)
+ return fetch_with_fetch(transport, nr_heads, to_fetch);
3: 4332cbf266 ! 3: 401bdaff7c transport: rename negotiation_tips
@@ Commit message
transport: rename negotiation_tips
The previous change added the --negotiation-restrict synonym for the
- --negotiation-tips option for 'git fetch'. In anticipation of adding a
- new option that behaves similarly but with distinct changes to its
- behavior, rename the internal representation of this data from
- 'negotiation_tips' to 'negotiation_restrict_tips'.
+ --negotiation-tip option for 'git fetch'. In anticipation of adding a new
+ option that behaves similarly but with distinct changes to its behavior,
+ rename the internal representation of this data from 'negotiation_tips' to
+ 'negotiation_restrict_tips'.
- The 'tips' part is kept because this is an oid_array in the transport
- layer. This requires the builtin to handle parsing refs into collections
- of oids so the transport layer can handle this cleaner form of the data.
+ The 'tips' part is kept because this is an oid_array in the transport layer.
+ This requires the builtin to handle parsing refs into collections of oids so
+ the transport layer can handle this cleaner form of the data.
Also update the string_list used to store the inputs from command-line
options.
@@ transport-helper.c: static int fetch_refs(struct transport *transport,
- if (data->transport_options.negotiation_tips)
+ if (data->transport_options.negotiation_restrict_tips)
- warning("Ignoring --negotiation-tip because the protocol does not support it.");
+ warning(_("ignoring %s because the protocol does not support it."),
+ "--negotiation-restrict");
- if (data->fetch)
## transport.c ##
@@ transport.c: static int fetch_refs_via_pack(struct transport *transport,
4: d2f48b78b5 ! 4: a14c568a1f remote: add remote.*.negotiationRestrict config
@@ Metadata
## Commit message ##
remote: add remote.*.negotiationRestrict config
- In a previous change, the --negotiation-restrict command-line option of
- 'git fetch' was added as a synonym of --negotiation-tips. Both of these
- options restrict the set of 'haves' the client can send as part of
- negotiation.
+ In a previous change, the --negotiation-restrict command-line option of 'git
+ fetch' was added as a synonym of --negotiation-tip. Both of these options
+ restrict the set of 'haves' the client can send as part of negotiation.
This was previously not available via a configuration option. Add a new
- 'remote.<name>.negotiationRestrict' multi-valued config option that
- updates 'git fetch <name>' to use these restrictions by default.
+ 'remote.<name>.negotiationRestrict' multi-valued config option that updates
+ 'git fetch <name>' to use these restrictions by default.
If the user provides even one --negotiation-restrict argument, then the
config is ignored.
@@ Documentation/config/remote.adoc: priority configuration file (e.g. `.git/config
`$HOME/.gitconfig`).
+remote.<name>.negotiationRestrict::
-+ When negotiating with this remote during `git fetch` and `git push`,
-+ restrict the commits advertised as "have" lines to only those
-+ reachable from refs matching the given patterns. This multi-valued
-+ config option behaves like `--negotiation-restrict` on the command
-+ line.
++ When negotiating with this remote during `git fetch`, restrict the
++ commits advertised as "have" lines to only those reachable from refs
++ matching the given patterns. This multi-valued config option behaves
++ like `--negotiation-restrict` on the command line.
++
+Each value is either an exact ref name (e.g. `refs/heads/release`) or a
+glob pattern (e.g. `refs/heads/release/*`). The pattern syntax is the
@@ builtin/fetch.c: int cmd_fetch(int argc,
if (!remote)
die(_("must supply remote when using --negotiate-only"));
gtransport = prepare_transport(remote, 1, &filter_options);
-+ if (!gtransport->smart_options ||
-+ !gtransport->smart_options->negotiation_restrict_tips)
+- if (gtransport->smart_options) {
+- gtransport->smart_options->acked_commits = &acked_commits;
+- } else {
++
++ if (!gtransport->smart_options) {
+ warning(_("protocol does not support --negotiate-only, exiting"));
+ result = 1;
+ trace2_region_leave("fetch", "negotiate-only", the_repository);
+ goto cleanup;
+ }
++ if (!gtransport->smart_options->negotiation_restrict_tips)
+ die(_("%s needs one or more %s"), "--negotiate-only",
+ "--negotiation-restrict=*");
- if (gtransport->smart_options) {
- gtransport->smart_options->acked_commits = &acked_commits;
- } else {
++
++ gtransport->smart_options->acked_commits = &acked_commits;
++
+ if (server_options.nr)
+ gtransport->server_options = &server_options;
+ result = transport_fetch_refs(gtransport, NULL);
## remote.c ##
@@ remote.c: static struct remote *make_remote(struct remote_state *remote_state,
@@ remote.c: static int handle_config(const char *key, const char *value,
return parse_transport_option(key, value,
&remote->server_options);
+ } else if (!strcmp(subkey, "negotiationrestrict")) {
-+ /* reset list on empty value. */
-+ if (!value || !*value)
-+ string_list_clear(&remote->negotiation_restrict, 0);
-+ else
-+ string_list_append(&remote->negotiation_restrict, value);
++ return parse_transport_option(key, value,
++ &remote->negotiation_restrict);
} else if (!strcmp(subkey, "followremotehead")) {
const char *no_warn_branch;
if (!strcmp(value, "never"))
-: ---------- > 5: 94b79784fe negotiator: add have_sent() interface
5: ae81ef36a1 ! 6: b4cd458fe0 fetch: add --negotiation-include option for negotiation
@@ Commit message
negotiation ref set to a specific list. In the earlier example, using
--negotiation-restrict to focus the negotiation to 'dev' and 'release'
would avoid those problematic downloads, but would still not allow
- advertising potentially-relevant user brances. In this way, the
+ advertising potentially-relevant user branches. In this way, the
'include' version solves the problem I mention while allowing
negotiation to pick other references opportunistically. The two options
can also be combined to allow the best of both worlds.
@@ Commit message
logic for the related --negotiation-restrict option to match.
The implementation outputs the requested objects as haves before the
- negotiation algorithm kicks in and performs a priority-queue walk from the
- tip commits. In order to avoid duplicates, we mark the requested objects as
- COMMON so they (and their descendants) are not output by the negotiator. The
- negotiator still outputs at least one have before a round is flushed, when
- the server could ACK to stop the negotiation.
+ negotiator performs its own algorithm to choose the next haves. Use the new
+ have_sent() interface to signal these have commits were sent before engaging
+ with the negotiator's next() iterator.
Also add --negotiation-include to 'git pull' passthrough options.
@@ Documentation/fetch-options.adoc: See also the `fetch.negotiationAlgorithm` and
configuration variables documented in linkgit:git-config[1], and the
`--negotiate-only` option below.
-+`--negotiation-include=<revision>`::
-+ Ensure that the given ref tip is always sent as a "have" line
-+ during fetch negotiation, regardless of what the negotiation
++`--negotiation-include=(<commit>|<glob>)`::
++ Ensure that the commits at the given tips are always sent as "have"
++ lines during fetch negotiation, regardless of what the negotiation
+ algorithm selects. This is useful to guarantee that common
+ history reachable from specific refs is always considered, even
+ when `--negotiation-restrict` restricts the set of tips or when
+ the negotiation algorithm would otherwise skip them.
++
-+This option may be specified more than once; if so, each ref is sent
++This option may be specified more than once; if so, each commit is sent
+unconditionally.
++
-+The argument may be an exact ref name (e.g. `refs/heads/release`) or a
-+glob pattern (e.g. `refs/heads/release/{asterisk}`). The pattern syntax
-+is the same as for `--negotiation-restrict`.
++The argument may be an exact ref name (e.g. `refs/heads/release`), an
++object hash, or a glob pattern (e.g. `refs/heads/release/{asterisk}`).
++The pattern syntax is the same as for `--negotiation-restrict`.
++
+If `--negotiation-restrict` is used, the have set is first restricted by
+that option and then increased to include the tips specified by
@@ Documentation/fetch-options.adoc: See also the `fetch.negotiationAlgorithm` and
+
`--negotiate-only`::
Do not fetch anything from the server, and instead print the
- ancestors of the provided `--negotiation-tip=` arguments,
+ ancestors of the provided `--negotiation-restrict=` arguments,
## builtin/fetch.c ##
@@ builtin/fetch.c: static struct transport *gsecondary;
@@ builtin/fetch.c: static struct transport *gsecondary;
struct fetch_config {
enum display_format display_format;
-@@ builtin/fetch.c: static void add_negotiation_restrict_tips(struct git_transport_options *smart_op
+@@ builtin/fetch.c: static int add_oid(const struct reference *ref, void *cb_data)
+ return 0;
+ }
+
+-static void add_negotiation_restrict_tips(struct git_transport_options *smart_options)
++static void add_negotiation_tips(struct string_list *input_list,
++ struct oid_array **output_list)
+ {
+ struct oid_array *oids = xcalloc(1, sizeof(*oids));
+ int i;
+
+- for (i = 0; i < negotiation_restrict.nr; i++) {
+- const char *s = negotiation_restrict.items[i].string;
++ for (i = 0; i < input_list->nr; i++) {
++ const char *s = input_list->items[i].string;
+ struct refs_for_each_ref_options opts = {
+ .pattern = s,
+ };
int old_nr;
if (!has_glob_specials(s)) {
struct object_id oid;
@@ builtin/fetch.c: static void add_negotiation_restrict_tips(struct git_transport_
oid_array_append(oids, &oid);
continue;
}
+@@ builtin/fetch.c: static void add_negotiation_restrict_tips(struct git_transport_options *smart_op
+ warning(_("ignoring %s=%s because it does not match any refs"),
+ "--negotiation-restrict", s);
+ }
+- smart_options->negotiation_restrict_tips = oids;
++ *output_list = oids;
+ }
+
+ static struct transport *prepare_transport(struct remote *remote, int deepen,
+@@ builtin/fetch.c: static struct transport *prepare_transport(struct remote *remote, int deepen,
+ }
+ if (negotiation_restrict.nr) {
+ if (transport->smart_options)
+- add_negotiation_restrict_tips(transport->smart_options);
++ add_negotiation_tips(&negotiation_restrict,
++ &transport->smart_options->negotiation_restrict_tips);
+ else
+ warning(_("ignoring %s because the protocol does not support it"),
+ "--negotiation-restrict");
+@@ builtin/fetch.c: static struct transport *prepare_transport(struct remote *remote, int deepen,
+ for_each_string_list_item(item, &remote->negotiation_restrict)
+ string_list_append(&negotiation_restrict, item->string);
+ if (transport->smart_options)
+- add_negotiation_restrict_tips(transport->smart_options);
++ add_negotiation_tips(&negotiation_restrict,
++ &transport->smart_options->negotiation_restrict_tips);
+ else {
+ struct strbuf config_name = STRBUF_INIT;
+ strbuf_addf(&config_name, "remote.%s.negotiationRestrict", remote->name);
@@ builtin/fetch.c: static struct transport *prepare_transport(struct remote *remote, int deepen,
strbuf_release(&config_name);
}
}
+ if (negotiation_include.nr) {
+ if (transport->smart_options)
-+ transport->smart_options->negotiation_include = &negotiation_include;
++ add_negotiation_tips(&negotiation_include,
++ &transport->smart_options->negotiation_include_tips);
+ else
+ warning(_("ignoring %s because the protocol does not support it"),
+ "--negotiation-include");
@@ builtin/fetch.c: int cmd_fetch(int argc,
## builtin/pull.c ##
@@ builtin/pull.c: int cmd_pull(int argc,
- OPT_PASSTHRU_ARGV(0, "negotiation-restrict", &opt_fetch, N_("revision"),
N_("report that we have only objects reachable from this object"),
0),
+ OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
+ OPT_PASSTHRU_ARGV(0, "negotiation-include", &opt_fetch, N_("revision"),
+ N_("ensure this ref is always sent as a negotiation have"),
+ 0),
@@ fetch-pack.c: static void send_filter(struct fetch_pack_args *args,
}
}
-+static int add_oid_to_oidset(const struct reference *ref, void *cb_data)
++static void add_oids_to_set(const struct oid_array *array,
++ struct oidset *set)
+{
-+ struct oidset *set = cb_data;
-+ if (!odb_has_object(the_repository->objects, ref->oid, 0))
-+ die(_("the object %s does not exist"), oid_to_hex(ref->oid));
-+ oidset_insert(set, ref->oid);
-+ return 0;
-+}
-+
-+static void resolve_negotiation_include(const struct string_list *negotiation_include,
-+ struct oidset *result)
-+{
-+ struct string_list_item *item;
-+
-+ if (!negotiation_include || !negotiation_include->nr)
++ if (!array)
+ return;
+
-+ for_each_string_list_item(item, negotiation_include) {
-+ if (!has_glob_specials(item->string)) {
-+ struct object_id oid;
++ for (size_t i = 0; i < array->nr; i++) {
++ struct object_id *oid = &array->oid[i];
++ if (!odb_has_object(the_repository->objects, oid, 0))
++ die(_("the object %s does not exist"), oid_to_hex(oid));
+
-+ /* Ignore missing reference. */
-+ if (repo_get_oid(the_repository, item->string, &oid))
-+ continue;
-+
-+ /* Fail on missing object pointed by ref. */
-+ if (!odb_has_object(the_repository->objects, &oid, 0))
-+ die(_("the object %s does not exist"),
-+ item->string);
-+
-+ oidset_insert(result, &oid);
-+ } else {
-+ struct refs_for_each_ref_options opts = {
-+ .pattern = item->string,
-+ };
-+ refs_for_each_ref_ext(
-+ get_main_ref_store(the_repository),
-+ add_oid_to_oidset, result, &opts);
-+ }
++ oidset_insert(set, oid);
+ }
+}
+
@@ fetch-pack.c: static int find_common(struct fetch_negotiator *negotiator,
retval = -1;
+
+ /* Send unconditional haves from --negotiation-include */
-+ resolve_negotiation_include(args->negotiation_include,
-+ &negotiation_include_oids);
++ add_oids_to_set(args->negotiation_include_tips,
++ &negotiation_include_oids);
+ if (oidset_size(&negotiation_include_oids)) {
+ struct oidset_iter iter;
+ oidset_iter_init(&negotiation_include_oids, &iter);
@@ fetch-pack.c: static int find_common(struct fetch_negotiator *negotiator,
+ print_verbose(args, "have %s", oid_to_hex(oid));
+ count++;
+
-+ /*
-+ * If this is a commit, then mark as COMMON to
-+ * avoid the negotiator also outputting it as
-+ * a have.
-+ */
+ commit = lookup_commit(the_repository, oid);
-+ if (commit &&
-+ !repo_parse_commit(the_repository, commit))
-+ commit->object.flags |= COMMON;
++ if (commit)
++ negotiator->have_sent(negotiator, commit);
+ }
+ }
+
@@ fetch-pack.c: static void add_common(struct strbuf *req_buf, struct oidset *comm
+ struct oidset_iter iter;
+ oidset_iter_init(negotiation_include_oids, &iter);
+
-+ while ((oid = oidset_iter_next(&iter)))
-+ packet_buf_write(req_buf, "have %s\n",
-+ oid_to_hex(oid));
++ while ((oid = oidset_iter_next(&iter))) {
++ struct commit *commit = lookup_commit(the_repository, oid);
++ if (commit) {
++ packet_buf_write(req_buf, "have %s\n",
++ oid_to_hex(oid));
++ negotiator->have_sent(negotiator, commit);
++ }
++ }
+ }
+
while ((oid = negotiator->next(negotiator))) {
-+ if (negotiation_include_oids &&
-+ oidset_contains(negotiation_include_oids, oid))
-+ continue;
packet_buf_write(req_buf, "have %s\n", oid_to_hex(oid));
if (++haves_added >= *haves_to_send)
- break;
@@ fetch-pack.c: static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
struct fetch_pack_args *args,
const struct ref *wants, struct oidset *common,
@@ fetch-pack.c: static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
state = FETCH_SEND_REQUEST;
mark_tips(negotiator, args->negotiation_restrict_tips);
-+ resolve_negotiation_include(args->negotiation_include,
-+ &negotiation_include_oids);
++ add_oids_to_set(args->negotiation_include_tips,
++ &negotiation_include_oids);
for_each_cached_alternate(negotiator,
insert_one_alternate_object);
break;
@@ fetch-pack.c: void negotiate_using_fetch(const struct oid_array *negotiation_res
int fd[],
- struct oidset *acked_commits)
+ struct oidset *acked_commits,
-+ const struct string_list *negotiation_include)
++ const struct oid_array *negotiation_include_tips)
{
struct fetch_negotiator negotiator;
struct packet_reader reader;
@@ fetch-pack.c: void negotiate_using_fetch(const struct oid_array *negotiation_res
fetch_negotiator_init(the_repository, &negotiator);
mark_tips(&negotiator, negotiation_restrict_tips);
-+ resolve_negotiation_include(negotiation_include,
-+ &negotiation_include_oids);
++ add_oids_to_set(negotiation_include_tips,
++ &negotiation_include_oids);
+
packet_reader_init(&reader, fd[0], NULL, 0,
PACKET_READ_CHOMP_NEWLINE |
@@ fetch-pack.c: void negotiate_using_fetch(const struct oid_array *negotiation_res
## fetch-pack.h ##
@@ fetch-pack.h: struct fetch_pack_args {
+
+ /*
+ * If not NULL, during packfile negotiation, fetch-pack will send "have"
+- * lines only with these tips and their ancestors.
++ * lines for all _include_ tips and then a subset of the _restrict_ tips.
*/
const struct oid_array *negotiation_restrict_tips;
++ const struct oid_array *negotiation_include_tips;
-+ /*
-+ * If non-empty, ref patterns whose tips should always be sent
-+ * as "have" lines during negotiation, regardless of what the
-+ * negotiation algorithm selects.
-+ */
-+ const struct string_list *negotiation_include;
-+
unsigned deepen_relative:1;
unsigned quiet:1;
- unsigned keep_pack:1;
@@ fetch-pack.h: void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
const struct string_list *server_options,
int stateless_rpc,
int fd[],
- struct oidset *acked_commits);
+ struct oidset *acked_commits,
-+ const struct string_list *negotiation_include);
++ const struct oid_array *negotiation_include_tips);
/*
* Print an appropriate error message for each sought ref that wasn't
## t/t5510-fetch.sh ##
+@@ t/t5510-fetch.sh: EOF
+ test_cmp fatal-expect fatal-actual
+ '
+
++test_expect_success '--negotiation-tip ignores missing refs and invalid hashes' '
++ setup_negotiation_tip server server 0 &&
++ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
++ --negotiation-tip=alpha_1 --negotiation-tip=beta_1 \
++ --negotiation-tip=no-such-ref \
++ --negotiation-tip=invalid-hash \
++ origin alpha_s beta_s &&
++ check_negotiation_tip
++'
++
+ test_expect_success '--negotiation-restrict limits "have" lines sent' '
+ setup_negotiation_tip server server 0 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
@@ t/t5510-fetch.sh: test_expect_success 'CLI --negotiation-restrict overrides remote config' '
test_grep ! "fetch> have $BETA_1" trace
'
@@ t/t5510-fetch.sh: test_expect_success 'CLI --negotiation-restrict overrides remo
+ test_grep "fetch> have $ALPHA_1" trace >matches &&
+ test_line_count = 1 matches
+'
++
++test_expect_success '--negotiation-include avoids duplicates with v0' '
++ test_when_finished rm -f trace &&
++ setup_negotiation_tip server server 0 &&
++
++ ALPHA_1=$(git -C client rev-parse alpha_1) &&
++ GIT_TRACE_PACKET="$(pwd)/trace" git -C client \
++ -c protocol.version=0 fetch \
++ --negotiation-restrict=alpha_1 \
++ --negotiation-include=refs/tags/alpha_1 \
++ origin alpha_s beta_s &&
++
++ test_grep "fetch> have $ALPHA_1" trace >matches &&
++ test_line_count = 1 matches
++'
+
test_expect_success SYMLINKS 'clone does not get confused by a D/F conflict' '
git init df-conflict &&
@@ transport.c: static int fetch_refs_via_pack(struct transport *transport,
args.stateless_rpc = transport->stateless_rpc;
args.server_options = transport->server_options;
args.negotiation_restrict_tips = data->options.negotiation_restrict_tips;
-+ args.negotiation_include = data->options.negotiation_include;
++ args.negotiation_include_tips = data->options.negotiation_include_tips;
args.reject_shallow_remote = transport->smart_options->reject_shallow;
if (!data->finished_handshake) {
@@ transport.c: static int fetch_refs_via_pack(struct transport *transport,
data->fd,
- data->options.acked_commits);
+ data->options.acked_commits,
-+ data->options.negotiation_include);
++ data->options.negotiation_include_tips);
ret = 0;
}
goto cleanup;
+@@ transport.c: static int disconnect_git(struct transport *transport)
+ oid_array_clear(data->options.negotiation_restrict_tips);
+ free(data->options.negotiation_restrict_tips);
+ }
++ if (data->options.negotiation_include_tips) {
++ oid_array_clear(data->options.negotiation_include_tips);
++ free(data->options.negotiation_include_tips);
++ }
+ list_objects_filter_release(&data->options.filter_options);
+ oid_array_clear(&data->extra_have);
+ oid_array_clear(&data->shallow);
## transport.h ##
@@ transport.h: struct git_transport_options {
+
+ /*
+ * This is only used during fetch. See the documentation of
+- * negotiation_restrict_tips in struct fetch_pack_args.
++ * these member names in struct fetch_pack_args.
+ *
+- * This field is only supported by transports that support connect or
++ * These fields are only supported by transports that support connect or
+ * stateless_connect. Set this field directly instead of using
+ * transport_set_option().
*/
struct oid_array *negotiation_restrict_tips;
++ struct oid_array *negotiation_include_tips;
-+ /*
-+ * If non-empty, ref patterns whose tips should always be sent
-+ * as "have" lines during negotiation.
-+ */
-+ const struct string_list *negotiation_include;
-+
/*
* If allocated, whenever transport_fetch_refs() is called, add known
- * common commits to this oidset instead of fetching any packfiles.
6: a2d15fa12a ! 7: 7bd70a970b remote: add remote.*.negotiationInclude config
@@ Documentation/config/remote.adoc: values are not used.
the list from broader config scenarios.
+remote.<name>.negotiationInclude::
-+ When negotiating with this remote during `git fetch` and `git push`,
-+ the client advertises a list of commits that exist locally. In
-+ repos with many references, this list of "haves" can be truncated.
-+ Depending on data shape, dropping certain references may be
-+ expensive. This multi-valued config option specifies ref patterns
-+ whose tips should always be sent as "have" commits during fetch
-+ negotiation with this remote.
++ When negotiating with this remote during `git fetch`, the client
++ advertises a list of commits that exist locally. In repos with
++ many references, this list of "haves" can be truncated. Depending
++ on data shape, dropping certain references may be expensive. This
++ multi-valued config option specifies references, commit hashes,
++ or ref pattern globs whose tips should always be sent as "have"
++ commits during fetch negotiation with this remote.
++
-+Each value is either an exact ref name (e.g. `refs/heads/release`) or a
-+glob pattern (e.g. `refs/heads/release/*`). The pattern syntax is the same
-+as for `--negotiation-restrict`.
++Each value is either an exact ref name (e.g. `refs/heads/release`), a
++commit hash, or a glob pattern (e.g. `refs/heads/release/*`). The
++pattern syntax is the same as for `--negotiation-include`.
++
+These config values are used as defaults for the `--negotiation-include`
+command-line option. If `--negotiation-include` is specified on the
@@ Documentation/config/remote.adoc: values are not used.
when fetching using the configured refspecs of a remote.
## Documentation/fetch-options.adoc ##
-@@ Documentation/fetch-options.adoc: is the same as for `--negotiation-restrict`.
+@@ Documentation/fetch-options.adoc: The pattern syntax is the same as for `--negotiation-restrict`.
If `--negotiation-restrict` is used, the have set is first restricted by
that option and then increased to include the tips specified by
`--negotiation-include`.
@@ builtin/fetch.c: static struct transport *prepare_transport(struct remote *remot
"--negotiation-include");
+ } else if (remote->negotiation_include.nr) {
+ if (transport->smart_options) {
-+ transport->smart_options->negotiation_include = &remote->negotiation_include;
++ add_negotiation_tips(&remote->negotiation_include,
++ &transport->smart_options->negotiation_include_tips);
+ } else {
+ struct strbuf config_name = STRBUF_INIT;
+ strbuf_addf(&config_name, "remote.%s.negotiationInclude", remote->name);
@@ remote.c: static void remote_clear(struct remote *remote)
static void add_merge(struct branch *branch, const char *name)
@@ remote.c: static int handle_config(const char *key, const char *value,
- string_list_clear(&remote->negotiation_restrict, 0);
- else
- string_list_append(&remote->negotiation_restrict, value);
+ } else if (!strcmp(subkey, "negotiationrestrict")) {
+ return parse_transport_option(key, value,
+ &remote->negotiation_restrict);
+ } else if (!strcmp(subkey, "negotiationinclude")) {
-+ /* reset list on empty value. */
-+ if (!value || !*value)
-+ string_list_clear(&remote->negotiation_include, 0);
-+ else
-+ string_list_append(&remote->negotiation_include, value);
++ return parse_transport_option(key, value,
++ &remote->negotiation_include);
} else if (!strcmp(subkey, "followremotehead")) {
const char *no_warn_branch;
if (!strcmp(value, "never"))
@@ t/t5510-fetch.sh: test_expect_success '--negotiation-include avoids duplicates w
+ git -C client config --add remote.origin.negotiationInclude "" &&
+ git -C client config --add remote.origin.negotiationInclude refs/tags/beta_1 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
-+ --negotiation-restrict=alpha_1 \
++ --negotiation-restrict=beta_2 \
+ origin alpha_s beta_s &&
+
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
-+ test_grep "fetch> have $ALPHA_1" trace &&
++ test_grep ! "fetch> have $ALPHA_1" trace &&
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace
+'
@@ t/t5510-fetch.sh: test_expect_success '--negotiation-include avoids duplicates w
+ test_grep ! "fetch> have $BETA_2" trace
+'
+
- test_expect_success SYMLINKS 'clone does not get confused by a D/F conflict' '
- git init df-conflict &&
- (
+ test_expect_success '--negotiation-include avoids duplicates with v0' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
7: e6c79f0661 ! 8: 5b968245eb send-pack: pass negotiation config in push
@@ Commit message
Signed-off-by: Derrick Stolee <stolee@gmail.com>
+ ## Documentation/config/remote.adoc ##
+@@ Documentation/config/remote.adoc: command-line option. If `--negotiation-restrict` (or its synonym
+ `--negotiation-tip`) is specified on the command line, then the config
+ values are not used.
+ +
++These values also influence negotiation during `git push` if
++`push.negotiate` is enabled.
+++
+ Blank values signal to ignore all previous values, allowing a reset of
+ the list from broader config scenarios.
+
+@@ Documentation/config/remote.adoc: unconditionally on top of those heuristically selected commits. This
+ option is also used during push negotiation when `push.negotiate` is
+ enabled.
+ +
++These values also influence negotiation during `git push` if
++`push.negotiate` is enabled.
+++
+ Blank values signal to ignore all previous values, allowing a reset of
+ the list from broader config scenarios.
+
+
## send-pack.c ##
@@ send-pack.c: static void reject_invalid_nonce(const char *nonce, int len)
@@ send-pack.c: static void reject_invalid_nonce(const char *nonce, int len)
strvec_pushl(&child.args, "fetch", "--negotiate-only", NULL);
- for (ref = remote_refs; ref; ref = ref->next) {
- if (!is_null_oid(&ref->new_oid)) {
-- strvec_pushf(&child.args, "--negotiation-tip=%s",
-- oid_to_hex(&ref->new_oid));
-- nr_negotiation_tip++;
+
+ if (negotiation_restrict && negotiation_restrict->nr) {
+ struct string_list_item *item;
+ for_each_string_list_item(item, negotiation_restrict)
-+ strvec_pushf(&child.args, "--negotiation-restrict=%s",
+ strvec_pushf(&child.args, "--negotiation-restrict=%s",
+- oid_to_hex(&ref->new_oid));
+- nr_negotiation_tip++;
+ item->string);
+ nr_negotiation = negotiation_restrict->nr;
+ } else {
--
gitgitgadget
^ permalink raw reply
* [PATCH v3 4/4] approxidate: use deferred mday adjustments for "specials"
From: Tuomas Ahola @ 2026-05-14 11:55 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Tuomas Ahola
In-Reply-To: <20260514115520.6660-1-taahol@utu.fi>
There are cases where the "wrap-to-yesterday" behavior of "tea" and
"noon" should be reverted later on down the line, so that "today tea"
and "tea today" won't yield different results. However, the logic of
approxidate doesn't seem to lend itself particularly well to
such cases.
Start tackling the issue by reusing negative values of `tm->tm_mday`
field for deferred date adjustments which can be easily reverted, so
that the default logic of the special formats only applies if we don't
get any explicit date (mday) specification. In particular, overwrite
the field with -1 in "now" and "yesterday", so that those formats will
be relative to the current date. That makes specifications like "tea
yesterday" behave more sensibly: instead of going backwards to the
last tea-time and then a day back, Git will now understand that as the
tea-time of yesterday.
Replace the call of `update_tm()` in `date_time()` with the assignment
`tm->tm_mday = -2`. Add the corresponding code to handle that in
`update_tm()`, wrapping to the previous day if the field still holds
such assignment, meaning that we haven't seen any better specification
for the day-of-month. On the other hand, `mday=-3` would mean going
two days back and so on. Even though such functionality isn't
actually needed by this patch, it won't add much complexity in the
code and is rather natural way to handle such values.
As `date_time()` won't no longer need the `now` struct, mark the
associated function parameters as unused. The parameters themselves
have to stay, however, as those functions are called through pointers
in `approxidate_alpha`. Add relevant tests to cover the changes.
Signed-off-by: Tuomas Ahola <taahol@utu.fi>
---
date.c | 29 ++++++++++++++++++++---------
t/t0006-date.sh | 4 ++++
2 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/date.c b/date.c
index 73879d202c..914c733737 100644
--- a/date.c
+++ b/date.c
@@ -1071,13 +1071,22 @@ void datestamp(struct strbuf *out)
/*
* Relative time update (eg "2 days ago"). If we haven't set the time
* yet, we need to set it from current time.
+ *
+ * The tm->tm_mday field has an additional logic of using negative values
+ * for date adjustments: -2 means yesterday and -3 the day before that,
+ * and so on. The idea is to deref such adjustments until we are sure
+ * there's no explicit mday specification in the approxidate string.
*/
static time_t update_tm(struct tm *tm, struct tm *now, time_t sec)
{
time_t n;
- if (tm->tm_mday < 0)
+ if (tm->tm_mday < 0) {
+ int offset = tm->tm_mday + 1;
+ if (sec == 0 && offset < 0)
+ sec = -offset * 24*60*60;
tm->tm_mday = now->tm_mday;
+ }
if (tm->tm_mon < 0)
tm->tm_mon = now->tm_mon;
if (tm->tm_year < 0) {
@@ -1121,44 +1130,46 @@ static void pending_number(struct tm *tm, int *num)
static void date_now(struct tm *tm, struct tm *now, int *num)
{
*num = 0;
+ tm->tm_mday = -1;
update_tm(tm, now, 0);
}
static void date_yesterday(struct tm *tm, struct tm *now, int *num)
{
*num = 0;
+ tm->tm_mday = -1;
update_tm(tm, now, 24*60*60);
}
-static void date_time(struct tm *tm, struct tm *now, int hour)
+static void date_time(struct tm *tm, int hour)
{
/*
* If we do not yet have a specified day, we'll use the most recent
* version of "hour" relative to now. But that may be yesterday.
*/
if (tm->tm_mday < 0 && tm->tm_hour < hour)
- update_tm(tm, now, 24*60*60);
+ tm->tm_mday = -2; /* eventually handled by update_tm() */
tm->tm_hour = hour;
tm->tm_min = 0;
tm->tm_sec = 0;
}
-static void date_midnight(struct tm *tm, struct tm *now, int *num)
+static void date_midnight(struct tm *tm, struct tm *now UNUSED, int *num)
{
pending_number(tm, num);
- date_time(tm, now, 0);
+ date_time(tm, 0);
}
-static void date_noon(struct tm *tm, struct tm *now, int *num)
+static void date_noon(struct tm *tm, struct tm *now UNUSED, int *num)
{
pending_number(tm, num);
- date_time(tm, now, 12);
+ date_time(tm, 12);
}
-static void date_tea(struct tm *tm, struct tm *now, int *num)
+static void date_tea(struct tm *tm, struct tm *now UNUSED, int *num)
{
pending_number(tm, num);
- date_time(tm, now, 17);
+ date_time(tm, 17);
}
static void date_pm(struct tm *tm, struct tm *now UNUSED, int *num)
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index d800cb30fe..432b92f841 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -209,9 +209,13 @@ check_approxidate '3:00' '2009-08-30 03:00:00'
check_approxidate '15:00' '2009-08-30 15:00:00'
check_approxidate 'noon today' '2009-08-30 12:00:00'
check_approxidate 'today at noon' '2009-08-30 12:00:00' '-12 hours'
+check_approxidate 'noon today' '2009-09-01 12:00:00' '+36 hours'
check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
check_approxidate 'last Friday at noon' '2009-08-28 12:00:00'
check_approxidate 'last Friday at noon' '2009-08-28 12:00:00' '-12 hours'
+check_approxidate 'noon yesterday' '2009-08-29 12:00:00' '-12 hours'
+check_approxidate 'tea last saturday' '2009-08-29 17:00:00'
+check_approxidate 'tea last saturday' '2009-08-29 17:00:00' '-12 hours'
check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00'
check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00' '-12 hours'
check_approxidate '10am noon' '2009-08-29 12:00:00'
--
2.30.2
^ permalink raw reply related
* [PATCH v3 3/4] approxidate: make "specials" respect fixed day-of-month
From: Tuomas Ahola @ 2026-05-14 11:55 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Tuomas Ahola
In-Reply-To: <20260514115520.6660-1-taahol@utu.fi>
The special approxidate time formats, "noon" and "tea" differ from
"12pm" and "5pm" by having the feature of wrapping to the previous day
if the current time is before those hours:
now -> 2026-05-13 11:00:00 +0000
12pm -> 2026-05-13 12:00:00 +0000
5pm -> 2026-05-13 17:00:00 +0000
noon -> 2026-05-12 12:00:00 +0000
tea -> 2026-05-12 17:00:00 +0000
However, that logic carries too far. Even when the date is specified,
the behavior of the "specials" depends on the current time. Assuming
the same time as above, we get:
today at noon -> 2026-05-12 12:00:00 +0000 (should be 13 May)
13 May at tea -> 2026-05-12 17:00:00 +0000
or, using an example mentioned in date-formats.adoc:
last Friday at noon -> 2026-05-07 12:00:00 +0000 (should be 8 May)
The quirk seems to be rather old. Already in 2006, Linus Torvalds
remarked that the date yielded by "one year ago yesterday at tea-time"
was "just silly and not even correct". Indeed, even today it gives:
One year ago yesterday at tea-time -> 2025-05-11 17:00:00 +0000
(should be 12 May)
Let's fix all of those with a simple patch. Check whether we already
have a specified day-of-month in `tm->tm_mday` and make `date_time()`
stick to it. Ensure the correct behavior with relevant tests.
Links:
1. https://lore.kernel.org/git/Pine.LNX.4.64.0610101102560.3952@g5.osdl.org/
Signed-off-by: Tuomas Ahola <taahol@utu.fi>
---
Notes:
> Again, this may be just me, but I happen to find the version of
> comment in Peff's review on the earlier iteration of this series
> much easier to understand.
>
Thanks, applied.
date.c | 6 +++++-
t/t0006-date.sh | 4 ++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/date.c b/date.c
index 412aca6dc4..73879d202c 100644
--- a/date.c
+++ b/date.c
@@ -1132,7 +1132,11 @@ static void date_yesterday(struct tm *tm, struct tm *now, int *num)
static void date_time(struct tm *tm, struct tm *now, int hour)
{
- if (tm->tm_hour < hour)
+ /*
+ * If we do not yet have a specified day, we'll use the most recent
+ * version of "hour" relative to now. But that may be yesterday.
+ */
+ if (tm->tm_mday < 0 && tm->tm_hour < hour)
update_tm(tm, now, 24*60*60);
tm->tm_hour = hour;
tm->tm_min = 0;
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index c7667bade2..d800cb30fe 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -208,8 +208,12 @@ check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
check_approxidate '3:00' '2009-08-30 03:00:00'
check_approxidate '15:00' '2009-08-30 15:00:00'
check_approxidate 'noon today' '2009-08-30 12:00:00'
+check_approxidate 'today at noon' '2009-08-30 12:00:00' '-12 hours'
check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
+check_approxidate 'last Friday at noon' '2009-08-28 12:00:00'
+check_approxidate 'last Friday at noon' '2009-08-28 12:00:00' '-12 hours'
check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00'
+check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00' '-12 hours'
check_approxidate '10am noon' '2009-08-29 12:00:00'
check_approxidate 'January 5th yesterday' '2009-01-29 19:20:00'
check_approxidate 'January 5th yesterday' '2008-12-31 19:20:00' '+2 days'
--
2.30.2
^ permalink raw reply related
* [PATCH v3 0/4] approxidate: tweak special date formats
From: Tuomas Ahola @ 2026-05-14 11:55 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Tuomas Ahola
In-Reply-To: <20260512145430.13212-1-taahol@utu.fi>
The approxidate system is an endless source of absurdities. Let's make the
usual "eh, that's crazy, let's do better with this input" type of fix[1], and
tweak some sharp edge cases, including one noticed by Linus back in 2006[2].
After this series, "tea" and "noon" will work predictably with all kinds of
date formats (today, yesterday, last Friday, January 5th, one year ago
yesterday...) regardless of the current time of day.
Links:
1. https://lore.kernel.org/git/20181115144854.GB16450@sigill.intra.peff.net/
2. https://lore.kernel.org/git/Pine.LNX.4.64.0610101102560.3952@g5.osdl.org/
Tuomas Ahola (4):
t0006: add support for approxidate test date adjustment
approxidate: alias "today" to "now"
approxidate: make "specials" respect fixed day-of-month
approxidate: use deferred mday adjustments for "specials"
date.c | 36 ++++++++++++++++++++++++++----------
t/t0006-date.sh | 41 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 66 insertions(+), 11 deletions(-)
Intervall-diff mot v2:
1: 118f1825ac < -: ---------- t0006: add support for approxidate test date adjustment
2: 21c4858c47 < -: ---------- approxidate: make "specials" respect fixed day-of-month
-: ---------- > 1: 7ea9c9967b t0006: add support for approxidate test date adjustment
-: ---------- > 2: 3a21727dbe approxidate: alias "today" to "now"
-: ---------- > 3: d1992d23d0 approxidate: make "specials" respect fixed day-of-month
3: cf72403102 ! 4: 0b1a10305c approxidate: use deferred mday adjustments for "specials"
@@ date.c: static void pending_number(struct tm *tm, int *num)
+static void date_time(struct tm *tm, int hour)
{
/*
- * By default, "tea" and "noon" refer to last such time in the
- * past, be it today or yesterday. With a specified mday,
-- * that logic is overridden.
-+ * or e.g. "noon today", that logic is overridden.
+ * If we do not yet have a specified day, we'll use the most recent
+ * version of "hour" relative to now. But that may be yesterday.
*/
if (tm->tm_mday < 0 && tm->tm_hour < hour)
- update_tm(tm, now, 24*60*60);
@@ t/t0006-date.sh
@@ t/t0006-date.sh: check_approxidate '3:00' '2009-08-30 03:00:00'
check_approxidate '15:00' '2009-08-30 15:00:00'
check_approxidate 'noon today' '2009-08-30 12:00:00'
- check_approxidate 'today at noon' '2009-08-30 12:00:00' success -12
-+check_approxidate 'noon today' '2009-09-01 12:00:00' success +36
+ check_approxidate 'today at noon' '2009-08-30 12:00:00' '-12 hours'
++check_approxidate 'noon today' '2009-09-01 12:00:00' '+36 hours'
check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
check_approxidate 'last Friday at noon' '2009-08-28 12:00:00'
- check_approxidate 'last Friday at noon' '2009-08-28 12:00:00' success -12
-+check_approxidate 'noon yesterday' '2009-08-29 12:00:00' success -12
+ check_approxidate 'last Friday at noon' '2009-08-28 12:00:00' '-12 hours'
++check_approxidate 'noon yesterday' '2009-08-29 12:00:00' '-12 hours'
+check_approxidate 'tea last saturday' '2009-08-29 17:00:00'
-+check_approxidate 'tea last saturday' '2009-08-29 17:00:00' success -12
++check_approxidate 'tea last saturday' '2009-08-29 17:00:00' '-12 hours'
check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00'
- check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00' success -12
+ check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00' '-12 hours'
check_approxidate '10am noon' '2009-08-29 12:00:00'
base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0
--
2.30.2
^ permalink raw reply
* [PATCH v3 1/4] t0006: add support for approxidate test date adjustment
From: Tuomas Ahola @ 2026-05-14 11:55 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Tuomas Ahola
In-Reply-To: <20260514115520.6660-1-taahol@utu.fi>
t0006 uses a hard-coded test date and provides no convenient
way to override it temporarily. Add an optional parameter to
check_approxidate to adjust the time as needed, and demonstrate
the feature with a new test.
Signed-off-by: Tuomas Ahola <taahol@utu.fi>
---
Notes:
> As you are not doing the test-date-now adjustment when $4 is not
> given, wouldn't it be a lot easier to read if you did something like
>
> old_date=$GIT_TEST_DATE_NOW
> if test -n "$4"
> then
> # the convention for $4 is a bit weird in that it
> # comes with its own +/- operator in front.
> GIT_TEST_DATE_NOW=$(( $old_date $4 * 60 * 60 ))
> caption="$1; offset $4h"
> else
> caption=$1
> fi
>
> instead? Other two minor points are
>
> - Documentation/SubmittingPatches prefers an explicit "test" over
> "[ ... ]", and have "then", "else", etc. on their own lines.
>
> - As you never "unset" GIT_TEST_DATE_NOW, you do not have to keep
> exporting it. It is not like there are two variables (one for
> shell, the other for environment) and every time you set the
> shell one you need to export to reflect the value to the
> environment one. Rather, a single "export" marks a shell
> variable and every time it changes value, it is updated in the
> environment as well.
>
Thanks, applied.
> One, it sucks to have to say "success" here, but is awkward because now
> we have two optional arguments. There's nobody passing "failure" right
> now, so we could just drop support, though that might be annoying later
> when somebody wants to add a failing test. But we could perhaps switch
> to allowing:
>
Ok, now it works without "success" in between, and "failure" works without
an offset, too.
check_approxidate <test-string> <expected-result> [<test-time-offset>] [failure]
> The second thing is that "+48" is pretty opaque. It's a relative offset
> to some arbitrary point. To some degree the script already suffers from
> that (all of the tests are using some arbitrary point), but I think the
> offset (without units!) adds a layer of indirection that makes it even
> more confusing.
>
Good catch. Now it is at least marginally better with the added units.
t/t0006-date.sh | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 53ced36df4..c7667bade2 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -155,12 +155,41 @@ check_parse '2100-00-00 00:00:00 -11' bad
check_parse '2100-00-00 00:00:00 +11' bad
REQUIRE_64BIT_TIME=
+add_time_offset() {
+ case "$3" in
+ hours)
+ unit=$(( 60*60 ))
+ ;;
+ days)
+ unit=$(( 24*60*60 ))
+ ;;
+ esac
+ offset=$(( $2 * unit ))
+ echo $(( $1 + offset ))
+}
+
check_approxidate() {
+ old_date=$GIT_TEST_DATE_NOW
+ if test "$3" = "failure"
+ then
+ expection="$3"
+ else
+ expection=${4:-success}
+ offset="$3"
+ fi
+ if test -n "$offset"
+ then
+ GIT_TEST_DATE_NOW=$(add_time_offset $old_date $offset)
+ caption="$1; offset $offset"
+ else
+ caption=$1
+ fi
echo "$1 -> $2 +0000" >expect
- test_expect_${3:-success} "parse approxidate ($1)" "
+ test_expect_$expection "parse approxidate ($caption)" "
test-tool date approxidate '$1' >actual &&
test_cmp expect actual
"
+ GIT_TEST_DATE_NOW=$old_date
}
check_approxidate now '2009-08-30 19:20:00'
@@ -182,6 +211,8 @@ check_approxidate 'noon today' '2009-08-30 12:00:00'
check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00'
check_approxidate '10am noon' '2009-08-29 12:00:00'
+check_approxidate 'January 5th yesterday' '2009-01-29 19:20:00'
+check_approxidate 'January 5th yesterday' '2008-12-31 19:20:00' '+2 days'
check_approxidate 'last tuesday' '2009-08-25 19:20:00'
check_approxidate 'July 5th' '2009-07-05 19:20:00'
--
2.30.2
^ permalink raw reply related
* [PATCH v3 2/4] approxidate: alias "today" to "now"
From: Tuomas Ahola @ 2026-05-14 11:55 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Tuomas Ahola
In-Reply-To: <20260514115520.6660-1-taahol@utu.fi>
As far as approxidate in concerned, "today" is a no-op. That makes
it functionally equivalent to "now" in commands like
$ git log --since=today
Make that behavior explicit by binding "today" to `date_now()`.
That way later patches can give "today" some functionality in
cases like "today at noon".
Signed-off-by: Tuomas Ahola <taahol@utu.fi>
---
Notes:
> Hmph, this may not work very well for "git log --since=today", which
> you may want to stop immediately when the traversal reaches a patch
> written before the most recent midnight, instead of stopping without
> giving anything back.
>
Sorry, I don't know if I understood. Does the patch change the behavior of
that command somehow? Is there some kind of edge case I missed?
That said, if we do want to change it so that "git log --since=today"
worked like "--since=midnight", this seems to do the trick:
```
static void date_today(struct tm *tm, struct tm *now, int *num)
{
if (tm->tm_hour == now->tm_hour &&
tm->tm_min == now->tm_min &&
tm->tm_sec == now->tm_sec)
date_midnight(tm, now, num);
date_now(tm, now, num);
}
```
date.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/date.c b/date.c
index 17a95077cf..412aca6dc4 100644
--- a/date.c
+++ b/date.c
@@ -1204,6 +1204,7 @@ static const struct special {
{ "AM", date_am },
{ "never", date_never },
{ "now", date_now },
+ { "today", date_now },
{ NULL }
};
--
2.30.2
^ permalink raw reply related
* Re: [PATCH 1/4] strbuf: add strbuf_add_uint()
From: Junio C Hamano @ 2026-05-14 11:53 UTC (permalink / raw)
To: René Scharfe; +Cc: Jeff King, git
In-Reply-To: <f51cdd89-dab1-44f3-8f63-7d34f6fbbba5@web.de>
René Scharfe <l.s.r@web.de> writes:
>> Right now we use unsigned_add_overflows(), and then do the actual add.
>> Using __builtin_add_overflow() might be faster.
> Curious. Clang and GCC emit the same instructions for our
> unsigned_add_overflows() vs. __builtin_add_overflow() on x64, but clang
> on ARM64 fails to elide the comparison: https://godbolt.org/z/91d35KofM
>
> Which explains why this patch:
>
> --- 8< ---
> diff --git a/strbuf.c b/strbuf.c
> index 3e04addc22..4c2bd1e66f 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -106,12 +106,13 @@ void strbuf_attach(struct strbuf *sb, void *buf, size_t len, size_t alloc)
> void strbuf_grow(struct strbuf *sb, size_t extra)
> {
> int new_buf = !sb->alloc;
> - if (unsigned_add_overflows(extra, 1) ||
> - unsigned_add_overflows(sb->len, extra + 1))
> + size_t len;
> + if (__builtin_add_overflow(extra, 1, &len) ||
> + __builtin_add_overflow(sb->len, len, &len))
> die("you want to use way too much memory");
> if (new_buf)
> sb->buf = NULL;
> - ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
> + ALLOC_GROW(sb->buf, len, sb->alloc);
> if (new_buf)
> sb->buf[0] = '\0';
> }
> --- >8 ---
>
> ... gives a speedup on my Apple M1 with Apple's clang:
>
> Benchmark 1: ./git_main cat-file --batch-all-objects --batch-check='%(objectname)'
> Time (mean ± σ): 119.6 ms ± 0.2 ms [User: 112.9 ms, System: 5.6 ms]
> Range (min … max): 119.3 ms … 120.1 ms 24 runs
>
> Benchmark 2: ./git cat-file --batch-all-objects --batch-check='%(objectname)'
> Time (mean ± σ): 117.3 ms ± 0.2 ms [User: 110.4 ms, System: 5.8 ms]
> Range (min … max): 117.1 ms … 117.6 ms 24 runs
>
> Summary
> ./git cat-file --batch-all-objects --batch-check='%(objectname)' ran
> 1.02 ± 0.00 times faster than ./git_main cat-file --batch-all-objects --batch-check='%(objectname)'
>
> ... but has no effect with GCC 15.2.
Interesting. A few percent is indeed impressive in such a micro
optimization, and it is satisfying to see that the compilers can see
what our unsigned_add_overflows() plus the actual add does and makes
your above patch a no-op.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox