Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2 6/6] clone: print advice on checking out detached HEAD
From: Nguyen Thai Ngoc Duy @ 2012-01-10  5:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7vlipgb1r1.fsf@alter.siamese.dyndns.org>

On Tue, Jan 10, 2012 at 11:49 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> 2012/1/10 Junio C Hamano <gitster@pobox.com>:
>>> This patch makes 100% sense _if_ we let clone result in a repository with
>>> a detached HEAD, which I am not sure if it is a good idea, or if it is
>>> better to fail the attempt to clone to give incentive to the owner of the
>>> remote repository to fix it.
>>
>> Then a hostile remote can stop users from cloning his repository by
>> detaching HEAD? That's not nice.
>
> That's crazy talk. Why does anybody from a hostile remote to begin with?

The point is, why punish client for remote's problems? If I have to
talk to upstream and wait for them to fix their repository, I might as
well give up cloning and move on. It's OK to annoy users to the point
that they ask upstream for a fix, but we should not disallow clone in
that case.
-- 
Duy

^ permalink raw reply

* Re: git grep doesn't follow symbolic link
From: Ramkumar Ramachandra @ 2012-01-10  5:56 UTC (permalink / raw)
  To: Bertrand BENOIT; +Cc: git
In-Reply-To: <CAPRVejc7xND_8Y=Pb5rYGEcaKYUaX7_WkSro-_EL8tTGxkfY3Q@mail.gmail.com>

Hi Bertrand,

Bertrand BENOIT wrote:
> When using git grep, symbolic links are not followed.
> Is it a wanted behavior ?

I'd imagine so: symbolic links are not portable across different file
systems; Git's internal representation of a symbolic link is a file
containing the path of the file to be linked to.

> I've not found information about that in documentation, so I do a report.

Hm, the description says:

Look for specified patterns in the tracked files in the work tree, blobs
registered in the index file, or blobs in given tree objects.

Hm, "tracked files in the work tree" is definitely sub-optimal: "blobs
corresponding to the tracked files in the work tree" is probably
better.  Then again, I think the description is too cryptic for an
end-user: do you have any suggestions?  Have we mentioned how Git
handles symbolic links anywhere in the documentation?  If not, where
should this information go?

-- Ram

^ permalink raw reply

* Re: [PATCH v2 3/6] clone: factor out checkout code
From: Nguyen Thai Ngoc Duy @ 2012-01-10  5:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7vhb04b1bc.fsf@alter.siamese.dyndns.org>

On Tue, Jan 10, 2012 at 11:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> There's also one thing I missed in the commit message that it makes
>> update head code and checkout code more independent. Update head code
>> does not need to maintain our_head_points_at at the end for checkout
>> anymore.
>
> I like that reasoning in general. The logic ought to be:
>
>  - Learn what the remote has;
>
>  - Combine it with --branch parameter, determine what local branch our
>   head _should_ point at;
>
>  - Make our head point at it, and check it out.
>
> I wonder if we can somehow make the above logic more clear in the
> code. Perhaps the first two could be made into a single helper function
> "decide_local_branch()", and the third would be the "checkout()" function
> in your patch, updated to take "const char *" parameter or something?

yeah, I split the first two into update_head() but dropped it for some
reasons I don't remember. That would make the main function easier to
follow. I'll look at it again.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] gitignore: warn about pointless syntax
From: Junio C Hamano @ 2012-01-10  6:01 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: Jeff King, git, trast
In-Reply-To: <alpine.LNX.2.01.1201100639340.11534@frira.zrqbmnf.qr>

Jan Engelhardt <jengelh@medozas.de> writes:

