[PATCH v2 0/3] Clear flags before each v2 request

From: Jonathan Tan <jonathantanmy@google.com>
Date: Oct 18, 2018, 20:43 UTC
Message-ID: <cover.1539893192.git.jonathantanmy@google.com>
In-Reply-To: <20181016215850.47821-1-jonathantanmy@google.com>

To explain the differences in this version of the patch set, I'll quote an email [1] from Junio:

[1] https://public-inbox.org/git/xmqq5zxzvnq1.fsf@gitster-ct.c.googlers.com/

Show 5 quoted lines

> The change to the code itself sort-of makes sense (I say sort-of
> because I didn't carefully look at the callers to see if they mind
> getting all these flags cleared, but the ones that are cleared are
> the ones that are involved mostly in the negotiation and shold be
> OK).

I have included 2 additional patches for these reasons:

 - After reading the section of Junio's email quoted above, I took
   another look at the flags, and found that not only is state stored in
   the flags between invocations of upload_pack_v2(), state is also
   stored in the want_obj and have_obj global variables. The additional
   patches help clean those up.

 - To help reviewers who want to see if the callers mind getting all 8
   flags cleared, I have included a discussion of all 8 flags in the
   commit message of patch 3. The additional patches made the discussion
   easier.

Responses to other points:

> Hmph, what if commit O had a long history behind it?  
> 
> Should fetching of B result in fetching the whole history?

I think so - when we fetch without --depth or any similar arguments, I think it's reasonable to have all objects referenced by the fetched tips.

> Would we
> notice that now all of A's parents are available locally and declare
> that the repository is no longer shallow?

We could, but I think this is outside the scope of this patch set.

> Use test_seq instead, or you'll get hit by test-lint?

Thanks for the pointer to test-lint. I've used test_seq, and checked that test-lint doesn't print any errors.

> Applied on 'master' or 'maint', this new test does not pass even
> with s/seq/test_&/, so there may be something else wrong with it,
> though.

Thanks - there was a copy-and-paste error (should have grepped for "fetch< version 2", not "git< version 2").

Jonathan Tan (3):
  upload-pack: make have_obj not global
  upload-pack: make want_obj not global
  upload-pack: clear flags before each v2 request

 t/t5702-protocol-v2.sh |  25 +++++++
 upload-pack.c          | 153 ++++++++++++++++++++++++-----------------
 2 files changed, 115 insertions(+), 63 deletions(-)

-- 
2.19.0.271.gfe8321ec05.dirty