negotiator/skipping: skip commits during fetch
Subsystems (2)
PATCH 1/1 negotiator/skipping: skip commits during fetch
Introduce a new negotiation algorithm used during fetch that skips commits in an effort to find common ancestors faster. The skips grow similarly to the Fibonacci sequence as the commit walk proceeds further away from the tips. The skips may cause unnecessary commits to be included in the packfile, but the negotiation step typically ends more quickly.
Usage of this algorithm is guarded behind the configuration flag fetch.negotiationAlgorithm.
---
This is on jt/fetch-pack-negotiator, but also applies cleanly on jt/fetch-nego-tip.
A previous version of this had rough backtracking after a "match" is found, but that backtracking implementation was complicated and inexact, so I've removed it.
At $DAY_JOB we have been using this patch with some success, so I'm sending it to this mailing list.
--- Documentation/config.txt | 9 + Makefile | 1 + fetch-negotiator.c | 8 +- fetch-negotiator.h | 3 +- fetch-pack.c | 7 +- negotiator/skipping.c | 250 +++++++++++++++++++++++++++ negotiator/skipping.h | 8 + t/t5552-skipping-fetch-negotiator.sh | 179 +++++++++++++++++++ 8 files changed, 461 insertions(+), 4 deletions(-) create mode 100644 negotiator/skipping.c create mode 100644 negotiator/skipping.h create mode 100755 t/t5552-skipping-fetch-negotiator.sh
Documentation/config.txt +9 -0
@@ -1491,6 +1491,15 @@ fetch.output:: `full` and `compact`. Default value is `full`. See section OUTPUT in linkgit:git-fetch[1] for detail. +fetch.negotiationAlgorithm:: + Control how information about the commits in the local repository is + sent when negotiating the contents of the packfile to be sent by the + server. Set to "skipping" to use an algorithm that skips commits in an + effort to converge faster, but may result in a larger-than-necessary + packfile; any other value instructs Git to use the default algorithm + that never skips commits (unless the server has acknowledged it or one + of its descendants). + format.attach:: Enable multipart/mixed attachments as the default for 'format-patch'. The value can also be a double quoted string
Makefile +1 -0
@@ -893,6 +893,7 @@ LIB_OBJS += merge-recursive.o LIB_OBJS += mergesort.o LIB_OBJS += name-hash.o LIB_OBJS += negotiator/default.o +LIB_OBJS += negotiator/skipping.o LIB_OBJS += notes.o LIB_OBJS += notes-cache.o LIB_OBJS += notes-merge.o
fetch-negotiator.c +7 -1
@@ -1,8 +1,14 @@#include "git-compat-util.h" #include "fetch-negotiator.h" #include "negotiator/default.h" +#include "negotiator/skipping.h" -void fetch_negotiator_init (struct fetch_negotiator *negotiator) +void fetch_negotiator_init (struct fetch_negotiator *negotiator, +const char *algorithm) { +if (algorithm && !strcmp (algorithm,"skipping" )) { +skipping_negotiator_init (negotiator); +return ; + }default_negotiator_init (negotiator); }
fetch-negotiator.h +2 -1
@@ -52,6 +52,7 @@ struct fetch_negotiator { void *data; }; -void fetch_negotiator_init (struct fetch_negotiator *negotiator); +void fetch_negotiator_init (struct fetch_negotiator *negotiator, +const char *algorithm);#endif
fetch-pack.c +5 -2
@@ -33,6 +33,7 @@ static int agent_supported;static int server_supports_filtering;static struct lock_file shallow_lock;static const char *alternate_shallow_file; +static char *negotiation_algorithm;/* Remember to update object flag allocation in object.h */ #define COMPLETE (1U << 0)
@@ -905,7 +906,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,const char *agent_feature;int agent_len;struct fetch_negotiator negotiator; -fetch_negotiator_init (&negotiator); +fetch_negotiator_init (&negotiator, negotiation_algorithm);sort_ref_list (&ref, ref_compare_name);QSORT (sought, nr_sought, cmp_ref_by_name);
@@ -1287,7 +1288,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,int in_vain =0 ;int haves_to_send =INITIAL_FLUSH ;struct fetch_negotiator negotiator; -fetch_negotiator_init (&negotiator); +fetch_negotiator_init (&negotiator, negotiation_algorithm);packet_reader_init (&reader, fd[0 ],NULL ,0 ,PACKET_READ_CHOMP_NEWLINE );
@@ -1366,6 +1367,8 @@ static void fetch_pack_config(void)git_config_get_bool ("repack.usedeltabaseoffset" , &prefer_ofs_delta);git_config_get_bool ("fetch.fsckobjects" , &fetch_fsck_objects);git_config_get_bool ("transfer.fsckobjects" , &transfer_fsck_objects); +git_config_get_string ("fetch.negotiationalgorithm" , + &negotiation_algorithm);git_config (git_default_config,NULL ); }
negotiator/skipping.c new file +250 -0
250 lines not on this page · raw message
negotiator/skipping.h new file +8 -0
@@ -0,0 +1,8 @@ +#ifndef NEGOTIATOR_SKIPPING_H +#define NEGOTIATOR_SKIPPING_H + +struct fetch_negotiator ; + +void skipping_negotiator_init (struct fetch_negotiator *negotiator); + +#endif
t/t5552-skipping-fetch-negotiator.sh new file +179 -0
179 lines not on this page · raw message
Signature · 1 line
2.18.0.203.gfac676dfb9-goog
Quoted 7 lines · Jonathan Tan
Introduce a new negotiation algorithm used during fetch that skips commits in an effort to find common ancestors faster. The skips grow similarly to the Fibonacci sequence as the commit walk proceeds further away from the tips. The skips may cause unnecessary commits to be included in the packfile, but the negotiation step typically ends more quickly.
Quoted 7 lines
Usage of this algorithm is guarded behind the configuration flag fetch.negotiationAlgorithm.
Signed-off-by Jonathan Tan <[email protected]>---
This is on jt/fetch-pack-negotiator, but also applies cleanly on jt/fetch-nego-tip.
Sounds sensible.
Unfortunately, this one is among many others that get hurt by needless semantic conflicts caused by reusing the old function name and changing the function signature to pass the_repository thru some codepaths, without adding transition macros. I am running out of time today as I need some post-office-move clean-ups before getting organized enough, so I expect I won't be able to clean it up and push it out on 'pu' by the end of day today.
Thanks.
Hi Jonathan,
Quoted 1 line · Jonathan Tan
t/t5552-skipping-fetch-negotiator.sh | 179 +++++++++++++++++++
This test seems to be failing consistently in the recent pu builds:
https://git-for-windows.visualstudio.com/git/_build/results?buildId=14337&view=logs
Could you have a look, please?
Ciao,
Dscho
P.S.: For your convenience, I will paste the last part of the output with -i -v -x here:
-- snipsnap --
2018-07-26T08:18:39.7864833Z expecting success: 2018-07-26T08:18:39.7868553Z rm -rf server client trace && 2018-07-26T08:18:39.7869403Z git init server && 2018-07-26T08:18:39.7869606Z test_commit -C server to_fetch && 2018-07-26T08:18:39.7870066Z 2018-07-26T08:18:39.7870281Z git init client && 2018-07-26T08:18:39.7870403Z 2018-07-26T08:18:39.7870579Z # 2 regular commits 2018-07-26T08:18:39.7870779Z test_tick=2000000000 && 2018-07-26T08:18:39.7870943Z test_commit -C client c1 && 2018-07-26T08:18:39.7871103Z test_commit -C client c2 && 2018-07-26T08:18:39.7871228Z 2018-07-26T08:18:39.7871419Z # 4 old commits 2018-07-26T08:18:39.7871575Z test_tick=1000000000 && 2018-07-26T08:18:39.7871734Z git -C client checkout c1 && 2018-07-26T08:18:39.7871916Z test_commit -C client old1 && 2018-07-26T08:18:39.7872081Z test_commit -C client old2 && 2018-07-26T08:18:39.7872396Z test_commit -C client old3 && 2018-07-26T08:18:39.7872598Z test_commit -C client old4 && 2018-07-26T08:18:39.7872743Z 2018-07-26T08:18:39.7872918Z # "c2" and "c1" are popped first, then "old4" to "old1". "old1" would 2018-07-26T08:18:39.7873114Z # normally be skipped, but is treated as a commit without a parent here 2018-07-26T08:18:39.7873329Z # and sent, because (due to clock skew) its only parent has already been 2018-07-26T08:18:39.7873524Z # popped off the priority queue. 2018-07-26T08:18:39.7873700Z test_config -C client fetch.negotiationalgorithm skipping && 2018-07-26T08:18:39.7873908Z GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch "$(pwd)/server" && 2018-07-26T08:18:39.7874091Z have_sent c2 c1 old4 old2 old1 && 2018-07-26T08:18:39.7874262Z have_not_sent old3 2018-07-26T08:18:39.7874383Z 2018-07-26T08:18:39.8353323Z ++ rm -rf server client trace 2018-07-26T08:18:40.3404166Z ++ git init server 2018-07-26T08:18:40.3756394Z Initialized empty Git repository in D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/server/.git/ 2018-07-26T08:18:40.3769512Z ++ test_commit -C server to_fetch 2018-07-26T08:18:40.3776271Z ++ notick= 2018-07-26T08:18:40.3777103Z ++ signoff= 2018-07-26T08:18:40.3777282Z ++ indir= 2018-07-26T08:18:40.3777465Z ++ test 3 '!=' 0 2018-07-26T08:18:40.3777648Z ++ case "$1" in 2018-07-26T08:18:40.3777801Z ++ indir=server 2018-07-26T08:18:40.3777948Z ++ shift 2018-07-26T08:18:40.3778093Z ++ shift 2018-07-26T08:18:40.3778493Z ++ test 1 '!=' 0 2018-07-26T08:18:40.3778921Z ++ case "$1" in 2018-07-26T08:18:40.3779072Z ++ break 2018-07-26T08:18:40.3779241Z ++ indir=server/ 2018-07-26T08:18:40.3779431Z ++ file=to_fetch.t 2018-07-26T08:18:40.3779603Z ++ echo to_fetch 2018-07-26T08:18:40.3779923Z ++ git -C server/ add to_fetch.t 2018-07-26T08:18:40.4072248Z ++ test -z '' 2018-07-26T08:18:40.4072727Z ++ test_tick 2018-07-26T08:18:40.4072948Z ++ test -z set 2018-07-26T08:18:40.4073113Z ++ test_tick=1112913673 2018-07-26T08:18:40.4073758Z ++ GIT_COMMITTER_DATE='1112913673 -0700' 2018-07-26T08:18:40.4074001Z ++ GIT_AUTHOR_DATE='1112913673 -0700' 2018-07-26T08:18:40.4074178Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE 2018-07-26T08:18:40.4074357Z ++ git -C server/ commit -m to_fetch 2018-07-26T08:18:40.4485364Z [master (root-commit) ff85695] to_fetch 2018-07-26T08:18:40.4485997Z Author: A U Thor <[email protected]> 2018-07-26T08:18:40.4486201Z 1 file changed, 1 insertion(+) 2018-07-26T08:18:40.4486414Z create mode 100644 to_fetch.t 2018-07-26T08:18:40.4499970Z ++ git -C server/ tag to_fetch 2018-07-26T08:18:40.4809208Z ++ git init client 2018-07-26T08:18:40.5139949Z Initialized empty Git repository in D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/client/.git/ 2018-07-26T08:18:40.5158270Z ++ test_tick=2000000000 2018-07-26T08:18:40.5158466Z ++ test_commit -C client c1 2018-07-26T08:18:40.5159077Z ++ notick= 2018-07-26T08:18:40.5159492Z ++ signoff= 2018-07-26T08:18:40.5159697Z ++ indir= 2018-07-26T08:18:40.5159855Z ++ test 3 '!=' 0 2018-07-26T08:18:40.5160010Z ++ case "$1" in 2018-07-26T08:18:40.5160209Z ++ indir=client 2018-07-26T08:18:40.5160362Z ++ shift 2018-07-26T08:18:40.5160507Z ++ shift 2018-07-26T08:18:40.5160657Z ++ test 1 '!=' 0 2018-07-26T08:18:40.5160831Z ++ case "$1" in 2018-07-26T08:18:40.5161289Z ++ break 2018-07-26T08:18:40.5161582Z ++ indir=client/ 2018-07-26T08:18:40.5161764Z ++ file=c1.t 2018-07-26T08:18:40.5161916Z ++ echo c1 2018-07-26T08:18:40.5162231Z ++ git -C client/ add c1.t 2018-07-26T08:18:40.5456318Z ++ test -z '' 2018-07-26T08:18:40.5460548Z ++ test_tick 2018-07-26T08:18:40.5461417Z ++ test -z set 2018-07-26T08:18:40.5463657Z ++ test_tick=2000000060 2018-07-26T08:18:40.5464369Z ++ GIT_COMMITTER_DATE='2000000060 -0700' 2018-07-26T08:18:40.5464617Z ++ GIT_AUTHOR_DATE='2000000060 -0700' 2018-07-26T08:18:40.5464805Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE 2018-07-26T08:18:40.5464988Z ++ git -C client/ commit -m c1 2018-07-26T08:18:40.5857440Z [master (root-commit) dc824fa] c1 2018-07-26T08:18:40.5858031Z Author: A U Thor <[email protected]> 2018-07-26T08:18:40.5858251Z 1 file changed, 1 insertion(+) 2018-07-26T08:18:40.5858451Z create mode 100644 c1.t 2018-07-26T08:18:40.5872839Z ++ git -C client/ tag c1 2018-07-26T08:18:40.6174770Z ++ test_commit -C client c2 2018-07-26T08:18:40.6175120Z ++ notick= 2018-07-26T08:18:40.6175398Z ++ signoff= 2018-07-26T08:18:40.6175583Z ++ indir= 2018-07-26T08:18:40.6175737Z ++ test 3 '!=' 0 2018-07-26T08:18:40.6175925Z ++ case "$1" in 2018-07-26T08:18:40.6176079Z ++ indir=client 2018-07-26T08:18:40.6176246Z ++ shift 2018-07-26T08:18:40.6176415Z ++ shift 2018-07-26T08:18:40.6176569Z ++ test 1 '!=' 0 2018-07-26T08:18:40.6176738Z ++ case "$1" in 2018-07-26T08:18:40.6176905Z ++ break 2018-07-26T08:18:40.6177052Z ++ indir=client/ 2018-07-26T08:18:40.6177200Z ++ file=c2.t 2018-07-26T08:18:40.6177369Z ++ echo c2 2018-07-26T08:18:40.6177525Z ++ git -C client/ add c2.t 2018-07-26T08:18:40.6474943Z ++ test -z '' 2018-07-26T08:18:40.6479175Z ++ test_tick 2018-07-26T08:18:40.6479861Z ++ test -z set 2018-07-26T08:18:40.6482344Z ++ test_tick=2000000120 2018-07-26T08:18:40.6483064Z ++ GIT_COMMITTER_DATE='2000000120 -0700' 2018-07-26T08:18:40.6483243Z ++ GIT_AUTHOR_DATE='2000000120 -0700' 2018-07-26T08:18:40.6483412Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE 2018-07-26T08:18:40.6483597Z ++ git -C client/ commit -m c2 2018-07-26T08:18:40.6883597Z [master 9ab4692] c2 2018-07-26T08:18:40.6884552Z Author: A U Thor <[email protected]> 2018-07-26T08:18:40.6884902Z 1 file changed, 1 insertion(+) 2018-07-26T08:18:40.6885078Z create mode 100644 c2.t 2018-07-26T08:18:40.6898418Z ++ git -C client/ tag c2 2018-07-26T08:18:40.7214970Z ++ test_tick=1000000000 2018-07-26T08:18:40.7215737Z ++ git -C client checkout c1 2018-07-26T08:18:40.7537971Z Note: checking out 'c1'. 2018-07-26T08:18:40.7538294Z 2018-07-26T08:18:40.7538485Z You are in 'detached HEAD' state. You can look around, make experimental 2018-07-26T08:18:40.7538901Z changes and commit them, and you can discard any commits you make in this 2018-07-26T08:18:40.7539153Z state without impacting any branches by performing another checkout. 2018-07-26T08:18:40.7539288Z 2018-07-26T08:18:40.7539455Z If you want to create a new branch to retain commits you create, you may 2018-07-26T08:18:40.7539646Z do so (now or later) by using -b with the checkout command again. Example: 2018-07-26T08:18:40.7539799Z 2018-07-26T08:18:40.7539979Z git checkout -b <new-branch-name> 2018-07-26T08:18:40.7540099Z 2018-07-26T08:18:40.7540264Z HEAD is now at dc824fa c1 2018-07-26T08:18:40.7552832Z ++ test_commit -C client old1 2018-07-26T08:18:40.7559118Z ++ notick= 2018-07-26T08:18:40.7559789Z ++ signoff= 2018-07-26T08:18:40.7559966Z ++ indir= 2018-07-26T08:18:40.7560066Z ++ test 3 '!=' 0 2018-07-26T08:18:40.7565193Z ++ case "$1" in 2018-07-26T08:18:40.7565286Z ++ indir=client 2018-07-26T08:18:40.7565373Z ++ shift 2018-07-26T08:18:40.7565456Z ++ shift 2018-07-26T08:18:40.7566662Z ++ test 1 '!=' 0 2018-07-26T08:18:40.7566796Z ++ case "$1" in 2018-07-26T08:18:40.7566879Z ++ break 2018-07-26T08:18:40.7566961Z ++ indir=client/ 2018-07-26T08:18:40.7567066Z ++ file=old1.t 2018-07-26T08:18:40.7567150Z ++ echo old1 2018-07-26T08:18:40.7567238Z ++ git -C client/ add old1.t 2018-07-26T08:18:40.7962371Z ++ test -z '' 2018-07-26T08:18:40.7962668Z ++ test_tick 2018-07-26T08:18:40.7963247Z ++ test -z set 2018-07-26T08:18:40.7963453Z ++ test_tick=1000000060 2018-07-26T08:18:40.7963649Z ++ GIT_COMMITTER_DATE='1000000060 -0700' 2018-07-26T08:18:40.7963832Z ++ GIT_AUTHOR_DATE='1000000060 -0700' 2018-07-26T08:18:40.7964000Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE 2018-07-26T08:18:40.7964164Z ++ git -C client/ commit -m old1 2018-07-26T08:18:40.8356576Z [detached HEAD e9a2c09] old1 2018-07-26T08:18:40.8357378Z Author: A U Thor <[email protected]> 2018-07-26T08:18:40.8357554Z 1 file changed, 1 insertion(+) 2018-07-26T08:18:40.8357652Z create mode 100644 old1.t 2018-07-26T08:18:40.8372002Z ++ git -C client/ tag old1 2018-07-26T08:18:40.8671359Z ++ test_commit -C client old2 2018-07-26T08:18:40.8676749Z ++ notick= 2018-07-26T08:18:40.8677398Z ++ signoff= 2018-07-26T08:18:40.8679262Z ++ indir= 2018-07-26T08:18:40.8679957Z ++ test 3 '!=' 0 2018-07-26T08:18:40.8680150Z ++ case "$1" in 2018-07-26T08:18:40.8680305Z ++ indir=client 2018-07-26T08:18:40.8680447Z ++ shift 2018-07-26T08:18:40.8680587Z ++ shift 2018-07-26T08:18:40.8680798Z ++ test 1 '!=' 0 2018-07-26T08:18:40.8680949Z ++ case "$1" in 2018-07-26T08:18:40.8681143Z ++ break 2018-07-26T08:18:40.8681311Z ++ indir=client/ 2018-07-26T08:18:40.8681458Z ++ file=old2.t 2018-07-26T08:18:40.8681604Z ++ echo old2 2018-07-26T08:18:40.8681789Z ++ git -C client/ add old2.t 2018-07-26T08:18:40.8990053Z ++ test -z '' 2018-07-26T08:18:40.8990392Z ++ test_tick 2018-07-26T08:18:40.8990953Z ++ test -z set 2018-07-26T08:18:40.8991246Z ++ test_tick=1000000120 2018-07-26T08:18:40.8991421Z ++ GIT_COMMITTER_DATE='1000000120 -0700' 2018-07-26T08:18:40.8991585Z ++ GIT_AUTHOR_DATE='1000000120 -0700' 2018-07-26T08:18:40.8991771Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE 2018-07-26T08:18:40.8991936Z ++ git -C client/ commit -m old2 2018-07-26T08:18:40.9402744Z [detached HEAD 41bd8dc] old2 2018-07-26T08:18:40.9403286Z Author: A U Thor <[email protected]> 2018-07-26T08:18:40.9403510Z 1 file changed, 1 insertion(+) 2018-07-26T08:18:40.9403678Z create mode 100644 old2.t 2018-07-26T08:18:40.9418122Z ++ git -C client/ tag old2 2018-07-26T08:18:40.9736807Z ++ test_commit -C client old3 2018-07-26T08:18:40.9737058Z ++ notick= 2018-07-26T08:18:40.9737152Z ++ signoff= 2018-07-26T08:18:40.9737238Z ++ indir= 2018-07-26T08:18:40.9737327Z ++ test 3 '!=' 0 2018-07-26T08:18:40.9737600Z ++ case "$1" in 2018-07-26T08:18:40.9737690Z ++ indir=client 2018-07-26T08:18:40.9737776Z ++ shift 2018-07-26T08:18:40.9737860Z ++ shift 2018-07-26T08:18:40.9737964Z ++ test 1 '!=' 0 2018-07-26T08:18:40.9738054Z ++ case "$1" in 2018-07-26T08:18:40.9738140Z ++ break 2018-07-26T08:18:40.9738248Z ++ indir=client/ 2018-07-26T08:18:40.9738338Z ++ file=old3.t 2018-07-26T08:18:40.9738445Z ++ echo old3 2018-07-26T08:18:40.9738540Z ++ git -C client/ add old3.t 2018-07-26T08:18:41.0035565Z ++ test -z '' 2018-07-26T08:18:41.0036056Z ++ test_tick 2018-07-26T08:18:41.0036299Z ++ test -z set 2018-07-26T08:18:41.0036467Z ++ test_tick=1000000180 2018-07-26T08:18:41.0036638Z ++ GIT_COMMITTER_DATE='1000000180 -0700' 2018-07-26T08:18:41.0037189Z ++ GIT_AUTHOR_DATE='1000000180 -0700' 2018-07-26T08:18:41.0037403Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE 2018-07-26T08:18:41.0037574Z ++ git -C client/ commit -m old3 2018-07-26T08:18:41.0429238Z [detached HEAD 4ff0db5] old3 2018-07-26T08:18:41.0429619Z Author: A U Thor <[email protected]> 2018-07-26T08:18:41.0429799Z 1 file changed, 1 insertion(+) 2018-07-26T08:18:41.0429965Z create mode 100644 old3.t 2018-07-26T08:18:41.0443795Z ++ git -C client/ tag old3 2018-07-26T08:18:41.0752553Z ++ test_commit -C client old4 2018-07-26T08:18:41.0752824Z ++ notick= 2018-07-26T08:18:41.0752936Z ++ signoff= 2018-07-26T08:18:41.0753905Z ++ indir= 2018-07-26T08:18:41.0754193Z ++ test 3 '!=' 0 2018-07-26T08:18:41.0754374Z ++ case "$1" in 2018-07-26T08:18:41.0754531Z ++ indir=client 2018-07-26T08:18:41.0754682Z ++ shift 2018-07-26T08:18:41.0754828Z ++ shift 2018-07-26T08:18:41.0755007Z ++ test 1 '!=' 0 2018-07-26T08:18:41.0755162Z ++ case "$1" in 2018-07-26T08:18:41.0755340Z ++ break 2018-07-26T08:18:41.0755491Z ++ indir=client/ 2018-07-26T08:18:41.0755664Z ++ file=old4.t 2018-07-26T08:18:41.0755814Z ++ echo old4 2018-07-26T08:18:41.0755971Z ++ git -C client/ add old4.t 2018-07-26T08:18:41.1064316Z ++ test -z '' 2018-07-26T08:18:41.1064885Z ++ test_tick 2018-07-26T08:18:41.1065169Z ++ test -z set 2018-07-26T08:18:41.1065432Z ++ test_tick=1000000240 2018-07-26T08:18:41.1065637Z ++ GIT_COMMITTER_DATE='1000000240 -0700' 2018-07-26T08:18:41.1065820Z ++ GIT_AUTHOR_DATE='1000000240 -0700' 2018-07-26T08:18:41.1066008Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE 2018-07-26T08:18:41.1066212Z ++ git -C client/ commit -m old4 2018-07-26T08:18:41.1488203Z [detached HEAD caef059] old4 2018-07-26T08:18:41.1488670Z Author: A U Thor <[email protected]> 2018-07-26T08:18:41.1489153Z 1 file changed, 1 insertion(+) 2018-07-26T08:18:41.1489370Z create mode 100644 old4.t 2018-07-26T08:18:41.1502887Z ++ git -C client/ tag old4 2018-07-26T08:18:41.1824546Z ++ test_config -C client fetch.negotiationalgorithm skipping 2018-07-26T08:18:41.1825670Z ++ config_dir= 2018-07-26T08:18:41.1826072Z ++ test -C = -C 2018-07-26T08:18:41.1826299Z ++ shift 2018-07-26T08:18:41.1826528Z ++ config_dir=client 2018-07-26T08:18:41.1826809Z ++ shift 2018-07-26T08:18:41.1827079Z ++ test_when_finished 'test_unconfig -C '\''client'\'' '\''fetch.negotiationalgorithm'\''' 2018-07-26T08:18:41.1827289Z ++ test 0 = 0 2018-07-26T08:18:41.1827561Z ++ test_cleanup='{ test_unconfig -C '\''client'\'' '\''fetch.negotiationalgorithm'\'' 2018-07-26T08:18:41.1827751Z } && (exit "$eval_ret"); eval_ret=$?; :' 2018-07-26T08:18:41.1827930Z ++ git -C client config fetch.negotiationalgorithm skipping 2018-07-26T08:18:41.2196451Z +++ pwd 2018-07-26T08:18:41.2196831Z +++ builtin pwd -W 2018-07-26T08:18:41.2274040Z +++ pwd 2018-07-26T08:18:41.2274458Z +++ builtin pwd -W 2018-07-26T08:18:41.2285081Z ++ GIT_TRACE_PACKET='D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/trace' 2018-07-26T08:18:41.2285515Z ++ git -C client fetch 'D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/server' 2018-07-26T08:18:41.3054360Z warning: no common commits 2018-07-26T08:18:41.3264762Z From D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/server 2018-07-26T08:18:41.3265204Z * branch HEAD -> FETCH_HEAD 2018-07-26T08:18:41.3362819Z ++ have_sent c2 c1 old4 old2 old1 2018-07-26T08:18:41.3370525Z ++ test 5 -ne 0 2018-07-26T08:18:41.3423124Z +++ git -C client rev-parse c2 2018-07-26T08:18:41.3756643Z ++ grep 'fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e' trace 2018-07-26T08:18:41.3878403Z packet: fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e 2018-07-26T08:18:41.3883984Z ++ test 0 -ne 0 2018-07-26T08:18:41.3884900Z ++ shift 2018-07-26T08:18:41.3885199Z ++ test 4 -ne 0 2018-07-26T08:18:41.3938298Z +++ git -C client rev-parse c1 2018-07-26T08:18:41.4243782Z ++ grep 'fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe' trace 2018-07-26T08:18:41.4375402Z packet: fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe 2018-07-26T08:18:41.4383803Z ++ test 0 -ne 0 2018-07-26T08:18:41.4384733Z ++ shift 2018-07-26T08:18:41.4385018Z ++ test 3 -ne 0 2018-07-26T08:18:41.4436622Z +++ git -C client rev-parse old4 2018-07-26T08:18:41.4749084Z ++ grep 'fetch> have caef059de69917b9119176a11b88afcef769331d' trace 2018-07-26T08:18:41.4888266Z ++ test 1 -ne 0 2018-07-26T08:18:41.4941092Z +++ git -C client rev-parse old4 2018-07-26T08:18:41.5253206Z ++ echo 'No have caef059de69917b9119176a11b88afcef769331d (old4)' 2018-07-26T08:18:41.5253602Z ++ return 1 2018-07-26T08:18:41.5254746Z error: last command exited with $?=1 2018-07-26T08:18:41.5254865Z No have caef059de69917b9119176a11b88afcef769331d (old4) 2018-07-26T08:18:41.5260970Z not ok 4 - handle clock skew 2018-07-26T08:18:41.5441990Z # 2018-07-26T08:18:41.5442184Z # rm -rf server client trace && 2018-07-26T08:18:41.5442422Z # git init server && 2018-07-26T08:18:41.5448007Z # test_commit -C server to_fetch && 2018-07-26T08:18:41.5448223Z # 2018-07-26T08:18:41.5448357Z # git init client && 2018-07-26T08:18:41.5448466Z # 2018-07-26T08:18:41.5448661Z # # 2 regular commits 2018-07-26T08:18:41.5448810Z # test_tick=2000000000 && 2018-07-26T08:18:41.5449231Z # test_commit -C client c1 && 2018-07-26T08:18:41.5449393Z # test_commit -C client c2 && 2018-07-26T08:18:41.5449509Z # 2018-07-26T08:18:41.5449679Z # # 4 old commits 2018-07-26T08:18:41.5449859Z # test_tick=1000000000 && 2018-07-26T08:18:41.5450017Z # git -C client checkout c1 && 2018-07-26T08:18:41.5450220Z # test_commit -C client old1 && 2018-07-26T08:18:41.5450343Z # test_commit -C client old2 && 2018-07-26T08:18:41.5450449Z # test_commit -C client old3 && 2018-07-26T08:18:41.5450667Z # test_commit -C client old4 && 2018-07-26T08:18:41.5450821Z # 2018-07-26T08:18:41.5450954Z # # "c2" and "c1" are popped first, then "old4" to "old1". "old1" would 2018-07-26T08:18:41.5451133Z # # normally be skipped, but is treated as a commit without a parent here 2018-07-26T08:18:41.5451392Z # # and sent, because (due to clock skew) its only parent has already been 2018-07-26T08:18:41.5451547Z # # popped off the priority queue. 2018-07-26T08:18:41.5451675Z # test_config -C client fetch.negotiationalgorithm skipping && 2018-07-26T08:18:41.5451829Z # GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch "$(pwd)/server" && 2018-07-26T08:18:41.5451961Z # have_sent c2 c1 old4 old2 old1 && 2018-07-26T08:18:41.5452091Z # have_not_sent old3 2018-07-26T08:18:41.5452186Z #
Hi Jonathan,
Quoted 7 lines · Johannes Schindelin
Quoted 1 line · Jonathan Tan
t/t5552-skipping-fetch-negotiator.sh | 179 +++++++++++++++++++This test seems to be failing consistently in the recent
pubuilds:https://git-for-windows.visualstudio.com/git/_build/results?buildId=14337&view=logs
It now also causes next builds to fail:
https://git-for-windows.visualstudio.com/git/_build/results?buildId=14345&view=logs
Please have a look,
Dscho
Quoted 334 lines
Could you have a look, please?
Ciao,
Dscho
P.S.: For your convenience, I will paste the last part of the output with-i -v -xhere:-- snipsnap --
2018-07-26T08:18:39.7864833Z expecting success: 2018-07-26T08:18:39.7868553Z rm -rf server client trace && 2018-07-26T08:18:39.7869403Z git init server && 2018-07-26T08:18:39.7869606Z test_commit -C server to_fetch && 2018-07-26T08:18:39.7870066Z 2018-07-26T08:18:39.7870281Z git init client && 2018-07-26T08:18:39.7870403Z 2018-07-26T08:18:39.7870579Z # 2 regular commits 2018-07-26T08:18:39.7870779Z test_tick=2000000000 && 2018-07-26T08:18:39.7870943Z test_commit -C client c1 && 2018-07-26T08:18:39.7871103Z test_commit -C client c2 && 2018-07-26T08:18:39.7871228Z 2018-07-26T08:18:39.7871419Z # 4 old commits 2018-07-26T08:18:39.7871575Z test_tick=1000000000 && 2018-07-26T08:18:39.7871734Z git -C client checkout c1 && 2018-07-26T08:18:39.7871916Z test_commit -C client old1 && 2018-07-26T08:18:39.7872081Z test_commit -C client old2 && 2018-07-26T08:18:39.7872396Z test_commit -C client old3 && 2018-07-26T08:18:39.7872598Z test_commit -C client old4 && 2018-07-26T08:18:39.7872743Z 2018-07-26T08:18:39.7872918Z # "c2" and "c1" are popped first, then "old4" to "old1". "old1" would 2018-07-26T08:18:39.7873114Z # normally be skipped, but is treated as a commit without a parent here 2018-07-26T08:18:39.7873329Z # and sent, because (due to clock skew) its only parent has already been 2018-07-26T08:18:39.7873524Z # popped off the priority queue. 2018-07-26T08:18:39.7873700Z test_config -C client fetch.negotiationalgorithm skipping && 2018-07-26T08:18:39.7873908Z GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch "$(pwd)/server" && 2018-07-26T08:18:39.7874091Z have_sent c2 c1 old4 old2 old1 && 2018-07-26T08:18:39.7874262Z have_not_sent old3 2018-07-26T08:18:39.7874383Z 2018-07-26T08:18:39.8353323Z ++ rm -rf server client trace 2018-07-26T08:18:40.3404166Z ++ git init server 2018-07-26T08:18:40.3756394Z Initialized empty Git repository in D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/server/.git/ 2018-07-26T08:18:40.3769512Z ++ test_commit -C server to_fetch 2018-07-26T08:18:40.3776271Z ++ notick= 2018-07-26T08:18:40.3777103Z ++ signoff= 2018-07-26T08:18:40.3777282Z ++ indir= 2018-07-26T08:18:40.3777465Z ++ test 3 '!=' 0 2018-07-26T08:18:40.3777648Z ++ case "$1" in 2018-07-26T08:18:40.3777801Z ++ indir=server 2018-07-26T08:18:40.3777948Z ++ shift 2018-07-26T08:18:40.3778093Z ++ shift 2018-07-26T08:18:40.3778493Z ++ test 1 '!=' 0 2018-07-26T08:18:40.3778921Z ++ case "$1" in 2018-07-26T08:18:40.3779072Z ++ break 2018-07-26T08:18:40.3779241Z ++ indir=server/ 2018-07-26T08:18:40.3779431Z ++ file=to_fetch.t 2018-07-26T08:18:40.3779603Z ++ echo to_fetch 2018-07-26T08:18:40.3779923Z ++ git -C server/ add to_fetch.t 2018-07-26T08:18:40.4072248Z ++ test -z '' 2018-07-26T08:18:40.4072727Z ++ test_tick 2018-07-26T08:18:40.4072948Z ++ test -z set 2018-07-26T08:18:40.4073113Z ++ test_tick=1112913673 2018-07-26T08:18:40.4073758Z ++ GIT_COMMITTER_DATE='1112913673 -0700' 2018-07-26T08:18:40.4074001Z ++ GIT_AUTHOR_DATE='1112913673 -0700' 2018-07-26T08:18:40.4074178Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE 2018-07-26T08:18:40.4074357Z ++ git -C server/ commit -m to_fetch 2018-07-26T08:18:40.4485364Z [master (root-commit) ff85695] to_fetch 2018-07-26T08:18:40.4485997Z Author: A U Thor <[email protected]> 2018-07-26T08:18:40.4486201Z 1 file changed, 1 insertion(+) 2018-07-26T08:18:40.4486414Z create mode 100644 to_fetch.t 2018-07-26T08:18:40.4499970Z ++ git -C server/ tag to_fetch 2018-07-26T08:18:40.4809208Z ++ git init client 2018-07-26T08:18:40.5139949Z Initialized empty Git repository in D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/client/.git/ 2018-07-26T08:18:40.5158270Z ++ test_tick=2000000000 2018-07-26T08:18:40.5158466Z ++ test_commit -C client c1 2018-07-26T08:18:40.5159077Z ++ notick= 2018-07-26T08:18:40.5159492Z ++ signoff= 2018-07-26T08:18:40.5159697Z ++ indir= 2018-07-26T08:18:40.5159855Z ++ test 3 '!=' 0 2018-07-26T08:18:40.5160010Z ++ case "$1" in 2018-07-26T08:18:40.5160209Z ++ indir=client 2018-07-26T08:18:40.5160362Z ++ shift 2018-07-26T08:18:40.5160507Z ++ shift 2018-07-26T08:18:40.5160657Z ++ test 1 '!=' 0 2018-07-26T08:18:40.5160831Z ++ case "$1" in 2018-07-26T08:18:40.5161289Z ++ break 2018-07-26T08:18:40.5161582Z ++ indir=client/ 2018-07-26T08:18:40.5161764Z ++ file=c1.t 2018-07-26T08:18:40.5161916Z ++ echo c1 2018-07-26T08:18:40.5162231Z ++ git -C client/ add c1.t 2018-07-26T08:18:40.5456318Z ++ test -z '' 2018-07-26T08:18:40.5460548Z ++ test_tick 2018-07-26T08:18:40.5461417Z ++ test -z set 2018-07-26T08:18:40.5463657Z ++ test_tick=2000000060 2018-07-26T08:18:40.5464369Z ++ GIT_COMMITTER_DATE='2000000060 -0700' 2018-07-26T08:18:40.5464617Z ++ GIT_AUTHOR_DATE='2000000060 -0700' 2018-07-26T08:18:40.5464805Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE 2018-07-26T08:18:40.5464988Z ++ git -C client/ commit -m c1 2018-07-26T08:18:40.5857440Z [master (root-commit) dc824fa] c1 2018-07-26T08:18:40.5858031Z Author: A U Thor <[email protected]> 2018-07-26T08:18:40.5858251Z 1 file changed, 1 insertion(+) 2018-07-26T08:18:40.5858451Z create mode 100644 c1.t 2018-07-26T08:18:40.5872839Z ++ git -C client/ tag c1 2018-07-26T08:18:40.6174770Z ++ test_commit -C client c2 2018-07-26T08:18:40.6175120Z ++ notick= 2018-07-26T08:18:40.6175398Z ++ signoff= 2018-07-26T08:18:40.6175583Z ++ indir= 2018-07-26T08:18:40.6175737Z ++ test 3 '!=' 0 2018-07-26T08:18:40.6175925Z ++ case "$1" in 2018-07-26T08:18:40.6176079Z ++ indir=client 2018-07-26T08:18:40.6176246Z ++ shift 2018-07-26T08:18:40.6176415Z ++ shift 2018-07-26T08:18:40.6176569Z ++ test 1 '!=' 0 2018-07-26T08:18:40.6176738Z ++ case "$1" in 2018-07-26T08:18:40.6176905Z ++ break 2018-07-26T08:18:40.6177052Z ++ indir=client/ 2018-07-26T08:18:40.6177200Z ++ file=c2.t 2018-07-26T08:18:40.6177369Z ++ echo c2 2018-07-26T08:18:40.6177525Z ++ git -C client/ add c2.t 2018-07-26T08:18:40.6474943Z ++ test -z '' 2018-07-26T08:18:40.6479175Z ++ test_tick 2018-07-26T08:18:40.6479861Z ++ test -z set 2018-07-26T08:18:40.6482344Z ++ test_tick=2000000120 2018-07-26T08:18:40.6483064Z ++ GIT_COMMITTER_DATE='2000000120 -0700' 2018-07-26T08:18:40.6483243Z ++ GIT_AUTHOR_DATE='2000000120 -0700' 2018-07-26T08:18:40.6483412Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE 2018-07-26T08:18:40.6483597Z ++ git -C client/ commit -m c2 2018-07-26T08:18:40.6883597Z [master 9ab4692] c2 2018-07-26T08:18:40.6884552Z Author: A U Thor <[email protected]> 2018-07-26T08:18:40.6884902Z 1 file changed, 1 insertion(+) 2018-07-26T08:18:40.6885078Z create mode 100644 c2.t 2018-07-26T08:18:40.6898418Z ++ git -C client/ tag c2 2018-07-26T08:18:40.7214970Z ++ test_tick=1000000000 2018-07-26T08:18:40.7215737Z ++ git -C client checkout c1 2018-07-26T08:18:40.7537971Z Note: checking out 'c1'. 2018-07-26T08:18:40.7538294Z 2018-07-26T08:18:40.7538485Z You are in 'detached HEAD' state. You can look around, make experimental 2018-07-26T08:18:40.7538901Z changes and commit them, and you can discard any commits you make in this 2018-07-26T08:18:40.7539153Z state without impacting any branches by performing another checkout. 2018-07-26T08:18:40.7539288Z 2018-07-26T08:18:40.7539455Z If you want to create a new branch to retain commits you create, you may 2018-07-26T08:18:40.7539646Z do so (now or later) by using -b with the checkout command again. Example: 2018-07-26T08:18:40.7539799Z 2018-07-26T08:18:40.7539979Z git checkout -b <new-branch-name> 2018-07-26T08:18:40.7540099Z 2018-07-26T08:18:40.7540264Z HEAD is now at dc824fa c1 2018-07-26T08:18:40.7552832Z ++ test_commit -C client old1 2018-07-26T08:18:40.7559118Z ++ notick= 2018-07-26T08:18:40.7559789Z ++ signoff= 2018-07-26T08:18:40.7559966Z ++ indir= 2018-07-26T08:18:40.7560066Z ++ test 3 '!=' 0 2018-07-26T08:18:40.7565193Z ++ case "$1" in 2018-07-26T08:18:40.7565286Z ++ indir=client 2018-07-26T08:18:40.7565373Z ++ shift 2018-07-26T08:18:40.7565456Z ++ shift 2018-07-26T08:18:40.7566662Z ++ test 1 '!=' 0 2018-07-26T08:18:40.7566796Z ++ case "$1" in 2018-07-26T08:18:40.7566879Z ++ break 2018-07-26T08:18:40.7566961Z ++ indir=client/ 2018-07-26T08:18:40.7567066Z ++ file=old1.t 2018-07-26T08:18:40.7567150Z ++ echo old1 2018-07-26T08:18:40.7567238Z ++ git -C client/ add old1.t 2018-07-26T08:18:40.7962371Z ++ test -z '' 2018-07-26T08:18:40.7962668Z ++ test_tick 2018-07-26T08:18:40.7963247Z ++ test -z set 2018-07-26T08:18:40.7963453Z ++ test_tick=1000000060 2018-07-26T08:18:40.7963649Z ++ GIT_COMMITTER_DATE='1000000060 -0700' 2018-07-26T08:18:40.7963832Z ++ GIT_AUTHOR_DATE='1000000060 -0700' 2018-07-26T08:18:40.7964000Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE 2018-07-26T08:18:40.7964164Z ++ git -C client/ commit -m old1 2018-07-26T08:18:40.8356576Z [detached HEAD e9a2c09] old1 2018-07-26T08:18:40.8357378Z Author: A U Thor <[email protected]> 2018-07-26T08:18:40.8357554Z 1 file changed, 1 insertion(+) 2018-07-26T08:18:40.8357652Z create mode 100644 old1.t 2018-07-26T08:18:40.8372002Z ++ git -C client/ tag old1 2018-07-26T08:18:40.8671359Z ++ test_commit -C client old2 2018-07-26T08:18:40.8676749Z ++ notick= 2018-07-26T08:18:40.8677398Z ++ signoff= 2018-07-26T08:18:40.8679262Z ++ indir= 2018-07-26T08:18:40.8679957Z ++ test 3 '!=' 0 2018-07-26T08:18:40.8680150Z ++ case "$1" in 2018-07-26T08:18:40.8680305Z ++ indir=client 2018-07-26T08:18:40.8680447Z ++ shift 2018-07-26T08:18:40.8680587Z ++ shift 2018-07-26T08:18:40.8680798Z ++ test 1 '!=' 0 2018-07-26T08:18:40.8680949Z ++ case "$1" in 2018-07-26T08:18:40.8681143Z ++ break 2018-07-26T08:18:40.8681311Z ++ indir=client/ 2018-07-26T08:18:40.8681458Z ++ file=old2.t 2018-07-26T08:18:40.8681604Z ++ echo old2 2018-07-26T08:18:40.8681789Z ++ git -C client/ add old2.t 2018-07-26T08:18:40.8990053Z ++ test -z '' 2018-07-26T08:18:40.8990392Z ++ test_tick 2018-07-26T08:18:40.8990953Z ++ test -z set 2018-07-26T08:18:40.8991246Z ++ test_tick=1000000120 2018-07-26T08:18:40.8991421Z ++ GIT_COMMITTER_DATE='1000000120 -0700' 2018-07-26T08:18:40.8991585Z ++ GIT_AUTHOR_DATE='1000000120 -0700' 2018-07-26T08:18:40.8991771Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE 2018-07-26T08:18:40.8991936Z ++ git -C client/ commit -m old2 2018-07-26T08:18:40.9402744Z [detached HEAD 41bd8dc] old2 2018-07-26T08:18:40.9403286Z Author: A U Thor <[email protected]> 2018-07-26T08:18:40.9403510Z 1 file changed, 1 insertion(+) 2018-07-26T08:18:40.9403678Z create mode 100644 old2.t 2018-07-26T08:18:40.9418122Z ++ git -C client/ tag old2 2018-07-26T08:18:40.9736807Z ++ test_commit -C client old3 2018-07-26T08:18:40.9737058Z ++ notick= 2018-07-26T08:18:40.9737152Z ++ signoff= 2018-07-26T08:18:40.9737238Z ++ indir= 2018-07-26T08:18:40.9737327Z ++ test 3 '!=' 0 2018-07-26T08:18:40.9737600Z ++ case "$1" in 2018-07-26T08:18:40.9737690Z ++ indir=client 2018-07-26T08:18:40.9737776Z ++ shift 2018-07-26T08:18:40.9737860Z ++ shift 2018-07-26T08:18:40.9737964Z ++ test 1 '!=' 0 2018-07-26T08:18:40.9738054Z ++ case "$1" in 2018-07-26T08:18:40.9738140Z ++ break 2018-07-26T08:18:40.9738248Z ++ indir=client/ 2018-07-26T08:18:40.9738338Z ++ file=old3.t 2018-07-26T08:18:40.9738445Z ++ echo old3 2018-07-26T08:18:40.9738540Z ++ git -C client/ add old3.t 2018-07-26T08:18:41.0035565Z ++ test -z '' 2018-07-26T08:18:41.0036056Z ++ test_tick 2018-07-26T08:18:41.0036299Z ++ test -z set 2018-07-26T08:18:41.0036467Z ++ test_tick=1000000180 2018-07-26T08:18:41.0036638Z ++ GIT_COMMITTER_DATE='1000000180 -0700' 2018-07-26T08:18:41.0037189Z ++ GIT_AUTHOR_DATE='1000000180 -0700' 2018-07-26T08:18:41.0037403Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE 2018-07-26T08:18:41.0037574Z ++ git -C client/ commit -m old3 2018-07-26T08:18:41.0429238Z [detached HEAD 4ff0db5] old3 2018-07-26T08:18:41.0429619Z Author: A U Thor <[email protected]> 2018-07-26T08:18:41.0429799Z 1 file changed, 1 insertion(+) 2018-07-26T08:18:41.0429965Z create mode 100644 old3.t 2018-07-26T08:18:41.0443795Z ++ git -C client/ tag old3 2018-07-26T08:18:41.0752553Z ++ test_commit -C client old4 2018-07-26T08:18:41.0752824Z ++ notick= 2018-07-26T08:18:41.0752936Z ++ signoff= 2018-07-26T08:18:41.0753905Z ++ indir= 2018-07-26T08:18:41.0754193Z ++ test 3 '!=' 0 2018-07-26T08:18:41.0754374Z ++ case "$1" in 2018-07-26T08:18:41.0754531Z ++ indir=client 2018-07-26T08:18:41.0754682Z ++ shift 2018-07-26T08:18:41.0754828Z ++ shift 2018-07-26T08:18:41.0755007Z ++ test 1 '!=' 0 2018-07-26T08:18:41.0755162Z ++ case "$1" in 2018-07-26T08:18:41.0755340Z ++ break 2018-07-26T08:18:41.0755491Z ++ indir=client/ 2018-07-26T08:18:41.0755664Z ++ file=old4.t 2018-07-26T08:18:41.0755814Z ++ echo old4 2018-07-26T08:18:41.0755971Z ++ git -C client/ add old4.t 2018-07-26T08:18:41.1064316Z ++ test -z '' 2018-07-26T08:18:41.1064885Z ++ test_tick 2018-07-26T08:18:41.1065169Z ++ test -z set 2018-07-26T08:18:41.1065432Z ++ test_tick=1000000240 2018-07-26T08:18:41.1065637Z ++ GIT_COMMITTER_DATE='1000000240 -0700' 2018-07-26T08:18:41.1065820Z ++ GIT_AUTHOR_DATE='1000000240 -0700' 2018-07-26T08:18:41.1066008Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE 2018-07-26T08:18:41.1066212Z ++ git -C client/ commit -m old4 2018-07-26T08:18:41.1488203Z [detached HEAD caef059] old4 2018-07-26T08:18:41.1488670Z Author: A U Thor <[email protected]> 2018-07-26T08:18:41.1489153Z 1 file changed, 1 insertion(+) 2018-07-26T08:18:41.1489370Z create mode 100644 old4.t 2018-07-26T08:18:41.1502887Z ++ git -C client/ tag old4 2018-07-26T08:18:41.1824546Z ++ test_config -C client fetch.negotiationalgorithm skipping 2018-07-26T08:18:41.1825670Z ++ config_dir= 2018-07-26T08:18:41.1826072Z ++ test -C = -C 2018-07-26T08:18:41.1826299Z ++ shift 2018-07-26T08:18:41.1826528Z ++ config_dir=client 2018-07-26T08:18:41.1826809Z ++ shift 2018-07-26T08:18:41.1827079Z ++ test_when_finished 'test_unconfig -C '\''client'\'' '\''fetch.negotiationalgorithm'\''' 2018-07-26T08:18:41.1827289Z ++ test 0 = 0 2018-07-26T08:18:41.1827561Z ++ test_cleanup='{ test_unconfig -C '\''client'\'' '\''fetch.negotiationalgorithm'\'' 2018-07-26T08:18:41.1827751Z } && (exit "$eval_ret"); eval_ret=$?; :' 2018-07-26T08:18:41.1827930Z ++ git -C client config fetch.negotiationalgorithm skipping 2018-07-26T08:18:41.2196451Z +++ pwd 2018-07-26T08:18:41.2196831Z +++ builtin pwd -W 2018-07-26T08:18:41.2274040Z +++ pwd 2018-07-26T08:18:41.2274458Z +++ builtin pwd -W 2018-07-26T08:18:41.2285081Z ++ GIT_TRACE_PACKET='D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/trace' 2018-07-26T08:18:41.2285515Z ++ git -C client fetch 'D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/server' 2018-07-26T08:18:41.3054360Z warning: no common commits 2018-07-26T08:18:41.3264762Z From D:/a/1/s/t/trash directory.t5552-skipping-fetch-negotiator/server 2018-07-26T08:18:41.3265204Z * branch HEAD -> FETCH_HEAD 2018-07-26T08:18:41.3362819Z ++ have_sent c2 c1 old4 old2 old1 2018-07-26T08:18:41.3370525Z ++ test 5 -ne 0 2018-07-26T08:18:41.3423124Z +++ git -C client rev-parse c2 2018-07-26T08:18:41.3756643Z ++ grep 'fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e' trace 2018-07-26T08:18:41.3878403Z packet: fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e 2018-07-26T08:18:41.3883984Z ++ test 0 -ne 0 2018-07-26T08:18:41.3884900Z ++ shift 2018-07-26T08:18:41.3885199Z ++ test 4 -ne 0 2018-07-26T08:18:41.3938298Z +++ git -C client rev-parse c1 2018-07-26T08:18:41.4243782Z ++ grep 'fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe' trace 2018-07-26T08:18:41.4375402Z packet: fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe 2018-07-26T08:18:41.4383803Z ++ test 0 -ne 0 2018-07-26T08:18:41.4384733Z ++ shift 2018-07-26T08:18:41.4385018Z ++ test 3 -ne 0 2018-07-26T08:18:41.4436622Z +++ git -C client rev-parse old4 2018-07-26T08:18:41.4749084Z ++ grep 'fetch> have caef059de69917b9119176a11b88afcef769331d' trace 2018-07-26T08:18:41.4888266Z ++ test 1 -ne 0 2018-07-26T08:18:41.4941092Z +++ git -C client rev-parse old4 2018-07-26T08:18:41.5253206Z ++ echo 'No have caef059de69917b9119176a11b88afcef769331d (old4)' 2018-07-26T08:18:41.5253602Z ++ return 1 2018-07-26T08:18:41.5254746Z error: last command exited with $?=1 2018-07-26T08:18:41.5254865Z No have caef059de69917b9119176a11b88afcef769331d (old4) 2018-07-26T08:18:41.5260970Z not ok 4 - handle clock skew 2018-07-26T08:18:41.5441990Z # 2018-07-26T08:18:41.5442184Z # rm -rf server client trace && 2018-07-26T08:18:41.5442422Z # git init server && 2018-07-26T08:18:41.5448007Z # test_commit -C server to_fetch && 2018-07-26T08:18:41.5448223Z # 2018-07-26T08:18:41.5448357Z # git init client && 2018-07-26T08:18:41.5448466Z # 2018-07-26T08:18:41.5448661Z # # 2 regular commits 2018-07-26T08:18:41.5448810Z # test_tick=2000000000 && 2018-07-26T08:18:41.5449231Z # test_commit -C client c1 && 2018-07-26T08:18:41.5449393Z # test_commit -C client c2 && 2018-07-26T08:18:41.5449509Z # 2018-07-26T08:18:41.5449679Z # # 4 old commits 2018-07-26T08:18:41.5449859Z # test_tick=1000000000 && 2018-07-26T08:18:41.5450017Z # git -C client checkout c1 && 2018-07-26T08:18:41.5450220Z # test_commit -C client old1 && 2018-07-26T08:18:41.5450343Z # test_commit -C client old2 && 2018-07-26T08:18:41.5450449Z # test_commit -C client old3 && 2018-07-26T08:18:41.5450667Z # test_commit -C client old4 && 2018-07-26T08:18:41.5450821Z # 2018-07-26T08:18:41.5450954Z # # "c2" and "c1" are popped first, then "old4" to "old1". "old1" would 2018-07-26T08:18:41.5451133Z # # normally be skipped, but is treated as a commit without a parent here 2018-07-26T08:18:41.5451392Z # # and sent, because (due to clock skew) its only parent has already been 2018-07-26T08:18:41.5451547Z # # popped off the priority queue. 2018-07-26T08:18:41.5451675Z # test_config -C client fetch.negotiationalgorithm skipping && 2018-07-26T08:18:41.5451829Z # GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch "$(pwd)/server" && 2018-07-26T08:18:41.5451961Z # have_sent c2 c1 old4 old2 old1 && 2018-07-26T08:18:41.5452091Z # have_not_sent old3 2018-07-26T08:18:41.5452186Z #
Quoted 11 lines
Hi Jonathan,
Quoted 1 line · Jonathan Tan
t/t5552-skipping-fetch-negotiator.sh | 179 +++++++++++++++++++This test seems to be failing consistently in the recent
pubuilds:https://git-for-windows.visualstudio.com/git/_build/results?buildId=14337&view=logs
Could you have a look, please?
Hmm...on my Linux computer, this test passes on both pu (as of the time of writing) and 838143aa5c ("Merge branch 'ab/newhash-is-sha256' into pu", 2018-07-25) (pu at the time of that build, according to the website you linked above). If you could rerun that test with additional code, could you add a "cat trace" and show me what the client sends? When I do that, the relevant parts are:
packet: fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e packet: fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe packet: fetch> have caef059de69917b9119176a11b88afcef769331d packet: fetch> have 41bd8dc092ee110ba80e350a346ec507ab2e42a0 packet: fetch> have e9a2c092a8e911567a377c881a7f6031e7f892ea packet: fetch> done
which is exactly as I (and the test) expect.
Two possible reasons for the discrepancy that I can think of offhand are that (1) my computer generates different commits from your test system, and (2) the priority queue pops commits in a different order. For (1), that's not possible because the SHA-1s are the same (as can be seen by comparing your link and the "have" lines I quoted above), and for (2), the code seems OK:
static int compare(const void *a_, const void *b_, void *unused)
{
const struct entry *a = a_;
const struct entry *b = b_;
return compare_commits_by_commit_date(a->commit, b->commit, NULL);
}Let me know if you can observe the output of "cat trace" or if you have any other ideas.
Hi Jonathan,
Quoted 8 lines · Jonathan Tan
Quoted 8 lines · Jonathan Tan
Quoted 2 lines
t/t5552-skipping-fetch-negotiator.sh | 179 +++++++++++++++++++This test seems to be failing consistently in the recent
pubuilds:https://git-for-windows.visualstudio.com/git/_build/results?buildId=14337&view=logs
Quoted 7 lines
Quoted 1 line
Could you have a look, please?
Hmm...on my Linux computer, this test passes on both pu (as of the time of writing) and
838143aa5c("Merge branch 'ab/newhash-is-sha256' into pu", 2018-07-25) (pu at the time of that build, according to the website you linked above). If you could rerun that test with additional code, could you add a "cat trace" and show me what the client sends?
I can give you something even better: a playground. Just open a PR at
https://github.com/gitgitgadget/git (all of the branches on gitster/git ar mirrored, including yours, I am sure, so you can target that branch specifically).
Once you open a Pull Request, it will automatically build and run the test suite on Windows, macOS and Linux. You will see it in the "checks" section on the bottom. Example for my range-diff series:
https://git-for-windows.visualstudio.com/git/_build/results?buildId=14279
For a quicker turnaround, you could add a commit that forces the all target in t/Makefile to run only your test.
Quoted 25 lines
When I do that, the relevant parts are:
packet: fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e packet: fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe packet: fetch> have caef059de69917b9119176a11b88afcef769331d packet: fetch> have 41bd8dc092ee110ba80e350a346ec507ab2e42a0 packet: fetch> have e9a2c092a8e911567a377c881a7f6031e7f892ea packet: fetch> donewhich is exactly as I (and the test) expect.
Two possible reasons for the discrepancy that I can think of offhand are that (1) my computer generates different commits from your test system, and (2) the priority queue pops commits in a different order. For (1), that's not possible because the SHA-1s are the same (as can be seen by comparing your link and the "have" lines I quoted above), and for (2), the code seems OK:
static int compare(const void *a_, const void *b_, void *unused) { const struct entry *a = a_; const struct entry *b = b_; return compare_commits_by_commit_date(a->commit, b->commit, NULL); }
Quoted 2 lines
Let me know if you can observe the output of "cat trace" or if you have any other ideas.
Like I said, you can use those "CI" builds, I think that would be more effective than if you waited for me to react, I am quite overwhelmed these days.
Ciao,
Dscho
Hi Jonathan,
Quoted 57 lines · Johannes Schindelin
Quoted 8 lines · Jonathan Tan
Quoted 8 lines · Jonathan Tan
Quoted 2 lines
t/t5552-skipping-fetch-negotiator.sh | 179 +++++++++++++++++++This test seems to be failing consistently in the recent
pubuilds:https://git-for-windows.visualstudio.com/git/_build/results?buildId=14337&view=logs
Quoted 7 lines
Quoted 1 line
Could you have a look, please?
Hmm...on my Linux computer, this test passes on both pu (as of the time of writing) and
838143aa5c("Merge branch 'ab/newhash-is-sha256' into pu", 2018-07-25) (pu at the time of that build, according to the website you linked above). If you could rerun that test with additional code, could you add a "cat trace" and show me what the client sends?I can give you something even better: a playground. Just open a PR at
https://github.com/gitgitgadget/git (all of the branches on gitster/git ar mirrored, including yours, I am sure, so you can target that branch specifically).Once you open a Pull Request, it will automatically build and run the test suite on Windows, macOS and Linux. You will see it in the "checks" section on the bottom. Example for my range-diff series:
https://git-for-windows.visualstudio.com/git/_build/results?buildId=14279
For a quicker turnaround, you could add a commit that forces the
alltarget int/Makefileto run only your test.Quoted 25 lines
When I do that, the relevant parts are:
packet: fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e packet: fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe packet: fetch> have caef059de69917b9119176a11b88afcef769331d packet: fetch> have 41bd8dc092ee110ba80e350a346ec507ab2e42a0 packet: fetch> have e9a2c092a8e911567a377c881a7f6031e7f892ea packet: fetch> donewhich is exactly as I (and the test) expect.
Two possible reasons for the discrepancy that I can think of offhand are that (1) my computer generates different commits from your test system, and (2) the priority queue pops commits in a different order. For (1), that's not possible because the SHA-1s are the same (as can be seen by comparing your link and the "have" lines I quoted above), and for (2), the code seems OK:
static int compare(const void *a_, const void *b_, void *unused) { const struct entry *a = a_; const struct entry *b = b_; return compare_commits_by_commit_date(a->commit, b->commit, NULL); }
Quoted 6 lines
Quoted 2 lines
Let me know if you can observe the output of "cat trace" or if you have any other ideas.
Like I said, you can use those "CI" builds, I think that would be more effective than if you waited for me to react, I am quite overwhelmed these days.
Hopefully you have a chance to do so. I got the impression that it is actually more of a flakey test than a consistent test failure:
https://git-for-windows.visualstudio.com/git/_build/results?buildId=15015&view=logs
Ciao,
Dscho
On Mon, Jul 16 2018, Jonathan Tan wrote:
Didn't catch this until this was in next, sorry.
Re-arranged the diff a bit:
fetch-negotiator.c line 14
Quoted 10 lines
fetch-negotiator.c +6
-1-void fetch_negotiator_init (struct fetch_negotiator *negotiator) +void fetch_negotiator_init (struct fetch_negotiator *negotiator, +const char *algorithm) { +if (algorithm && !strcmp (algorithm,"skipping" )) { +skipping_negotiator_init (negotiator); +return ; + }default_negotiator_init (negotiator); }
Okey, I understand that's how it works now, but....
Documentation/config.txt fetch.output:: · line 1502
Quoted 9 lines
Documentation/config.txt +9
-0+fetch.negotiationAlgorithm:: + Control how information about the commits in the local repository is + sent when negotiating the contents of the packfile to be sent by the + server. Set to "skipping" to use an algorithm that skips commits in an + effort to converge faster, but may result in a larger-than-necessary + packfile; any other value instructs Git to use the default algorithm + that never skips commits (unless the server has acknowledged it or one + of its descendants). +
...let's instead document that there's just the values "skipping" and "default", and say "default" is provided by default, and perhaps change the code to warn about anything that isn't those two.
Then we're not painting ourselves into a corner by needing to break a promise in the docs ("any other value instructs Git to use the default") if we add a new one of these, and aren't silently falling back on the default if we add new-fancy-algo the user's version doesn't know about.
Also, switching gears entirely, I'm very excited about this whole thing because it allows me to address something I've been meaning to get to for a while.
At work I sometimes want to see what commits I've made to all our git repos, for remembering what I was doing last February or whatever (this is for filling in quarterly reports).
So I have this script that basically does this:
for repo in $(get-list-of-all-the-things)
do
git config "remote.$repo.url" [email protected]:$repo.git git config "remote.$repo.fetch" "+HEAD:$repo/HEAD"
git config "remote.$repo.tagOpt" "--no-tags"
done &&
git fetch --all
I.e. for every repo like git/git I'll fetch its upstream HEAD as the branch git/git/HEAD. Then I can do stuff like:
git shortlog --author=Ævar --since=2018-02-01 --until=2018-03-01
Now, running that "git fetch --all" takes ages, and I know why. It's because the in the negotiation for "git fetch some/small-repo" I'm emitting hundreds of thousands of "have" lines for SHA1s found in other unrelated repos, only to get a NAK for all of them.
One way to fix that with this facility would be to have some way to pass in arguments, similar to what we have for merge drivers, so I can say "just emit 'have' lines for stuff found in this branch". The most pathological cases are when I'm fetching a remote that has one commit, and I'm desperately trying to find something in common by asking if the remote has hundreds of K of commits it has no chance of having.
Or there may be some smarter way to do this, what do you think?
Quoted 14 lines
Documentation/config.txt fetch.output:: · line 1502
Quoted 9 lines
Documentation/config.txt +9
-0+fetch.negotiationAlgorithm:: + Control how information about the commits in the local repository is + sent when negotiating the contents of the packfile to be sent by the + server. Set to "skipping" to use an algorithm that skips commits in an + effort to converge faster, but may result in a larger-than-necessary + packfile; any other value instructs Git to use the default algorithm + that never skips commits (unless the server has acknowledged it or one + of its descendants). +...let's instead document that there's just the values "skipping" and "default", and say "default" is provided by default, and perhaps change the code to warn about anything that isn't those two.
Quoted 4 lines
Then we're not painting ourselves into a corner by needing to break a promise in the docs ("any other value instructs Git to use the default") if we add a new one of these, and aren't silently falling back on the default if we add new-fancy-algo the user's version doesn't know about.
My intention was to allow future versions of Git to introduce more algorithms, but have older versions of Git still work even if a repository is configured to use a newer algorithm. But your suggestion is reasonable too.
Quoted 5 lines
Now, running that "git fetch --all" takes ages, and I know why. It's because the in the negotiation for "git fetch some/small-repo" I'm emitting hundreds of thousands of "have" lines for SHA1s found in other unrelated repos, only to get a NAK for all of them.
Quoted 8 lines
One way to fix that with this facility would be to have some way to pass in arguments, similar to what we have for merge drivers, so I can say "just emit 'have' lines for stuff found in this branch". The most pathological cases are when I'm fetching a remote that has one commit, and I'm desperately trying to find something in common by asking if the remote has hundreds of K of commits it has no chance of having.
Or there may be some smarter way to do this, what do you think?
Well, there is already a commit in "next" that does this :-)
3390e42adb ("fetch-pack: support negotiation tip whitelist", 2018-07-03)
cover negotiator: improve recent behavior + docs
Quoted 19 lines · Jonathan Tan
Quoted 14 lines
Documentation/config.txt fetch.output:: · line 1502
Quoted 9 lines
Documentation/config.txt +9
-0+fetch.negotiationAlgorithm:: + Control how information about the commits in the local repository is + sent when negotiating the contents of the packfile to be sent by the + server. Set to "skipping" to use an algorithm that skips commits in an + effort to converge faster, but may result in a larger-than-necessary + packfile; any other value instructs Git to use the default algorithm + that never skips commits (unless the server has acknowledged it or one + of its descendants). +...let's instead document that there's just the values "skipping" and "default", and say "default" is provided by default, and perhaps change the code to warn about anything that isn't those two.
Quoted 4 lines
Then we're not painting ourselves into a corner by needing to break a promise in the docs ("any other value instructs Git to use the default") if we add a new one of these, and aren't silently falling back on the default if we add new-fancy-algo the user's version doesn't know about.
Quoted 4 lines
My intention was to allow future versions of Git to introduce more algorithms, but have older versions of Git still work even if a repository is configured to use a newer algorithm. But your suggestion is reasonable too.
I think 01/02 in this patch series implements something that's better & more future-proof.
Quoted 12 lines
Quoted 12 lines
Now, running that "git fetch --all" takes ages, and I know why. It's because the in the negotiation for "git fetch some/small-repo" I'm emitting hundreds of thousands of "have" lines for SHA1s found in other unrelated repos, only to get a NAK for all of them.
One way to fix that with this facility would be to have some way to pass in arguments, similar to what we have for merge drivers, so I can say "just emit 'have' lines for stuff found in this branch". The most pathological cases are when I'm fetching a remote that has one commit, and I'm desperately trying to find something in common by asking if the remote has hundreds of K of commits it has no chance of having.
Quoted 5 lines
Quoted 1 line
Or there may be some smarter way to do this, what do you think?
Well, there is already a commit in "next" that does this :-)
3390e42adb("fetch-pack: support negotiation tip whitelist", 2018-07-03)
That's awesome. This is exactly what I wanted, this patch series also fixes another small issue in 02/02; which is that the docs for the two really should cross-link to make these discoverable from one another.
Another thing I noticed, which I have not fixed, is that there's no way to say "I don't want to you to say that anything is in common". Currently I'm hacking around in my script by doing:
parallel --eta --verbose --progress '
if git rev-parse {}/HEAD 2>/dev/null
then
git fetch --negotiation-tip={}/HEAD {}
else
git fetch --negotiation-tip=ref-i-have-with-one-commit {}
fi
' ::: $(git remote)I.e. the way I'm doing this is I add all the remotes first, then I fetch them all in parallel, but because the first time around I don't have anything for that remote (and they don't share any commits) I need to fake it up and pretend to be fetching from a repo that has just one commit.
It would be better if I could somehow say that I don't mind that the ref doesn't exist, but currently you either error out with this, or ignore the glob, depending on the mode.
So I want this, but can't think of a less shitty UI than:
git fetch --negotiation-tip=$REF --negotiation-tip-error-handling=missing-ref-means-no-want
Or something equally atrocious, do you have any better ideas?
Ævar Arnfjörð Bjarmason (2):
negotiator: unknown fetch.negotiationAlgorithm should error out fetch doc: cross-link two new negotiation options
Documentation/config.txt | 5 ++++- Documentation/fetch-options.txt | 3 +++ fetch-negotiator.c | 12 +++++++++--- t/t5552-skipping-fetch-negotiator.sh | 23 +++++++++++++++++++++++ 4 files changed, 39 insertions(+), 4 deletions(-)
Signature · 1 line
2.18.0.345.g5c9ce644c3
Re: [PATCH 0/2] negotiator: improve recent behavior + docs
Quoted 2 lines
I think 01/02 in this patch series implements something that's better & more future-proof.
Thanks. Both patches are:
A small note:
Review
Quoted 2 lines
(quoted diff) +1
-1- packfile; any other value instructs Git to use the default algorithm + packfile; The default is "default" which instructs Git to use the default algorithm
I think we generally don't capitalize words after semicolons.
Thanks for noticing that the check of fetch.negotiationAlgorithm only happens when a negotiation actually occurs - before your patches, it didn't really matter because we tolerated anything, but now we do. I think this is fine - as far as I know, Git commands generally only read the configs relevant to them, and if fetch.negotiationAlgorithm is not relevant in a certain situation, we don't need to read it.
Quoted 3 lines
That's awesome. This is exactly what I wanted, this patch series also fixes another small issue in 02/02; which is that the docs for the two really should cross-link to make these discoverable from one another.
That's a good idea; thanks for doing it.
Quoted 10 lines
I.e. the way I'm doing this is I add all the remotes first, then I fetch them all in parallel, but because the first time around I don't have anything for that remote (and they don't share any commits) I need to fake it up and pretend to be fetching from a repo that has just one commit.
It would be better if I could somehow say that I don't mind that the ref doesn't exist, but currently you either error out with this, or ignore the glob, depending on the mode.
Quoted 5 lines
So I want this, but can't think of a less shitty UI than:
git fetch --negotiation-tip=$REF --negotiation-tip-error-handling=missing-ref-means-no-wantOr something equally atrocious, do you have any better ideas?
If you wanted to do this, it seems better to me to just declare a "null" negotiation algorithm that does not perform any negotiation at all.
Re: [PATCH 0/2] negotiator: improve recent behavior + docs
Quoted 4 lines · Jonathan Tan
Quoted 2 lines
I think 01/02 in this patch series implements something that's better & more future-proof.
Thanks. Both patches are:
Quoted 8 lines
Reviewed-by Jonathan Tan <[email protected]>A small note:
Review
Quoted 2 lines
(quoted diff) +1
-1- packfile; any other value instructs Git to use the default algorithm + packfile; The default is "default" which instructs Git to use the default algorithmI think we generally don't capitalize words after semicolons.
Yeah I think that's right. Will fix (or if there's no other comments perhaps Junio will munge it...) :)
Quoted 6 lines
Thanks for noticing that the check of fetch.negotiationAlgorithm only happens when a negotiation actually occurs - before your patches, it didn't really matter because we tolerated anything, but now we do. I think this is fine - as far as I know, Git commands generally only read the configs relevant to them, and if fetch.negotiationAlgorithm is not relevant in a certain situation, we don't need to read it.
Yeah I think that's OK.
Quoted 16 lines
Quoted 3 lines
That's awesome. This is exactly what I wanted, this patch series also fixes another small issue in 02/02; which is that the docs for the two really should cross-link to make these discoverable from one another.
That's a good idea; thanks for doing it.
Quoted 10 lines
I.e. the way I'm doing this is I add all the remotes first, then I fetch them all in parallel, but because the first time around I don't have anything for that remote (and they don't share any commits) I need to fake it up and pretend to be fetching from a repo that has just one commit.
It would be better if I could somehow say that I don't mind that the ref doesn't exist, but currently you either error out with this, or ignore the glob, depending on the mode.
Quoted 8 lines
Quoted 5 lines
So I want this, but can't think of a less shitty UI than:
git fetch --negotiation-tip=$REF --negotiation-tip-error-handling=missing-ref-means-no-wantOr something equally atrocious, do you have any better ideas?
If you wanted to do this, it seems better to me to just declare a "null" negotiation algorithm that does not perform any negotiation at all.
I think such an algorithm is a good idea in general, especially for testing, and yeah, maybe that's the best way out of this, i.e. to do:
if git rev-parse {}/HEAD 2>/dev/null
then
git fetch --negotiation-tip={}/HEAD {}
else
git -c fetch.negotiationAlgorithm=null fetch {}
fiWould such an algorithm be added by overriding default.c's add_tip function to never add anything by calling default_negotiator_init() followed by null_negotiator_init(), which would only override add_tip? (yay C OO)
If so from fetch-pack.c it looks like there may be the limitation on the interface that the negotiator can't exit early (in fetch-pack.c:mark_tips). But I've just skimmed this, so maybe I've missed something.
Also, looks like because of the current interface =null and --negotiation-tip=* would (somewhat confusingly) do a "real" negotiation if done that way, since it'll bypass the API and insert tips for it to negotiate, but it looks like overriding next() will get around that.
Re: [PATCH 0/2] negotiator: improve recent behavior + docs
Quoted 18 lines
Quoted 2 lines
If you wanted to do this, it seems better to me to just declare a "null" negotiation algorithm that does not perform any negotiation at all.
I think such an algorithm is a good idea in general, especially for testing, and yeah, maybe that's the best way out of this, i.e. to do:
if git rev-parse {}/HEAD 2>/dev/null then git fetch --negotiation-tip={}/HEAD {} else git -c fetch.negotiationAlgorithm=null fetch {} fiWould such an algorithm be added by overriding default.c's
add_tipfunction to never add anything by callingdefault_negotiator_init()followed bynull_negotiator_init(), which would only overrideadd_tip? (yay C OO)
Quoted 4 lines
If so from
fetch-pack.cit looks like there may be the limitation on the interface that the negotiator can't exit early (in fetch-pack.c:mark_tips). But I've just skimmed this, so maybe I've missed something.
(I was reminded to reply to this offlist - sorry for the late reply.)
I think too many things need to be replaced (known_common, add_tip, and ack all need to do nothing), so it's best to start from scratch. That way, we also don't need to deal with the subtleties of C OO :-)
Quoted 4 lines
Also, looks like because of the current interface =null and --negotiation-tip=* would (somewhat confusingly) do a "real" negotiation if done that way, since it'll bypass the API and insert tips for it to negotiate, but it looks like overriding
next()will get around that.
If you do it as I suggest (in particular, add_tip doing nothing) then there is the opposite problem that it won't be easy to inform the user that --negotiation-tip does nothing in this case. Maybe there needs to be an "accepts_tips" field in struct fetch_negotiator that, if false, means that custom tips (or any tips) are not accepted, allowing the caller of the negotiator to print a warning message in this case.
Re: [PATCH 0/2] negotiator: improve recent behavior + docs
Quoted 34 lines · Jonathan Tan
Quoted 18 lines
Quoted 2 lines
If you wanted to do this, it seems better to me to just declare a "null" negotiation algorithm that does not perform any negotiation at all.
I think such an algorithm is a good idea in general, especially for testing, and yeah, maybe that's the best way out of this, i.e. to do:
if git rev-parse {}/HEAD 2>/dev/null then git fetch --negotiation-tip={}/HEAD {} else git -c fetch.negotiationAlgorithm=null fetch {} fiWould such an algorithm be added by overriding default.c's
add_tipfunction to never add anything by callingdefault_negotiator_init()followed bynull_negotiator_init(), which would only overrideadd_tip? (yay C OO)Quoted 4 lines
If so from
fetch-pack.cit looks like there may be the limitation on the interface that the negotiator can't exit early (in fetch-pack.c:mark_tips). But I've just skimmed this, so maybe I've missed something.(I was reminded to reply to this offlist - sorry for the late reply.)
I think too many things need to be replaced (
known_common,add_tip, and ack all need to do nothing), so it's best to start from scratch. That way, we also don't need to deal with the subtleties of C OO :-)Quoted 4 lines
Also, looks like because of the current interface =null and --negotiation-tip=* would (somewhat confusingly) do a "real" negotiation if done that way, since it'll bypass the API and insert tips for it to negotiate, but it looks like overriding
next()will get around that.
Quoted 6 lines
If you do it as I suggest (in particular,
add_tipdoing nothing) then there is the opposite problem that it won't be easy to inform the user that --negotiation-tip does nothing in this case. Maybe there needs to be an "accepts_tips" field in structfetch_negotiatorthat, if false, means that custom tips (or any tips) are not accepted, allowing the caller of the negotiator to print a warning message in this case.
Thanks, yeah it seems the interface would need to be tweaked for such a "null" negotiator.
Some more general questions (which I can turn into docs once I understand this). If I run this, as a testcase for two random repos where I "fetch" an unrelated one and use the first ever commit to git.git as an alias for this "null" negotiatior, i.e. "just present this one commit":
(
rm -rf /tmp/git &&
git clone https://github.com/git/git.git /tmp/git &&
cd /tmp/git &&
git remote add gitlab-shell https://github.com/cr-marcstevens/sha1collisiondetection &&
GIT_TRACE_PACKET=/tmp/git/packet.trace git fetch --negotiation-tip=$(git log --reverse|head -n 1|cut -d ' ' -f2) gitlab-shell &&
grep -c "fetch-pack> have" /tmp/git/packet.trace
)I get:
warning: Ignoring --negotiation-tip because the protocol does not support it.
And the grep -c shows we tried to present 55170 commits in "have" lines to the server. Now, change that to SSH and all is well:
(
rm -rf /tmp/git &&
git clone [email protected]:git/git.git /tmp/git &&
cd /tmp/git &&
git remote add gitlab-shell [email protected]:cr-marcstevens/sha1collisiondetection &&
GIT_TRACE_PACKET=/tmp/git/packet.trace git fetch --negotiation-tip=$(git log --reverse|head -n 1|cut -d ' ' -f2) gitlab-shell &&
grep -c "fetch-pack> have" /tmp/git/packet.trace
)I don't understand this limitation. With the SSH version we skip straight to saying we "want" with just the 1 "have" line of "e83c5163316f89bfbde7d9ab23ca2e25604af290".
Why aren't we doing the same over http? I don't get how protocol support is needed, it's us who decide to send over the "have" lines. Some variant of this does work over "skipping":
(
rm -rf /tmp/git &&
git clone https://github.com/git/git.git /tmp/git &&
cd /tmp/git &&
git remote add gitlab-shell https://github.com/cr-marcstevens/sha1collisiondetection &&
GIT_TRACE_PACKET=/tmp/git/packet.trace git -c fetch.negotiationAlgorithm=skipping fetch gitlab-shell &&
grep -c "fetch-pack> have" /tmp/git/packet.trace
)There we send 14002 "have" lines, which seems expected, but then with the same thing over SSH we don't send any:
(
rm -rf /tmp/git &&
git clone [email protected]:git/git.git /tmp/git &&
cd /tmp/git &&
git remote add gitlab-shell [email protected]:cr-marcstevens/sha1collisiondetection &&
GIT_TRACE_PACKET=/tmp/git/packet.trace git -c fetch.negotiationAlgorithm=skipping fetch gitlab-shell &&
grep -c "fetch-pack> have" /tmp/git/packet.trace
)So that seems like another bug, and as an aside, a "skipping" implementation that sends ~1/4 of the commits in the repo seems way less aggressive than it should be. I was expecting something that would gradually "ramp up" from the tips. Where say starting at master/next/pu we present every 100th commit as a "have" until the 1000th commit, then every 1000 commits until 10k and quickly after that step up the size rapidly.
Re: [PATCH 0/2] negotiator: improve recent behavior + docs
Quoted 3 lines
I get:
warning: Ignoring --negotiation-tip because the protocol does not support it.
When I implemented --negotiation-tip, I only implemented it for protocols that support connect or stateless-connect, because implementing it fully would have required expanding the protocol helper functionality. For reference, the commit is 3390e42adb ("fetch-pack:
support negotiation tip whitelist", 2018-07-03).
So HTTPS wouldn't work unless you were using protocol v2.
Quoted 7 lines
So that seems like another bug, and as an aside, a "skipping" implementation that sends ~1/4 of the commits in the repo seems way less aggressive than it should be. I was expecting something that would gradually "ramp up" from the tips. Where say starting at master/next/pu we present every 100th commit as a "have" until the 1000th commit, then every 1000 commits until 10k and quickly after that step up the size rapidly.
I reproduced using your commands, and yes, there is a bug - I'll take a look.
PATCH 1/2 negotiator: unknown fetch.negotiationAlgorithm should error out
In reply to Jonathan TanChange the handling of fetch.negotiationAlgorithm=<str> to error out on unknown strings, i.e. everything except "default" or "skipping".
This changes the behavior added in 42cc7485a2 ("negotiator/skipping:
skip commits during fetch", 2018-07-16) which would ignore all unknown values and silently fall back to the "default" value.
For a feature like this it's much better to produce an error than proceed. We don't want users to debug some amazingly slow fetch that should benefit from "skipping", only to find that they'd forgotten to deploy the new git version on that particular machine.
--- Documentation/config.txt | 3 ++- fetch-negotiator.c | 12 +++++++++--- t/t5552-skipping-fetch-negotiator.sh | 23 +++++++++++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-)
Documentation/config.txt +2 -1
@@ -1536,9 +1536,10 @@ fetch.negotiationAlgorithm:: sent when negotiating the contents of the packfile to be sent by the server. Set to "skipping" to use an algorithm that skips commits in an effort to converge faster, but may result in a larger-than-necessary - packfile; any other value instructs Git to use the default algorithm + packfile; The default is "default" which instructs Git to use the default algorithm that never skips commits (unless the server has acknowledged it or one of its descendants). + Unknown values will cause 'git fetch' to error out. format.attach:: Enable multipart/mixed attachments as the default for
fetch-negotiator.c +9 -3
@@ -6,9 +6,15 @@void fetch_negotiator_init (struct fetch_negotiator *negotiator,const char *algorithm) { -if (algorithm && !strcmp (algorithm,"skipping" )) { -skipping_negotiator_init (negotiator); -return ; +if (algorithm) { +if (!strcmp (algorithm,"skipping" )) { +skipping_negotiator_init (negotiator); +return ; + }else if (!strcmp (algorithm,"default" )) { +/* Fall through to default initialization */ + }else { +die ("unknown fetch negotiation algorithm '%s'" , algorithm); + } }default_negotiator_init (negotiator); }
t/t5552-skipping-fetch-negotiator.sh +23 -0
@@ -47,6 +47,29 @@ test_expect_success 'commits with no parents are sent regardless of skip distanc have_not_sent c6 c4 c3 ' +test_expect_success 'unknown fetch.negotiationAlgorithm values error out' ' + rm -rf server client trace && + git init server && + test_commit -C server to_fetch && + + git init client && + test_commit -C client on_client && + git -C client checkout on_client && + + test_config -C client fetch.negotiationAlgorithm invalid && + test_must_fail git -C client fetch "$(pwd)/server" 2>err && + test_i18ngrep "unknown fetch negotiation algorithm" err && + + # Explicit "default" value + test_config -C client fetch.negotiationAlgorithm default && + git -C client -c fetch.negotiationAlgorithm=default fetch "$(pwd)/server" && + + # Implementation detail: If there is nothing to fetch, we will not error out + test_config -C client fetch.negotiationAlgorithm invalid && + git -C client fetch "$(pwd)/server" 2>err && + test_i18ngrep ! "unknown fetch negotiation algorithm" err +' + test_expect_success 'when two skips collide, favor the larger one' ' rm -rf server client trace && git init server &&
Signature · 1 line
2.18.0.345.g5c9ce644c3
PATCH 2/2 fetch doc: cross-link two new negotiation options
In reply to Jonathan TanUsers interested in the fetch.negotiationAlgorithm variable added in 42cc7485a2 ("negotiator/skipping: skip commits during fetch", 2018-07-16) are probably interested in the related --negotiation-tip option added in 3390e42adb ("fetch-pack: support negotiation tip whitelist", 2018-07-02).
Change the documentation for those two to reference one another to point readers in the right direction.
--- Documentation/config.txt | 2 ++ Documentation/fetch-options.txt | 3 +++ 2 files changed, 5 insertions(+)
Documentation/config.txt +2 -0
@@ -1540,6 +1540,8 @@ fetch.negotiationAlgorithm:: that never skips commits (unless the server has acknowledged it or one of its descendants). Unknown values will cause 'git fetch' to error out. ++ +See also the `--negotiation-tip` option for linkgit:git-fetch[1]. format.attach:: Enable multipart/mixed attachments as the default for
Documentation/fetch-options.txt +3 -0
@@ -57,6 +57,9 @@ commits reachable from any of the given commits. The argument to this option may be a glob on ref names, a ref, or the (possibly abbreviated) SHA-1 of a commit. Specifying a glob is equivalent to specifying this option multiple times, one for each matching ref name. ++ +See also the `fetch.negotiationAlgorithm` configuration variable +documented in linkgit:git-config[1]. ifndef::git-pull[] --dry-run::
Signature · 1 line
2.18.0.345.g5c9ce644c3