> On Monday 2012-01-09 23:33, Jeff King wrote:
>>On Mon, Jan 09, 2012 at 11:43:21AM -0800, Junio C Hamano wrote:
>>
>>>>>+static inline void check_bogus_wildcard(const char *file, const char *p)
>>>>>+{
>>>>>+	if (strstr(p, "**") == NULL)
>>>>>+		return;
>>>>>+	warning(_("Pattern \"%s\" from file \"%s\": Double asterisk does not "
>>>>>+		"have a special meaning and is interpreted just like a single "
>>>>>+		"asterisk.\n"), file, p);
>>
>>You only have to implement proper backslash decoding, so I think it is
>>not as hard as reimplementing fnmatch:
>>[...]
>>
>>That being said, if this is such a commonly-requested feature
>
> Was it actually requested, or did you mean "commonly attempted use"?
>
> As I see it, foo/**/*.o for example is equal to placing "*.o" in
> foo/.gitignore, so the feature is already implemented, just not
> through the syntax people falsely assume it is.

You can either adjust the people, i.e. teach that their "false" assumption
is wrong and the feature they expect is available but not in a way that
they expect.

Or you can adjust the tool to match their expectation.

The point that Peff correctly read between my lines is that in real life,
people are harder to train than tools and often the latter is a better
approach, especially if it does not amount to too much more work than
doing the former.

^ permalink raw reply

* Re: [PATCH] gitignore: warn about pointless syntax
From: Jan Engelhardt @ 2012-01-10  7:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git, trast
In-Reply-To: <7vd3asayfx.fsf@alter.siamese.dyndns.org>

On Tuesday 2012-01-10 07:01, Junio C Hamano wrote:

>>>That being said, if this is such a commonly-requested feature
>>
>>Was it actually requested, or did you mean "commonly attempted use"?
>>As I see it, foo/**/*.o for example is equal to placing "*.o" in
>>foo/.gitignore, so the feature is already implemented, just not
>>through the syntax people falsely assume it is.
>
>You can either adjust the people, i.e. teach that their "false" assumption
>is wrong and the feature they expect is available but not in a way that
>they expect. Or you can adjust the tool to match their expectation.
>[...]in real life, people are harder to train than tools[...]

Though, having one more way to do things leads to a certain mess at a 
later point, if such mess is not already present. I am thinking here of 
the precedent iptables option parser set by removing support for 
exclamation marks in odd positions, as it was redundant ("more than 1 
way"), was only supported by ~45% of all options and had to be 
explicitly invoked at every callsite - so in fact was harder on users 
than git would be for **. There were a few mails by people who could not 
seem to read error messages, but overall, within 6-9 months, everything 
was quiet again. So, that's the empiric result of what teaching-the-tool 
would do.

^ permalink raw reply

* Re: [PATCH] gitignore: warn about pointless syntax
From: Jan Engelhardt @ 2012-01-10  7:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git, trast
In-Reply-To: <alpine.LNX.2.01.1201100744340.17336@frira.zrqbmnf.qr>

On Tuesday 2012-01-10 08:01, Jan Engelhardt wrote:
>>
>>You can either adjust the people, i.e. teach that their "false" assumption
>>is wrong and the feature they expect is available but not in a way that
>>they expect. Or you can adjust the tool to match their expectation.
>>[...]in real life, people are harder to train than tools[...]
>
>Though, having one more way to do things leads to a certain mess at a 
>later point, if such mess is not already present. I am thinking here of 
>the precedent iptables option parser set by removing support for 
>exclamation marks in odd positions, as it was redundant ("more than 1 
>way"), was only supported by ~45% of all options and had to be 
>explicitly invoked at every callsite - so in fact was harder on users 
>than git would be for **. There were a few mails by people who could not 
>seem to read error messages, but overall, within 6-9 months, everything 
>was quiet again. So, that's the empiric result of what teaching-the-tool 
>would do.

~ teaching-the-user would entail - it's factually problemfree.

^ permalink raw reply

* [BUG] gitattribute macro expansion oddity
From: Jeff King @ 2012-01-10  7:03 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Henrik Grubbström, git-dev

I'm seeing some very odd behavior with git's attribute expansion for
diffs. You can see it with this repository:

  git clone git://github.com/libgit2/libgit2sharp.git

Try a diff of a non-binary file:

  $ git show 2a0f4bf7 LibGit2Sharp/Configuration.cs
  ...
  diff --git a/LibGit2Sharp/Configuration.cs b/LibGit2Sharp/Configuration.cs
  index 83cc9d6..9ab0b60 100644
  --- a/LibGit2Sharp/Configuration.cs
  +++ b/LibGit2Sharp/Configuration.cs

Looks OK. Now try a diff that also has a binary file (that is marked
such via gitattributes):

  $ git show 2a0f4bf7 Lib/NativeBinaries/x86/git2.dll \
                      LibGit2Sharp/Configuration.cs
  ...
  diff --git a/Lib/NativeBinaries/x86/git2.dll b/Lib/NativeBinaries/x86/git2.dll
  index dab0d04..8de18ab 100644
  Binary files a/Lib/NativeBinaries/x86/git2.dll and b/Lib/NativeBinaries/x86/git2.dll differ
  diff --git a/LibGit2Sharp/Configuration.cs b/LibGit2Sharp/Configuration.cs
  index 83cc9d6..9ab0b60 100644
  Binary files a/LibGit2Sharp/Configuration.cs and b/LibGit2Sharp/Configuration.cs differ

Now the Configuration.cs blobs appear binary!

It has nothing to do with pathspecs; if you do a non-limited diff of
2a0f4bf7, you'll see many of the files appear as binary. Running it
through the debugger, it looks like we are getting wrong diff attribute
values for later paths, as if the earlier lookups are somehow polluting
the attribute stack.

The gitattributes in this repository look reasonably sane, but even if
they were not, nothing should make a file have different attributes
based on other files that were diffed.

Bisection points to ec775c4 (attr: Expand macros immediately when
encountered., 2010-04-06), but it's too late for me to dig further
tonight. Cc'ing Junio as the author of the attr code and Henrik as the
author of ec775c4.

-Peff

^ permalink raw reply

* Re: [linux.conf.au] VCS Interoperability
From: Ramkumar Ramachandra @ 2012-01-10  8:48 UTC (permalink / raw)
  To: David Michael Barr; +Cc: git, Junio C Hamano, Jonathan Nieder, Dmitry Ivankov
In-Reply-To: <CAFfmPPMH2643JMMZdVbOQJL7DB-DiRYQS8x0TqEaSbGmhMdBNw@mail.gmail.com>

Hi David,

David Michael Barr wrote:
> Next week, I'll be presenting  a summary of the past 2 years work
> on improving svn interoperability for git.
> I'm requesting feedback from anyone who cares with regard to
> what they'd like to hear about.

Nice.  As a lay person attending the conference, here are a few things
I think I'd like to hear:
- Why this project is so challenging compared to say, a git-hg bridge
or a git-darcs bridge.  What makes Subversion especially hard to deal
with?
- What is the biggest motivation for developing the svnrdump/ svnrload
combination?  Are there any other usecases for the tools?
- How has this project contributed to the development of the
fast-import infrastructure?  Can these changes be used to improve
other/ future remote helpers?
- You've spoken about exporting Subversion history to Git so far, but
what about the reverse?  Which parts of the picture are still missing?

Thanks.

-- Ram

^ permalink raw reply

* Re: [BUG] gitattribute macro expansion oddity
From: Michael Haggerty @ 2012-01-10  9:01 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, Henrik Grubbström, git-dev
In-Reply-To: <20120110070300.GA17086@sigill.intra.peff.net>

On 01/10/2012 08:03 AM, Jeff King wrote:
> I'm seeing some very odd behavior with git's attribute expansion for
> diffs. You can see it with this repository:
> 
>   git clone git://github.com/libgit2/libgit2sharp.git
> 
> Try a diff of a non-binary file: [...]

The problem has nothing with diffing; simply interrogating the attribute
values gives different results depending on the order of the files:

$ git check-attr --all Lib/NativeBinaries/x86/git2.dll
LibGit2Sharp/Configuration.cs
Lib/NativeBinaries/x86/git2.dll: binary: set
Lib/NativeBinaries/x86/git2.dll: diff: unset
Lib/NativeBinaries/x86/git2.dll: text: unset
LibGit2Sharp/Configuration.cs: binary: set
LibGit2Sharp/Configuration.cs: diff: unset
LibGit2Sharp/Configuration.cs: text: unset
LibGit2Sharp/Configuration.cs: crlf: set
$ git check-attr --all LibGit2Sharp/Configuration.cs
Lib/NativeBinaries/x86/git2.dll
LibGit2Sharp/Configuration.cs: diff: csharp
LibGit2Sharp/Configuration.cs: crlf: set
Lib/NativeBinaries/x86/git2.dll: binary: set
Lib/NativeBinaries/x86/git2.dll: diff: unset
Lib/NativeBinaries/x86/git2.dll: text: unset

It also doesn't depend on the fact that Lib/.gitattributes uses CRLF as
its EOL, nor does it depend on the use of the "binary" macro.  However,
it does depend on the fact that the directory name "Lib" matches the
first part of the directory name "LibGit2Sharp".  Here is a simplified
demonstration of the problem:

a=LibA/a.txt
b=Lib/b.bin
rm -rf foo
git init foo
cd foo
mkdir $(dirname $a) $(dirname $b)
touch $a $b
echo '*.txt foo' >.gitattributes
echo '* bar' >$(dirname $b)/.gitattributes
git add .
git commit -am 'Demonstrate problem'
echo '================================================='
git check-attr --all $b $a
echo '================================================='
git check-attr --all $a $b
echo '================================================='

The attributes of $a are different depending on what order $a and $b
appear in the "git check-attr" command line.

Changing the example to "a=foo/a.txt" makes the problem go away.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: [PATCH] gitignore: warn about pointless syntax
From: Thomas Rast @ 2012-01-10  9:44 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: Jeff King, Junio C Hamano, git
In-Reply-To: <alpine.LNX.2.01.1201100639340.11534@frira.zrqbmnf.qr>

Jan Engelhardt <jengelh@medozas.de> writes:
>
> As I see it, foo/**/*.o for example is equal to placing "*.o" in
> foo/.gitignore, so the feature is already implemented, just not
> through the syntax people falsely assume it is. And that is the
> reason for wanting to output a warning. If it was me, I'd even make
> it use error(),

No, please don't even think about it.  Having an error() there would
mean that git would be essentially useless in repositories that have
this mistake, and the user may not be in a position to fix the
repository!

(He could drag along a local change to that effect, which is highly
annoying if the repository is otherwise read-only for him.)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: git-send-email: bug with sendemail.multiedit
From: Thomas Rast @ 2012-01-10  9:56 UTC (permalink / raw)
  To: Jean-Francois Dagenais; +Cc: Pierre Habouzit, pierre.habouzit, git
In-Reply-To: <1AC16B4B-8376-4A50-A900-BB8E704FAB82@gmail.com>

Jean-Francois Dagenais <jeff.dagenais@gmail.com> writes:

> Bonjour Pierre! ... and all git developers!
>
> I think there is a bug with git-send-email.perl's evaluation of the sendemail.multiedit config variable.
>
> I was only able to make the "do_edit()" function detect it as false by setting the variable to "0" instead
> of "false", like so:
>
> git config --global sendemail.multiedit 0
>
> otherwise do_edit evaluates it as true and invokes the editor with all files as argument.

The patch below looks like the obvious fix.  Perhaps you can test it?

diff --git i/git-send-email.perl w/git-send-email.perl
index d491db9..7ac0a7d 100755
--- i/git-send-email.perl
+++ w/git-send-email.perl
@@ -210,6 +210,7 @@ sub do_edit {
     "signedoffbycc" => [\$signed_off_by_cc, undef],
     "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
     "validate" => [\$validate, 1],
+    "multiedit" => [\$multiedit, undef],
 );
 
 my %config_settings = (
@@ -227,7 +228,6 @@ sub do_edit {
     "bcc" => \@bcclist,
     "suppresscc" => \@suppress_cc,
     "envelopesender" => \$envelope_sender,
-    "multiedit" => \$multiedit,
     "confirm"   => \$confirm,
     "from" => \$sender,
     "assume8bitencoding" => \$auto_8bit_encoding,

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply related

* [PATCH v3 00/10] nd/clone-detached
From: Nguyễn Thái Ngọc Duy @ 2012-01-10  9:56 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>

Compare to v2, this round does more refactoring, which makes
cmd_clone() looks easier to follow in the end, in my opinion.

There's also 7/10 that refuses --branch=<nonexistent>. I don't know if
I react too strong. The current behavior is fall back to remote's HEAD
(and detached HEAD if remote's HEAD is detached). Maybe we should only
refuse it when it leads to detached HEAD and let it fall back to
remote's HEAD otherwise.

The last two patches remain debatable. If we disallow detached HEAD
from new clones, perhaps we could put <tag>^{commit} to
refs/heads/master then drop the last patch. t3501.6, t5527.2, t5707.5,
t7406.29 likes to have detached HEAD, but those can be fixed.

Nguyễn Thái Ngọc Duy (10):
  t5601: add missing && cascade
  clone: write detached HEAD in bare repositories
  clone: factor out checkout code
  clone: factor out HEAD update code
  clone: factor out remote ref writing
  clone: delay cloning until after remote HEAD checking
  clone: --branch=<branch> always means refs/heads/<branch>
  clone: refuse to clone if --branch points to bogus ref
  clone: allow --branch to take a tag
  clone: print advice on checking out detached HEAD

 Documentation/git-clone.txt |    5 +-
 advice.c                    |   14 +++
 advice.h                    |    1 +
 builtin/checkout.c          |   16 +---
 builtin/clone.c             |  252 +++++++++++++++++++++++++------------------
 t/t5601-clone.sh            |   40 ++++++-
 t/t5706-clone-branch.sh     |    8 +-
 transport.c                 |    5 +-
 8 files changed, 207 insertions(+), 134 deletions(-)

-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply

* [PATCH v3 01/10] t5601: add missing && cascade
From: Nguyễn Thái Ngọc Duy @ 2012-01-10  9:56 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t5601-clone.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 87ee016..49821eb 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -9,9 +9,9 @@ test_expect_success setup '
 	rm -fr .git &&
 	test_create_repo src &&
 	(
-		cd src
-		>file
-		git add file
+		cd src &&
+		>file &&
+		git add file &&
 		git commit -m initial
 	)
 
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH v3 02/10] clone: write detached HEAD in bare repositories
From: Nguyễn Thái Ngọc Duy @ 2012-01-10  9:56 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>

If we don't write, HEAD is still at refs/heads/master as initialized
by init-db, which may or may not match remote's HEAD.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/clone.c  |    9 +++------
 t/t5601-clone.sh |   25 ++++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 86db954..8dde1ea 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -720,12 +720,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		}
 	} else if (remote_head) {
 		/* Source had detached HEAD pointing somewhere. */
-		if (!option_bare) {
-			update_ref(reflog_msg.buf, "HEAD",
-				   remote_head->old_sha1,
-				   NULL, REF_NODEREF, DIE_ON_ERR);
-			our_head_points_at = remote_head;
-		}
+		update_ref(reflog_msg.buf, "HEAD", remote_head->old_sha1,
+			   NULL, REF_NODEREF, DIE_ON_ERR);
+		our_head_points_at = remote_head;
 	} else {
 		/* Nothing to checkout out */
 		if (!option_no_checkout)
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 49821eb..e0b8db6 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -12,7 +12,10 @@ test_expect_success setup '
 		cd src &&
 		>file &&
 		git add file &&
-		git commit -m initial
+		git commit -m initial &&
+		echo 1 >file &&
+		git add file &&
+		git commit -m updated
 	)
 
 '
@@ -88,6 +91,26 @@ test_expect_success 'clone --mirror' '
 
 '
 
+test_expect_success 'clone --mirror with detached HEAD' '
+
+	( cd src && git checkout HEAD^ && git rev-parse HEAD >../expected ) &&
+	git clone --mirror src mirror.detached &&
+	( cd src && git checkout - ) &&
+	GIT_DIR=mirror.detached git rev-parse HEAD >actual &&
+	test_cmp expected actual
+
+'
+
+test_expect_success 'clone --bare with detached HEAD' '
+
+	( cd src && git checkout HEAD^ && git rev-parse HEAD >../expected ) &&
+	git clone --bare src bare.detached &&
+	( cd src && git checkout - ) &&
+	GIT_DIR=bare.detached git rev-parse HEAD >actual &&
+	test_cmp expected actual
+
+'
+
 test_expect_success 'clone --bare names the local repository <name>.git' '
 
 	git clone --bare src &&
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH v3 03/10] clone: factor out checkout code
From: Nguyễn Thái Ngọc Duy @ 2012-01-10  9:57 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>

Read HEAD from disk instead of relying on local variable
our_head_points_at. This reduces complexity of cmd_clone() a little
bit.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/clone.c |  101 +++++++++++++++++++++++++++++++-----------------------
 1 files changed, 58 insertions(+), 43 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 8dde1ea..100056d 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -448,6 +448,63 @@ static void write_remote_refs(const struct ref *local_refs)
 	clear_extra_refs();
 }
 
+static int checkout(void)
+{
+	unsigned char sha1[20];
+	char *head;
+	struct lock_file *lock_file;
+	struct unpack_trees_options opts;
+	struct tree *tree;
+	struct tree_desc t;
+	int err = 0, fd;
+
+	if (option_no_checkout)
+		return 0;
+
+	head = resolve_refdup("HEAD", sha1, 1, NULL);
+	if (!head) {
+		warning(_("remote HEAD refers to nonexistent ref, "
+			  "unable to checkout.\n"));
+		return 0;
+	}
+	if (strcmp(head, "HEAD")) {
+		if (prefixcmp(head, "refs/heads/"))
+			die(_("HEAD not found below refs/heads!"));
+	}
+	free(head);
+
+	/* We need to be in the new work tree for the checkout */
+	setup_work_tree();
+
+	lock_file = xcalloc(1, sizeof(struct lock_file));
+	fd = hold_locked_index(lock_file, 1);
+
+	memset(&opts, 0, sizeof opts);
+	opts.update = 1;
+	opts.merge = 1;
+	opts.fn = oneway_merge;
+	opts.verbose_update = (option_verbosity > 0);
+	opts.src_index = &the_index;
+	opts.dst_index = &the_index;
+
+	tree = parse_tree_indirect(sha1);
+	parse_tree(tree);
+	init_tree_desc(&t, tree->buffer, tree->size);
+	unpack_trees(1, &t, &opts);
+
+	if (write_cache(fd, active_cache, active_nr) ||
+	    commit_locked_index(lock_file))
+		die(_("unable to write new index file"));
+
+	err |= run_hook(NULL, "post-checkout", sha1_to_hex(null_sha1),
+			sha1_to_hex(sha1), "1", NULL);
+
+	if (!err && option_recursive)
+		err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
+
+	return err;
+}
+
 static int write_one_config(const char *key, const char *value, void *data)
 {
 	return git_config_set_multivar(key, value ? value : "true", "^$", 0);
@@ -722,13 +779,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		/* Source had detached HEAD pointing somewhere. */
 		update_ref(reflog_msg.buf, "HEAD", remote_head->old_sha1,
 			   NULL, REF_NODEREF, DIE_ON_ERR);
-		our_head_points_at = remote_head;
-	} else {
-		/* Nothing to checkout out */
-		if (!option_no_checkout)
-			warning(_("remote HEAD refers to nonexistent ref, "
-				"unable to checkout.\n"));
-		option_no_checkout = 1;
 	}
 
 	if (transport) {
@@ -736,42 +786,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		transport_disconnect(transport);
 	}
 
-	if (!option_no_checkout) {
-		struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
-		struct unpack_trees_options opts;
-		struct tree *tree;
-		struct tree_desc t;
-		int fd;
-
-		/* We need to be in the new work tree for the checkout */
-		setup_work_tree();
-
-		fd = hold_locked_index(lock_file, 1);
-
-		memset(&opts, 0, sizeof opts);
-		opts.update = 1;
-		opts.merge = 1;
-		opts.fn = oneway_merge;
-		opts.verbose_update = (option_verbosity > 0);
-		opts.src_index = &the_index;
-		opts.dst_index = &the_index;
-
-		tree = parse_tree_indirect(our_head_points_at->old_sha1);
-		parse_tree(tree);
-		init_tree_desc(&t, tree->buffer, tree->size);
-		unpack_trees(1, &t, &opts);
-
-		if (write_cache(fd, active_cache, active_nr) ||
-		    commit_locked_index(lock_file))
-			die(_("unable to write new index file"));
-
-		err |= run_hook(NULL, "post-checkout", sha1_to_hex(null_sha1),
-				sha1_to_hex(our_head_points_at->old_sha1), "1",
-				NULL);
-
-		if (!err && option_recursive)
-			err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
-	}
+	err = checkout();
 
 	strbuf_release(&reflog_msg);
 	strbuf_release(&branch_top);
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH v3 04/10] clone: factor out HEAD update code
From: Nguyễn Thái Ngọc Duy @ 2012-01-10  9:57 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/clone.c |   36 +++++++++++++++++++-----------------
 1 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 100056d..26a037c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -448,6 +448,24 @@ static void write_remote_refs(const struct ref *local_refs)
 	clear_extra_refs();
 }
 
