Git development
 help / color / mirror / Atom feed
From: "Philip Oakley" <philipoakley@iee.org>
To: "Jeff King" <peff@peff.net>, "Junio C Hamano" <gitster@pobox.com>
Cc: "Félix Saparelli" <felix@passcod.name>, git@vger.kernel.org
Subject: Re: [Non-Bug] cloning a repository with a default MASTER branch tries to check out the master branch
Date: Fri, 26 May 2017 21:00:57 +0100	[thread overview]
Message-ID: <CEF93B59EA3F49608B41892952454B1D@PhilipOakley> (raw)
In-Reply-To: 20170525191115.tqd6zlj5mxqls4wp@sigill.intra.peff.net

been trying to keep up...

From: "Jeff King" <peff@peff.net>
> On Thu, May 25, 2017 at 11:59:24AM -0400, Jeff King wrote:
>
>> The just-HEAD case could look like:
>
> This patch does work, in the sense that upload-pack advertises the
> unborn HEAD symref. But the client doesn't actually do anything with it.
> The capability parsing happens in get_remote_heads(), which passes the
> data out by adding an annotation to the "struct ref" list. But of course
> we have no HEAD ref to annotate.
>
> So either get_remote_heads() would have to start returning a bogus HEAD
> ref (with a null sha1, I guess, which all callers would have to
> recognize). Or clone (and probably "remote set-head -a") would have to
> start reaching across the transport-module boundary and asking for any
> symref values for "HEAD". I'm not excited about more special-casing of
> "HEAD", though. In theory we'd want this for other symrefs in the long
> run, and it would be nice if clients were ready to handle that (even if
> the protocol isn't quite there).
>
> I dunno. I was thinking there might be a quick tweak, but I'm wondering
> if this arcane case is worth the restructuring we'd have to do to
> support it. It only comes up when you've moved the server repo's HEAD to
> an unborn branch _and_ you have other refs (since otherwise we don't
> even send capabilities at all!).
>
> -Peff

My original comment regarding Felix's report was based on when I was looking 
at the bundle code's disambiguation of refs which matched HEAD's sha1. Hence 
I had a mis-remembered impression that the HEAD - symref matching was 
avaibable.

At that time, Junio had suggested that, at least in the bundle file, the 
HEAD symref could be advertised immediately after a nul on the ref line.

At least for regular git, the sha1and symref value would included in the 
read line, and the current string processing [1] would not notice the extra 
symref data. This extra data could then be read (if present) from the end of 
the line, and the HEAD symref set.

What I then noticed (but didn't report to the list) was the option of adding 
that extra info to the PKLINE protocol.

<from my notes>
In technical\pack-protocol.txt #L136;158-160
Reference Discovery:

If HEAD is a valid ref, HEAD MUST appear as the first advertised
ref.  If HEAD is not a valid ref, HEAD MUST NOT appear in the
advertisement list at all, but other refs may still appear.
-

So, (for both upload pack, and bundle refs) the place to hide the HEAD is 
after the later ref that HEAD points to.
 e.g.(updating the example at #L147):
   00441d3fcd5ced445d1abc402225c0b8a1299641f497 
refs/heads/integration\0HEAD[LF]

The potential issue is if there is a passed ref that is HEAD, but that HEAD 
itself isn't passed (especially for bundle)
<\from my notes>
--

However given the discussion about an unborn HEAD, the option here would be 
to also pass the NULL sha for the symref and then add the annotation 'HEAD' 
after an extra \0, in the same way that an active symref could be annotated 
with the '\0HEAD'. This would kill two birds with one stone!

These are still protocol changes but should squeeze into the existing 
processing using the \0 trick.

In the absence of the information, and the multi-use of the warning 
function, the current message is possible the best we can get.

Philip

[1] the question would be whether other git versions also use the same 
string processing so could be 'fooled' in the same way? I'd be interested to 
know if that was a possibility.




  parent reply	other threads:[~2017-05-26 20:01 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-22 21:42 [Bug] cloning a repository with a default MASTER branch tries to check out the master branch Félix Saparelli
2017-05-23  3:40 ` [Non-Bug] " Junio C Hamano
2017-05-23 23:24   ` Philip Oakley
2017-05-24 14:19     ` Jeff King
2017-05-25  1:36       ` Junio C Hamano
2017-05-25  3:13         ` Junio C Hamano
2017-05-25 15:59           ` Jeff King
2017-05-25 19:11             ` Jeff King
2017-05-25 23:28               ` Junio C Hamano
2017-05-26 20:00               ` Philip Oakley [this message]
2017-05-26 21:17                 ` Philip Oakley
2017-05-27 23:55                 ` Junio C Hamano
2017-05-28 11:21                   ` Philip Oakley
2017-05-28 12:57                     ` Junio C Hamano
2017-05-31  4:43                     ` Jeff King
2017-05-23  8:01 ` [Bug] " Samuel Lijin
2017-05-23 12:12   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CEF93B59EA3F49608B41892952454B1D@PhilipOakley \
    --to=philipoakley@iee.org \
    --cc=felix@passcod.name \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox