Hello, I can confirm that shallow clones are no longer failing. -- Arturas Moskvinas On Wed, Oct 17, 2018 at 12:58 AM Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote: > > Suppose a server has the following commit graph: > > A B > \ / > O > > We create a client by cloning A from the server with depth 1, and add > many commits to it (so that future fetches span multiple requests due to > lengthy negotiation). If it then fetches B using protocol v2, the fetch > spanning multiple requests, the resulting packfile does not contain O > even though the client did report that A is shallow. > > This is because upload_pack_v2() can be called multiple times while > processing the same session. During the 2nd and all subsequent > invocations, some object flags remain from the previous invocations. In > particular, CLIENT_SHALLOW remains, preventing process_shallow() from > adding client-reported shallows to the "shallows" array, and hence > pack-objects not knowing about these client-reported shallows. > > Therefore, teach upload_pack_v2() to clear object flags at the start of > each invocation. > > (One alternative is to reduce or eliminate usage of object flags in > protocol v2, but that doesn't seem feasible because almost all 8 flags > are used pervasively in v2 code.) > > Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx> > --- > This was noticed by Arturas Moskvinas <arturas@xxxxxxxx> in [1]. The > reproduction steps given were to repeat a shallow fetch twice in > succession, but I found it easier to write a more understandable test if > I made the 2nd fetch an ordinary fetch. In any case, I also reran the > original reproduction steps, and the fetch completes without error. > > This patch doesn't cover the negotiation issue that I mentioned in my > previous reply [2]. > > [1] https://public-inbox.org/git/CAGY-PBgsG-T3JY=awszwGMpFx+jDx-a1FcV0s6vR067bSqgZaA@xxxxxxxxxxxxxx/ > [2] https://public-inbox.org/git/20181013004356.257709-1-jonathantanmy@xxxxxxxxxx/ > --- > t/t5702-protocol-v2.sh | 25 +++++++++++++++++++++++++ > upload-pack.c | 5 +++++ > 2 files changed, 30 insertions(+) > > diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh > index 88a886975d..70b88385ba 100755 > --- a/t/t5702-protocol-v2.sh > +++ b/t/t5702-protocol-v2.sh > @@ -429,6 +429,31 @@ test_expect_success 'fetch supports include-tag and tag following' ' > git -C client cat-file -e $(git -C client rev-parse annotated_tag) > ' > > +test_expect_success 'upload-pack respects client shallows' ' > + rm -rf server client trace && > + > + git init server && > + test_commit -C server base && > + test_commit -C server client_has && > + > + git clone --depth=1 "file://$(pwd)/server" client && > + > + # Add extra commits to the client so that the whole fetch takes more > + # than 1 request (due to negotiation) > + for i in $(seq 1 32) > + do > + test_commit -C client c$i > + done && > + > + git -C server checkout -b newbranch base && > + test_commit -C server client_wants && > + > + GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \ > + fetch origin newbranch && > + # Ensure that protocol v2 is used > + grep "git< version 2" trace > +' > + > # Test protocol v2 with 'http://' transport > # > . "$TEST_DIRECTORY"/lib-httpd.sh > diff --git a/upload-pack.c b/upload-pack.c > index 62a1000f44..de7de1de38 100644 > --- a/upload-pack.c > +++ b/upload-pack.c > @@ -37,6 +37,9 @@ > #define CLIENT_SHALLOW (1u << 18) > #define HIDDEN_REF (1u << 19) > > +#define ALL_FLAGS (THEY_HAVE | OUR_REF | WANTED | COMMON_KNOWN | SHALLOW | \ > + NOT_SHALLOW | CLIENT_SHALLOW | HIDDEN_REF) > + > static timestamp_t oldest_have; > > static int deepen_relative; > @@ -1393,6 +1396,8 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys, > enum fetch_state state = FETCH_PROCESS_ARGS; > struct upload_pack_data data; > > + clear_object_flags(ALL_FLAGS); > + > git_config(upload_pack_config, NULL); > > upload_pack_data_init(&data); > -- > 2.19.0.271.gfe8321ec05.dirty >