+static void update_head(const struct ref *our, const struct ref *remote,
+			const char *msg)
+{
+	if (our) {
+		/* Local default branch link */
+		create_symref("HEAD", our->name, NULL);
+		if (!option_bare) {
+			const char *head = skip_prefix(our->name, "refs/heads/");
+			update_ref(msg, "HEAD", our->old_sha1, NULL, 0, DIE_ON_ERR);
+			install_branch_config(0, head, option_origin, our->name);
+		}
+	} else if (remote) {
+		/* Source had detached HEAD pointing somewhere. */
+		update_ref(msg, "HEAD", remote->old_sha1,
+			   NULL, REF_NODEREF, DIE_ON_ERR);
+	}
+}
+
 static int checkout(void)
 {
 	unsigned char sha1[20];
@@ -763,23 +781,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 			      reflog_msg.buf);
 	}
 
-	if (our_head_points_at) {
-		/* Local default branch link */
-		create_symref("HEAD", our_head_points_at->name, NULL);
-		if (!option_bare) {
-			const char *head = skip_prefix(our_head_points_at->name,
-						       "refs/heads/");
-			update_ref(reflog_msg.buf, "HEAD",
-				   our_head_points_at->old_sha1,
-				   NULL, 0, DIE_ON_ERR);
-			install_branch_config(0, head, option_origin,
-					      our_head_points_at->name);
-		}
-	} else if (remote_head) {
-		/* Source had detached HEAD pointing somewhere. */
-		update_ref(reflog_msg.buf, "HEAD", remote_head->old_sha1,
-			   NULL, REF_NODEREF, DIE_ON_ERR);
-	}
+	update_head(our_head_points_at, remote_head, reflog_msg.buf);
 
 	if (transport) {
 		transport_unlock_pack(transport);
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH v3 05/10] clone: factor out remote ref writing
From: Nguyễn Thái Ngọc Duy @ 2012-01-10  9:57 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/clone.c |   36 ++++++++++++++++++++++++------------
 1 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 26a037c..73d07ed 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -448,6 +448,28 @@ static void write_remote_refs(const struct ref *local_refs)
 	clear_extra_refs();
 }
 
