Thread (126 messages) 126 messages, 9 authors, 2024-12-05

Re: [PATCH v15 09/10] fetch: set remote/HEAD if it does not exist

From: Josh Steadmon <hidden>
Date: 2024-12-05 20:11:49

On 2024.12.05 21:09, Bence Ferdinandy wrote:
On Thu Dec 05, 2024 at 20:50, Josh Steadmon [off-list ref] wrote:
quoted
On 2024.12.05 10:58, Josh Steadmon wrote:
quoted
On 2024.11.22 13:28, Bence Ferdinandy wrote:
quoted
When cloning a repository remote/HEAD is created, but when the user
creates a repository with git init, and later adds a remote, remote/HEAD
is only created if the user explicitly runs a variant of "remote
set-head". Attempt to set remote/HEAD during fetch, if the user does not
have it already set. Silently ignore any errors.

Signed-off-by: Bence Ferdinandy <redacted>
At $DAYJOB, we noticed that this breaks `git fetch --tags`, although I
haven't had a chance to figure out what causes the error just yet.

I was able to bisect down to this commit using Jonathan Tan's
reproduction script:

rm -rf test_tag_1 test_tag_2
GIT=~/git/bin-wrappers/git
mkdir test_tag_1 && cd test_tag_1
REMOTE=$(pwd)
$GIT init .
touch foo.txt
$GIT add foo.txt
$GIT commit foo.txt -m "commit one"
$GIT tag foo
cd ..
mkdir test_tag_2 && cd test_tag_2
$GIT init .
echo fetch --tags
$GIT fetch --tags "file://$REMOTE"
echo regular fetch
$GIT fetch "file://$REMOTE" 'refs/tags/*:refs/tags/*'
$GIT --version


Prior to this change, the first `$GIT fetch --tags "file://$REMOTE"`
fetches the `foo` tag; with this change, it does not.

FWIW, moving this:
@@ -1643,6 +1703,8 @@ static int do_fetch(struct transport *transport,
                                    "refs/tags/");
        }

+       strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
+
to just above the prior `if` block fixes our issue and doesn't break any
tests. However, I'm not sure yet why the order of ref_prefixes should
matter here.
Thanks for looking into this! I think the issue is with 

 $GIT fetch --tags "file://$REMOTE"

instead of adding a proper remote. Tbh, I've never seen the above syntax before, so first I just ran your script, which reproduced the issue for me, but then I modified it to use a proper remote which works as expected:

rm -rf test_tag_1 test_tag_2
GIT=~/git/bin-wrappers/git
mkdir test_tag_1 && cd test_tag_1
REMOTE=$(pwd)
$GIT init .
touch foo.txt
$GIT add foo.txt
$GIT commit foo.txt -m "commit one"
$GIT tag foo
cd ..
mkdir test_tag_2 && cd test_tag_2
$GIT init .
$GIT remote add origin $REMOTE
echo fetch --tags
$GIT fetch origin --tags 
echo regular fetch
$GIT fetch origin 'refs/tags/*:refs/tags/*'
$GIT --version

So I'm assuming this is why also the tests never caught this, since probably
all of them are using `git remote add`.
Yeah, I think the issue is that we check the `--tags` flag first, but
only add the ref_prefixes entry if it's not empty already. Then after
that we unconditionally add HEAD. So that's why moving your strvec_push
earlier fixes it. I'll send a fix + test patch in just a minute.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help