From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Dmitry Ivankov <divanorama@gmail.com>, git@vger.kernel.org
Subject: Re: git clone (ssh://) skips detached HEAD
Date: Fri, 3 Jun 2011 14:48:22 -0400 [thread overview]
Message-ID: <20110603184822.GA20600@sigill.intra.peff.net> (raw)
In-Reply-To: <7vaadyc2u0.fsf@alter.siamese.dyndns.org>
On Fri, Jun 03, 2011 at 09:11:19AM -0700, Junio C Hamano wrote:
> An alternative would be not to checkout anything when HEAD points at an
> object that does not exist, or point the HEAD at the default "master"
> branch just like in the case when we cannot guess uniquely. That way, we
> do not have to worry about having to fetch the orphaned detached HEAD,
> which is an unlikely thing the publisher wanted to feed to its recipients
> in the first place. I tend to prefer the former (i.e. resulting in no
> paths in the working tree, possibly with a big warning message "the
> repository does not suggest which branch to track---are you sure you
> wanted to clone from there?").
Yeah, I was tempted to go the "check out nothing if we don't have the
object" route. But my thinking was:
1. We have already been creating local detached HEADs to match a
remote detached HEAD since at least 8434c2f (Build in clone,
2008-04-27). Should we keep doing that? If so, I think it
introduces a somewhat confusing inconsistency from the user's
perspective. Why is a sight-seeing detached HEAD pointing into
history OK to checkout, but one with a commit on top is not OK?
2. Similar to the above, we already do have the object for a local
clone, which just copies the object db whole. So now there is an
inconsistency that:
git clone foo bar
will checkout out such a HEAD, but neither of:
git clone file://$PWD/foo bar
git clone git://host/foo bar
does.
3. Fetching and checking it out just seems like the most friendly and
the least surprising thing for the user. In 99% of cases, people
are cloning bare repositories whose HEADs likely won't detach
anyway (and if they did, they certainly wouldn't have made commits
on them). But in the rare case that I _do_ clone a non-bare repo,
what am I trying to accomplish?
Most likely I'm trying to make a new workspace, either to work on
the identical branch, or some other branch. In the former case,
making a detached HEAD (whether it points to history or to a new
commit) is what I would want. In the latter case, it doesn't really
matter what we put in HEAD, as the user is going to switch it
anyway. The only downside is that we may have transferred some
extra objects; in practice, this is probably not a big deal due to
deltas unless your detached commit is gigantic.
4. We're guessing at whether the user will want the objects on the
detached HEAD or not. Which means we're going to be wrong
sometimes. But I would rather err on the side of copying the extra
commits than not. The few extra bytes spent are a better downside
than:
$ git clone git://host/foo bar
$ ssh host && rm -rf foo
[oops, I actually wanted the detached commit!]
Though to be fair, if we printed a warning about the detached HEAD
during the first command, the user would hopefully not execute the
second one.
> We treat the symbolic-ref on the publishing end not as the "current"
> branch at all. It is used as the "suggested primary branch to track".
> So allowing to fetch from a repository with detached HEAD is already a
> weird setup.
By that argument, we should stop checking out any detached HEAD at all.
Which I agree makes sense from the point of view that clone is just
"init + remote add + fetch + checkout". But I think it's probably more
helpful to the user (and doesn't cost us much) to just checkout the
detached HEAD than to refuse to check out anything and print a warning.
If we're right, they're happy. If we're wrong, the solution in both
cases is to "git checkout" what they did want.
Possibly we should warn that the cloned HEAD is detached (maybe even
with the regular detached HEAD warning).
> I am hoping that we are not setting up origin/HEAD to point at
> anything in this case, as the remote is telling us that there is no
> suggested primary branch for the clients to track by having a detached
> HEAD to begin with.
No, we don't set up an origin/HEAD at all in that case; the handling for
that and our local HEAD are separate (and I was careful to maintain this
with my patch by not setting a peer_ref to store the HEAD we fetch; it
stays in core until we write it to our HEAD).
> Even if you are fetching your own (or your pal's) repository with a
> working tree to transfer a work-in-progress, any work on detached HEAD
> that is orphaned is too transitory, and it goes against my taste to
> let people fetch from it.
>
> But people are free to have bad taste ;-).
They can already fetch from it via "git fetch /your/pal HEAD". So this
is really just about clone. I think it is not even bad taste if your
workflow is:
1. You're in the middle of a rebase on a detached HEAD. You make an
amended commit, then continue. You get a conflict which is
confusing, and inspection causes you to blame your coworker.
2. You holler over the cubicle wall to your coworker, who runs "git
clone ~bob/project bobs-project". They inspect your current state
and try to cherry-pick the failed commit themselves. Either the
merge conflicts tell them what to tell you to fix your problem, or
perhaps they even resolve the conflicts themselves and let you
fetch the state back.
So I think it is not a matter of taste, but of "in some (rare) cases
this is useful, and in many cases it is useless". I just think there is
no reason not to be helpful to the people in the rare cases, as it costs
so little in the other case (and remember that _without_ detached orphan
commits, this is literally a no-op).
-Peff
next prev parent reply other threads:[~2011-06-03 18:48 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-01 21:30 git clone (ssh://) skips detached HEAD Dmitry Ivankov
2011-06-01 22:05 ` Jeff King
2011-06-01 22:18 ` Dmitry Ivankov
2011-06-01 22:22 ` Junio C Hamano
2011-06-01 22:47 ` Jeff King
2011-06-01 22:53 ` Jeff King
2011-06-03 5:09 ` Jeff King
2011-06-03 5:10 ` [PATCH 1/3] t: add tests for cloning remotes with " Jeff King
2011-06-03 5:11 ` [PATCH 2/3] consider only branches in guess_remote_head Jeff King
2011-06-03 5:18 ` [PATCH 3/3] clone: always fetch remote HEAD Jeff King
2011-06-03 5:36 ` Sverre Rabbelier
2011-06-03 5:43 ` Jeff King
2011-06-03 14:51 ` Jeff King
2011-06-03 16:28 ` Junio C Hamano
2011-06-03 18:10 ` Jeff King
2011-06-04 1:50 ` Sverre Rabbelier
2011-06-06 16:08 ` Jeff King
2011-06-06 0:47 ` Junio C Hamano
2011-06-06 1:00 ` Junio C Hamano
2011-06-06 13:05 ` Sverre Rabbelier
2011-06-06 13:57 ` Junio C Hamano
2011-06-06 16:11 ` Jeff King
2011-06-06 19:05 ` Sverre Rabbelier
2011-06-07 17:10 ` Jeff King
2011-06-07 17:20 ` Sverre Rabbelier
2011-06-07 17:18 ` [PATCH 0/8] minor import/export remote helper fixes Jeff King
2011-06-07 17:19 ` [PATCH 1/8] transport-helper: fix minor leak in push_refs_with_export Jeff King
2011-06-07 17:19 ` [PATCH 2/8] git-remote-testgit: exit gracefully after push Jeff King
2011-06-07 17:48 ` Sverre Rabbelier
2011-06-07 17:20 ` [PATCH 3/8] t5800: factor out some ref tests Jeff King
2011-06-07 17:22 ` Sverre Rabbelier
2011-06-07 17:20 ` [PATCH 4/8] t5800: document some non-functional parts of remote helpers Jeff King
2011-06-07 17:25 ` Sverre Rabbelier
2011-06-07 17:28 ` Jeff King
2011-06-07 17:34 ` Sverre Rabbelier
2011-06-07 17:51 ` Jeff King
2011-06-07 17:53 ` Sverre Rabbelier
2011-06-07 17:55 ` Jeff King
2011-06-08 23:19 ` Junio C Hamano
2011-06-09 0:11 ` Jeff King
2011-06-09 0:43 ` Junio C Hamano
2011-06-09 0:45 ` Jeff King
2011-06-09 6:20 ` Sverre Rabbelier
2011-06-07 17:20 ` [PATCH 5/8] teach remote-testgit to import non-HEAD refs Jeff King
2011-06-08 23:21 ` Junio C Hamano
2011-06-09 0:17 ` Jeff King
2011-06-07 17:21 ` [PATCH 6/8] teach remote-testgit to import multiple refs Jeff King
2011-06-07 17:21 ` [PATCH 7/8] transport-helper: don't feed bogus refs to export push Jeff King
2011-06-07 17:31 ` Sverre Rabbelier
2011-06-07 17:21 ` [PATCH 8/8] git_remote_helpers: push all refs during a non-local export Jeff King
2011-06-07 17:32 ` Sverre Rabbelier
2011-06-07 17:42 ` Jeff King
2011-06-07 17:44 ` Sverre Rabbelier
2011-06-06 20:31 ` [PATCH 3/3] clone: always fetch remote HEAD Junio C Hamano
2011-06-06 22:08 ` Jeff King
2011-06-07 23:01 ` Jeff King
2011-06-07 23:03 ` [PATCH 1/2] make copy_ref globally available Jeff King
2011-06-07 23:03 ` [PATCH 2/2] clone: always fetch remote HEAD Jeff King
2011-06-07 23:18 ` [PATCH 3/3] " Junio C Hamano
2011-06-03 16:11 ` git clone (ssh://) skips detached HEAD Junio C Hamano
2011-06-03 18:48 ` Jeff King [this message]
2011-06-01 22:42 ` Jakub Narebski
2011-06-01 22:51 ` Jeff King
2011-06-02 20:02 ` Jakub Narebski
2011-06-03 2:52 ` 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=20110603184822.GA20600@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=divanorama@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/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;
as well as URLs for NNTP newsgroup(s).