+static void update_remote_refs(const struct ref *refs,
+			       const struct ref *mapped_refs,
+			       const struct ref *remote_head_points_at,
+			       const char *branch_top,
+			       const char *msg)
+{
+	if (refs) {
+		clear_extra_refs();
+		write_remote_refs(mapped_refs);
+	}
+
+	if (remote_head_points_at && !option_bare) {
+		struct strbuf head_ref = STRBUF_INIT;
+		strbuf_addstr(&head_ref, branch_top);
+		strbuf_addstr(&head_ref, "HEAD");
+		create_symref(head_ref.buf,
+			      remote_head_points_at->peer_ref->name,
+			      msg);
+	}
+
+}
+
 static void update_head(const struct ref *our, const struct ref *remote,
 			const char *msg)
 {
@@ -735,10 +757,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	}
 
 	if (refs) {
-		clear_extra_refs();
-
-		write_remote_refs(mapped_refs);
-
 		remote_head = find_ref_by_name(refs, "HEAD");
 		remote_head_points_at =
 			guess_remote_head(remote_head, mapped_refs, 0);
@@ -772,14 +790,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 					      "refs/heads/master");
 	}
 
-	if (remote_head_points_at && !option_bare) {
-		struct strbuf head_ref = STRBUF_INIT;
-		strbuf_addstr(&head_ref, branch_top.buf);
-		strbuf_addstr(&head_ref, "HEAD");
-		create_symref(head_ref.buf,
-			      remote_head_points_at->peer_ref->name,
-			      reflog_msg.buf);
-	}
+	update_remote_refs(refs, mapped_refs, remote_head_points_at,
+			   branch_top.buf, reflog_msg.buf);
 
 	update_head(our_head_points_at, remote_head, reflog_msg.buf);
 
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH v3 06/10] clone: delay cloning until after remote HEAD checking
From: Nguyễn Thái Ngọc Duy @ 2012-01-10  9:57 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>

