On Wed, Jan 20, 2021 at 09:50:19AM +0100, Ævar Arnfjörð Bjarmason wrote: > To elaborate on other things that aren't really your problem & Taylor's > E-Mail downthread we originally added this because: > > If new annotated tags have been introduced then we can also include > them in the packfile, saving the client from needing to request them > through a second connection. > > I've barked up this whole tag fetch tree before 97716d217c (fetch: add a > --prune-tags option and fetch.pruneTags config, 2018-02-09) but really > not this specific area. > > I wonder if longer term simply moving this to be work the client does > wouldn't make more sense. I.e. if we just delete this for_each_ref() > loop. > > We're just saving a client from saying they "want" a tag. I.e. with the > whole thing removed we need this to make t5503-tagfollow.sh pass (see > [1] at the end for the dialog, the tag is 3253df4d...): Isn't this an ordering problem, though? The client cannot mention auto-followed tags in a "want", because they first need to "want" the main history, receive the pack, and then realize they want the others. So we are not just saving the client from sending a "want", but making a second full connection to do so. That seems to be an optimization worth continuing to have. But here... > diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh > index 6041a4dd32..1ddc430aef 100755 > --- a/t/t5503-tagfollow.sh > +++ b/t/t5503-tagfollow.sh > @@ -134,6 +134,7 @@ test_expect_success 'fetch B, S (commit and tag : 1 connection)' ' > test_expect_success 'setup expect' ' > cat - <<EOF >expect > want $B > +want $T > want $S > EOF > ' > @@ -146,6 +147,7 @@ test_expect_success 'new clone fetch master and tags' ' > cd clone2 && > git init && > git remote add origin .. && > + git config --add remote.origin.fetch "+refs/tags/*:refs/tags/*" && > GIT_TRACE_PACKET=$UPATH git fetch && > test $B = $(git rev-parse --verify origin/master) && > test $S = $(git rev-parse --verify tag2) && > > We're also saving the client the work of having to go through > refs/tags/* and figure out whether there are tags there that aren't on > our main history. You seem to be against auto-following at all. And certainly I can see an argument that it is not worth the trouble it causes. But it is the default behavior, and I suspect many people are relying on it. Fetching every tag indiscriminately is going to grab a bunch of extra unwanted objects in some repos. An obvious case is any time "clone --single-branch --depth" is used. Maybe I'm not quite understanding what you're proposing. -Peff
