git_clone() always fails if the remote doesn't have a default head

[torvalds]

We use libgit2 for the dive log data saving in subsurface (https://github.com/subsurface/subsurface).

The subsurface use of branches on the server side is bit odd, in that the server doesn't have a valid HEAD that points to any particular branch. But that doesn't matter - the client side will just ask to clone the repository with a specific branch.

However, that seems to have broken in libgit-1.2, because we have this call chain

  git_clone(branch) ->
     .. fetch data ..
       checkout_branch(branch)
        update_head_to_branch() ->
            update_head_to_new_branch(branch)

and that all works fine.

BUT

Then update_head_to_branch() for some reason does this:

    if ((retcode = git_remote_default_branch(&default_branch, remote)) < 0)
            goto cleanup;

and that fails with GIT_ENOTFOUND (-3) because the remote doesn't have a default branch. It literally has one single branch which is NOT a ref-link, it has just the one single branch we want to check out.

This behavior seems to have been introduced in commit 6bb3587 ("clone: set refs/remotes/origin/HEAD to default branch when branch is specified, attempt 2") and I don't see the reason for it. It makes checkout_branch() fail for no obvious reason.

The fix would seem to be to just remove the "retcode = " part, and say "if we don't find a remote default branch, then obviously we won't be updating it either". The problem is that since it sets retcode to GIT_ENOTFOUND, the whole operation will fail, and instead of checking out the branch, the whole git_clone() will fail and just remove all the data it downlaoded.

But since I don't really know the code, I'm not really willing to make a patch, even if the patch looks trivial.

So this issue is more of a "was that really intentional, and why?"

This was apparently merged in

https://github.com/libgit2/libgit2/pull/6010

almost a year ago (Aug 2021), but I didn't notice anything, because the problem only affects the git_clone() path (and since I already had the repo, and "git_fetch()" worked fine, it didn't cause problems). And probably most platforms where subsurface is used don't even have libgit-1.2 yet.

@ethomson @A-Ovchinnikov-mx

[ghost]

I believe this wasn't intentional because I don't know the code either 😅 I think I just wrote it this way because I saw the same pattern right above it, but didn't really realize that there might not be any default branch at all. So I believe the fix you've suggested in #6369 makes total sense. But it's been a year since the original story so I'd wait for @ethomson's take on it.

[ethomson]

Oops, going through old issues and didn't realize that this was still open. Fixed via #6369