This gives us an opportunity to abort the command during remote HEAD
check without wasting much bandwidth.

Cloning with remote-helper remains before the check because the remote
helper updates mapped_refs, which is necessary for remote ref checks.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 I'm not familiar with remote-helper to see if there's any better way
 to do this..

 builtin/clone.c |   54 +++++++++++++++++++++++++++---------------------------
 transport.c     |    5 ++++-
 2 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 73d07ed..05224d7 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -361,13 +361,8 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
 	closedir(dir);
 }
 
-static const struct ref *clone_local(const char *src_repo,
-				     const char *dest_repo)
+static void clone_local(const char *src_repo, const char *dest_repo)
 {
-	const struct ref *ret;
-	struct remote *remote;
-	struct transport *transport;
-
 	if (option_shared) {
 		struct strbuf alt = STRBUF_INIT;
 		strbuf_addf(&alt, "%s/objects", src_repo);
@@ -383,13 +378,8 @@ static const struct ref *clone_local(const char *src_repo,
 		strbuf_release(&dest);
 	}
 
-	remote = remote_get(src_repo);
-	transport = transport_get(remote, src_repo);
-	ret = transport_get_remote_refs(transport);
-	transport_disconnect(transport);
 	if (0 <= option_verbosity)
 		printf(_("done.\n"));
-	return ret;
 }
 
 static const char *junk_work_tree;
@@ -576,6 +566,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	struct strbuf branch_top = STRBUF_INIT, reflog_msg = STRBUF_INIT;
 	struct transport *transport = NULL;
 	char *src_ref_prefix = "refs/heads/";
+	struct remote *remote;
 	int err = 0;
 
 	struct refspec *refspec;
@@ -727,13 +718,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 	strbuf_reset(&value);
 
-	if (is_local) {
-		refs = clone_local(path, git_dir);
-		mapped_refs = wanted_peer_refs(refs, refspec);
-	} else {
-		struct remote *remote = remote_get(option_origin);
-		transport = transport_get(remote, remote->url[0]);
+	remote = remote_get(option_origin);
+	transport = transport_get(remote, remote->url[0]);
 
+	if (!is_local) {
 		if (!transport->get_refs_list || !transport->fetch)
 			die(_("Don't know how to clone %s"), transport->url);
 
@@ -748,14 +736,23 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		if (option_upload_pack)
 			transport_set_option(transport, TRANS_OPT_UPLOADPACK,
 					     option_upload_pack);
-
-		refs = transport_get_remote_refs(transport);
-		if (refs) {
-			mapped_refs = wanted_peer_refs(refs, refspec);
-			transport_fetch_refs(transport, mapped_refs);
-		}
 	}
 
+	refs = transport_get_remote_refs(transport);
+	mapped_refs = refs ? wanted_peer_refs(refs, refspec) : NULL;
+
+	/*
+	 * mapped_refs may be updated if transport-helper is used so
+	 * we need fetch it early because remote_head code below
+	 * relies on it.
+	 *
+	 * for normal clones, transport_get_remote_refs() should
+	 * return reliable ref set, we can delay cloning until after
+	 * remote HEAD check.
+	 */
+	if (!is_local && remote->foreign_vcs && refs)
+		transport_fetch_refs(transport, mapped_refs);
+
 	if (refs) {
 		remote_head = find_ref_by_name(refs, "HEAD");
 		remote_head_points_at =
@@ -790,15 +787,18 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 					      "refs/heads/master");
 	}
 
+	if (is_local)
+		clone_local(path, git_dir);
+	else if (refs && !remote->foreign_vcs)
+		transport_fetch_refs(transport, mapped_refs);
+
 	update_remote_refs(refs, mapped_refs, remote_head_points_at,
 			   branch_top.buf, reflog_msg.buf);
 
 	update_head(our_head_points_at, remote_head, reflog_msg.buf);
 
-	if (transport) {
-		transport_unlock_pack(transport);
-		transport_disconnect(transport);
-	}
+	transport_unlock_pack(transport);
+	transport_disconnect(transport);
 
 	err = checkout();
 
diff --git a/transport.c b/transport.c
index a99b7c9..aae9889 100644
--- a/transport.c
+++ b/transport.c
@@ -895,8 +895,10 @@ struct transport *transport_get(struct remote *remote, const char *url)
 
 		while (is_urlschemechar(p == url, *p))
 			p++;
-		if (!prefixcmp(p, "::"))
+		if (!prefixcmp(p, "::")) {
 			helper = xstrndup(url, p - url);
+			remote->foreign_vcs = helper;
+		}
 	}
 
 	if (helper) {
@@ -938,6 +940,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 		char *handler = xmalloc(len + 1);
 		handler[len] = 0;
 		strncpy(handler, url, len);
+		remote->foreign_vcs = helper;
 		transport_helper_init(ret, handler);
 	}
 
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH v3 07/10] clone: --branch=<branch> always means refs/heads/<branch>
From: Nguyễn Thái Ngọc Duy @ 2012-01-10  9:57 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>

It does not make sense to look outside refs/heads for HEAD's target
(src_ref_prefix can be set to "refs/" if --mirror is used) because ref
code only allows symref in form refs/heads/...

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/clone.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 05224d7..960242f 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -463,7 +463,7 @@ static void update_remote_refs(const struct ref *refs,
 static void update_head(const struct ref *our, const struct ref *remote,
 			const char *msg)
 {
-	if (our) {
+	if (our && !prefixcmp(our->name, "refs/heads/")) {
 		/* Local default branch link */
 		create_symref("HEAD", our->name, NULL);
 		if (!option_bare) {
@@ -760,7 +760,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 		if (option_branch) {
 			struct strbuf head = STRBUF_INIT;
-			strbuf_addstr(&head, src_ref_prefix);
+			strbuf_addstr(&head, "refs/heads/");
 			strbuf_addstr(&head, option_branch);
 			our_head_points_at =
 				find_ref_by_name(mapped_refs, head.buf);
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH v3 08/10] clone: refuse to clone if --branch points to bogus ref
From: Nguyễn Thái Ngọc Duy @ 2012-01-10  9:57 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>

It's possible that users make a typo in the branch name. Stop and let
users recheck. Falling back to remote's HEAD is not documented any
way.

Except when using remote helper, the pack has not been transferred at
this stage yet so we don't waste much bandwidth.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/clone.c         |   10 ++++------
 t/t5706-clone-branch.sh |    8 ++------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 960242f..f751997 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -766,12 +766,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 				find_ref_by_name(mapped_refs, head.buf);
 			strbuf_release(&head);
 
-			if (!our_head_points_at) {
-				warning(_("Remote branch %s not found in "
-					"upstream %s, using HEAD instead"),
-					option_branch, option_origin);
-				our_head_points_at = remote_head_points_at;
-			}
+			if (!our_head_points_at)
+				die(_("Remote branch %s not found in "
+				      "upstream %s, using HEAD instead"),
+				    option_branch, option_origin);
 		}
 		else
 			our_head_points_at = remote_head_points_at;
diff --git a/t/t5706-clone-branch.sh b/t/t5706-clone-branch.sh
index f3f9a76..56be67e 100755
--- a/t/t5706-clone-branch.sh
+++ b/t/t5706-clone-branch.sh
@@ -57,12 +57,8 @@ test_expect_success 'clone -b does not munge remotes/origin/HEAD' '
 	)
 '
 
-test_expect_success 'clone -b with bogus branch chooses HEAD' '
-	git clone -b bogus parent clone-bogus &&
-	(cd clone-bogus &&
-	 check_HEAD master &&
-	 check_file one
-	)
+test_expect_success 'clone -b with bogus branch' '
+	test_must_fail git clone -b bogus parent clone-bogus
 '
 
 test_done
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH v3 09/10] clone: allow --branch to take a tag
From: Nguyễn Thái Ngọc Duy @ 2012-01-10  9:57 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>

Because a tag ref cannot be put to HEAD, HEAD will become detached.
This is consistent with "git checkout <tag>".

This is mostly useful in shallow clone, where it allows you to clone a
tag in addtion to branches.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-clone.txt |    5 +++--
 builtin/clone.c             |   13 +++++++++++++
 t/t5601-clone.sh            |    9 +++++++++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 4b8b26b..db2b29c 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -146,8 +146,9 @@ objects from the source repository into a pack in the cloned repository.
 -b <name>::
 	Instead of pointing the newly created HEAD to the branch pointed
 	to by the cloned repository's HEAD, point to `<name>` branch
-	instead. In a non-bare repository, this is the branch that will
-	be checked out.
+	instead. `--branch` can also take tags and treat them like
+	detached HEAD. In a non-bare repository, this is the branch
+	that will be checked out.
 
 --upload-pack <upload-pack>::
 -u <upload-pack>::
diff --git a/builtin/clone.c b/builtin/clone.c
index f751997..ed18c1a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -471,6 +471,11 @@ static void update_head(const struct ref *our, const struct ref *remote,
 			update_ref(msg, "HEAD", our->old_sha1, NULL, 0, DIE_ON_ERR);
 			install_branch_config(0, head, option_origin, our->name);
 		}
+	} else if (our) {
+		struct commit *c = lookup_commit_reference(our->old_sha1);
+		/* Source had detached HEAD pointing somewhere. */
+		update_ref(msg, "HEAD", c->object.sha1,
+			   NULL, REF_NODEREF, DIE_ON_ERR);
 	} else if (remote) {
 		/* Source had detached HEAD pointing somewhere. */
 		update_ref(msg, "HEAD", remote->old_sha1,
@@ -766,6 +771,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 				find_ref_by_name(mapped_refs, head.buf);
 			strbuf_release(&head);
 
+			if (!our_head_points_at) {
+				strbuf_addstr(&head, "refs/tags/");
+				strbuf_addstr(&head, option_branch);
+				our_head_points_at =
+					find_ref_by_name(mapped_refs, head.buf);
+				strbuf_release(&head);
+			}
+
 			if (!our_head_points_at)
 				die(_("Remote branch %s not found in "
 				      "upstream %s, using HEAD instead"),
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index e0b8db6..67869b4 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -271,4 +271,13 @@ test_expect_success 'clone from original with relative alternate' '
 	grep /src/\\.git/objects target-10/objects/info/alternates
 '
 
+test_expect_success 'clone checking out a tag' '
+	git clone --branch=some-tag src dst.tag &&
+	GIT_DIR=src/.git git rev-parse some-tag >expected &&
+	test_cmp expected dst.tag/.git/HEAD &&
+	GIT_DIR=dst.tag/.git git config remote.origin.fetch >fetch.actual &&
+	echo "+refs/heads/*:refs/remotes/origin/*" >fetch.expected &&
+	test_cmp fetch.expected fetch.actual
+'
+
 test_done
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH v3 10/10] clone: print advice on checking out detached HEAD
From: Nguyễn Thái Ngọc Duy @ 2012-01-10  9:57 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 advice.c           |   14 ++++++++++++++
 advice.h           |    1 +
 builtin/checkout.c |   16 +---------------
 builtin/clone.c    |    5 ++++-
 4 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/advice.c b/advice.c
index e02e632..3e1a145 100644
--- a/advice.c
+++ b/advice.c
@@ -64,3 +64,17 @@ void NORETURN die_resolve_conflict(const char *me)
 	error_resolve_conflict(me);
 	die("Exiting because of an unresolved conflict.");
 }
+
+void detach_advice(const char *new_name)
+{
+	const char fmt[] =
+	"Note: checking out '%s'.\n\n"
+	"You are in 'detached HEAD' state. You can look around, make experimental\n"
+	"changes and commit them, and you can discard any commits you make in this\n"
+	"state without impacting any branches by performing another checkout.\n\n"
+	"If you want to create a new branch to retain commits you create, you may\n"
+	"do so (now or later) by using -b with the checkout command again. Example:\n\n"
+	"  git checkout -b new_branch_name\n\n";
+
+	fprintf(stderr, fmt, new_name);
+}
diff --git a/advice.h b/advice.h
index e5d0af7..7bda45b 100644
--- a/advice.h
+++ b/advice.h
@@ -14,5 +14,6 @@ int git_default_advice_config(const char *var, const char *value);
 void advise(const char *advice, ...);
 int error_resolve_conflict(const char *me);
 extern void NORETURN die_resolve_conflict(const char *me);
+void detach_advice(const char *new_name);
 
 #endif /* ADVICE_H */
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f1984d9..5bf96ba 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -514,20 +514,6 @@ static void report_tracking(struct branch_info *new)
 	strbuf_release(&sb);
 }
 
-static void detach_advice(const char *old_path, const char *new_name)
-{
-	const char fmt[] =
-	"Note: checking out '%s'.\n\n"
-	"You are in 'detached HEAD' state. You can look around, make experimental\n"
-	"changes and commit them, and you can discard any commits you make in this\n"
-	"state without impacting any branches by performing another checkout.\n\n"
-	"If you want to create a new branch to retain commits you create, you may\n"
-	"do so (now or later) by using -b with the checkout command again. Example:\n\n"
-	"  git checkout -b new_branch_name\n\n";
-
-	fprintf(stderr, fmt, new_name);
-}
-
 static void update_refs_for_switch(struct checkout_opts *opts,
 				   struct branch_info *old,
 				   struct branch_info *new)
@@ -575,7 +561,7 @@ static void update_refs_for_switch(struct checkout_opts *opts,
 			   REF_NODEREF, DIE_ON_ERR);
 		if (!opts->quiet) {
 			if (old->path && advice_detached_head)
-				detach_advice(old->path, new->name);
+				detach_advice(new->name);
 			describe_detached_head(_("HEAD is now at"), new->commit);
 		}
 	} else if (new->path) {	/* Switch branches. */
diff --git a/builtin/clone.c b/builtin/clone.c
index ed18c1a..70f0280 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -502,7 +502,10 @@ static int checkout(void)
 			  "unable to checkout.\n"));
 		return 0;
 	}
-	if (strcmp(head, "HEAD")) {
+	if (!strcmp(head, "HEAD")) {
+		if (advice_detached_head)
+			detach_advice(sha1_to_hex(sha1));
+	} else {
 		if (prefixcmp(head, "refs/heads/"))
 			die(_("HEAD not found below refs/heads!"));
 	}
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* Re: git grep doesn't follow symbolic link
From: Thomas Rast @ 2012-01-10 10:00 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Bertrand BENOIT, git
In-Reply-To: <CALkWK0=-LZH4MYhX50v-RWpGA2r+6q50YxsKaOxc0mJ__yuK7g@mail.gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Hi Bertrand,
>
> Bertrand BENOIT wrote:
>> When using git grep, symbolic links are not followed.
>> Is it a wanted behavior ?
>
> I'd imagine so: symbolic links are not portable across different file
> systems; Git's internal representation of a symbolic link is a file
> containing the path of the file to be linked to.

I'd actually welcome a fix to this general area, for an entirely
different reason.  With bash and ordinary diff I can do things like

  diff -u <(ls) <(cd elsewhere && ls) | less

But I lose all the cute features of git-diff.  I *could* say

  git diff --no-index <(ls) <(cd elsewhere && ls)

and it helpfully tells me

  diff --git 1/dev/fd/63 2/dev/fd/62
  index 55ccbe5..d796c45 120000
  --- 1/dev/fd/63
  +++ 2/dev/fd/62
  @@ -1 +1 @@
  -pipe:[607341]
  \ No newline at end of file
  +pipe:[607343]
  \ No newline at end of file

Of course that's diff and not grep, but I think they suffer from the
same flaw: they share the file-kind handling logic of the rest of git in
a case where it's not very helpful.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [bug] submodule --update insufficiently verbose
From: Thomas Rast @ 2012-01-10 10:43 UTC (permalink / raw)
  To: Dave Abrahams; +Cc: Git Mailing List
In-Reply-To: <m239bqnctf.fsf@pluto.luannocracy.com>

Dave Abrahams <dave@boostpro.com> writes:

> When a "git submodule --update" fails, e.g., due to a dirty working
> directory in one of the submodules, nothing is printed out indicating
> the submodule or directory in which the failure occurred.  This seems
> like a usability bug to me.

I assume you mean 'git submodule update'.  What git version is this, and
how do you reproduce?  I get a message like

  error: Your local changes to the following files would be overwritten by checkout:
          foo
  Please, commit your changes or stash them before you can switch branches.
  Aborting
  Unable to checkout '1fac3675a8c65b14e0a98e0136b5c43db97706fe' in submodule path 'sub'

in a simple test.  I am using a development version, but a brief glance
at the history seems to indictate that the 'Unable to ... in submodule
...' has always been there.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [PATCH v2 2/3] index-pack: eliminate recursion in find_unresolved_deltas
From: Nguyen Thai Ngoc Duy @ 2012-01-10 12:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Shawn O. Pearce
In-Reply-To: <7vzkdwcys4.fsf@alter.siamese.dyndns.org>

2012/1/10 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>
> I find both the original and the updated code rather dense to read without
> annotation, but from a cursory look all changes look good.

Maybe I stared at it for too long it seems obvious to me (hence no
further description in commit message). Let me describe it (and put in
commit message later if it makes sense)

Current code already links all bases together in a form of tree, using
struct base_data, with prev_base pointer to point to parent node. The
only problem is that struct base_data is all allocated on stack. So we
need to put all on heap (parse_pack_objects and
fix_unresolved_deltas). After that, it's simple depth-first traversal
where each node also maintains its own state (ofs and ref indices to
iterate over all children nodes).

So we process one node:

 - if it returns a new (child) node (a parent base), we link it to our
tree, then process the new node.
 - if it returns nothing, the node is done, free it. We go back to
parent node and resume whatever it's doing.

and do it until we have no nodes to process.
-- 
Duy

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox