[PATCH] checkout: implement "-" shortcut name for last branch

[PATCH] checkout: implement "-" shortcut name for last branch

[Thomas Rast]

Let git-checkout save the old branch as a symref in LAST_HEAD, and
make 'git checkout -' switch back to LAST_HEAD, like 'cd -' does in
the shell.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

I really wished I had this earlier today.  I'm just not sure if it's a
good idea, or even possible, to reserve the '-'.  I can't seem to
check out a branch '-foo' with git-checkout, but it's easy to create
one with 'git branch -- -foo'.  git-check-ref-format(1) doesn't forbid
it either, although the actual 'git check-ref-format -foo' exits with
status 1.

 Documentation/git-checkout.txt         |    3 +++
 Documentation/gitrepository-layout.txt |    4 ++++
 builtin-checkout.c                     |   26 +++++++++++++++++++++++++-
 3 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 9cd5151..1397745 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -133,6 +133,9 @@ the conflicted merge in the specified paths.
 +
 When this parameter names a non-branch (but still a valid commit object),
 your HEAD becomes 'detached'.
++
+You may also specify "`-`", which denotes the last branch you were on
+before the current HEAD.
 
 
 Detached HEAD
diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index 1befca9..f506c98 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -123,6 +123,10 @@ is often called 'detached HEAD', and almost all commands work
 identically as normal.  See linkgit:git-checkout[1] for
 details.
 
+LAST_HEAD::
+	A symref that holds the value of HEAD before the last
+	branch switch.
+
 branches::
 	A slightly deprecated way to store shorthands to be used
 	to specify URL to 'git-fetch', 'git-pull' and 'git-push'
diff --git a/builtin-checkout.c b/builtin-checkout.c
index b5dd9c0..356ad6c 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -480,6 +480,15 @@ static void report_tracking(struct branch_info *new)
 	strbuf_release(&sb);
 }
 
+static void save_old_branch(struct branch_info *old, char *msg)
+{
+	if (old->path) {
+		create_symref("LAST_HEAD", old->path, msg);
+	} else
+		update_ref(msg, "LAST_HEAD", old->commit->object.sha1, NULL,
+			   REF_NODEREF, DIE_ON_ERR);
+}
+
 static void update_refs_for_switch(struct checkout_opts *opts,
 				   struct branch_info *old,
 				   struct branch_info *new)
@@ -505,12 +514,15 @@ static void update_refs_for_switch(struct checkout_opts *opts,
 			if (old->path && !strcmp(new->path, old->path))
 				fprintf(stderr, "Already on \"%s\"\n",
 					new->name);
-			else
+			else {
 				fprintf(stderr, "Switched to%s branch \"%s\"\n",
 					opts->new_branch ? " a new" : "",
 					new->name);
+				save_old_branch(old, msg.buf);
+			}
 		}
 	} else if (strcmp(new->name, "HEAD")) {
+		save_old_branch(old, msg.buf);
 		update_ref(msg.buf, "HEAD", new->commit->object.sha1, NULL,
 			   REF_NODEREF, DIE_ON_ERR);
 		if (!opts->quiet) {
@@ -533,6 +545,8 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 	int flag;
 	memset(&old, 0, sizeof(old));
 	old.path = resolve_ref("HEAD", rev, 0, &flag);
+	if (old.path)
+		old.path = strdup(old.path);
 	old.commit = lookup_commit_reference_gently(rev, 1);
 	if (!(flag & REF_ISSYMREF))
 		old.path = NULL;
@@ -604,6 +618,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_END(),
 	};
 	int has_dash_dash;
+	int flag;
 
 	memset(&opts, 0, sizeof(opts));
 	memset(&new, 0, sizeof(new));
@@ -671,6 +686,15 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		arg = argv[0];
 		has_dash_dash = (argc > 1) && !strcmp(argv[1], "--");
 
+		if (!strcmp(arg, "-")) {
+			arg = resolve_ref("LAST_HEAD", rev, 0, &flag);
+			if (!arg)
+				die("No last branch saved.");
+			if(!prefixcmp(arg, "refs/heads/"))
+				arg += 11;
+			arg = strdup(arg);
+		}
+
 		if (get_sha1(arg, rev)) {
 			if (has_dash_dash)          /* case (1) */
 				die("invalid reference: %s", arg);
-- 
1.6.1.282.gae4091.dirty

[PATCH v2] checkout: implement "-" shortcut name for last branch

[Thomas Rast]

Let git-checkout save the old branch as a symref in LAST_HEAD, and
make 'git checkout -' switch back to LAST_HEAD, like 'cd -' does in
the shell.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Bah, sorry.  I managed to keep it uncommitted AGAIN.

But this fixed version passes tests.  All of them.  Really!  ;-)


 Documentation/git-checkout.txt         |    3 ++
 Documentation/gitrepository-layout.txt |    4 ++
 builtin-checkout.c                     |   27 ++++++++++++++++-
 t/t2012-checkout-last.sh               |   50 ++++++++++++++++++++++++++++++++
 4 files changed, 83 insertions(+), 1 deletions(-)
 create mode 100755 t/t2012-checkout-last.sh

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 9cd5151..1397745 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -133,6 +133,9 @@ the conflicted merge in the specified paths.
 +
 When this parameter names a non-branch (but still a valid commit object),
 your HEAD becomes 'detached'.
++
+You may also specify "`-`", which denotes the last branch you were on
+before the current HEAD.
 
 
 Detached HEAD
diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index 1befca9..f506c98 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -123,6 +123,10 @@ is often called 'detached HEAD', and almost all commands work
 identically as normal.  See linkgit:git-checkout[1] for
 details.
 
+LAST_HEAD::
+	A symref that holds the value of HEAD before the last
+	branch switch.
+
 branches::
 	A slightly deprecated way to store shorthands to be used
 	to specify URL to 'git-fetch', 'git-pull' and 'git-push'
diff --git a/builtin-checkout.c b/builtin-checkout.c
index b5dd9c0..da74831 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -480,6 +480,16 @@ static void report_tracking(struct branch_info *new)
 	strbuf_release(&sb);
 }
 
+static void save_old_branch(struct branch_info *old, char *msg)
+{
+	if (old->path) {
+		create_symref("LAST_HEAD", old->path, msg);
+	} else if (old->commit) {
+		update_ref(msg, "LAST_HEAD", old->commit->object.sha1, NULL,
+			   REF_NODEREF, DIE_ON_ERR);
+	}
+}
+
 static void update_refs_for_switch(struct checkout_opts *opts,
 				   struct branch_info *old,
 				   struct branch_info *new)
@@ -505,12 +515,15 @@ static void update_refs_for_switch(struct checkout_opts *opts,
 			if (old->path && !strcmp(new->path, old->path))
 				fprintf(stderr, "Already on \"%s\"\n",
 					new->name);
-			else
+			else {
 				fprintf(stderr, "Switched to%s branch \"%s\"\n",
 					opts->new_branch ? " a new" : "",
 					new->name);
+				save_old_branch(old, msg.buf);
+			}
 		}
 	} else if (strcmp(new->name, "HEAD")) {
+		save_old_branch(old, msg.buf);
 		update_ref(msg.buf, "HEAD", new->commit->object.sha1, NULL,
 			   REF_NODEREF, DIE_ON_ERR);
 		if (!opts->quiet) {
@@ -533,6 +546,8 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 	int flag;
 	memset(&old, 0, sizeof(old));
 	old.path = resolve_ref("HEAD", rev, 0, &flag);
+	if (old.path)
+		old.path = strdup(old.path);
 	old.commit = lookup_commit_reference_gently(rev, 1);
 	if (!(flag & REF_ISSYMREF))
 		old.path = NULL;
@@ -604,6 +619,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_END(),
 	};
 	int has_dash_dash;
+	int flag;
 
 	memset(&opts, 0, sizeof(opts));
 	memset(&new, 0, sizeof(new));
@@ -671,6 +687,15 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		arg = argv[0];
 		has_dash_dash = (argc > 1) && !strcmp(argv[1], "--");
 
+		if (!strcmp(arg, "-")) {
+			arg = resolve_ref("LAST_HEAD", rev, 0, &flag);
+			if (!arg)
+				die("No last branch saved.");
+			if(!prefixcmp(arg, "refs/heads/"))
+				arg += 11;
+			arg = strdup(arg);
+		}
+
 		if (get_sha1(arg, rev)) {
 			if (has_dash_dash)          /* case (1) */
 				die("invalid reference: %s", arg);
diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
new file mode 100755
index 0000000..320f6eb
--- /dev/null
+++ b/t/t2012-checkout-last.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='checkout can switch to last branch'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo hello >world &&
+	git add world &&
+	git commit -m initial &&
+	git branch other &&
+	echo "hello again" >>world &&
+	git add world &&
+	git commit -m second
+'
+
+test_expect_success '"checkout -" does not work initially' '
+	test_must_fail git checkout -
+'
+
+test_expect_success 'first branch switch' '
+	git checkout other
+'
+
+test_expect_success '"checkout -" switches back' '
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master"
+'
+
+test_expect_success '"checkout -" switches forth' '
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/other"
+'
+
+test_expect_success 'detach HEAD' '
+	git checkout $(git rev-parse HEAD)
+'
+
+test_expect_success '"checkout -" attaches again' '
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/other"
+'
+
+test_expect_success '"checkout -" detaches again' '
+	git checkout - &&
+	test "z$(git rev-parse HEAD)" = "z$(git rev-parse other)" &&
+	test_must_fail git symbolic-ref HEAD
+'
+
+test_done
-- 
1.6.1.282.gae4091.dirty

Re: [PATCH] checkout: implement "-" shortcut name for last branch

[Johannes Schindelin]

Hi,

On Thu, 15 Jan 2009, Thomas Rast wrote:

> Let git-checkout save the old branch as a symref in LAST_HEAD, and
> make 'git checkout -' switch back to LAST_HEAD, like 'cd -' does in
> the shell.

Actually, what you want is in the reflog, no?  So... parsing 
.git/logs/HEAD for the latest occurrence of "checkout: moving from " and 
then using everything up until the next space should give you the branch 
name, right?

It could be a SHA-1, though, if the last branch switch was from a detached 
HEAD, though.

Ciao,
Dscho

Re: [PATCH v2] checkout: implement "-" shortcut name for last branch

[Johannes Sixt]

Thomas Rast schrieb:
> Let git-checkout save the old branch as a symref in LAST_HEAD, and
> make 'git checkout -' switch back to LAST_HEAD, like 'cd -' does in
> the shell.

/me likes this feature.

git rebase (-i or not) calls checkout behind the scenes if the
two-argument form is used:

   git rebase [-i] master topic

and 'topic' is not the current branch. You may want to add a test that
ensures that rebase sets LAST_HEAD in this case.

You must make sure that commits referenced by LAST_HEAD are not
garbage-collected. (I don't know if this happens anyway for symrefs in .git.)

-- Hannes

Re: [PATCH v2] checkout: implement "-" shortcut name for last branch

[Johannes Schindelin]

Hi,

On Thu, 15 Jan 2009, Johannes Sixt wrote:

> Thomas Rast schrieb:
> > Let git-checkout save the old branch as a symref in LAST_HEAD, and
> > make 'git checkout -' switch back to LAST_HEAD, like 'cd -' does in
> > the shell.
> 
> /me likes this feature.
> 
> git rebase (-i or not) calls checkout behind the scenes if the
> two-argument form is used:
> 
>    git rebase [-i] master topic
> 
> and 'topic' is not the current branch. You may want to add a test that
> ensures that rebase sets LAST_HEAD in this case.
> 
> You must make sure that commits referenced by LAST_HEAD are not
> garbage-collected. (I don't know if this happens anyway for symrefs in .git.)

Note: if you used reflogs for that feature, the garbage collection could 
not have killed the commit.  However, it is quite possible that the 
branch was deleted.

Ciao,
Dscho

Re: [PATCH v2] checkout: implement "-" shortcut name for last branch

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 1476 bytes --]

Johannes Schindelin wrote:
> On Thu, 15 Jan 2009, Johannes Sixt wrote:
> > You must make sure that commits referenced by LAST_HEAD are not
> > garbage-collected. (I don't know if this happens anyway for symrefs in .git.)
> 
> Note: if you used reflogs for that feature, the garbage collection could 
> not have killed the commit.  However, it is quite possible that the 
> branch was deleted.

Suddenly I'm not so sure about either behaviour any more.

Consider:

  $ git commit -m initial
  [master (root-commit)]: created 812c476: "initial"
   1 files changed, 1 insertions(+), 0 deletions(-)
   create mode 100644 foo
  $ git checkout $(git rev-parse HEAD)
  Note: moving to "812c476ca23e25efa7e4d7081153ba657a127d95" which isn't a local branch
  If you want to create a new branch from this checkout, you may do so
  (now or later) by using -b with the checkout command again. Example:
    git checkout -b <new_branch_name>
  HEAD is now at 812c476... initial
  $ git branch -D master
  Deleted branch master (812c476).
  $ git for-each-ref
  $ git reflog expire --expire=now --all
  $ git prune --expire now
  $ git show
  fatal: bad object HEAD
  $ git show 812c476
  fatal: ambiguous argument '812c476': unknown revision or path not in the working tree.
  Use '--' to separate paths from revisions

Oops.

Some quick RTFS shows that it indeed "only" cares about refs and
reflogs.

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


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] checkout: implement "-" shortcut name for last branch

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 697 bytes --]

Johannes Schindelin wrote:
> On Thu, 15 Jan 2009, Thomas Rast wrote:
> 
> > Let git-checkout save the old branch as a symref in LAST_HEAD, and
> > make 'git checkout -' switch back to LAST_HEAD, like 'cd -' does in
> > the shell.
> 
> Actually, what you want is in the reflog, no?  So... parsing 
> .git/logs/HEAD for the latest occurrence of "checkout: moving from " and 
> then using everything up until the next space should give you the branch 
> name, right?

It just feels wrong to grab that information from there; it's a
free-form comment field for user consumption.  And it wasn't even that
hard to implement a LAST_HEAD.

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


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH v2] checkout: implement "-" shortcut name for last branch

[Johannes Schindelin]

Hi,

On Thu, 15 Jan 2009, Thomas Rast wrote:

> Johannes Schindelin wrote:
> > On Thu, 15 Jan 2009, Johannes Sixt wrote:
> > > You must make sure that commits referenced by LAST_HEAD are not
> > > garbage-collected. (I don't know if this happens anyway for symrefs in .git.)
> > 
> > Note: if you used reflogs for that feature, the garbage collection could 
> > not have killed the commit.  However, it is quite possible that the 
> > branch was deleted.
> 
> Suddenly I'm not so sure about either behaviour any more.
> 
> Consider:
> 
>   $ git commit -m initial
>   [master (root-commit)]: created 812c476: "initial"
>    1 files changed, 1 insertions(+), 0 deletions(-)
>    create mode 100644 foo
>   $ git checkout $(git rev-parse HEAD)
>   Note: moving to "812c476ca23e25efa7e4d7081153ba657a127d95" which isn't a local branch
>   If you want to create a new branch from this checkout, you may do so
>   (now or later) by using -b with the checkout command again. Example:
>     git checkout -b <new_branch_name>
>   HEAD is now at 812c476... initial
>   $ git branch -D master
>   Deleted branch master (812c476).
>   $ git for-each-ref
>   $ git reflog expire --expire=now --all
>   $ git prune --expire now
>   $ git show
>   fatal: bad object HEAD
>   $ git show 812c476
>   fatal: ambiguous argument '812c476': unknown revision or path not in the working tree.
>   Use '--' to separate paths from revisions
> 
> Oops.
> 
> Some quick RTFS shows that it indeed "only" cares about refs and
> reflogs.

Maybe something like this would help (completely untested, though the 
idea should be clear)?

-- snipsnap --
[PATCH] pack-objects --all: include HEAD, which could be detached

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 builtin-pack-objects.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index cb51916..da55671 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -2219,6 +2219,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 						 rp_ac_alloc * sizeof(*rp_av));
 			}
 			rp_av[rp_ac++] = arg;
+			if (!strcmp("--all", arg)) {
+				ALLOC_GROW(rp_av, rp_ac + 1, rp_ac_alloc);
+				rp_av[rp_ac++] = "HEAD";
+			}
 			continue;
 		}
 		if (!strcmp("--thin", arg)) {

Re: [PATCH] checkout: implement "-" shortcut name for last branch

[Johannes Schindelin]

Hi,

On Thu, 15 Jan 2009, Thomas Rast wrote:

> Johannes Schindelin wrote:
> > On Thu, 15 Jan 2009, Thomas Rast wrote:
> > 
> > > Let git-checkout save the old branch as a symref in LAST_HEAD, and
> > > make 'git checkout -' switch back to LAST_HEAD, like 'cd -' does in
> > > the shell.
> > 
> > Actually, what you want is in the reflog, no?  So... parsing 
> > .git/logs/HEAD for the latest occurrence of "checkout: moving from " and 
> > then using everything up until the next space should give you the branch 
> > name, right?
> 
> It just feels wrong to grab that information from there; it's a
> free-form comment field for user consumption.  And it wasn't even that
> hard to implement a LAST_HEAD.

There are a number of issues why I would like to avoid introducing 
LAST_HEAD:

- it does not work when you are using different Git versions on the same 
  repository,

- it does not work when you switched recently,

- you are storing redundant information,

- yes, the field is meant for user consumption, but no, it is not 
  free-form,

- AFAICT your version could never be convinced to resurrect deleted 
  branches, without resorting to reflogs anyway.

- the reflog method reflects pretty much exactly how people work around 
  the lack of "checkout -" currently, so why not just use the same proven 
  approach?

Ciao,
Dscho

Re: [PATCH v2] checkout: implement "-" shortcut name for last branch

[Johannes Schindelin]

Hi,

On Thu, 15 Jan 2009, Johannes Schindelin wrote:

> [PATCH] pack-objects --all: include HEAD, which could be detached

In hind sight, it would probably be better to add this to revision.c.

Ciao,
Dscho

Re: [PATCH v2] checkout: implement "-" shortcut name for last branch

[Johan Herland]

On Thursday 15 January 2009, Johannes Schindelin wrote:
> Hi,
>
> On Thu, 15 Jan 2009, Johannes Sixt wrote:
> > Thomas Rast schrieb:
> > > Let git-checkout save the old branch as a symref in LAST_HEAD,
> > > and make 'git checkout -' switch back to LAST_HEAD, like 'cd -'
> > > does in the shell.
> >
> > /me likes this feature.
> >
> > git rebase (-i or not) calls checkout behind the scenes if the
> > two-argument form is used:
> >
> >    git rebase [-i] master topic
> >
> > and 'topic' is not the current branch. You may want to add a test
> > that ensures that rebase sets LAST_HEAD in this case.
> >
> > You must make sure that commits referenced by LAST_HEAD are not
> > garbage-collected. (I don't know if this happens anyway for symrefs
> > in .git.)
>
> Note: if you used reflogs for that feature, the garbage collection
> could not have killed the commit.  However, it is quite possible that
> the branch was deleted.

I also like this feature, but as this is only a _convenience_ feature, I 
would prefer if it didn't keep the previous branch/commit alive (if 
otherwise unreachable). In any case, this new feature will _have_ to 
handle the case where there simply is no previous branch/commit (e.g. 
after a git clone or git init).

I suggest that "git checkout -" looks at the reflog, and if there is no 
previous entry in the reflog, or that entry is unreachable, then fail 
in the same manner as "git checkout garbage"


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: [PATCH v2] checkout: implement "-" shortcut name for last branch

[Johannes Schindelin]

Hi,

On Thu, 15 Jan 2009, Johan Herland wrote:

> On Thursday 15 January 2009, Johannes Schindelin wrote:
>
> > On Thu, 15 Jan 2009, Johannes Sixt wrote:
> > > Thomas Rast schrieb:
> > > > Let git-checkout save the old branch as a symref in LAST_HEAD,
> > > > and make 'git checkout -' switch back to LAST_HEAD, like 'cd -'
> > > > does in the shell.
> > >
> > > /me likes this feature.
> > >
> > > git rebase (-i or not) calls checkout behind the scenes if the
> > > two-argument form is used:
> > >
> > >    git rebase [-i] master topic
> > >
> > > and 'topic' is not the current branch. You may want to add a test
> > > that ensures that rebase sets LAST_HEAD in this case.
> > >
> > > You must make sure that commits referenced by LAST_HEAD are not
> > > garbage-collected. (I don't know if this happens anyway for symrefs
> > > in .git.)
> >
> > Note: if you used reflogs for that feature, the garbage collection
> > could not have killed the commit.  However, it is quite possible that
> > the branch was deleted.
> 
> I also like this feature, but as this is only a _convenience_ feature, I 
> would prefer if it didn't keep the previous branch/commit alive (if 
> otherwise unreachable).

You misread me: if the information is in HEAD's reflog, the _is_ 
reachable.  From HEAD's reflog.  And therefore, the objects will not be 
gc'ed (yet).

> In any case, this new feature will _have_ to handle the case where there 
> simply is no previous branch/commit (e.g. after a git clone or git 
> init).
>
> I suggest that "git checkout -" looks at the reflog, and if there is no 
> previous entry in the reflog, or that entry is unreachable, then fail 
> in the same manner as "git checkout garbage"

Exactly my thinking.

Ciao,
Dscho

Re: [PATCH] checkout: implement "-" shortcut name for last branch

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]

Johannes Schindelin wrote:
> There are a number of issues why I would like to avoid introducing 
> LAST_HEAD:
> 
> - it does not work when you are using different Git versions on the same 
>   repository,
> 
> - it does not work when you switched recently,

If you switch once, you'll be able to use the feature one checkout
later than if it was reflog-based.

If you switch a lot, the feature won't be in your git half the time
anyway.

> - you are storing redundant information,

AFAIK it's the first instance of this data in a non-free-form field.
There's also the precedent of ORIG_HEAD.

> - yes, the field is meant for user consumption, but no, it is not 
>   free-form,

It's a field of almost arbitrary character data, filled by 70% of the
update-ref calls I can find in git.git in a "<tool>: <comment>" format
and by the rest with things such as "initial pull" or
"refs/remotes/git-svn: updating HEAD".  (The latter is so informative
that it probably deserves a fix.)  How is that not free-form?

> - AFAICT your version could never be convinced to resurrect deleted 
>   branches, without resorting to reflogs anyway.

Neither can any other use of git-checkout without the user manually
recovering some valid revspec referring to the old branch tip from the
reflog.  I wanted to be able to abbreviate the previous branch's name,
and it does just that.

> - the reflog method reflects pretty much exactly how people work around 
>   the lack of "checkout -" currently, so why not just use the same proven 
>   approach?

So you can make me fight an uphill battle against your idea how it
should be done.


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









[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] checkout: implement "-" shortcut name for last branch

[Johannes Schindelin]

Hi,

On Thu, 15 Jan 2009, Thomas Rast wrote:

> Johannes Schindelin wrote:
> > There are a number of issues why I would like to avoid introducing 
> > LAST_HEAD:
> > 
> > - it does not work when you are using different Git versions on the same 
> >   repository,
> > 
> > - it does not work when you switched recently,
> 
> If you switch once, you'll be able to use the feature one checkout
> later than if it was reflog-based.
> 
> If you switch a lot, the feature won't be in your git half the time
> anyway.

But once it is, you could also have something like "git checkout -{5}" 
meaning the 5th last branch you were on.

No, I am not married to that syntax

> > - you are storing redundant information,
> 
> AFAIK it's the first instance of this data in a non-free-form field.
> There's also the precedent of ORIG_HEAD.

See below.

> > - yes, the field is meant for user consumption, but no, it is not 
> >   free-form,
> 
> It's a field of almost arbitrary character data, filled by 70% of the
> update-ref calls I can find in git.git in a "<tool>: <comment>" format
> and by the rest with things such as "initial pull" or
> "refs/remotes/git-svn: updating HEAD".  (The latter is so informative
> that it probably deserves a fix.)  How is that not free-form?

That is not free-form, as the "<tool>:" is a hard convention all obey 
(and therefore, git checkout - only relies on _checkout_ not changing the 
format), and checkout is sufficiently plumbing that we will not change it 
all that lightly, certainly not when "git checkout -" depends on it.

So I think that those free-form concerns are totally unfounded.

Oh, and before you say that people could mess with GIT_REFLOG_ACTION, git 
checkout is no longer a script, and creates the message itself.  So we 
have full control over it.

They could edit the logs directly, but that applies to virtually the whole 
repository, and can safely be ignored as a lemming behavior.

> > - AFAICT your version could never be convinced to resurrect deleted 
> >   branches, without resorting to reflogs anyway.
> 
> Neither can any other use of git-checkout without the user manually
> recovering some valid revspec referring to the old branch tip from the
> reflog.

To the contrary.  The reflog has this information together with the 
message "moved from ...".

> > - the reflog method reflects pretty much exactly how people work around 
> >   the lack of "checkout -" currently, so why not just use the same proven 
> >   approach?
> 
> So you can make me fight an uphill battle against your idea how it
> should be done.

If you can convince me that there are benefits from introducing yet 
another file in $GIT_DIR and duplicating information that is in the 
reflogs already, then no, it's not an uphill battle.

I mean, I _like_ the feature.  Otherwise I would not spend so much time 
suggesting what I think would be a method more in line with what we have 
already.

Ciao,
Dscho

Re: [PATCH v2] checkout: implement "-" shortcut name for last branch

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> Let git-checkout save the old branch as a symref in LAST_HEAD, and
> make 'git checkout -' switch back to LAST_HEAD, like 'cd -' does in
> the shell.

I do not like this for two reasons.

I will not dispute that you would need to have "checkout" and other branch
switching operations to record where you were in order to be able to refer
to "where I was".  And as Dscho and others point out, there already is an
existing mechanism that does exactly that, so it _might_ be easier to work
with an extra LAST_HEAD, it is not absolutely necessary.

I do not see a reason to limit the new notation "where I was" only to "git
checkout".  Wouldn't it be handy if you can use the notation as the other
branch to merge from, or the commit to rebase on?

"cd -" is a very good analogy why your "-" shortcut is a short-sighted
convenience feature that is too narrow and not well designed.  "cd -" can
go back, but you cannot say "ls -" to list the contents of the previous
directory.

So if this topic were "Introduce LAST_HEAD to always keep track of the
branch I was on before the current branch", and were advertised as "You
can use this throughout git to say things like 'git checkout LAST_HEAD',
'git merge LAST_HEAD', and 'git rebase LAST_HEAD'", I think it might have
made a bit more sense.  You could _additionally_ say "because switching to
LAST_HEAD happens very often, there is another short cut 'checkout -' but
that is exactly the same as 'checkout LAST_HEAD'".

Another reason is the one level limitation.  If we do not use LAST_HEAD,
and instead used HEAD reflog, to get to this information, there is no
reason we cannot to give an equally easy access to the second from the
last branch the user was on.

So I think it is just the matter of coming up with a clever syntax that
works on reflogs to name the nth last branch we were on and teach that
syntax to both get_sha1() and resolve_ref().

With the attached illustration patch,

     $ git checkout junk
     $ git chekcout master
     $ git checkout @{-1}

will take you back to junk branch.  It probably would serve as a starting
point, if anybody is interested.

NOTE!

 * It will report "Switched to branch "junk", not "junk (@{-1})" or
   anything that hints the user used this new syntax.  switch_branches()
   may need to be given more information to distinguish the name the end
   user spelled to specify the branch (e.g. "@{-1}") and the actual name
   of the branch (e.g. "junk"), and use the former together with the
   latter when reporting to the end user and use the latter only to record
   what happened to the reflog.  But this is a very minor point.

 * The reflog parser only parses "checkout" and not rebase action.  It
   also does not notice "git checkout HEAD^" is not switching to a real
   branch.

 * The code read the reflog twice, first to count how many branch
   switching there are and then to locate the N-th entry we are interested
   in, because I was lazy.  We may want an API to enumerate reflog entries
   in reverse.

 * interpret_nth_last_branch() is not hooked to get_sha1() codepath in
   this patch, so this is still only applicable to "git checkout".  But it
   should be trivial to do so.

 builtin-checkout.c |   10 +++++-
 cache.h            |    1 +
 sha1_name.c        |   78 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 2 deletions(-)

diff --git c/builtin-checkout.c w/builtin-checkout.c
index b5dd9c0..a3b69d6 100644
--- c/builtin-checkout.c
+++ w/builtin-checkout.c
@@ -361,8 +361,14 @@ struct branch_info {
 static void setup_branch_path(struct branch_info *branch)
 {
 	struct strbuf buf = STRBUF_INIT;
-	strbuf_addstr(&buf, "refs/heads/");
-	strbuf_addstr(&buf, branch->name);
+
+	if (!interpret_nth_last_branch(branch->name, &buf)) {
+		branch->name = xstrdup(buf.buf);
+		strbuf_splice(&buf, 0, 0, "refs/heads/", 11);
+	} else {
+		strbuf_addstr(&buf, "refs/heads/");
+		strbuf_addstr(&buf, branch->name);
+	}
 	branch->path = strbuf_detach(&buf, NULL);
 }
 
diff --git c/cache.h w/cache.h
index 8e1af26..0dd9168 100644
--- c/cache.h
+++ w/cache.h
@@ -663,6 +663,7 @@ extern int read_ref(const char *filename, unsigned char *sha1);
 extern const char *resolve_ref(const char *path, unsigned char *sha1, int, int *);
 extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
 extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
+extern int interpret_nth_last_branch(const char *str, struct strbuf *);
 
 extern int refname_match(const char *abbrev_name, const char *full_name, const char **rules);
 extern const char *ref_rev_parse_rules[];
diff --git c/sha1_name.c w/sha1_name.c
index 159c2ab..6377264 100644
--- c/sha1_name.c
+++ w/sha1_name.c
@@ -674,6 +674,84 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	return retval;
 }
 
+struct grab_nth_branch_switch_cbdata {
+	int counting;
+	int nth;
+	struct strbuf *buf;
+};
+
+static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
+				  const char *email, unsigned long timestamp, int tz,
+				  const char *message, void *cb_data)
+{
+	struct grab_nth_branch_switch_cbdata *cb = cb_data;
+	const char *match = NULL;
+
+	if (!prefixcmp(message, "checkout: moving to "))
+		match = message + strlen("checkout: moving to ");
+	else if (!prefixcmp(message, "checkout: moving from ")) {
+		const char *cp = message + strlen("checkout: moving from ");
+		if ((cp = strstr(cp, " to ")) != NULL) {
+			match = cp + 4;
+		}
+	}
+
+	if (!match)
+		return 0;
+
+	if (cb->counting) {
+		cb->nth++;
+		return 0;
+	}
+
+	if (--cb->nth <= 0) {
+		size_t len = strlen(match);
+		while (match[len-1] == '\n')
+			len--;
+		strbuf_reset(cb->buf);
+		strbuf_add(cb->buf, match, len);
+		return 1;
+	}
+	return 0;
+}
+
+/*
+ * This reads "@{-N}" syntax, finds the name of the Nth previous
+ * branch we were on, and places the name of the branch in the given
+ * buf and returns 0 if successful.
+ *
+ * If the input is not of the accepted format, it returns a negative
+ * number to signal an error.
+ */
+int interpret_nth_last_branch(const char *name, struct strbuf *buf)
+{
+	int nth, i;
+	struct grab_nth_branch_switch_cbdata cb;
+
+	if (name[0] != '@' || name[1] != '{' || name[2] != '-')
+		return -1;
+	for (i = 3, nth = 0; name[i] && name[i] != '}'; i++) {
+		char ch = name[i];
+		if ('0' <= ch && ch <= '9')
+			nth = nth * 10 + ch - '0';
+		else
+			return -1;
+	}
+	if (nth < 0 || 10 <= nth)
+		return -1;
+
+	cb.counting = 1;
+	cb.nth = 0;
+	cb.buf = buf;
+	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+
+	cb.counting = 0;
+	cb.nth -= nth;
+	cb.buf = buf;
+	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+	return 0;
+}
+
 /*
  * This is like "get_sha1_basic()", except it allows "sha1 expressions",
  * notably "xyz^" for "parent of xyz"

Re: [PATCH v2] checkout: implement "-" shortcut name for last branch

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Thu, 15 Jan 2009, Johannes Schindelin wrote:
>
>> [PATCH] pack-objects --all: include HEAD, which could be detached
>
> In hind sight, it would probably be better to add this to revision.c.

If you mean that "git log --all" should also include a possibly detached
HEAD in its traversal, and a patch that implements such a fix would
automatically fix "repack -a" without the patch you are responding to, I
think I agree 100%.

Re: [PATCH v2] checkout: implement "-" shortcut name for last branch

[Johannes Schindelin]

Hi,

On Thu, 15 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Thu, 15 Jan 2009, Johannes Schindelin wrote:
> >
> >> [PATCH] pack-objects --all: include HEAD, which could be detached
> >
> > In hind sight, it would probably be better to add this to revision.c.
> 
> If you mean that "git log --all" should also include a possibly detached
> HEAD in its traversal, and a patch that implements such a fix would
> automatically fix "repack -a" without the patch you are responding to, I
> think I agree 100%.

Yes, indeed.

Something like

-- snip --
diff --git a/revision.c b/revision.c
index db60f06..b065184 100644
--- a/revision.c
+++ b/revision.c
@@ -1263,6 +1263,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 
 			if (!strcmp(arg, "--all")) {
 				handle_refs(revs, flags, for_each_ref);
+				handle_refs(revs, flags, head_ref);
 				continue;
 			}
 			if (!strcmp(arg, "--branches")) {
-- snap --

but that was just a quick guess, and if nobody beats me to it, I'll turn 
it into a proper patch later.

Ciao,
Dscho

Re: [PATCH v2] checkout: implement "-" shortcut name for last branch

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> So I think it is just the matter of coming up with a clever syntax that
> works on reflogs to name the nth last branch we were on and teach that
> syntax to both get_sha1() and resolve_ref().
>
> With the attached illustration patch,
>
>      $ git checkout junk
>      $ git chekcout master
>      $ git checkout @{-1}
>
> will take you back to junk branch.  It probably would serve as a starting
> point, if anybody is interested.
>
> NOTE!
> ...
>  * interpret_nth_last_branch() is not hooked to get_sha1() codepath in
>    this patch, so this is still only applicable to "git checkout".  But it
>    should be trivial to do so.
> ...
> +/*
> + * This reads "@{-N}" syntax, finds the name of the Nth previous
> + * branch we were on, and places the name of the branch in the given
> + * buf and returns 0 if successful.
> + *
> + * If the input is not of the accepted format, it returns a negative
> + * number to signal an error.
> + */
> +int interpret_nth_last_branch(const char *name, struct strbuf *buf)

A few more things to note.

 * interpret_nth_last_branch() probably should return how many bytes it
   consumed, instead of returning 0 in the successful case.  This is to
   allow things like "git merge @{-1}~2" to be easily parsed, either by
   "git merge" itself into "git merge junk~2", which would result in
   "Merge branch junk (early part)", or by get_sha1() which would result
   in "Merge commit deadbeefacebeads".

 * I mentioned resolve_ref() may need to be told about this syntax but I
   do not think it is necessary.  If a command that can take an arbitrary
   refname or committish in the most general case does something special
   when the end user input is a branch name ("git checkout" is a prime
   example for this, but "git merge" also has this property, illustrated
   by the previous "Merge branch junk" example), these commands has to do
   their own special case logic before the user input hits get_sha1() or
   resolve_ref() anyway (setup_branch_path() in builtin-checkout.c is a
   good example of this), and such special case logic can and probably
   should use interpret_nth_last_branch() directly.

Re: [PATCH] checkout: implement "-" shortcut name for last branch

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]

Johannes Schindelin wrote:
> - AFAICT your version could never be convinced to resurrect deleted 
>   branches, without resorting to reflogs anyway.

Speaking of resurrection, there are other possible sources that a
branch tip could be gleaned from.  How about the script below?  The
advantage is that it can even be used to recover Junio's topic
branches by looking at the merges in 'pu'.

(I'll answer the rest later.)

--- 8< ---
#!/bin/sh

. git-sh-setup

USAGE="<branch>"

test "$#" = 1 || usage

branch="$1"
candidates=

search_reflog () {
	next=
	git reflog show HEAD |
	while read sha ref msg; do
		if test -n "$next"; then
			next=
			echo ${sha%...}
		fi
		if echo "$msg" | grep -q "^checkout: moving from $branch "; then
			next=t
		fi
		if echo "$msg" | grep -q "^merge $branch:"; then
			git rev-list --parents -1 ${sha%...} \
				| cut -d' ' -f3
		fi
	done
}

search_merges () {
	git rev-list --pretty=tformat:"%h %p:%s" --all |
	grep "Merge branch.*'$branch'.*into" |
	while read sha rest; do
		parents="$(echo "$rest" | cut -d: -f1)"
		case "$parents" in
		    *' '*' '*)
			warn "$branch took part in octopus merge $sha"
			warn "check manually!"
			;;
		    *' '*)
			echo "$parents" | cut -d' ' -f2
			;;
		esac
	done
}

search_merge_targets () {
	git rev-list --pretty=tformat:"%h %s" --all |
	grep "Merge branch '[^']*' into $branch$" |
	cut -d' ' -f1
}

candidates="$(search_reflog | sort -u)"
if test -z "$candidates"; then
	echo "** Searching merges... **"
	candidates="$( (search_merges;search_merge_targets) | sort -u)"
fi

echo "** Candidates **"
for cmt in $candidates; do
	git --no-pager log --pretty=oneline --abbrev-commit -1 $cmt
done

newest=$(git rev-list -1 $candidates)

if ! git rev-parse --verify --quiet $branch >/dev/null; then
	printf "** Restoring $branch to "
	git --no-pager log -1 --pretty=tformat:"%h %s" $newest
	git branch $branch $newest
else
	printf "Most recent among them: "
	git --no-pager log -1 --pretty=tformat:"%h %s" $newest
	echo "** $branch already exists, doing nothing"
fi


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] checkout: implement "-" shortcut name for last branch

[Johannes Schindelin]

Hi,

On Fri, 16 Jan 2009, Thomas Rast wrote:

> search_reflog () {
> 	next=
> 	git reflog show HEAD |
> 	while read sha ref msg; do
> 		if test -n "$next"; then
> 			next=
> 			echo ${sha%...}
> 		fi
> 		if echo "$msg" | grep -q "^checkout: moving from $branch "; then
> 			next=t
> 		fi
> 		if echo "$msg" | grep -q "^merge $branch:"; then
> 			git rev-list --parents -1 ${sha%...} \
> 				| cut -d' ' -f3
> 		fi
> 	done
> }

How about this instead:

search_reflog () {
	sed -n 's/\([^ ]*\) .*\tcheckout: moving from $branch .*/\1/p' \
		< .git/logs/HEAD
}

Of course, this leaves out the merges...  but I'd make that a command line 
option anyway: would you like to resurrect a branch that you recently were 
on, or one that you recently merged, or one that was merged by someone 
else?

Ciao,
Dscho

Re: [PATCH v2] checkout: implement "-" shortcut name for last branch

[Johannes Schindelin]

Hi,

On Thu, 15 Jan 2009, Junio C Hamano wrote:

> I do not see a reason to limit the new notation "where I was" only to 
> "git checkout".  Wouldn't it be handy if you can use the notation as the 
> other branch to merge from, or the commit to rebase on?
> 
> [...]
> 
> Another reason is the one level limitation.  If we do not use LAST_HEAD,
> and instead used HEAD reflog, to get to this information, there is no
> reason we cannot to give an equally easy access to the second from the
> last branch the user was on.
> 
> So I think it is just the matter of coming up with a clever syntax that
> works on reflogs to name the nth last branch we were on and teach that
> syntax to both get_sha1() and resolve_ref().
> 
> With the attached illustration patch,
> 
>      $ git checkout junk
>      $ git chekcout master
>      $ git checkout @{-1}
> 
> will take you back to junk branch.  It probably would serve as a starting
> point, if anybody is interested.

I like it.  Additionaly, we could teach "checkout" that "-" is 
equivalent to "@{-1}", as checkout cannot possibly take stdin, so 
it would not hurt.  Thomas?

Ciao,
Dscho

[PATCH] revision walker: include a detached HEAD in --all

[Johannes Schindelin]

When HEAD is detached, --all should list it, too, logically, as a
detached HEAD is by definition a temporary, unnamed branch.

It is especially necessary to list it when garbage collecting, as
the detached HEAD would be trashed.

Noticed by Thomas Rast.

Note that this affects creating bundles with --all; I contend that it
is a good change to add the HEAD, so that cloning from such a bundle
will give you a current branch.  However, I had to fix t5701 as it
assumed that --all does not imply HEAD.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Thu, 15 Jan 2009, Junio C Hamano wrote:

	> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
	> 
	> > On Thu, 15 Jan 2009, Johannes Schindelin wrote:
	> >
	> >> [PATCH] pack-objects --all: include HEAD, which could be 
	> >> detached
	> >
	> > In hind sight, it would probably be better to add this to 
	> > revision.c.
	> 
	> If you mean that "git log --all" should also include a possibly 
	> detached HEAD in its traversal, and a patch that implements such a fix 
	> would automatically fix "repack -a" without the patch you are 
	> responding to, I think I agree 100%.

	Here it is.  (Sorry for the delay, it was due to some 
	well-deserved inebriation.)

 revision.c              |    1 +
 t/t5701-clone-local.sh  |    4 ++--
 t/t6014-rev-list-all.sh |   38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 2 deletions(-)
 create mode 100755 t/t6014-rev-list-all.sh

diff --git a/revision.c b/revision.c
index db60f06..b065184 100644
--- a/revision.c
+++ b/revision.c
@@ -1263,6 +1263,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 
 			if (!strcmp(arg, "--all")) {
 				handle_refs(revs, flags, for_each_ref);
+				handle_refs(revs, flags, head_ref);
 				continue;
 			}
 			if (!strcmp(arg, "--branches")) {
diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
index 8dfaaa4..14413f8 100755
--- a/t/t5701-clone-local.sh
+++ b/t/t5701-clone-local.sh
@@ -11,8 +11,8 @@ test_expect_success 'preparing origin repository' '
 	git clone --bare . x &&
 	test "$(GIT_CONFIG=a.git/config git config --bool core.bare)" = true &&
 	test "$(GIT_CONFIG=x/config git config --bool core.bare)" = true
-	git bundle create b1.bundle --all HEAD &&
-	git bundle create b2.bundle --all &&
+	git bundle create b1.bundle master HEAD &&
+	git bundle create b2.bundle master &&
 	mkdir dir &&
 	cp b1.bundle dir/b3
 	cp b1.bundle b4
diff --git a/t/t6014-rev-list-all.sh b/t/t6014-rev-list-all.sh
new file mode 100755
index 0000000..991ab4a
--- /dev/null
+++ b/t/t6014-rev-list-all.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+test_description='--all includes detached HEADs'
+
+. ./test-lib.sh
+
+
+commit () {
+	test_tick &&
+	echo $1 > foo &&
+	git add foo &&
+	git commit -m "$1"
+}
+
+test_expect_success 'setup' '
+
+	commit one &&
+	commit two &&
+	git checkout HEAD^ &&
+	commit detached
+
+'
+
+test_expect_success 'rev-list --all lists detached HEAD' '
+
+	test 3 = $(git rev-list --all | wc -l)
+
+'
+
+test_expect_success 'repack does not lose detached HEAD' '
+
+	git gc &&
+	git prune --expire=now &&
+	git show HEAD
+
+'
+
+test_done
-- 
1.6.1.299.gfdbb

Re: [PATCH] revision walker: include a detached HEAD in --all

[Santi Béjar]

2009/1/16 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>
> When HEAD is detached, --all should list it, too, logically, as a
> detached HEAD is by definition a temporary, unnamed branch.
>
> It is especially necessary to list it when garbage collecting, as
> the detached HEAD would be trashed.
>
> Noticed by Thomas Rast.
>
> Note that this affects creating bundles with --all; I contend that it
> is a good change to add the HEAD, so that cloning from such a bundle
> will give you a current branch.  However, I had to fix t5701 as it
> assumed that --all does not imply HEAD.

>From the description I understand that it only affects when the HEAD
is detached, but in t5701 the HEAD is not detached so nothing should
be fixed.

For gc for sure it is a good thing, but I'm not convinced of the
others, as a detached HEAD is a very special thing (temporary and
unnamed branch).

Santi

Re: [PATCH] revision walker: include a detached HEAD in --all

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1258 bytes --]

Hi,

On Fri, 16 Jan 2009, Santi Béjar wrote:

> 2009/1/16 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> >
> > Note that this affects creating bundles with --all; I contend that it 
> > is a good change to add the HEAD, so that cloning from such a bundle 
> > will give you a current branch.  However, I had to fix t5701 as it 
> > assumed that --all does not imply HEAD.
> 
> From the description I understand that it only affects when the HEAD is 
> detached, but in t5701 the HEAD is not detached so nothing should be 
> fixed.

The error in t5701 was that it _wanted_ to test a bundle without a HEAD, 
but it actually created it with --all.  That was implying that --all does 
not mean HEAD, and I disagree with that.

> For gc for sure it is a good thing, but I'm not convinced of the others, 
> as a detached HEAD is a very special thing (temporary and unnamed 
> branch).

So?  What does "--all" mean?  All branches or what? :-)

Seriously, I think that --all should imply HEAD at all times, as the only 
time when it makes a difference is when you have that unnamed _branch_ 
that is a detached HEAD.

Maybe I would be more amenable to your criticism if you could come up with 
a scenario where implying HEAD with --all is _wrong_.

Ciao,
Dscho

Re: [PATCH] revision walker: include a detached HEAD in --all

[David Kastrup]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Fri, 16 Jan 2009, Santi Béjar wrote:
>
>> 2009/1/16 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> >
>> > Note that this affects creating bundles with --all; I contend that it 
>> > is a good change to add the HEAD, so that cloning from such a bundle 
>> > will give you a current branch.  However, I had to fix t5701 as it 
>> > assumed that --all does not imply HEAD.
>> 
>> From the description I understand that it only affects when the HEAD is 
>> detached, but in t5701 the HEAD is not detached so nothing should be 
>> fixed.
>
> The error in t5701 was that it _wanted_ to test a bundle without a HEAD, 
> but it actually created it with --all.  That was implying that --all does 
> not mean HEAD, and I disagree with that.

I don't think that a detached HEAD should be a special case, since you
can have other detached symbolic references no longer on a branch.  None
of those should be garbage-collected either, I think.

-- 
David Kastrup

Re: [PATCH] revision walker: include a detached HEAD in --all

[Santi Béjar]

2009/1/16 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> Hi,
>
> On Fri, 16 Jan 2009, Santi Béjar wrote:
>
>> 2009/1/16 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> >
>> > Note that this affects creating bundles with --all; I contend that it
>> > is a good change to add the HEAD, so that cloning from such a bundle
>> > will give you a current branch.  However, I had to fix t5701 as it
>> > assumed that --all does not imply HEAD.
>>
>> From the description I understand that it only affects when the HEAD is
>> detached, but in t5701 the HEAD is not detached so nothing should be
>> fixed.
>
> The error in t5701 was that it _wanted_ to test a bundle without a HEAD,
> but it actually created it with --all.  That was implying that --all does
> not mean HEAD

Yes, that is the current behaviour.

> , and I disagree with that.

I know you disagree, but in the commit log you said:

---
[PATCH] revision walker: include a detached HEAD in --all

When HEAD is detached, --all should list it, too, logically, as a
detached HEAD is by definition a temporary, unnamed branch.
---

so nothing talks about changing the behaviour when the HEAD is not detached.

But the problem with t5701 is another thing. If you run this:

git init
: >file
git add .
git commit -m1
git bundle create b1.bundle --all HEAD
git ls-remote b1.bundle
git rev-parse --all HEAD

you will see that the same rev-parse parameters in "git bundle"
produce tree lines while with "git rev-parse" only two are produced.


>
>> For gc for sure it is a good thing, but I'm not convinced of the others,
>> as a detached HEAD is a very special thing (temporary and unnamed
>> branch).
>
> So?  What does "--all" mean?  All branches or what? :-)
>
> Seriously, I think that --all should imply HEAD at all times, as the only
> time when it makes a difference is when you have that unnamed _branch_
> that is a detached HEAD.
>
> Maybe I would be more amenable to your criticism if you could come up with
> a scenario where implying HEAD with --all is _wrong_.

I don't think it is plainly wrong. I think both makes sense, but I
think it is not a good idea to change the behaviour now as some
scripts may rely on it.

Santi

Re: [PATCH] revision walker: include a detached HEAD in --all

[Santi Béjar]

2009/1/16 Santi Béjar <santi@agolina.net>:
> 2009/1/16 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> Hi,
>>
>> On Fri, 16 Jan 2009, Santi Béjar wrote:
>>
>>> 2009/1/16 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>>> >
>>> > Note that this affects creating bundles with --all; I contend that it
>>> > is a good change to add the HEAD, so that cloning from such a bundle
>>> > will give you a current branch.  However, I had to fix t5701 as it
>>> > assumed that --all does not imply HEAD.
>>>
>>> From the description I understand that it only affects when the HEAD is
>>> detached, but in t5701 the HEAD is not detached so nothing should be
>>> fixed.
>>
>> The error in t5701 was that it _wanted_ to test a bundle without a HEAD,
>> but it actually created it with --all.  That was implying that --all does
>> not mean HEAD
>
> Yes, that is the current behaviour.
>
>> , and I disagree with that.
>
> I know you disagree, but in the commit log you said:
>
> ---
> [PATCH] revision walker: include a detached HEAD in --all
>
> When HEAD is detached, --all should list it, too, logically, as a
> detached HEAD is by definition a temporary, unnamed branch.
> ---
>
> so nothing talks about changing the behaviour when the HEAD is not detached.
>
> But the problem with t5701 is another thing. If you run this:

>
> git init
> : >file
> git add .
> git commit -m1
> git bundle create b1.bundle --all HEAD
> git ls-remote b1.bundle
> git rev-parse --all HEAD
>
> you will see that the same rev-parse parameters in "git bundle"
> produce tree lines while with "git rev-parse" only two are produced.
>

Sorry, there are two problems with t5701, the one of the changing
behaviour of the --all flag and this one.

Santi

[PATCH/RFC v3 0/6] N-th last checked out branch

[Thomas Rast]

Junio C Hamano <gitster@pobox.com> writes:
> "cd -" is a very good analogy why your "-" shortcut is a short-sighted
> convenience feature that is too narrow and not well designed.  "cd -" can
> go back, but you cannot say "ls -" to list the contents of the previous
> directory.

True.

> Another reason is the one level limitation.

It shares that limitation with my mind ;-)

But well, since you all seem to want a more general solution, here's a
draft.

I started poking around in refs.c for the backwards search, but the
two reflog functions had a lot of functionality implemented very
differently.  So 1-4 are just cleanup and adding that backwards
iterator.  Note that 3/6 changes for_each_reflog_ent() to die() when
it finds a format error, unlike the previous behaviour.  No tests
seemed to care, but maybe someone out there relies on having broken
reflogs? ;-)

>  * The code read the reflog twice, first to count how many branch
>    switching there are and then to locate the N-th entry we are interested
>    in, because I was lazy.  We may want an API to enumerate reflog entries
>    in reverse.

So this might be settled.

>  * The reflog parser only parses "checkout" and not rebase action.  It
>    also does not notice "git checkout HEAD^" is not switching to a real
>    branch.

I don't handle this, but I'm not sure how much of a problem it is.  It
can correctly detach and re-attach even in cases such as HEAD^.

Ok, now that you mention it, it will probably put you on the parent of
whatever you happened to be on, instead of the earlier value of HEAD^.

>      $ git checkout @{-1}

One problem with the syntax is that the assumption that there can only
be a single @{} construct is quite entrenched in sha1_name.c.  So if
we want to support @{-1}@{1}, that'll need some extra work.

@{-1}~2 and similar work fine though.


Other things of note:

I changed the semantics to really only look at checkouts that "did
something".  If you keep saying 'git checkout master' over and over,
those will not count towards the N.

I changed the parser to read the 'checkout: moving from $old' instead
of the 'to $new'.  The above semantics introduced too many extra
conditions for my taste if we wanted to support the pre-1.5.3 reflog
comment format.  (You'd have to check if the $to was changed from the
last [newer] entry, but then there's another border case when you do
'git init; git checkout -b side; git checkout -' because you never
moved to master.)

The @{date} syntax will be marginally slower after the refactoring
because the old parser carefully avoided parsing numbers where it
could.  (I guess it could actually be done as a bisection for an extra
order of magnitude.)

It's far too early here to be sending mail.

Interactive rebase needs a "move hunk" feature.


Thomas Rast (6):
  reflog: refactor parsing and checking
  reflog: refactor log open+mmap
  reflog: make for_each_reflog_ent use mmap
  reflog: add backwards iterator
  sha1_name: implement @{-N} syntax for N-th last checked out
  checkout: implement '@{-N}' and '-' special abbreviations

 Documentation/git-checkout.txt  |    4 +
 Documentation/git-rev-parse.txt |    3 +
 builtin-checkout.c              |   15 ++-
 cache.h                         |    1 +
 refs.c                          |  285 +++++++++++++++++++++++----------------
 refs.h                          |    1 +
 sha1_name.c                     |   79 +++++++++++-
 t/t1505-rev-parse-last.sh       |   71 ++++++++++
 t/t2012-checkout-last.sh        |   50 +++++++
 9 files changed, 387 insertions(+), 122 deletions(-)
 create mode 100755 t/t1505-rev-parse-last.sh
 create mode 100755 t/t2012-checkout-last.sh

[PATCH/RFC v3 1/6] reflog: refactor parsing and checking

[Thomas Rast]

read_ref_at() and for_each_reflog_ent() both had parsing and error
checking routines.  Refactor into a separate function that fully
parses a single entry.  Note that this switches for_each_reflog_ent()
from silently ignoring errors to die().

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 refs.c |  197 ++++++++++++++++++++++++++++++++--------------------------------
 1 files changed, 98 insertions(+), 99 deletions(-)

diff --git a/refs.c b/refs.c
index 33ced65..4571fac 100644
--- a/refs.c
+++ b/refs.c
@@ -1337,24 +1337,68 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
 	return 0;
 }
 
-static char *ref_msg(const char *line, const char *endp)
+/*
+ * Check and parse a single reflog line.  Assumes that there is only
+ * one newline in the range buf[0]..buf[len-1] (but does check that it
+ * is at buf[len-1]).
+ */
+static void parse_reflog_line(const char *buf, int len,
+			      unsigned char *osha1, unsigned char *nsha1,
+			      char **email,
+			      unsigned long *timestamp, int *tz,
+			      char **message,
+			      const char *logname)
 {
-	const char *ep;
-	line += 82;
-	ep = memchr(line, '\n', endp - line);
-	if (!ep)
-		ep = endp;
-	return xmemdupz(line, ep - line);
+	static char *retbuf = NULL;
+	static int retbufsz = 0;
+	char *tzstr, *email_end;
+
+	if (len < 83 || buf[len-1] != '\n')
+		die("Log %s is corrupt (entry too short or unterminated).", logname);
+
+	if (get_sha1_hex(buf, osha1) || buf[40] != ' ')
+		die("Log %s is corrupt (malformed old sha1).", logname);
+
+	if (get_sha1_hex(buf + 41, nsha1) || buf[81] != ' ')
+		die("Log %s is corrupt (malformed new sha1).", logname);
+
+	ALLOC_GROW(retbuf, len-82+1, retbufsz);
+	memcpy(retbuf, buf+82, len-82);
+	retbuf[len-82] = '\0';
+
+	email_end = strchr(retbuf, '>');
+	if (!email_end || email_end[1] != ' ')
+		die("Log %s is corrupt (malformed email field).", logname);
+
+	*email = retbuf;
+	email_end[1] = '\0';
+
+	*timestamp = strtoul(email_end + 2, &tzstr, 10);
+	if (!(*timestamp) || !tzstr || tzstr[0] != ' ' ||
+	    (tzstr[1] != '+' && tzstr[1] != '-') ||
+	    !isdigit(tzstr[2]) || !isdigit(tzstr[3]) ||
+	    !isdigit(tzstr[4]) || !isdigit(tzstr[5]))
+		die("Log %s is corrupt (malformed timezone).", logname);
+	if (!(tzstr[6] == '\t' || tzstr[6] == '\n'))
+		die("Log %s is corrupt (bad message field separator).", logname);
+	*tz = strtoul(tzstr, NULL, 10);
+
+	if (tzstr[6] == '\t')
+		*message = tzstr+7;
+	else
+		*message = tzstr+6;
 }
 
 int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *sha1, char **msg, unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
 {
-	const char *logfile, *logdata, *logend, *rec, *lastgt, *lastrec;
-	char *tz_c;
+	const char *logfile, *logdata, *logend, *rec, *start;
+	char *email, *message;
 	int logfd, tz, reccnt = 0;
 	struct stat st;
 	unsigned long date;
-	unsigned char logged_sha1[20];
+	unsigned char new_sha1[20];
+	unsigned char old_sha1[20];
+	unsigned char next_sha1[20];
 	void *log_mapped;
 	size_t mapsz;
 
@@ -1370,86 +1414,55 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *
 	logdata = log_mapped;
 	close(logfd);
 
-	lastrec = NULL;
 	rec = logend = logdata + st.st_size;
+	if (logdata < rec && *(rec-1) == '\n')
+		rec--;
 	while (logdata < rec) {
-		reccnt++;
-		if (logdata < rec && *(rec-1) == '\n')
-			rec--;
-		lastgt = NULL;
-		while (logdata < rec && *(rec-1) != '\n') {
-			rec--;
-			if (*rec == '>')
-				lastgt = rec;
-		}
-		if (!lastgt)
-			die("Log %s is corrupt.", logfile);
-		date = strtoul(lastgt + 1, &tz_c, 10);
+		start = memrchr(logdata, '\n', rec-logdata);
+		if (start)
+			start++;
+		else
+			start = logdata;
+		parse_reflog_line(start, rec-start+1,
+				  old_sha1, new_sha1,
+				  &email, &date, &tz, &message,
+				  logfile);
+
+		if (cutoff_time)
+			*cutoff_time = date;
+		if (cutoff_tz)
+			*cutoff_tz = tz;
+		if (cutoff_cnt)
+			*cutoff_cnt = reccnt;
+		if (msg)
+			*msg = message;
+
 		if (date <= at_time || cnt == 0) {
-			tz = strtoul(tz_c, NULL, 10);
-			if (msg)
-				*msg = ref_msg(rec, logend);
-			if (cutoff_time)
-				*cutoff_time = date;
-			if (cutoff_tz)
-				*cutoff_tz = tz;
-			if (cutoff_cnt)
-				*cutoff_cnt = reccnt - 1;
-			if (lastrec) {
-				if (get_sha1_hex(lastrec, logged_sha1))
-					die("Log %s is corrupt.", logfile);
-				if (get_sha1_hex(rec + 41, sha1))
-					die("Log %s is corrupt.", logfile);
-				if (hashcmp(logged_sha1, sha1)) {
-					fprintf(stderr,
-						"warning: Log %s has gap after %s.\n",
-						logfile, show_date(date, tz, DATE_RFC2822));
-				}
-			}
-			else if (date == at_time) {
-				if (get_sha1_hex(rec + 41, sha1))
-					die("Log %s is corrupt.", logfile);
-			}
-			else {
-				if (get_sha1_hex(rec + 41, logged_sha1))
-					die("Log %s is corrupt.", logfile);
-				if (hashcmp(logged_sha1, sha1)) {
-					fprintf(stderr,
-						"warning: Log %s unexpectedly ended on %s.\n",
-						logfile, show_date(date, tz, DATE_RFC2822));
-				}
-			}
+			if (reccnt && hashcmp(new_sha1, next_sha1))
+				fprintf(stderr,
+					"warning: Log %s has gap after %s.\n",
+					logfile, show_date(date, tz, DATE_RFC2822));
+			if (!reccnt && date < at_time && hashcmp(new_sha1, next_sha1))
+				fprintf(stderr,
+					"warning: Log %s unexpectedly ended on %s.\n",
+					logfile, show_date(date, tz, DATE_RFC2822));
+				/* leave caller's sha1 untouched */
+			else
+				hashcpy(sha1, new_sha1);
 			munmap(log_mapped, mapsz);
 			return 0;
 		}
-		lastrec = rec;
+
+		hashcpy(next_sha1, old_sha1);
+		rec = start-1;
 		if (cnt > 0)
 			cnt--;
+		reccnt++;
 	}
 
-	rec = logdata;
-	while (rec < logend && *rec != '>' && *rec != '\n')
-		rec++;
-	if (rec == logend || *rec == '\n')
-		die("Log %s is corrupt.", logfile);
-	date = strtoul(rec + 1, &tz_c, 10);
-	tz = strtoul(tz_c, NULL, 10);
-	if (get_sha1_hex(logdata, sha1))
-		die("Log %s is corrupt.", logfile);
-	if (is_null_sha1(sha1)) {
-		if (get_sha1_hex(logdata + 41, sha1))
-			die("Log %s is corrupt.", logfile);
-	}
-	if (msg)
-		*msg = ref_msg(logdata, logend);
-	munmap(log_mapped, mapsz);
+	hashcpy(sha1, new_sha1);
 
-	if (cutoff_time)
-		*cutoff_time = date;
-	if (cutoff_tz)
-		*cutoff_tz = tz;
-	if (cutoff_cnt)
-		*cutoff_cnt = reccnt;
+	munmap(log_mapped, mapsz);
 	return 1;
 }
 
@@ -1466,30 +1479,16 @@ int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
 		return -1;
 	while (fgets(buf, sizeof(buf), logfp)) {
 		unsigned char osha1[20], nsha1[20];
-		char *email_end, *message;
+		char *email, *message;
 		unsigned long timestamp;
 		int len, tz;
 
-		/* old SP new SP name <email> SP time TAB msg LF */
 		len = strlen(buf);
-		if (len < 83 || buf[len-1] != '\n' ||
-		    get_sha1_hex(buf, osha1) || buf[40] != ' ' ||
-		    get_sha1_hex(buf + 41, nsha1) || buf[81] != ' ' ||
-		    !(email_end = strchr(buf + 82, '>')) ||
-		    email_end[1] != ' ' ||
-		    !(timestamp = strtoul(email_end + 2, &message, 10)) ||
-		    !message || message[0] != ' ' ||
-		    (message[1] != '+' && message[1] != '-') ||
-		    !isdigit(message[2]) || !isdigit(message[3]) ||
-		    !isdigit(message[4]) || !isdigit(message[5]))
-			continue; /* corrupt? */
-		email_end[1] = '\0';
-		tz = strtol(message + 1, NULL, 10);
-		if (message[6] != '\t')
-			message += 6;
-		else
-			message += 7;
-		ret = fn(osha1, nsha1, buf+82, timestamp, tz, message, cb_data);
+		parse_reflog_line(buf, len,
+				  osha1, nsha1,
+				  &email, &timestamp, &tz, &message,
+				  logfile);
+		ret = fn(osha1, nsha1, email, timestamp, tz, message, cb_data);
 		if (ret)
 			break;
 	}
-- 
1.6.1.315.g92577

[PATCH/RFC v3 2/6] reflog: refactor log open+mmap

[Thomas Rast]

Move the open+mmap code from read_ref_at() to a separate function for
the next patch.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 refs.c |   54 ++++++++++++++++++++++++++++++++----------------------
 1 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/refs.c b/refs.c
index 4571fac..0a57896 100644
--- a/refs.c
+++ b/refs.c
@@ -1389,12 +1389,31 @@ static void parse_reflog_line(const char *buf, int len,
 		*message = tzstr+6;
 }
 
+static char *open_reflog(const char *ref, size_t *mapsz, const char **logfile)
+{
+	struct stat st;
+	int logfd;
+	char *map;
+
+	*logfile = git_path("logs/%s", ref);
+	logfd = open(*logfile, O_RDONLY, 0);
+	if (logfd < 0)
+		return NULL;
+	if (fstat(logfd, &st))
+		return NULL;
+
+	*mapsz = xsize_t(st.st_size);
+	map = xmmap(NULL, *mapsz, PROT_READ, MAP_PRIVATE, logfd, 0);
+	close(logfd);
+	return map;
+}
+
+
 int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *sha1, char **msg, unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
 {
-	const char *logfile, *logdata, *logend, *rec, *start;
+	const char *logfile, *logdata, *logend, *rec, *end;
 	char *email, *message;
-	int logfd, tz, reccnt = 0;
-	struct stat st;
+	int tz, reccnt = 0;
 	unsigned long date;
 	unsigned char new_sha1[20];
 	unsigned char old_sha1[20];
@@ -1402,28 +1421,20 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *
 	void *log_mapped;
 	size_t mapsz;
 
-	logfile = git_path("logs/%s", ref);
-	logfd = open(logfile, O_RDONLY, 0);
-	if (logfd < 0)
+	logdata = log_mapped = open_reflog(ref, &mapsz, &logfile);
+	if (!logdata)
 		die("Unable to read log %s: %s", logfile, strerror(errno));
-	fstat(logfd, &st);
-	if (!st.st_size)
+	if (!mapsz)
 		die("Log %s is empty.", logfile);
-	mapsz = xsize_t(st.st_size);
-	log_mapped = xmmap(NULL, mapsz, PROT_READ, MAP_PRIVATE, logfd, 0);
-	logdata = log_mapped;
-	close(logfd);
 
-	rec = logend = logdata + st.st_size;
-	if (logdata < rec && *(rec-1) == '\n')
-		rec--;
+	rec = logend = logdata + mapsz;
 	while (logdata < rec) {
-		start = memrchr(logdata, '\n', rec-logdata);
-		if (start)
-			start++;
-		else
-			start = logdata;
-		parse_reflog_line(start, rec-start+1,
+		if (logdata < rec && rec[-1] == '\n')
+			rec--;
+		end = rec;
+		while (logdata < rec && rec[-1] != '\n')
+			rec--;
+		parse_reflog_line(rec, end-rec+1,
 				  old_sha1, new_sha1,
 				  &email, &date, &tz, &message,
 				  logfile);
@@ -1454,7 +1465,6 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *
 		}
 
 		hashcpy(next_sha1, old_sha1);
-		rec = start-1;
 		if (cnt > 0)
 			cnt--;
 		reccnt++;
-- 
1.6.1.315.g92577

[PATCH/RFC v3 3/6] reflog: make for_each_reflog_ent use mmap

[Thomas Rast]

Now that all building blocks have been refactored, we can implement
for_each_reflog_ent in terms of them, so that it uses mmap.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 refs.c |   25 ++++++++++++++++---------
 1 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/refs.c b/refs.c
index 0a57896..3848aa0 100644
--- a/refs.c
+++ b/refs.c
@@ -1478,22 +1478,28 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *
 
 int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
 {
-	const char *logfile;
-	FILE *logfp;
-	char buf[1024];
+	const char *log_mapped, *buf, *log_end, *logfile;
+	size_t mapsz;
 	int ret = 0;
 
-	logfile = git_path("logs/%s", ref);
-	logfp = fopen(logfile, "r");
-	if (!logfp)
+	buf = log_mapped = open_reflog(ref, &mapsz, &logfile);
+	if (!log_mapped)
 		return -1;
-	while (fgets(buf, sizeof(buf), logfp)) {
+	log_end = buf + mapsz;
+
+	while (buf < log_end) {
 		unsigned char osha1[20], nsha1[20];
 		char *email, *message;
+		const char *next;
 		unsigned long timestamp;
 		int len, tz;
 
-		len = strlen(buf);
+		next = memchr(buf, '\n', log_end - buf);
+		if (next)
+			next++;
+		else
+			next = log_end;
+		len = next - buf;
 		parse_reflog_line(buf, len,
 				  osha1, nsha1,
 				  &email, &timestamp, &tz, &message,
@@ -1501,8 +1507,9 @@ int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
 		ret = fn(osha1, nsha1, email, timestamp, tz, message, cb_data);
 		if (ret)
 			break;
+		buf = next;
 	}
-	fclose(logfp);
+	munmap((void*) log_mapped, mapsz);
 	return ret;
 }
 
-- 
1.6.1.315.g92577

[PATCH/RFC v3 4/6] reflog: add backwards iterator

[Thomas Rast]

Add for_each_reflog_ent_backward() which does the same as
for_each_reflog_ent(), except it traverses the reflog in backwards
(newest to oldest) order.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 refs.c |   35 +++++++++++++++++++++++++++++++++++
 refs.h |    1 +
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/refs.c b/refs.c
index 3848aa0..cc78f63 100644
--- a/refs.c
+++ b/refs.c
@@ -1476,6 +1476,41 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *
 	return 1;
 }
 
+int for_each_reflog_ent_backward(const char *ref, each_reflog_ent_fn fn, void *cb_data)
+{
+	const char *log_mapped, *buf, *log_end, *logfile;
+	size_t mapsz;
+	int ret = 0;
+
+	log_mapped = open_reflog(ref, &mapsz, &logfile);
+	if (!log_mapped)
+		return -1;
+	buf = log_end = log_mapped + mapsz;
+
+	while (buf > log_mapped) {
+		unsigned char osha1[20], nsha1[20];
+		char *email, *message;
+		const char *end;
+		unsigned long timestamp;
+		int tz;
+
+		while (buf > log_mapped && buf[-1] == '\n')
+			buf--;
+		end = buf;
+		while (buf > log_mapped && buf[-1] != '\n')
+			buf--;
+		parse_reflog_line(buf, end-buf+1,
+				  osha1, nsha1,
+				  &email, &timestamp, &tz, &message,
+				  logfile);
+		ret = fn(osha1, nsha1, email, timestamp, tz, message, cb_data);
+		if (ret)
+			break;
+	}
+	munmap((void*) log_mapped, mapsz);
+	return ret;
+}
+
 int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
 {
 	const char *log_mapped, *buf, *log_end, *logfile;
diff --git a/refs.h b/refs.h
index 06ad260..723bddc 100644
--- a/refs.h
+++ b/refs.h
@@ -60,6 +60,7 @@ struct ref_lock {
 /* iterate over reflog entries */
 typedef int each_reflog_ent_fn(unsigned char *osha1, unsigned char *nsha1, const char *, unsigned long, int, const char *, void *);
 int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data);
+int for_each_reflog_ent_backward(const char *ref, each_reflog_ent_fn fn, void *cb_data);
 
 /*
  * Calls the specified function for each reflog file until it returns nonzero,
-- 
1.6.1.315.g92577

[PATCH/RFC v3 5/6] sha1_name: implement @{-N} syntax for N-th last checked out

[Thomas Rast]

Implements a new syntax @{-N} that parses the reflog for the N-th last
interesting 'checkout: moving from $branch to $new' entry and
substitutes $branch.  Here, "interesting" is defined as $branch !=
$new.  We then substitute the real branch name for the parse.

For example:

  git checkout foo
  git checkout bar
  git checkout master
  git checkout master   # did not move, so doesn't count
  git rev-parse @{-1}   # same as bar
  git rev-parse @{-2}   # same as foo
  git rev-parse @{-2}~3 # same as foo~3

Thanks to Junio for much of the code.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Documentation/git-rev-parse.txt |    3 +
 cache.h                         |    1 +
 sha1_name.c                     |   79 +++++++++++++++++++++++++++++++++++++-
 t/t1505-rev-parse-last.sh       |   71 +++++++++++++++++++++++++++++++++++
 4 files changed, 151 insertions(+), 3 deletions(-)
 create mode 100755 t/t1505-rev-parse-last.sh

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 2921da3..3ccef2f 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -212,6 +212,9 @@ when you run 'git-merge'.
   reflog of the current branch. For example, if you are on the
   branch 'blabla', then '@\{1\}' means the same as 'blabla@\{1\}'.
 
+* The special construct '@\{-<n>\}' means the <n>th branch checked out
+  before the current one.
+
 * A suffix '{caret}' to a revision parameter means the first parent of
   that commit object.  '{caret}<n>' means the <n>th parent (i.e.
   'rev{caret}'
diff --git a/cache.h b/cache.h
index 8e1af26..0dd9168 100644
--- a/cache.h
+++ b/cache.h
@@ -663,6 +663,7 @@ static inline unsigned int hexval(unsigned char c)
 extern const char *resolve_ref(const char *path, unsigned char *sha1, int, int *);
 extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
 extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
+extern int interpret_nth_last_branch(const char *str, struct strbuf *);
 
 extern int refname_match(const char *abbrev_name, const char *full_name, const char **rules);
 extern const char *ref_rev_parse_rules[];
diff --git a/sha1_name.c b/sha1_name.c
index 159c2ab..b2dd302 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -297,6 +297,8 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 	return logs_found;
 }
 
+static int get_sha1_1(const char *name, int len, unsigned char *sha1);
+
 static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 {
 	static const char *warning = "warning: refname '%.*s' is ambiguous.\n";
@@ -307,7 +309,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 	if (len == 40 && !get_sha1_hex(str, sha1))
 		return 0;
 
-	/* basic@{time or number} format to query ref-log */
+	/* basic@{time or number or -number} format to query ref-log */
 	reflog_len = at = 0;
 	if (str[len-1] == '}') {
 		for (at = 0; at < len - 1; at++) {
@@ -324,6 +326,14 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 		return -1;
 
 	if (!len && reflog_len) {
+		struct strbuf buf = STRBUF_INIT;
+		int ret;
+		/* try the @{-N} syntax for n-th checkout */
+		ret = interpret_nth_last_branch(str+at, &buf);
+		if (ret > 0) {
+			/* substitute this branch name and restart */
+			return get_sha1_1(buf.buf, buf.len, sha1);
+		}
 		/* allow "@{...}" to mean the current branch reflog */
 		refs_found = dwim_ref("HEAD", 4, sha1, &real_ref);
 	} else if (reflog_len)
@@ -379,8 +389,6 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 	return 0;
 }
 
-static int get_sha1_1(const char *name, int len, unsigned char *sha1);
-
 static int get_parent(const char *name, int len,
 		      unsigned char *result, int idx)
 {
@@ -674,6 +682,71 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	return retval;
 }
 
+struct grab_nth_branch_switch_cbdata {
+	int nth;
+	struct strbuf *buf;
+};
+
+static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
+				  const char *email, unsigned long timestamp, int tz,
+				  const char *message, void *cb_data)
+{
+	struct grab_nth_branch_switch_cbdata *cb = cb_data;
+	const char *match = NULL, *target = NULL;
+	size_t len;
+
+	if (!prefixcmp(message, "checkout: moving from ")) {
+		match = message + strlen("checkout: moving from ");
+		if ((target = strstr(match, " to ")) != NULL)
+			target += 4;
+	}
+
+	if (!match)
+		return 0;
+
+	len = target - match - 4;
+	if (target[len] == '\n' && !strncmp(match, target, len))
+		return 0;
+
+	if (--cb->nth <= 0) {
+		strbuf_reset(cb->buf);
+		strbuf_add(cb->buf, match, len);
+		return 1;
+	}
+	return 0;
+}
+
+/*
+ * This reads "@{-N}" syntax, finds the name of the Nth previous
+ * branch we were on, and places the name of the branch in the given
+ * buf and returns 0 if successful.
+ *
+ * If the input is not of the accepted format, it returns a negative
+ * number to signal an error.
+ */
+int interpret_nth_last_branch(const char *name, struct strbuf *buf)
+{
+	int nth;
+	struct grab_nth_branch_switch_cbdata cb;
+	const char *brace;
+	char *num_end;
+
+	if (name[0] != '@' || name[1] != '{' || name[2] != '-')
+		return -1;
+	brace = strchr(name, '}');
+	if (!brace)
+		return -1;
+	nth = strtol(name+3, &num_end, 10);
+	if (num_end != brace)
+		return -1;
+
+	cb.nth = nth;
+	cb.buf = buf;
+	for_each_reflog_ent_backward("HEAD", grab_nth_branch_switch, &cb);
+
+	return brace-name+1;
+}
+
 /*
  * This is like "get_sha1_basic()", except it allows "sha1 expressions",
  * notably "xyz^" for "parent of xyz"
diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh
new file mode 100755
index 0000000..1e49dd2
--- /dev/null
+++ b/t/t1505-rev-parse-last.sh
@@ -0,0 +1,71 @@
+#!/bin/sh
+
+test_description='test @{-N} syntax'
+
+. ./test-lib.sh
+
+
+make_commit () {
+	echo "$1" > "$1" &&
+	git add "$1" &&
+	git commit -m "$1"
+}
+
+
+test_expect_success 'setup' '
+
+	make_commit 1 &&
+	git branch side &&
+	make_commit 2 &&
+	make_commit 3 &&
+	git checkout side &&
+	make_commit 4 &&
+	git merge master &&
+	git checkout master
+
+'
+
+# 1 -- 2 -- 3 master
+#  \         \
+#   \         \
+#    --- 4 --- 5 side
+#
+# and 'side' should be the last branch
+
+git log --graph --all --pretty=oneline --decorate
+
+test_rev_equivalent () {
+
+	git rev-parse "$1" > expect &&
+	git rev-parse "$2" > output &&
+	test_cmp expect output
+
+}
+
+test_expect_success '@{-1} works' '
+	test_rev_equivalent side @{-1}
+'
+
+test_expect_success '@{-1}~2 works' '
+	test_rev_equivalent side~2 @{-1}~2
+'
+
+test_expect_success '@{-1}^2 works' '
+	test_rev_equivalent side^2 @{-1}^2
+'
+
+test_expect_failure '@{-1}@{1} works' '
+	test_rev_equivalent side@{1} @{-1}@{1}
+'
+
+test_expect_success '@{-2} works' '
+	test_rev_equivalent master @{-2}
+'
+
+test_expect_success '@{-3} fails' '
+	test_must_fail git rev-parse @{-3}
+'
+
+test_done
+
+
-- 
1.6.1.315.g92577

[PATCH/RFC v3 6/6] checkout: implement '@{-N}' and '-' special abbreviations

[Thomas Rast]

Checks if the branch to be checked out is either '@{-N}' or the
special shorthand '-' for '@{-1}' (i.e. the last checked out branch).
If so, we take it to mean the branch name, not the corresponding SHA,
so that we check out an attached HEAD on that branch.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Documentation/git-checkout.txt |    4 +++
 builtin-checkout.c             |   15 ++++++++++-
 t/t2012-checkout-last.sh       |   50 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 2 deletions(-)
 create mode 100755 t/t2012-checkout-last.sh

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 9cd5151..3bccffa 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -133,6 +133,10 @@ the conflicted merge in the specified paths.
 +
 When this parameter names a non-branch (but still a valid commit object),
 your HEAD becomes 'detached'.
++
+As a special case, the "`@\{-N\}`" syntax for the N-th last branch
+checks out the branch (instead of detaching).  You may also specify
+"`-`" which is synonymous with "`@\{-1\}`".
 
 
 Detached HEAD
diff --git a/builtin-checkout.c b/builtin-checkout.c
index b5dd9c0..b0a101b 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -361,8 +361,16 @@ struct branch_info {
 static void setup_branch_path(struct branch_info *branch)
 {
 	struct strbuf buf = STRBUF_INIT;
-	strbuf_addstr(&buf, "refs/heads/");
-	strbuf_addstr(&buf, branch->name);
+	int ret;
+
+	if ((ret = interpret_nth_last_branch(branch->name, &buf))
+	    && ret == strlen(branch->name)) {
+		branch->name = xstrdup(buf.buf);
+		strbuf_splice(&buf, 0, 0, "refs/heads/", 11);
+	} else {
+		strbuf_addstr(&buf, "refs/heads/");
+		strbuf_addstr(&buf, branch->name);
+	}
 	branch->path = strbuf_detach(&buf, NULL);
 }
 
@@ -671,6 +679,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		arg = argv[0];
 		has_dash_dash = (argc > 1) && !strcmp(argv[1], "--");
 
+		if (!strcmp(arg, "-"))
+			arg = "@{-1}";
+
 		if (get_sha1(arg, rev)) {
 			if (has_dash_dash)          /* case (1) */
 				die("invalid reference: %s", arg);
diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
new file mode 100755
index 0000000..320f6eb
--- /dev/null
+++ b/t/t2012-checkout-last.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='checkout can switch to last branch'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo hello >world &&
+	git add world &&
+	git commit -m initial &&
+	git branch other &&
+	echo "hello again" >>world &&
+	git add world &&
+	git commit -m second
+'
+
+test_expect_success '"checkout -" does not work initially' '
+	test_must_fail git checkout -
+'
+
+test_expect_success 'first branch switch' '
+	git checkout other
+'
+
+test_expect_success '"checkout -" switches back' '
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master"
+'
+
+test_expect_success '"checkout -" switches forth' '
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/other"
+'
+
+test_expect_success 'detach HEAD' '
+	git checkout $(git rev-parse HEAD)
+'
+
+test_expect_success '"checkout -" attaches again' '
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/other"
+'
+
+test_expect_success '"checkout -" detaches again' '
+	git checkout - &&
+	test "z$(git rev-parse HEAD)" = "z$(git rev-parse other)" &&
+	test_must_fail git symbolic-ref HEAD
+'
+
+test_done
-- 
1.6.1.315.g92577

Re: [PATCH/RFC v3 1/6] reflog: refactor parsing and checking

[Johannes Schindelin]

Hi,

On Sat, 17 Jan 2009, Thomas Rast wrote:

> Note that this switches for_each_reflog_ent() from silently ignoring 
> errors to die().

I am not convinced this is necessary, and neither that it is good.

You could return int from parse_reflog_line() just like the other 
non-dying functions.

And keep in mind that reflogs could be corrupted for all kinds of reasons; 
with your patch, reflog handling would fail gracelessly, even if a 
lot could still be salvaged (by ignoring the error with a warning).

Ciao,
Dscho

Re: [PATCH/RFC v3 2/6] reflog: refactor log open+mmap

[Johannes Schindelin]

Hi,

On Sat, 17 Jan 2009, Thomas Rast wrote:

> +static char *open_reflog(const char *ref, size_t *mapsz, const char **logfile)
> +{
> +	struct stat st;
> +	int logfd;
> +	char *map;
> +
> +	*logfile = git_path("logs/%s", ref);

That is dangerous.  git_path() returns a pointer to a static buffer.  
Before your patch, logfile was a local variable, and one could be 
relatively sure that git_path() was not called during the lifetime.  Now 
the lifetime of logfile is no longer as clear-cut, and it is much easier 
to overlook that git_path() must not be called while logfile holds a 
reference to its static buffer.

Ciao,
Dscho

Re: [PATCH/RFC v3 0/6] N-th last checked out branch

[Johannes Schindelin]

Hi,

On Sat, 17 Jan 2009, Thomas Rast wrote:

>  Documentation/git-checkout.txt  |    4 +
>  Documentation/git-rev-parse.txt |    3 +
>  builtin-checkout.c              |   15 ++-
>  cache.h                         |    1 +
>  refs.c                          |  285 +++++++++++++++++++++++----------------
>  refs.h                          |    1 +
>  sha1_name.c                     |   79 +++++++++++-
>  t/t1505-rev-parse-last.sh       |   71 ++++++++++
>  t/t2012-checkout-last.sh        |   50 +++++++
>  9 files changed, 387 insertions(+), 122 deletions(-)
>  create mode 100755 t/t1505-rev-parse-last.sh
>  create mode 100755 t/t2012-checkout-last.sh

Let's quickly compare that to what Junio sent:

 builtin-checkout.c |   10 +++++-
 cache.h            |    1 +
 sha1_name.c        |   78 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 2 deletions(-)

Granted, the documentation and tests are nice, but why that big change in 
refs.c?  After all, the more you change in one go, the more you can break.

Let's take one step back.  The two things that Junio's patch left to be 
desired (in addition to documentation and tests) are

- reflogs are traversed twice.  While this is not a showstopper, it is 
  easily improved by building a string_list and then picking the one entry 
  we're interested in, and

- the '-' handling you seem to want.

I really have to ask: why did you not work on top of Junio's patch, just 
adding docs, tests, and checkout -?  And then -- maybe -- the 
string_list...

Although I have to admit that I am not _that_ convinced the string_list is 
worth it: reflogs are not evaluated all the time, so it is definitely not 
performance critical.

Ciao,
Dscho

Re: [PATCH/RFC v3 0/6] N-th last checked out branch

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 1170 bytes --]

Johannes Schindelin wrote:
> On Sat, 17 Jan 2009, Thomas Rast wrote:
> >  9 files changed, 387 insertions(+), 122 deletions(-)
> 
> Let's quickly compare that to what Junio sent:
> 
>  builtin-checkout.c |   10 +++++-
>  cache.h            |    1 +
>  sha1_name.c        |   78 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 87 insertions(+), 2 deletions(-)
[...]
> - the '-' handling you seem to want.

Let's be clear that the handling *I* wanted is this big:

 builtin-checkout.c                     |   27 ++++++++++++++++-

In any case, I'll follow up with a version that still traverses the
logs twice and thus doesn't need the first two.

> I really have to ask: why did you not work on top of Junio's patch, just 
> adding docs, tests, and checkout -?  And then -- maybe -- the 
> string_list...
> 
> Although I have to admit that I am not _that_ convinced the string_list is 
> worth it: reflogs are not evaluated all the time, so it is definitely not 
> performance critical.

I take it you have some idea where and how string_list fits into this
topic?

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


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH/RFC v3bis 1/2] sha1_name: implement @{-N} syntax for N-th last checked out

[Thomas Rast]

Implements a new syntax @{-N} that parses the reflog for the N-th last
interesting 'checkout: moving from $branch to $new' entry and
substitutes $branch.  Here, "interesting" is defined as $branch !=
$new.  We then substitute the real branch name for the parse.

For example:

  git checkout foo
  git checkout bar
  git checkout master
  git checkout master   # did not move, so doesn't count
  git rev-parse @{-1}   # same as bar
  git rev-parse @{-2}   # same as foo
  git rev-parse @{-2}~3 # same as foo~3

Thanks to Junio for much of the code.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Documentation/git-rev-parse.txt |    3 +
 cache.h                         |    1 +
 sha1_name.c                     |   99 +++++++++++++++++++++++++++++++++++++-
 t/t1505-rev-parse-last.sh       |   71 ++++++++++++++++++++++++++++
 4 files changed, 171 insertions(+), 3 deletions(-)
 create mode 100755 t/t1505-rev-parse-last.sh

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 2921da3..3ccef2f 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -212,6 +212,9 @@ when you run 'git-merge'.
   reflog of the current branch. For example, if you are on the
   branch 'blabla', then '@\{1\}' means the same as 'blabla@\{1\}'.
 
+* The special construct '@\{-<n>\}' means the <n>th branch checked out
+  before the current one.
+
 * A suffix '{caret}' to a revision parameter means the first parent of
   that commit object.  '{caret}<n>' means the <n>th parent (i.e.
   'rev{caret}'
diff --git a/cache.h b/cache.h
index 8e1af26..0dd9168 100644
--- a/cache.h
+++ b/cache.h
@@ -663,6 +663,7 @@ static inline unsigned int hexval(unsigned char c)
 extern const char *resolve_ref(const char *path, unsigned char *sha1, int, int *);
 extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
 extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
+extern int interpret_nth_last_branch(const char *str, struct strbuf *);
 
 extern int refname_match(const char *abbrev_name, const char *full_name, const char **rules);
 extern const char *ref_rev_parse_rules[];
diff --git a/sha1_name.c b/sha1_name.c
index 159c2ab..9e1538e 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -297,6 +297,8 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 	return logs_found;
 }
 
+static int get_sha1_1(const char *name, int len, unsigned char *sha1);
+
 static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 {
 	static const char *warning = "warning: refname '%.*s' is ambiguous.\n";
@@ -307,7 +309,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 	if (len == 40 && !get_sha1_hex(str, sha1))
 		return 0;
 
-	/* basic@{time or number} format to query ref-log */
+	/* basic@{time or number or -number} format to query ref-log */
 	reflog_len = at = 0;
 	if (str[len-1] == '}') {
 		for (at = 0; at < len - 1; at++) {
@@ -324,6 +326,16 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 		return -1;
 
 	if (!len && reflog_len) {
+		struct strbuf buf = STRBUF_INIT;
+		int ret;
+		/* try the @{-N} syntax for n-th checkout */
+		ret = interpret_nth_last_branch(str+at, &buf);
+		if (ret > 0) {
+			/* substitute this branch name and restart */
+			return get_sha1_1(buf.buf, buf.len, sha1);
+		} else if (ret == 0) {
+			return -1;
+		}
 		/* allow "@{...}" to mean the current branch reflog */
 		refs_found = dwim_ref("HEAD", 4, sha1, &real_ref);
 	} else if (reflog_len)
@@ -379,8 +391,6 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 	return 0;
 }
 
-static int get_sha1_1(const char *name, int len, unsigned char *sha1);
-
 static int get_parent(const char *name, int len,
 		      unsigned char *result, int idx)
 {
@@ -674,6 +684,89 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	return retval;
 }
 
+struct grab_nth_branch_switch_cbdata {
+	int counting;
+	int nth;
+	struct strbuf *buf;
+};
+
+static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
+				  const char *email, unsigned long timestamp, int tz,
+				  const char *message, void *cb_data)
+{
+	struct grab_nth_branch_switch_cbdata *cb = cb_data;
+	const char *match = NULL, *target = NULL;
+	size_t len;
+
+	if (!prefixcmp(message, "checkout: moving from ")) {
+		match = message + strlen("checkout: moving from ");
+		if ((target = strstr(match, " to ")) != NULL)
+			target += 4;
+	}
+
+	if (!match)
+		return 0;
+
+	len = target - match - 4;
+	if (target[len] == '\n' && !strncmp(match, target, len))
+		return 0;
+
+	if (cb->counting) {
+		cb->nth++;
+		return 0;
+	}
+
+	if (cb->nth-- <= 0) {
+		strbuf_reset(cb->buf);
+		strbuf_add(cb->buf, match, len);
+		return 1;
+	}
+	return 0;
+}
+
+/*
+ * This reads "@{-N}" syntax, finds the name of the Nth previous
+ * branch we were on, and places the name of the branch in the given
+ * buf and returns the number of characters parsed if successful.
+ *
+ * If the input is not of the accepted format, it returns a negative
+ * number to signal an error.
+ *
+ * If the input was ok but there are not N branch switches in the
+ * reflog, it returns 0.
+ */
+int interpret_nth_last_branch(const char *name, struct strbuf *buf)
+{
+	int nth;
+	struct grab_nth_branch_switch_cbdata cb;
+	const char *brace;
+	char *num_end;
+
+	if (name[0] != '@' || name[1] != '{' || name[2] != '-')
+		return -1;
+	brace = strchr(name, '}');
+	if (!brace)
+		return -1;
+	nth = strtol(name+3, &num_end, 10);
+	if (num_end != brace)
+		return -1;
+
+	cb.counting = 1;
+	cb.nth = 0;
+	cb.buf = buf;
+	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+
+	if (cb.nth < nth)
+		return 0;
+
+	cb.counting = 0;
+	cb.nth -= nth;
+	cb.buf = buf;
+	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+
+	return brace-name+1;
+}
+
 /*
  * This is like "get_sha1_basic()", except it allows "sha1 expressions",
  * notably "xyz^" for "parent of xyz"
diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh
new file mode 100755
index 0000000..1e49dd2
--- /dev/null
+++ b/t/t1505-rev-parse-last.sh
@@ -0,0 +1,71 @@
+#!/bin/sh
+
+test_description='test @{-N} syntax'
+
+. ./test-lib.sh
+
+
+make_commit () {
+	echo "$1" > "$1" &&
+	git add "$1" &&
+	git commit -m "$1"
+}
+
+
+test_expect_success 'setup' '
+
+	make_commit 1 &&
+	git branch side &&
+	make_commit 2 &&
+	make_commit 3 &&
+	git checkout side &&
+	make_commit 4 &&
+	git merge master &&
+	git checkout master
+
+'
+
+# 1 -- 2 -- 3 master
+#  \         \
+#   \         \
+#    --- 4 --- 5 side
+#
+# and 'side' should be the last branch
+
+git log --graph --all --pretty=oneline --decorate
+
+test_rev_equivalent () {
+
+	git rev-parse "$1" > expect &&
+	git rev-parse "$2" > output &&
+	test_cmp expect output
+
+}
+
+test_expect_success '@{-1} works' '
+	test_rev_equivalent side @{-1}
+'
+
+test_expect_success '@{-1}~2 works' '
+	test_rev_equivalent side~2 @{-1}~2
+'
+
+test_expect_success '@{-1}^2 works' '
+	test_rev_equivalent side^2 @{-1}^2
+'
+
+test_expect_failure '@{-1}@{1} works' '
+	test_rev_equivalent side@{1} @{-1}@{1}
+'
+
+test_expect_success '@{-2} works' '
+	test_rev_equivalent master @{-2}
+'
+
+test_expect_success '@{-3} fails' '
+	test_must_fail git rev-parse @{-3}
+'
+
+test_done
+
+
-- 
1.6.1.315.g92577

[PATCH/RFC v3bis 2/2] checkout: implement '@{-N}' and '-' special abbreviations

[Thomas Rast]

Checks if the branch to be checked out is either '@{-N}' or the
special shorthand '-' for '@{-1}' (i.e. the last checked out branch).
If so, we take it to mean the branch name, not the corresponding SHA,
so that we check out an attached HEAD on that branch.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Documentation/git-checkout.txt |    4 +++
 builtin-checkout.c             |   15 ++++++++++-
 t/t2012-checkout-last.sh       |   50 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 2 deletions(-)
 create mode 100755 t/t2012-checkout-last.sh

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 9cd5151..3bccffa 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -133,6 +133,10 @@ the conflicted merge in the specified paths.
 +
 When this parameter names a non-branch (but still a valid commit object),
 your HEAD becomes 'detached'.
++
+As a special case, the "`@\{-N\}`" syntax for the N-th last branch
+checks out the branch (instead of detaching).  You may also specify
+"`-`" which is synonymous with "`@\{-1\}`".
 
 
 Detached HEAD
diff --git a/builtin-checkout.c b/builtin-checkout.c
index b5dd9c0..b0a101b 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -361,8 +361,16 @@ struct branch_info {
 static void setup_branch_path(struct branch_info *branch)
 {
 	struct strbuf buf = STRBUF_INIT;
-	strbuf_addstr(&buf, "refs/heads/");
-	strbuf_addstr(&buf, branch->name);
+	int ret;
+
+	if ((ret = interpret_nth_last_branch(branch->name, &buf))
+	    && ret == strlen(branch->name)) {
+		branch->name = xstrdup(buf.buf);
+		strbuf_splice(&buf, 0, 0, "refs/heads/", 11);
+	} else {
+		strbuf_addstr(&buf, "refs/heads/");
+		strbuf_addstr(&buf, branch->name);
+	}
 	branch->path = strbuf_detach(&buf, NULL);
 }
 
@@ -671,6 +679,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		arg = argv[0];
 		has_dash_dash = (argc > 1) && !strcmp(argv[1], "--");
 
+		if (!strcmp(arg, "-"))
+			arg = "@{-1}";
+
 		if (get_sha1(arg, rev)) {
 			if (has_dash_dash)          /* case (1) */
 				die("invalid reference: %s", arg);
diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
new file mode 100755
index 0000000..320f6eb
--- /dev/null
+++ b/t/t2012-checkout-last.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='checkout can switch to last branch'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo hello >world &&
+	git add world &&
+	git commit -m initial &&
+	git branch other &&
+	echo "hello again" >>world &&
+	git add world &&
+	git commit -m second
+'
+
+test_expect_success '"checkout -" does not work initially' '
+	test_must_fail git checkout -
+'
+
+test_expect_success 'first branch switch' '
+	git checkout other
+'
+
+test_expect_success '"checkout -" switches back' '
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master"
+'
+
+test_expect_success '"checkout -" switches forth' '
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/other"
+'
+
+test_expect_success 'detach HEAD' '
+	git checkout $(git rev-parse HEAD)
+'
+
+test_expect_success '"checkout -" attaches again' '
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/other"
+'
+
+test_expect_success '"checkout -" detaches again' '
+	git checkout - &&
+	test "z$(git rev-parse HEAD)" = "z$(git rev-parse other)" &&
+	test_must_fail git symbolic-ref HEAD
+'
+
+test_done
-- 
1.6.1.315.g92577

[PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Johannes Schindelin]

Instead of traversing them twice, we just build a list of branch switches,
pick the one we're interested in, and free the list again.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Sat, 17 Jan 2009, Thomas Rast wrote:

	> Johannes Schindelin wrote:
	> 
	> > I really have to ask: why did you not work on top of Junio's 
	> > patch, just adding docs, tests, and checkout -?  And then -- maybe -- 
	> > the string_list...

	Of course, I meant the patch as-is, with Junio as author.  But 
	hey, if he does not care...

	> > Although I have to admit that I am not _that_ convinced the 
	> > string_list is worth it: reflogs are not evaluated all the time, so 
	> > it is definitely not performance critical.
	>
	> I take it you have some idea where and how string_list fits into 
	> this topic?

	Indeed.

	This patch was generated using --collapse-non-alnums for readability.
	Did not really help much.

 sha1_name.c |   72 ++++++++++++++++++++++++----------------------------------
 1 files changed, 30 insertions(+), 42 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 2bbc5f1..306d04b 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -6,6 +6,7 @@
 #include "tree-walk.h"
 #include "refs.h"
 #include "cache-tree.h"
+#include "string-list.h"
 
 static int find_short_object_filename(int len, const char *name, unsigned char *sha1)
 {
@@ -691,43 +692,31 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	return retval;
 }
 
-struct grab_nth_branch_switch_cbdata {
-	int counting;
-	int nth;
-	struct strbuf *buf;
-};
-
-static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
+static int add_one_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 				  const char *email, unsigned long timestamp, int tz,
 				  const char *message, void *cb_data)
 {
-	struct grab_nth_branch_switch_cbdata *cb = cb_data;
+	struct string_list *list = cb_data;
 	const char *match = NULL, *target = NULL;
 	size_t len;
-
-	if (!prefixcmp(message, "checkout: moving from ")) {
-		match = message + strlen("checkout: moving from ");
-		if ((target = strstr(match, " to ")) != NULL)
-			target += 4;
-	}
-
-	if (!match)
+
+	if (prefixcmp(message, "checkout: moving from "))
 		return 0;
 
-	len = target - match - 4;
-	if (target[len] == '\n' && !strncmp(match, target, len))
-		return 0;
-
-	if (cb->counting) {
-		cb->nth++;
-		return 0;
-	}
-
-	if (cb->nth-- <= 0) {
-		strbuf_reset(cb->buf);
-		strbuf_add(cb->buf, match, len);
-		return 1;
-	}
+	match = message + strlen("checkout: moving from ");
+
+	/* Is it "moving" from a branch to itself?  Then ignore it. */
+	if ((target = strstr(match, " to ")) != NULL) {
+		target += 4;
+		len = target - match - 4;
+		if (target[len] == '\n' && !strncmp(match, target, len))
+			return 0;
+	}
+	else
+		len = strchrnul(match, ' ') - match;
+
+	string_list_append(xstrndup(match, len), list);
+
 	return 0;
 }
 
@@ -745,7 +734,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 int interpret_nth_last_branch(const char *name, struct strbuf *buf)
 {
 	int nth;
-	struct grab_nth_branch_switch_cbdata cb;
+	struct string_list branch_list = { NULL, 0, 0, 0 };
 	const char *brace;
 	char *num_end;
 
@@ -758,18 +747,17 @@ int interpret_nth_last_branch(const char *name, struct strbuf *buf)
 	if (num_end != brace)
 		return -1;
 
-	cb.counting = 1;
-	cb.nth = 0;
-	cb.buf = buf;
-	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
-
-	if (cb.nth < nth)
+	for_each_reflog_ent("HEAD", add_one_branch_switch, &branch_list);
+
+	if (branch_list.nr < nth)
 		return 0;
-
-	cb.counting = 0;
-	cb.nth -= nth;
-	cb.buf = buf;
-	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+
+	strbuf_reset(buf);
+	strbuf_addstr(buf, branch_list.items[branch_list.nr - nth].string);
+
+	/* force free()ing the items */
+	branch_list.strdup_strings = 1;
+	string_list_clear(&branch_list, 0);
 
 	return brace-name+1;
 }
-- 
1.6.1.325.g062d4

[PATCH/RFC v4 0/5] N-th last checked out branch

[Thomas Rast]

Johannes Schindelin wrote:
> 
> Instead of traversing them twice, we just build a list of branch switches,
> pick the one we're interested in, and free the list again.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 
> On Sat, 17 Jan 2009, Thomas Rast wrote:
> 
> > Johannes Schindelin wrote:
> > 
> > > I really have to ask: why did you not work on top of Junio's 
> > > patch, just adding docs, tests, and checkout -?  And then -- maybe -- 
> > > the string_list...
> 
> Of course, I meant the patch as-is, with Junio as author.  But 
> hey, if he does not care...

*shrug*

For one thing there was no commit message.

I made up a message, split the changes I made into smaller patches,
and added a fixed up version of your patch on top, since it had some
context that is not in any version I have.


Johannes Schindelin (1):
  interpret_nth_last_branch(): avoid traversing the reflogs twice

Junio C Hamano (1):
  checkout: implement "@{-N}" shortcut name for N-th last branch

Thomas Rast (3):
  sha1_name: tweak @{-N} lookup
  sha1_name: support @{-N} syntax in get_sha1()
  checkout: implement "-" abbreviation, add docs and tests

 Documentation/git-checkout.txt  |    4 ++
 Documentation/git-rev-parse.txt |    3 +
 builtin-checkout.c              |   15 ++++++-
 cache.h                         |    1 +
 sha1_name.c                     |   88 +++++++++++++++++++++++++++++++++++++-
 t/t1505-rev-parse-last.sh       |   71 +++++++++++++++++++++++++++++++
 t/t2012-checkout-last.sh        |   50 ++++++++++++++++++++++
 7 files changed, 227 insertions(+), 5 deletions(-)
 create mode 100755 t/t1505-rev-parse-last.sh
 create mode 100755 t/t2012-checkout-last.sh

[PATCH/RFC v4 1/5] checkout: implement "@{-N}" shortcut name for N-th last branch

[Thomas Rast]

From: Junio C Hamano <gitster@pobox.com>

Implement a shortcut @{-N} for the N-th last branch checked out, that
works by parsing the reflog for the message added by previous
git-checkout invocations.  We expand the @{-N} to the branch name, so
that you end up on an attached HEAD on that branch.
---
 builtin-checkout.c |   10 +++++-
 cache.h            |    1 +
 sha1_name.c        |   78 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index b5dd9c0..a3b69d6 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -361,8 +361,14 @@ struct branch_info {
 static void setup_branch_path(struct branch_info *branch)
 {
 	struct strbuf buf = STRBUF_INIT;
-	strbuf_addstr(&buf, "refs/heads/");
-	strbuf_addstr(&buf, branch->name);
+
+	if (!interpret_nth_last_branch(branch->name, &buf)) {
+		branch->name = xstrdup(buf.buf);
+		strbuf_splice(&buf, 0, 0, "refs/heads/", 11);
+	} else {
+		strbuf_addstr(&buf, "refs/heads/");
+		strbuf_addstr(&buf, branch->name);
+	}
 	branch->path = strbuf_detach(&buf, NULL);
 }
 
diff --git a/cache.h b/cache.h
index 8e1af26..0dd9168 100644
--- a/cache.h
+++ b/cache.h
@@ -663,6 +663,7 @@ static inline unsigned int hexval(unsigned char c)
 extern const char *resolve_ref(const char *path, unsigned char *sha1, int, int *);
 extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
 extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
+extern int interpret_nth_last_branch(const char *str, struct strbuf *);
 
 extern int refname_match(const char *abbrev_name, const char *full_name, const char **rules);
 extern const char *ref_rev_parse_rules[];
diff --git a/sha1_name.c b/sha1_name.c
index 159c2ab..6377264 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -674,6 +674,84 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	return retval;
 }
 
+struct grab_nth_branch_switch_cbdata {
+	int counting;
+	int nth;
+	struct strbuf *buf;
+};
+
+static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
+				  const char *email, unsigned long timestamp, int tz,
+				  const char *message, void *cb_data)
+{
+	struct grab_nth_branch_switch_cbdata *cb = cb_data;
+	const char *match = NULL;
+
+	if (!prefixcmp(message, "checkout: moving to "))
+		match = message + strlen("checkout: moving to ");
+	else if (!prefixcmp(message, "checkout: moving from ")) {
+		const char *cp = message + strlen("checkout: moving from ");
+		if ((cp = strstr(cp, " to ")) != NULL) {
+			match = cp + 4;
+		}
+	}
+
+	if (!match)
+		return 0;
+
+	if (cb->counting) {
+		cb->nth++;
+		return 0;
+	}
+
+	if (--cb->nth <= 0) {
+		size_t len = strlen(match);
+		while (match[len-1] == '\n')
+			len--;
+		strbuf_reset(cb->buf);
+		strbuf_add(cb->buf, match, len);
+		return 1;
+	}
+	return 0;
+}
+
+/*
+ * This reads "@{-N}" syntax, finds the name of the Nth previous
+ * branch we were on, and places the name of the branch in the given
+ * buf and returns 0 if successful.
+ *
+ * If the input is not of the accepted format, it returns a negative
+ * number to signal an error.
+ */
+int interpret_nth_last_branch(const char *name, struct strbuf *buf)
+{
+	int nth, i;
+	struct grab_nth_branch_switch_cbdata cb;
+
+	if (name[0] != '@' || name[1] != '{' || name[2] != '-')
+		return -1;
+	for (i = 3, nth = 0; name[i] && name[i] != '}'; i++) {
+		char ch = name[i];
+		if ('0' <= ch && ch <= '9')
+			nth = nth * 10 + ch - '0';
+		else
+			return -1;
+	}
+	if (nth < 0 || 10 <= nth)
+		return -1;
+
+	cb.counting = 1;
+	cb.nth = 0;
+	cb.buf = buf;
+	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+
+	cb.counting = 0;
+	cb.nth -= nth;
+	cb.buf = buf;
+	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+	return 0;
+}
+
 /*
  * This is like "get_sha1_basic()", except it allows "sha1 expressions",
  * notably "xyz^" for "parent of xyz"
-- 
1.6.1.315.g92577

[PATCH/RFC v4 2/5] sha1_name: tweak @{-N} lookup

[Thomas Rast]

Have the lookup only look at "interesting" checkouts, meaning those
that tell you "Already on ..." don't count even though they also cause
a reflog entry.

Let interpret_nth_last_branch() return the number of characters
parsed, so that git-checkout can verify that the branch spec was
@{-N}, not @{-1}^2 or something like that.  (The latter will be added
later.)

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 builtin-checkout.c |    4 ++-
 sha1_name.c        |   53 ++++++++++++++++++++++++++++-----------------------
 2 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index a3b69d6..dc1de06 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -361,8 +361,10 @@ struct branch_info {
 static void setup_branch_path(struct branch_info *branch)
 {
 	struct strbuf buf = STRBUF_INIT;
+	int ret;
 
-	if (!interpret_nth_last_branch(branch->name, &buf)) {
+	if ((ret = interpret_nth_last_branch(branch->name, &buf))
+	    && ret == strlen(branch->name)) {
 		branch->name = xstrdup(buf.buf);
 		strbuf_splice(&buf, 0, 0, "refs/heads/", 11);
 	} else {
diff --git a/sha1_name.c b/sha1_name.c
index 6377264..34e39db 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -685,29 +685,28 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 				  const char *message, void *cb_data)
 {
 	struct grab_nth_branch_switch_cbdata *cb = cb_data;
-	const char *match = NULL;
-
-	if (!prefixcmp(message, "checkout: moving to "))
-		match = message + strlen("checkout: moving to ");
-	else if (!prefixcmp(message, "checkout: moving from ")) {
-		const char *cp = message + strlen("checkout: moving from ");
-		if ((cp = strstr(cp, " to ")) != NULL) {
-			match = cp + 4;
-		}
+	const char *match = NULL, *target = NULL;
+	size_t len;
+
+	if (!prefixcmp(message, "checkout: moving from ")) {
+		match = message + strlen("checkout: moving from ");
+		if ((target = strstr(match, " to ")) != NULL)
+			target += 4;
 	}
 
 	if (!match)
 		return 0;
 
+	len = target - match - 4;
+	if (target[len] == '\n' && !strncmp(match, target, len))
+		return 0;
+
 	if (cb->counting) {
 		cb->nth++;
 		return 0;
 	}
 
-	if (--cb->nth <= 0) {
-		size_t len = strlen(match);
-		while (match[len-1] == '\n')
-			len--;
+	if (cb->nth-- <= 0) {
 		strbuf_reset(cb->buf);
 		strbuf_add(cb->buf, match, len);
 		return 1;
@@ -718,26 +717,28 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 /*
  * This reads "@{-N}" syntax, finds the name of the Nth previous
  * branch we were on, and places the name of the branch in the given
- * buf and returns 0 if successful.
+ * buf and returns the number of characters parsed if successful.
  *
  * If the input is not of the accepted format, it returns a negative
  * number to signal an error.
+ *
+ * If the input was ok but there are not N branch switches in the
+ * reflog, it returns 0.
  */
 int interpret_nth_last_branch(const char *name, struct strbuf *buf)
 {
-	int nth, i;
+	int nth;
 	struct grab_nth_branch_switch_cbdata cb;
+	const char *brace;
+	char *num_end;
 
 	if (name[0] != '@' || name[1] != '{' || name[2] != '-')
 		return -1;
-	for (i = 3, nth = 0; name[i] && name[i] != '}'; i++) {
-		char ch = name[i];
-		if ('0' <= ch && ch <= '9')
-			nth = nth * 10 + ch - '0';
-		else
-			return -1;
-	}
-	if (nth < 0 || 10 <= nth)
+	brace = strchr(name, '}');
+	if (!brace)
+		return -1;
+	nth = strtol(name+3, &num_end, 10);
+	if (num_end != brace)
 		return -1;
 
 	cb.counting = 1;
@@ -745,11 +746,15 @@ int interpret_nth_last_branch(const char *name, struct strbuf *buf)
 	cb.buf = buf;
 	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
 
+	if (cb.nth < nth)
+		return 0;
+
 	cb.counting = 0;
 	cb.nth -= nth;
 	cb.buf = buf;
 	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
-	return 0;
+
+	return brace-name+1;
 }
 
 /*
-- 
1.6.1.315.g92577

[PATCH/RFC v4 3/5] sha1_name: support @{-N} syntax in get_sha1()

[Thomas Rast]

Let get_sha1() parse the @{-N} syntax, with docs and tests.

Note that while @{-1}^2, @{-2}~5 and such are supported, @{-1}@{1} is
currently not allowed.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Documentation/git-rev-parse.txt |    3 ++
 sha1_name.c                     |   16 +++++++--
 t/t1505-rev-parse-last.sh       |   71 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 3 deletions(-)
 create mode 100755 t/t1505-rev-parse-last.sh

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 2921da3..3ccef2f 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -212,6 +212,9 @@ when you run 'git-merge'.
   reflog of the current branch. For example, if you are on the
   branch 'blabla', then '@\{1\}' means the same as 'blabla@\{1\}'.
 
+* The special construct '@\{-<n>\}' means the <n>th branch checked out
+  before the current one.
+
 * A suffix '{caret}' to a revision parameter means the first parent of
   that commit object.  '{caret}<n>' means the <n>th parent (i.e.
   'rev{caret}'
diff --git a/sha1_name.c b/sha1_name.c
index 34e39db..9e1538e 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -297,6 +297,8 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 	return logs_found;
 }
 
+static int get_sha1_1(const char *name, int len, unsigned char *sha1);
+
 static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 {
 	static const char *warning = "warning: refname '%.*s' is ambiguous.\n";
@@ -307,7 +309,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 	if (len == 40 && !get_sha1_hex(str, sha1))
 		return 0;
 
-	/* basic@{time or number} format to query ref-log */
+	/* basic@{time or number or -number} format to query ref-log */
 	reflog_len = at = 0;
 	if (str[len-1] == '}') {
 		for (at = 0; at < len - 1; at++) {
@@ -324,6 +326,16 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 		return -1;
 
 	if (!len && reflog_len) {
+		struct strbuf buf = STRBUF_INIT;
+		int ret;
+		/* try the @{-N} syntax for n-th checkout */
+		ret = interpret_nth_last_branch(str+at, &buf);
+		if (ret > 0) {
+			/* substitute this branch name and restart */
+			return get_sha1_1(buf.buf, buf.len, sha1);
+		} else if (ret == 0) {
+			return -1;
+		}
 		/* allow "@{...}" to mean the current branch reflog */
 		refs_found = dwim_ref("HEAD", 4, sha1, &real_ref);
 	} else if (reflog_len)
@@ -379,8 +391,6 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 	return 0;
 }
 
-static int get_sha1_1(const char *name, int len, unsigned char *sha1);
-
 static int get_parent(const char *name, int len,
 		      unsigned char *result, int idx)
 {
diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh
new file mode 100755
index 0000000..1e49dd2
--- /dev/null
+++ b/t/t1505-rev-parse-last.sh
@@ -0,0 +1,71 @@
+#!/bin/sh
+
+test_description='test @{-N} syntax'
+
+. ./test-lib.sh
+
+
+make_commit () {
+	echo "$1" > "$1" &&
+	git add "$1" &&
+	git commit -m "$1"
+}
+
+
+test_expect_success 'setup' '
+
+	make_commit 1 &&
+	git branch side &&
+	make_commit 2 &&
+	make_commit 3 &&
+	git checkout side &&
+	make_commit 4 &&
+	git merge master &&
+	git checkout master
+
+'
+
+# 1 -- 2 -- 3 master
+#  \         \
+#   \         \
+#    --- 4 --- 5 side
+#
+# and 'side' should be the last branch
+
+git log --graph --all --pretty=oneline --decorate
+
+test_rev_equivalent () {
+
+	git rev-parse "$1" > expect &&
+	git rev-parse "$2" > output &&
+	test_cmp expect output
+
+}
+
+test_expect_success '@{-1} works' '
+	test_rev_equivalent side @{-1}
+'
+
+test_expect_success '@{-1}~2 works' '
+	test_rev_equivalent side~2 @{-1}~2
+'
+
+test_expect_success '@{-1}^2 works' '
+	test_rev_equivalent side^2 @{-1}^2
+'
+
+test_expect_failure '@{-1}@{1} works' '
+	test_rev_equivalent side@{1} @{-1}@{1}
+'
+
+test_expect_success '@{-2} works' '
+	test_rev_equivalent master @{-2}
+'
+
+test_expect_success '@{-3} fails' '
+	test_must_fail git rev-parse @{-3}
+'
+
+test_done
+
+
-- 
1.6.1.315.g92577

[PATCH/RFC v4 4/5] checkout: implement "-" abbreviation, add docs and tests

[Thomas Rast]

Have '-' mean the same as '@{-1}', i.e., the last branch we were on.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Documentation/git-checkout.txt |    4 +++
 builtin-checkout.c             |    3 ++
 t/t2012-checkout-last.sh       |   50 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 0 deletions(-)
 create mode 100755 t/t2012-checkout-last.sh

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 9cd5151..3bccffa 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -133,6 +133,10 @@ the conflicted merge in the specified paths.
 +
 When this parameter names a non-branch (but still a valid commit object),
 your HEAD becomes 'detached'.
++
+As a special case, the "`@\{-N\}`" syntax for the N-th last branch
+checks out the branch (instead of detaching).  You may also specify
+"`-`" which is synonymous with "`@\{-1\}`".
 
 
 Detached HEAD
diff --git a/builtin-checkout.c b/builtin-checkout.c
index dc1de06..b0a101b 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -679,6 +679,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		arg = argv[0];
 		has_dash_dash = (argc > 1) && !strcmp(argv[1], "--");
 
+		if (!strcmp(arg, "-"))
+			arg = "@{-1}";
+
 		if (get_sha1(arg, rev)) {
 			if (has_dash_dash)          /* case (1) */
 				die("invalid reference: %s", arg);
diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
new file mode 100755
index 0000000..320f6eb
--- /dev/null
+++ b/t/t2012-checkout-last.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='checkout can switch to last branch'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo hello >world &&
+	git add world &&
+	git commit -m initial &&
+	git branch other &&
+	echo "hello again" >>world &&
+	git add world &&
+	git commit -m second
+'
+
+test_expect_success '"checkout -" does not work initially' '
+	test_must_fail git checkout -
+'
+
+test_expect_success 'first branch switch' '
+	git checkout other
+'
+
+test_expect_success '"checkout -" switches back' '
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master"
+'
+
+test_expect_success '"checkout -" switches forth' '
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/other"
+'
+
+test_expect_success 'detach HEAD' '
+	git checkout $(git rev-parse HEAD)
+'
+
+test_expect_success '"checkout -" attaches again' '
+	git checkout - &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/other"
+'
+
+test_expect_success '"checkout -" detaches again' '
+	git checkout - &&
+	test "z$(git rev-parse HEAD)" = "z$(git rev-parse other)" &&
+	test_must_fail git symbolic-ref HEAD
+'
+
+test_done
-- 
1.6.1.315.g92577

[PATCH/RFC v4 5/5] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Thomas Rast]

From: Johannes Schindelin <Johannes.Schindelin@gmx.de>

Instead of traversing them twice, we just build a list of branch switches,
pick the one we're interested in, and free the list again.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sha1_name.c |   61 ++++++++++++++++++++++++----------------------------------
 1 files changed, 25 insertions(+), 36 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 9e1538e..b21a1f0 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -5,6 +5,8 @@
 #include "blob.h"
 #include "tree-walk.h"
 #include "refs.h"
+#include "cache-tree.h"
+#include "string-list.h"
 
 static int find_short_object_filename(int len, const char *name, unsigned char *sha1)
 {
@@ -684,43 +686,31 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	return retval;
 }
 
-struct grab_nth_branch_switch_cbdata {
-	int counting;
-	int nth;
-	struct strbuf *buf;
-};
-
-static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
+static int add_one_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 				  const char *email, unsigned long timestamp, int tz,
 				  const char *message, void *cb_data)
 {
-	struct grab_nth_branch_switch_cbdata *cb = cb_data;
+	struct string_list *list = cb_data;
 	const char *match = NULL, *target = NULL;
 	size_t len;
 
-	if (!prefixcmp(message, "checkout: moving from ")) {
-		match = message + strlen("checkout: moving from ");
-		if ((target = strstr(match, " to ")) != NULL)
-			target += 4;
-	}
-
-	if (!match)
+	if (prefixcmp(message, "checkout: moving from "))
 		return 0;
 
-	len = target - match - 4;
-	if (target[len] == '\n' && !strncmp(match, target, len))
-		return 0;
+	match = message + strlen("checkout: moving from ");
 
-	if (cb->counting) {
-		cb->nth++;
-		return 0;
+	/* Is it "moving" from a branch to itself?  Then ignore it. */
+	if ((target = strstr(match, " to ")) != NULL) {
+		target += 4;
+		len = target - match - 4;
+		if (target[len] == '\n' && !strncmp(match, target, len))
+			return 0;
 	}
+	else
+		len = strchrnul(match, ' ') - match;
+
+	string_list_append(xstrndup(match, len), list);
 
-	if (cb->nth-- <= 0) {
-		strbuf_reset(cb->buf);
-		strbuf_add(cb->buf, match, len);
-		return 1;
-	}
 	return 0;
 }
 
@@ -738,7 +728,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 int interpret_nth_last_branch(const char *name, struct strbuf *buf)
 {
 	int nth;
-	struct grab_nth_branch_switch_cbdata cb;
+	struct string_list branch_list = { NULL, 0, 0, 0 };
 	const char *brace;
 	char *num_end;
 
@@ -751,18 +741,17 @@ int interpret_nth_last_branch(const char *name, struct strbuf *buf)
 	if (num_end != brace)
 		return -1;
 
-	cb.counting = 1;
-	cb.nth = 0;
-	cb.buf = buf;
-	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+	for_each_reflog_ent("HEAD", add_one_branch_switch, &branch_list);
 
-	if (cb.nth < nth)
+	if (branch_list.nr < nth)
 		return 0;
 
-	cb.counting = 0;
-	cb.nth -= nth;
-	cb.buf = buf;
-	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+	strbuf_reset(buf);
+	strbuf_addstr(buf, branch_list.items[branch_list.nr - nth].string);
+
+	/* force free()ing the items */
+	branch_list.strdup_strings = 1;
+	string_list_clear(&branch_list, 0);
 
 	return brace-name+1;
 }
-- 
1.6.1.315.g92577

Re: [PATCH/RFC v4 0/5] N-th last checked out branch

[Johannes Schindelin]

Hi,

On Sat, 17 Jan 2009, Thomas Rast wrote:

> I [...] added a fixed up version of your patch on top, since it had some 
> context that is not in any version I have.

Thanks.

Yeah, I know, I have too many patches in my fork, but I'm working on it... 
:-)

Ciao,
Dscho

Re: [PATCH/RFC v4 3/5] sha1_name: support @{-N} syntax in get_sha1()

[Johannes Schindelin]

Hi,

On Sat, 17 Jan 2009, Thomas Rast wrote:

> diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh
> new file mode 100755
> index 0000000..1e49dd2
> --- /dev/null
> +++ b/t/t1505-rev-parse-last.sh
> @@ -0,0 +1,71 @@
> +#!/bin/sh
> +
> +test_description='test @{-N} syntax'
> +
> +. ./test-lib.sh
> +
> +
> +make_commit () {
> +	echo "$1" > "$1" &&
> +	git add "$1" &&
> +	git commit -m "$1"
> +}
> +
> +
> +test_expect_success 'setup' '
> +
> +	make_commit 1 &&
> +	git branch side &&
> +	make_commit 2 &&
> +	make_commit 3 &&
> +	git checkout side &&
> +	make_commit 4 &&
> +	git merge master &&
> +	git checkout master
> +
> +'
> +
> +# 1 -- 2 -- 3 master
> +#  \         \
> +#   \         \
> +#    --- 4 --- 5 side
> +#
> +# and 'side' should be the last branch
> +
> +git log --graph --all --pretty=oneline --decorate
> +

Maybe you want to squash this in, so that the output of "make test" is 
not cluttered by the graph?

-- snipsnap --
diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh
index 1e49dd2..72e8322 100755
--- a/t/t1505-rev-parse-last.sh
+++ b/t/t1505-rev-parse-last.sh
@@ -32,7 +32,11 @@ test_expect_success 'setup' '
 #
 # and 'side' should be the last branch
 
-git log --graph --all --pretty=oneline --decorate
+test_expect_success 'show a log (for debugging)' '
+
+	git log --graph --all --pretty=oneline --decorate
+
+'
 
 test_rev_equivalent () {
 
-- 
1.6.1.332.g9a59d

[PATCH 6/5] Fix parsing of @{-1}@{1}

[Johannes Schindelin]

To do that, Git no longer looks forward for the '@{' corresponding to the
closing '}' but backward, and dwim_ref() as well as dwim_log() learnt
about the @{-<N>} notation.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	The modifications of dwim_ref() and dwim_log() are probably
	more important than the issue I tried to fix...

 sha1_name.c               |   25 ++++++++++++++++++++++++-
 t/t1505-rev-parse-last.sh |    2 +-
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 306d04b..ee0c456 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -240,8 +240,28 @@ static int ambiguous_path(const char *path, int len)
 	return slash;
 }
 
+/*
+ * *string and *len will only be substituted, and *string returned (for
+ * later free()ing) if the string passed in is of the form @{-<n>}.
+ */
+static char *substitute_nth_last_branch(const char **string, int *len)
+{
+	struct strbuf buf = STRBUF_INIT;
+	int ret = interpret_nth_last_branch(*string, &buf);
+
+	if (ret == *len) {
+		size_t size;
+		*string = strbuf_detach(&buf, &size);
+		*len = size;
+		return (char *)*string;
+	}
+
+	return NULL;
+}
+
 int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 {
+	char *last_branch = substitute_nth_last_branch(&str, &len);
 	const char **p, *r;
 	int refs_found = 0;
 
@@ -261,11 +281,13 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 				break;
 		}
 	}
+	free(last_branch);
 	return refs_found;
 }
 
 int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 {
+	char *last_branch = substitute_nth_last_branch(&str, &len);
 	const char **p;
 	int logs_found = 0;
 
@@ -296,6 +318,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 		if (!warn_ambiguous_refs)
 			break;
 	}
+	free(last_branch);
 	return logs_found;
 }
 
@@ -314,7 +337,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 	/* basic@{time or number or -number} format to query ref-log */
 	reflog_len = at = 0;
 	if (str[len-1] == '}') {
-		for (at = 0; at < len - 1; at++) {
+		for (at = len-2; at >= 0; at--) {
 			if (str[at] == '@' && str[at+1] == '{') {
 				reflog_len = (len-1) - (at+2);
 				len = at;
diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh
index 72e8322..2d6b31e 100755
--- a/t/t1505-rev-parse-last.sh
+++ b/t/t1505-rev-parse-last.sh
@@ -58,7 +58,7 @@ test_expect_success '@{-1}^2 works' '
 	test_rev_equivalent side^2 @{-1}^2
 '
 
-test_expect_failure '@{-1}@{1} works' '
+test_expect_success '@{-1}@{1} works' '
 	test_rev_equivalent side@{1} @{-1}@{1}
 '
 
-- 
1.6.1.332.g9a59d

Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Instead of traversing them twice, we just build a list of branch switches,
> pick the one we're interested in, and free the list again.

Isn't the code keeping them all in core, or am I reading the patch wrong?

If you know that you are interested in the nth-from-the-last switch, and
if you are reading from the beginning, you would need to keep at most n 
last switches you have seen in core, wouldn't you?  

Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Johannes Schindelin]

Hi,

On Sat, 17 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Instead of traversing them twice, we just build a list of branch switches,
> > pick the one we're interested in, and free the list again.
> 
> Isn't the code keeping them all in core, or am I reading the patch wrong?
> 
> If you know that you are interested in the nth-from-the-last switch, and
> if you are reading from the beginning, you would need to keep at most n 
> last switches you have seen in core, wouldn't you?  

That is correct.  But this is such a highly uncritical code path that I'd 
like to keep this simple rather than fast.

Ciao,
Dscho

Re: [PATCH/RFC v4 3/5] sha1_name: support @{-N} syntax in get_sha1()

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> Let get_sha1() parse the @{-N} syntax, with docs and tests.
>
> Note that while @{-1}^2, @{-2}~5 and such are supported, @{-1}@{1} is
> currently not allowed.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ...
> @@ -324,6 +326,16 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
>  		return -1;
>  
>  	if (!len && reflog_len) {
> +		struct strbuf buf = STRBUF_INIT;
> +		int ret;
> +		/* try the @{-N} syntax for n-th checkout */
> +		ret = interpret_nth_last_branch(str+at, &buf);
> +		if (ret > 0) {
> +			/* substitute this branch name and restart */
> +			return get_sha1_1(buf.buf, buf.len, sha1);
> +		} else if (ret == 0) {
> +			return -1;
> +		}

What are the possible failure cases, and what do we want to tell the
end-user?

 - You asked for 3rd but there weren't that many switches yet, and ask
   "git rev-parse --verify @{-3}".

   Are we Ok with "fatal: Needed a single revision" from rev-parse?  Do we
   want to show "fatal: @{-3}: not that many branch switches yet"?

   What happens to "git checkout @{-3}" in this case?  Having checkout say
   "fatal: invalid reference: @{-3}" would be fine in this case, I think.

 - You try "git checkout @{-3}", you were on "frotz" branch back then, but
   the branch does not exist anymore.

   I think you will get "fatal: invalid reference: frotz" from checkout,
   which should be fine.

There also is a case where nth_last_branch() may find something that is
not a branch (e.g. "git checkout HEAD^"), but I am hoping we can label
that as a bug in nth_last_branch() and fix it later.

Re: [PATCH/RFC v4 4/5] checkout: implement "-" abbreviation, add docs and tests

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> @@ -133,6 +133,10 @@ the conflicted merge in the specified paths.
>  +
>  When this parameter names a non-branch (but still a valid commit object),
>  your HEAD becomes 'detached'.
> ++
> +As a special case, the "`@\{-N\}`" syntax for the N-th last branch
> +checks out the branch (instead of detaching).  You may also specify
> +"`-`" which is synonymous with "`@\{-1\}`".

I mildly disagree with this wording.

The new syntax is supposed to be a new way to name a branch, not a random
non-branch committish that is special cased by "git checkout".  I would
further suggest that we should teach "git rev-parse --symbolic-full-name"
and "git rev-parse --symbolic" about the new syntax, so that scripts can
use the syntax to find out the same information.

The "-" thing deserves a mention here in the documentation.  That _is_ a
special case that only applies to the "git checkout" command.

> diff --git a/builtin-checkout.c b/builtin-checkout.c
> index dc1de06..b0a101b 100644
> --- a/builtin-checkout.c
> +++ b/builtin-checkout.c
> @@ -679,6 +679,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  		arg = argv[0];
>  		has_dash_dash = (argc > 1) && !strcmp(argv[1], "--");
>  
> +		if (!strcmp(arg, "-"))
> +			arg = "@{-1}";
> +

This is not quite nice as it could be, but it probably is Ok.  If the
interpretation of @{-1} errors out, the user won't see an error message
that talks about "-" but instead the user will see "@{-1}".

Also it will look somewhat inconsistent to the end user who does not know
the internals for "-" claim to be a synonym for @{-1} but it really isn't.
For example, "git checkout -^0" does not work as "git checkout @{-1}^0".

To avoid such confusion, we could instead make "git checkout - <ENTER>" a
synonym for "git checkout @{-1} <ENTER>", without claiming to make "-" a
synonym for "@{-1}".  In other words, "git checkout -" can become a very
narrow, focused special case that does not allow anything else, such as
pathspecs, "--" separator, nor --force and other options.

Re: [PATCH 6/5] Fix parsing of @{-1}@{1}

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> To do that, Git no longer looks forward for the '@{' corresponding to the
> closing '}' but backward, and dwim_ref() as well as dwim_log() learnt
> about the @{-<N>} notation.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> 	The modifications of dwim_ref() and dwim_log() are probably
> 	more important than the issue I tried to fix...

Good, so we can say things like:

	git log -g @{-1}
        git show-branch -g @{-1}

with this?

By the way, I noticed that without these patch series we show something
when "git rev-parse --verify @{-1}" is asked for.  What is it trying to
show?

Re: [PATCH 6/5] Fix parsing of @{-1}@{1}

[Johannes Schindelin]

Hi,

On Sat, 17 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > To do that, Git no longer looks forward for the '@{' corresponding to the
> > closing '}' but backward, and dwim_ref() as well as dwim_log() learnt
> > about the @{-<N>} notation.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >
> > 	The modifications of dwim_ref() and dwim_log() are probably
> > 	more important than the issue I tried to fix...
> 
> Good, so we can say things like:
> 
> 	git log -g @{-1}
>         git show-branch -g @{-1}
> 
> with this?

I _hope_ :-)

> By the way, I noticed that without these patch series we show something 
> when "git rev-parse --verify @{-1}" is asked for.  What is it trying to 
> show?

Apparently the same as @{1.Jan}: in get_sha1_basic(), we have this code:

                /* Is it asking for N-th entry, or approxidate? */
                for (i = nth = 0; 0 <= nth && i < reflog_len; i++) {
                        char ch = str[at+2+i];
                        if ('0' <= ch && ch <= '9')
                                nth = nth * 10 + ch - '0';
                        else
                                nth = -1;
                }
                if (100000000 <= nth) {
                        at_time = nth;
                        nth = -1;
                } else if (0 <= nth)
                        at_time = 0;
                else {
                        char *tmp = xstrndup(str + at + 2, reflog_len);
                        at_time = approxidate(tmp);
                        free(tmp);
                }

So in the loop, nth is set to -1 because of a non-digit, and later, 
approxidate is called for nth == -1, which does this:

	$ ./test-date now
	now -> bad -> Thu Jan  1 01:00:00 1970
	now -> Sat Jan 17 22:21:20 2009

Ciao,
Dscho

Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> That is correct.  But this is such a highly uncritical code path that I'd 
> like to keep this simple rather than fast.

I actually not worried about "fast" at all; it was more about unbounded
memory consumption.

Re: [PATCH/RFC v4 2/5] sha1_name: tweak @{-N} lookup

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> Have the lookup only look at "interesting" checkouts, meaning those
> that tell you "Already on ..." don't count even though they also cause
> a reflog entry.
>
> Let interpret_nth_last_branch() return the number of characters
> parsed, so that git-checkout can verify that the branch spec was
> @{-N}, not @{-1}^2 or something like that.  (The latter will be added
> later.)

Thanks; you seem to have handled the issues I pointed out in response to
my own weatherbaloon patch.  I think it is probably better to squash the
first two (and you take the authorship).

> diff --git a/sha1_name.c b/sha1_name.c
> index 6377264..34e39db 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -685,29 +685,28 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
>  				  const char *message, void *cb_data)
>  {
>  	struct grab_nth_branch_switch_cbdata *cb = cb_data;
> -	const char *match = NULL;
> -
> -	if (!prefixcmp(message, "checkout: moving to "))
> -		match = message + strlen("checkout: moving to ");
> -	else if (!prefixcmp(message, "checkout: moving from ")) {
> -		const char *cp = message + strlen("checkout: moving from ");
> -		if ((cp = strstr(cp, " to ")) != NULL) {
> -			match = cp + 4;
> -		}
> +	const char *match = NULL, *target = NULL;
> +	size_t len;
> +
> +	if (!prefixcmp(message, "checkout: moving from ")) {
> +		match = message + strlen("checkout: moving from ");
> +		if ((target = strstr(match, " to ")) != NULL)
> +			target += 4;
>  	}

This drops support for older reflog records, but I think it would be Ok.
This "N-th" support is really meant to be for small number of N anyway.

> -	if (--cb->nth <= 0) {
> -		size_t len = strlen(match);
> -		while (match[len-1] == '\n')
> -			len--;
> +	if (cb->nth-- <= 0) {
>  		strbuf_reset(cb->buf);
>  		strbuf_add(cb->buf, match, len);
>  		return 1;

Hmm, did I have an off-by-one I did not notice? ;-)

>  int interpret_nth_last_branch(const char *name, struct strbuf *buf)
>  {
> -	int nth, i;
> +	int nth;
>  	struct grab_nth_branch_switch_cbdata cb;
> +	const char *brace;
> +	char *num_end;
>  
>  	if (name[0] != '@' || name[1] != '{' || name[2] != '-')
>  		return -1;
> -	for (i = 3, nth = 0; name[i] && name[i] != '}'; i++) {
> -		char ch = name[i];
> -		if ('0' <= ch && ch <= '9')
> -			nth = nth * 10 + ch - '0';
> -		else
> -			return -1;
> -	}
> -	if (nth < 0 || 10 <= nth)

The removal of "limit to reasonably small recent N" I somewhat have
reservations on, but I think we can later re-add something based on
configuration variable if we need to.

Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Johannes Schindelin]

Hi,

On Sat, 17 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > That is correct.  But this is such a highly uncritical code path that 
> > I'd like to keep this simple rather than fast.
> 
> I actually not worried about "fast" at all; it was more about unbounded 
> memory consumption.

Let's just assume that I make a branch switch per minute, continuously, 
for 90 days (until the reflogs are expired), and let's say that all my 
branchnames have 20 characters.  Conservatively, the memory requirement 
would be 100 * 2000 * 30 = 3 megabyte.

Note: these are the memory requirements after some really unrealistically 
high activity, and the memory is free()d during parameter parsing.

A much more realistical expectation would be to switch branches maybe 20 
times a day, which would amount to something like 36 kilobyte.  And again, 
they are free()d before the action really starts.

Ciao,
Dscho

[TOY PATCH] git-resurrect: find traces of a branch name and resurrect it

[Thomas Rast]

Add a tool 'git resurrect <branch>...' that tries to find traces of
each <branch> in the HEAD reflog and, optionally, all merge commits in
the repository.  It can then resurrect the branch, pointing it at the
most recent of all candidate commits found.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

So here's a slightly more polished version so gmane can keep it
forever.  Thanks for the sed trick!  I was too lazy to add more
options, but at least there's a "fast" and a "complete" mode.



 Makefile         |    1 +
 git-resurrect.sh |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+), 0 deletions(-)
 create mode 100755 git-resurrect.sh

diff --git a/Makefile b/Makefile
index 2b873fa..87cb539 100644
--- a/Makefile
+++ b/Makefile
@@ -260,6 +260,7 @@ SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
 SCRIPT_SH += git-parse-remote.sh
 SCRIPT_SH += git-pull.sh
+SCRIPT_SH += git-resurrect.sh
 SCRIPT_SH += git-quiltimport.sh
 SCRIPT_SH += git-rebase--interactive.sh
 SCRIPT_SH += git-rebase.sh
diff --git a/git-resurrect.sh b/git-resurrect.sh
new file mode 100755
index 0000000..6d5a0c7
--- /dev/null
+++ b/git-resurrect.sh
@@ -0,0 +1,109 @@
+#!/bin/sh
+
+USAGE="git resurrect [-m | --merges] [-n | --dry-run] <name>..."
+LONG_USAGE="git-resurrect attempts to find traces of a branch tip called <name>,
+and tries to resurrect it.  Currently, the reflog is searched for
+checkout and merge messages.  With --merges, the history of all refs
+is scanned for merge commit subjects, which is rather slow but allows
+you to resurrect other people's topic branches."
+
+. git-sh-setup
+cd_to_toplevel
+
+OPTIONS_SPEC="\
+git resurrect [-m | --merges] [-n | --dry-run] <name>...
+--
+m,merges             also scan merges (slow)
+n,dry-run            don't recreate the branch"
+
+test "$#" = 0 && usage
+
+eval "$(echo "$OPTIONS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
+
+search_reflog () {
+        sed -n 's~^\([^ ]*\) .*\tcheckout: moving from '"$1"' .*~\1~p' \
+                < .git/logs/HEAD
+}
+
+search_reflog_merges () {
+        sed -n 's~^[^ ]* \([^ ]*\) .*\tmerge '"$1"':~\1~p' \
+                < .git/logs/HEAD
+}
+
+search_merges () {
+	git rev-list --pretty=tformat:"%h %p:%s" --all |
+	grep "Merge branch.*'$branch'.*into" |
+	while read sha rest; do
+		parents="$(echo "$rest" | cut -d: -f1)"
+		case "$parents" in
+		    *' '*' '*)
+			warn "$branch took part in octopus merge $sha"
+			warn "check manually!"
+			;;
+		    *' '*)
+			echo "$parents" | cut -d' ' -f2
+			;;
+		esac
+	done
+}
+
+search_merge_targets () {
+	git rev-list --pretty=tformat:"%h %s" --all |
+	grep "Merge branch '[^']*' into $branch$" |
+	cut -d' ' -f1
+}
+
+dry_run=
+scan_merges=
+
+while test "$#" != 0; do
+	case "$1" in
+	    -n|--dry-run)
+		dry_run=t
+		;;
+	    -m|--merges)
+		scan_merges=t
+		;;
+	    --)
+		shift
+		break
+		;;
+	    *)
+		usage
+		;;
+	esac
+	shift
+done
+
+for branch in "$@"; do
+	candidates="$(search_reflog $1; search_reflog_merges $1)"
+	if test ! -z "$scan_merges"; then
+		candidates="$candidates $(search_merges $1; search_merge_targets $1)"
+	fi
+
+	candidates="$(git rev-parse $candidates | sort -u)"
+
+	if test -z "$candidates"; then
+		echo "** No candidates for $branch found **"
+		test -z "$scan_merges" && echo "(maybe try again with -m)"
+	else
+		echo "** Candidates for $branch **"
+		for cmt in $candidates; do
+			git --no-pager log --pretty=oneline --abbrev-commit -1 $cmt
+		done
+
+		newest="$(git rev-list -1 $candidates)"
+		if test ! -z "$dry_run"; then
+			printf "Most recent: "
+			git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+		elif ! git rev-parse --verify --quiet $branch >/dev/null; then
+			printf "** Restoring $branch to "
+			git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+			git branch $branch $newest
+		else
+			printf "Most recent: "
+			git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+			echo "** $branch already exists, doing nothing"
+		fi
+	fi
+done
-- 
1.6.1.320.gd5dca.dirty

Re: [PATCH] revision walker: include a detached HEAD in --all

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> When HEAD is detached, --all should list it, too, logically, as a
> detached HEAD is by definition a temporary, unnamed branch.
>
> It is especially necessary to list it when garbage collecting, as
> the detached HEAD would be trashed.
>
> Noticed by Thomas Rast.
>
> Note that this affects creating bundles with --all; I contend that it
> is a good change to add the HEAD, so that cloning from such a bundle
> will give you a current branch.  However, I had to fix t5701 as it
> assumed that --all does not imply HEAD.

Sorry, but I do not understand.

> diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
> index 8dfaaa4..14413f8 100755
> --- a/t/t5701-clone-local.sh
> +++ b/t/t5701-clone-local.sh
> @@ -11,8 +11,8 @@ test_expect_success 'preparing origin repository' '
>  	git clone --bare . x &&
>  	test "$(GIT_CONFIG=a.git/config git config --bool core.bare)" = true &&
>  	test "$(GIT_CONFIG=x/config git config --bool core.bare)" = true
> -	git bundle create b1.bundle --all HEAD &&
> -	git bundle create b2.bundle --all &&
> +	git bundle create b1.bundle master HEAD &&
> +	git bundle create b2.bundle master &&

Because --all did not imply HEAD, "--all HEAD" used to be the way to say
"everything and HEAD".  Now --all does imply HEAD, but it should still be
a valid way to say "everything, by the way, do not forget HEAD".

Does the first one need to be changed to "master HEAD"?  If "--all HEAD"
makes the rest of the test unhappy because HEAD is listed twice, perhaps
that is an independent bug that needs to be fixed?

For that matter, what does "git bundle create x HEAD HEAD" do?  Does it
list HEAD twice?

Re: [PATCH] revision walker: include a detached HEAD in --all

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> ...
>> Note that this affects creating bundles with --all; I contend that it
>> is a good change to add the HEAD, so that cloning from such a bundle
>> will give you a current branch.  However, I had to fix t5701 as it
>> assumed that --all does not imply HEAD.
>
> Sorry, but I do not understand.
>
>> diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
>> index 8dfaaa4..14413f8 100755
>> --- a/t/t5701-clone-local.sh
>> +++ b/t/t5701-clone-local.sh
>> @@ -11,8 +11,8 @@ test_expect_success 'preparing origin repository' '
>>  	git clone --bare . x &&
>>  	test "$(GIT_CONFIG=a.git/config git config --bool core.bare)" = true &&
>>  	test "$(GIT_CONFIG=x/config git config --bool core.bare)" = true
>> -	git bundle create b1.bundle --all HEAD &&
>> -	git bundle create b2.bundle --all &&
>> +	git bundle create b1.bundle master HEAD &&
>> +	git bundle create b2.bundle master &&
>
> Because --all did not imply HEAD, "--all HEAD" used to be the way to say
> "everything and HEAD".  Now --all does imply HEAD, but it should still be
> a valid way to say "everything, by the way, do not forget HEAD".
>
> Does the first one need to be changed to "master HEAD"?  If "--all HEAD"
> makes the rest of the test unhappy because HEAD is listed twice, perhaps
> that is an independent bug that needs to be fixed?
>
> For that matter, what does "git bundle create x HEAD HEAD" do?  Does it
> list HEAD twice?

With a patch like this, I think b1.bundle can be created with "--all HEAD"
as before.

Of course, to advertise that --all now includes HEAD and it is a _good_
thing, we may want to even say "git bundle create b1.bundle --all" in the
above test sequence.

Creation of b2.bundle should say "master" explicitly as in your patch,
because the point of that bundle is to test a use of such HEAD-less bundle
in the later parts of the script.

-- >8 --
Subject: [PATCH] bundle: allow the same ref to be given more than once

"git bundle create x master master" used to create a bundle that lists
the same branch (master) twice.  Cloning from such a bundle resulted in
a needless warning "warning: Duplicated ref: refs/remotes/origin/master".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 bundle.c |    2 ++
 object.c |   19 +++++++++++++++++++
 object.h |    1 +
 3 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/bundle.c b/bundle.c
index daecd8e..b20f210 100644
--- a/bundle.c
+++ b/bundle.c
@@ -240,6 +240,8 @@ int create_bundle(struct bundle_header *header, const char *path,
 		return error("unrecognized argument: %s'", argv[i]);
 	}
 
+	object_array_remove_duplicates(&revs.pending);
+
 	for (i = 0; i < revs.pending.nr; i++) {
 		struct object_array_entry *e = revs.pending.objects + i;
 		unsigned char sha1[20];
diff --git a/object.c b/object.c
index 50b6528..7e6a92c 100644
--- a/object.c
+++ b/object.c
@@ -268,3 +268,22 @@ void add_object_array_with_mode(struct object *obj, const char *name, struct obj
 	objects[nr].mode = mode;
 	array->nr = ++nr;
 }
+
+void object_array_remove_duplicates(struct object_array *array)
+{
+	int ref, src, dst;
+	struct object_array_entry *objects = array->objects;
+
+	for (ref = 0; ref < array->nr - 1; ref++) {
+		for (src = ref + 1, dst = src;
+		     src < array->nr;
+		     src++) {
+			if (!strcmp(objects[ref].name, objects[src].name))
+				continue;
+			if (src != dst)
+				objects[dst] = objects[src];
+			dst++;
+		}
+		array->nr = dst;
+	}
+}
diff --git a/object.h b/object.h
index 036bd66..3193916 100644
--- a/object.h
+++ b/object.h
@@ -71,5 +71,6 @@ int object_list_contains(struct object_list *list, struct object *obj);
 /* Object array handling .. */
 void add_object_array(struct object *obj, const char *name, struct object_array *array);
 void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode);
+void object_array_remove_duplicates(struct object_array *);
 
 #endif /* OBJECT_H */
-- 
1.6.1.208.g58df

Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Note: these are the memory requirements after some really unrealistically 
> high activity, and the memory is free()d during parameter parsing.
>
> A much more realistical expectation would be to switch branches maybe 20 
> times a day, which would amount to something like 36 kilobyte.  And again, 
> they are free()d before the action really starts.

My HEAD reflog is 7MB long with 39000 entries, and among them, 13100
entries have "checkout: moving ".

I know I will never want to switch back to the 10000th from the last
branch.  I am quite sure that I would forget which branch I was on after
switching branches three or four times (hence my original hardcoded
limitation of 10 which "should be plenty").  When I know I only have to
keep track of 10 entries, having to keep track of 13100 entries, even if
it is 36kB (it would actually be 260kB in my case) feels there is
something wrong in the design.

Re: [PATCH] revision walker: include a detached HEAD in --all

[Johannes Schindelin]

Hi,

On Sat, 17 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > When HEAD is detached, --all should list it, too, logically, as a
> > detached HEAD is by definition a temporary, unnamed branch.
> >
> > It is especially necessary to list it when garbage collecting, as
> > the detached HEAD would be trashed.
> >
> > Noticed by Thomas Rast.
> >
> > Note that this affects creating bundles with --all; I contend that it
> > is a good change to add the HEAD, so that cloning from such a bundle
> > will give you a current branch.  However, I had to fix t5701 as it
> > assumed that --all does not imply HEAD.
> 
> Sorry, but I do not understand.
> 
> > diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
> > index 8dfaaa4..14413f8 100755
> > --- a/t/t5701-clone-local.sh
> > +++ b/t/t5701-clone-local.sh
> > @@ -11,8 +11,8 @@ test_expect_success 'preparing origin repository' '
> >  	git clone --bare . x &&
> >  	test "$(GIT_CONFIG=a.git/config git config --bool core.bare)" = true &&
> >  	test "$(GIT_CONFIG=x/config git config --bool core.bare)" = true
> > -	git bundle create b1.bundle --all HEAD &&
> > -	git bundle create b2.bundle --all &&
> > +	git bundle create b1.bundle master HEAD &&
> > +	git bundle create b2.bundle master &&
> 
> Because --all did not imply HEAD, "--all HEAD" used to be the way to say
> "everything and HEAD".  Now --all does imply HEAD, but it should still be
> a valid way to say "everything, by the way, do not forget HEAD".
> 
> Does the first one need to be changed to "master HEAD"?  If "--all HEAD"
> makes the rest of the test unhappy because HEAD is listed twice, perhaps
> that is an independent bug that needs to be fixed?

I changed it away from --all because I am a fan of being explicit.  We 
want a bundle here that has master and HEAD in it.  This being a test 
case, being lazy is so wrong here.  You should describe what you actually 
want, not use a set of parameters that just happens to work (by chance as 
we saw).

Ciao,
Dscho

Re: [PATCH] revision walker: include a detached HEAD in --all

[Johannes Schindelin]

Hi,

On Sat, 17 Jan 2009, Junio C Hamano wrote:

> Subject: [PATCH] bundle: allow the same ref to be given more than once
> 
> "git bundle create x master master" used to create a bundle that lists
> the same branch (master) twice.  Cloning from such a bundle resulted in
> a needless warning "warning: Duplicated ref: refs/remotes/origin/master".
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  bundle.c |    2 ++
>  object.c |   19 +++++++++++++++++++
>  object.h |    1 +
>  3 files changed, 22 insertions(+), 0 deletions(-)

Yes, that would be good.  You have my ACK on that if you want.

Another thing I am thinking about on and off:

You cannot really use bundles as a replacement for regular transports 
(e.g. when you administrator does not let you ssh or git:// out, and you 
do not have an HTTP server available [*1*]).

Suppose you have two branches, 'master' and 'side'.  Now you make changes 
to 'master' and send the complete repository as a bundle to your friend.  
Now you delete the branch 'side', and send the next bundle (created with 
--all implying HEAD, and --since=$(stat -c %Y <first-bundle>)).

Then your friend has no idea if 'side' was deleted or untouched.

So I think we'd need some option for "create bundle" to list all specified 
refs, and if they have not really changed, their SHA-1s as prerequisites, 
too.  Maybe "--full-bundle", or just "--full"?

Another problem: suppose you have a branch, called 'private', that you 
excluded from your bundles.  Now you happened to make changes to it, and 
by mistake, the branch gets included in the incremental bundle.  No 
problem for you, as your friend lacks the prerequisites to reconstruct it.

But unfortunately, your friend cannot even pull 'master' from it, because 
of our overzealous verification process which refuses all fetches when 
some prerequisites are missing locally, even if they are not even needed.

This problem is much harder to solve, I think, and maybe we just want to 
leave it: as bundles are just fed into index-pack --fix-thin, which has no 
idea what objects can be skipped.  Maybe there is no clean solution to 
that to begin with.

Ciao,
Dscho

[*1*] When people are stuck behind such a stupidly restrictive firewall, 
often people come with "helpful" suggestions to use a VPN, or to publish 
your private repository, or get an external machine to HTTP proxy their 
connections.  I find it outright mean to waste the time of people who 
already have a big problem.

However, I believe that a mail based bundle exchange should be a 
relatively easy way out for those situations, once it works.

Re: [TOY PATCH] git-resurrect: find traces of a branch name and resurrect it

[Johannes Schindelin]

Hi,

On Sun, 18 Jan 2009, Thomas Rast wrote:

>  Makefile         |    1 +
>  git-resurrect.sh |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++

Maybe have it in contrib/ instead?

Ciao,
Dscho

Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Johannes Schindelin]

Hi,

On Sat, 17 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Note: these are the memory requirements after some really unrealistically 
> > high activity, and the memory is free()d during parameter parsing.
> >
> > A much more realistical expectation would be to switch branches maybe 20 
> > times a day, which would amount to something like 36 kilobyte.  And again, 
> > they are free()d before the action really starts.
> 
> My HEAD reflog is 7MB long with 39000 entries, and among them, 13100
> entries have "checkout: moving ".
> 
> I know I will never want to switch back to the 10000th from the last 
> branch.  I am quite sure that I would forget which branch I was on after 
> switching branches three or four times (hence my original hardcoded 
> limitation of 10 which "should be plenty").  When I know I only have to 
> keep track of 10 entries, having to keep track of 13100 entries, even if 
> it is 36kB (it would actually be 260kB in my case) feels there is 
> something wrong in the design.

Hrm.  So let's leave it as a two-pass thing?

Ciao,
Dscho

Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Sat, 17 Jan 2009, Junio C Hamano wrote:
> ...
>> My HEAD reflog is 7MB long with 39000 entries, and among them, 13100
>> entries have "checkout: moving ".
>> 
>> I know I will never want to switch back to the 10000th from the last 
>> branch.  I am quite sure that I would forget which branch I was on after 
>> switching branches three or four times (hence my original hardcoded 
>> limitation of 10 which "should be plenty").  When I know I only have to 
>> keep track of 10 entries, having to keep track of 13100 entries, even if 
>> it is 36kB (it would actually be 260kB in my case) feels there is 
>> something wrong in the design.
>
> Hrm.  So let's leave it as a two-pass thing?

Well, I would rather be in favor of something like this.

-- >8 --
Subject: interpret_nth_last_branch(): avoid traversing the reflog twice

You can have quite a many reflog entries, but you typically won't recall
which branch you were on after switching branches for more than several
times.

Instead of reading the reflog twice, this reads the branch switching event
and keeps the latest 16 (which is an arbitrary limitation that should be
plenty) such entry, to switch back to the branch we were recently on.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_name.c              |   48 +++++++++++++++++++++------------------------
 t/t2012-checkout-last.sh |   44 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 26 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 9e1538e..d6622f2 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -684,10 +684,11 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	return retval;
 }
 
+#define MAX_PREVIOUS_BRANCH 16
+
 struct grab_nth_branch_switch_cbdata {
-	int counting;
-	int nth;
-	struct strbuf *buf;
+	long cnt;
+	struct strbuf buf[MAX_PREVIOUS_BRANCH];
 };
 
 static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
@@ -697,6 +698,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 	struct grab_nth_branch_switch_cbdata *cb = cb_data;
 	const char *match = NULL, *target = NULL;
 	size_t len;
+	int nth;
 
 	if (!prefixcmp(message, "checkout: moving from ")) {
 		match = message + strlen("checkout: moving from ");
@@ -711,16 +713,9 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 	if (target[len] == '\n' && !strncmp(match, target, len))
 		return 0;
 
-	if (cb->counting) {
-		cb->nth++;
-		return 0;
-	}
-
-	if (cb->nth-- <= 0) {
-		strbuf_reset(cb->buf);
-		strbuf_add(cb->buf, match, len);
-		return 1;
-	}
+	nth = cb->cnt++ % MAX_PREVIOUS_BRANCH;
+	strbuf_reset(&cb->buf[nth]);
+	strbuf_add(&cb->buf[nth], match, len);
 	return 0;
 }
 
@@ -737,7 +732,8 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
  */
 int interpret_nth_last_branch(const char *name, struct strbuf *buf)
 {
-	int nth;
+	long nth;
+	int i;
 	struct grab_nth_branch_switch_cbdata cb;
 	const char *brace;
 	char *num_end;
@@ -750,19 +746,19 @@ int interpret_nth_last_branch(const char *name, struct strbuf *buf)
 	nth = strtol(name+3, &num_end, 10);
 	if (num_end != brace)
 		return -1;
-
-	cb.counting = 1;
-	cb.nth = 0;
-	cb.buf = buf;
-	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
-
-	if (cb.nth < nth)
-		return 0;
-
-	cb.counting = 0;
-	cb.nth -= nth;
-	cb.buf = buf;
+	if (nth <= 0 || MAX_PREVIOUS_BRANCH < nth)
+		return -1;
+	for (i = 0; i < MAX_PREVIOUS_BRANCH; i++)
+		strbuf_init(&cb.buf[i], 20);
+	cb.cnt = 0;
 	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+	if (cb.cnt < nth)
+		return -1;
+	i = (cb.cnt + MAX_PREVIOUS_BRANCH - nth) % MAX_PREVIOUS_BRANCH;
+	strbuf_reset(buf);
+	strbuf_add(buf, cb.buf[i].buf, cb.buf[i].len);
+	for (i = 0; i < MAX_PREVIOUS_BRANCH; i++)
+		strbuf_release(&cb.buf[i]);
 
 	return brace-name+1;
 }
diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
index 320f6eb..87b30a2 100755
--- a/t/t2012-checkout-last.sh
+++ b/t/t2012-checkout-last.sh
@@ -47,4 +47,48 @@ test_expect_success '"checkout -" detaches again' '
 	test_must_fail git symbolic-ref HEAD
 '
 
+test_expect_success 'more switches' '
+	for i in 16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1
+	do
+		git checkout -b branch$i
+	done
+'
+
+more_switches () {
+	for i in 16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1
+	do
+		git checkout branch$i
+	done
+}
+
+test_expect_success 'switch to the last' '
+	more_switches &&
+	git checkout @{-1} &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/branch2"
+'
+
+test_expect_success 'switch to second from the last' '
+	more_switches &&
+	git checkout @{-2} &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/branch3"
+'
+
+test_expect_success 'switch to third from the last' '
+	more_switches &&
+	git checkout @{-3} &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/branch4"
+'
+
+test_expect_success 'switch to fourth from the last' '
+	more_switches &&
+	git checkout @{-4} &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/branch5"
+'
+
+test_expect_success 'switch to twelfth from the last' '
+	more_switches &&
+	git checkout @{-12} &&
+	test "z$(git symbolic-ref HEAD)" = "zrefs/heads/branch13"
+'
+
 test_done
-- 
1.6.1.245.gdd9f9

Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Well, I would rather be in favor of something like this.
>
> -- >8 --
> Subject: interpret_nth_last_branch(): avoid traversing the reflog twice
>
> You can have quite a many reflog entries, but you typically won't recall
> which branch you were on after switching branches for more than several
> times.
>
> Instead of reading the reflog twice, this reads the branch switching event
> and keeps the latest 16 (which is an arbitrary limitation that should be
> plenty) such entry, to switch back to the branch we were recently on.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  sha1_name.c              |   48 +++++++++++++++++++++------------------------
>  t/t2012-checkout-last.sh |   44 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+), 26 deletions(-)
>
> diff --git a/sha1_name.c b/sha1_name.c
> index 9e1538e..d6622f2 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -750,19 +746,19 @@ int interpret_nth_last_branch(const char *name, struct strbuf *buf)
>  	nth = strtol(name+3, &num_end, 10);
>  	if (num_end != brace)
>  		return -1;
> ...
> -	if (cb.nth < nth)
> -		return 0;
> ...
> +	if (cb.cnt < nth)
> +		return -1;

This should (obviously) be "return 0".

Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Johannes Schindelin]

Hi,

On Mon, 19 Jan 2009, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Well, I would rather be in favor of something like this.
> >
> > -- >8 --
> > Subject: interpret_nth_last_branch(): avoid traversing the reflog twice
> >
> > You can have quite a many reflog entries, but you typically won't recall
> > which branch you were on after switching branches for more than several
> > times.
> >
> > Instead of reading the reflog twice, this reads the branch switching event
> > and keeps the latest 16 (which is an arbitrary limitation that should be
> > plenty) such entry, to switch back to the branch we were recently on.
> >
> > Signed-off-by: Junio C Hamano <gitster@pobox.com>
> > ---
> >  sha1_name.c              |   48 +++++++++++++++++++++------------------------
> >  t/t2012-checkout-last.sh |   44 ++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 66 insertions(+), 26 deletions(-)
> >
> > diff --git a/sha1_name.c b/sha1_name.c
> > index 9e1538e..d6622f2 100644
> > --- a/sha1_name.c
> > +++ b/sha1_name.c
> > @@ -750,19 +746,19 @@ int interpret_nth_last_branch(const char *name, struct strbuf *buf)
> >  	nth = strtol(name+3, &num_end, 10);
> >  	if (num_end != brace)
> >  		return -1;
> > ...
> > -	if (cb.nth < nth)
> > -		return 0;
> > ...
> > +	if (cb.cnt < nth)
> > +		return -1;
> 
> This should (obviously) be "return 0".

This, together with a removal of the hard-coded limit of 16 could be 
squashed with this patch:

-- snipsnap --
diff --git a/sha1_name.c b/sha1_name.c
index 2c5461e..9e5f444 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -691,11 +691,9 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	return retval;
 }
 
-#define MAX_PREVIOUS_BRANCH 16
-
 struct grab_nth_branch_switch_cbdata {
-	long cnt;
-	struct strbuf buf[MAX_PREVIOUS_BRANCH];
+	long cnt, alloc;
+	struct strbuf *buf;
 };
 
 static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
@@ -720,7 +718,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 	if (target[len] == '\n' && !strncmp(match, target, len))
 		return 0;
 
-	nth = cb->cnt++ % MAX_PREVIOUS_BRANCH;
+	nth = cb->cnt++ % cb->alloc;
 	strbuf_reset(&cb->buf[nth]);
 	strbuf_add(&cb->buf[nth], match, len);
 	return 0;
@@ -753,19 +751,22 @@ int interpret_nth_last_branch(const char *name, struct strbuf *buf)
 	nth = strtol(name+3, &num_end, 10);
 	if (num_end != brace)
 		return -1;
-	if (nth <= 0 || MAX_PREVIOUS_BRANCH < nth)
+	if (nth <= 0)
 		return -1;
-	for (i = 0; i < MAX_PREVIOUS_BRANCH; i++)
+	cb.alloc = nth;
+	cb.buf = xmalloc(nth * sizeof(struct strbuf));
+	for (i = 0; i < nth; i++)
 		strbuf_init(&cb.buf[i], 20);
 	cb.cnt = 0;
 	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
 	if (cb.cnt < nth)
-		return -1;
-	i = (cb.cnt + MAX_PREVIOUS_BRANCH - nth) % MAX_PREVIOUS_BRANCH;
+		return 0;
+	i = cb.cnt % nth;
 	strbuf_reset(buf);
 	strbuf_add(buf, cb.buf[i].buf, cb.buf[i].len);
-	for (i = 0; i < MAX_PREVIOUS_BRANCH; i++)
+	for (i = 0; i < nth; i++)
 		strbuf_release(&cb.buf[i]);
+	free(cb.buf);
 
 	return brace-name+1;
 }
-- 
1.6.1.347.g7b62749

[PATCH] @{-<n>}: avoid crash with corrupt reflog

[Johannes Schindelin]

The earlier code checked if a " to " was found after "checkout: Moving
from ".  However, it then went on to access the pointer to " to ",
regardless if it was still NULL (if no " to " was found) or not.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	You might want to squash this into "sha1_name: tweak @{-N} 
	lookup", just as a safety belt.

 sha1_name.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 9e5f444..853bac6 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -705,18 +705,18 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 	size_t len;
 	int nth;
 
-	if (!prefixcmp(message, "checkout: moving from ")) {
-		match = message + strlen("checkout: moving from ");
-		if ((target = strstr(match, " to ")) != NULL)
-			target += 4;
-	}
-
-	if (!match)
+	if (prefixcmp(message, "checkout: moving from "))
 		return 0;
 
-	len = target - match - 4;
-	if (target[len] == '\n' && !strncmp(match, target, len))
-		return 0;
+	match = message + strlen("checkout: moving from ");
+	if ((target = strstr(match, " to ")) != NULL) {
+		len = target - match - 4;
+		target += 4;
+		if (target[len] == '\n' && !strncmp(match, target, len))
+			return 0;
+	}
+	else
+		len = strchrnul(match, ' ') - match;
 
 	nth = cb->cnt++ % cb->alloc;
 	strbuf_reset(&cb->buf[nth]);
-- 
1.6.1.347.g7b62749

Re: [PATCH] @{-<n>}: avoid crash with corrupt reflog

[Johannes Schindelin]

Hi,

On Mon, 19 Jan 2009, Johannes Schindelin wrote:

> diff --git a/sha1_name.c b/sha1_name.c
> index 9e5f444..853bac6 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -705,18 +705,18 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
>  	size_t len;
>  	int nth;
>  
> -	if (!prefixcmp(message, "checkout: moving from ")) {
> -		match = message + strlen("checkout: moving from ");
> -		if ((target = strstr(match, " to ")) != NULL)
> -			target += 4;
> -	}
> -
> -	if (!match)
> +	if (prefixcmp(message, "checkout: moving from "))
>  		return 0;
>  
> -	len = target - match - 4;
> -	if (target[len] == '\n' && !strncmp(match, target, len))
> -		return 0;
> +	match = message + strlen("checkout: moving from ");
> +	if ((target = strstr(match, " to ")) != NULL) {
> +		len = target - match - 4;

Aargh, the "- 4" is wrong, of course.

> +		target += 4;
> +		if (target[len] == '\n' && !strncmp(match, target, len))
> +			return 0;
> +	}
> +	else
> +		len = strchrnul(match, ' ') - match;
>  
>  	nth = cb->cnt++ % cb->alloc;
>  	strbuf_reset(&cb->buf[nth]);

Sorry for the noise,
Dscho

Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]

Johannes Schindelin wrote:
> > Junio C Hamano <gitster@pobox.com> writes:
> > > You can have quite a many reflog entries, but you typically won't recall
> > > which branch you were on after switching branches for more than several
> > > times.
> 
> This, together with a removal of the hard-coded limit of 16 could be 
> squashed with this patch:

You know, I'm quite puzzled as to why we had working code that could
read the reflog backwards earlier in this thread, but it got shot down
solely based on impact and line counts, and you now have to jump
through hoops to work around the lack of this exact functionality.

So how about I resurrect the part about for_each_reflog_ent() and
_backward(), without touching read_ref_at().  This would actually
avoid the worst (hard to check) part of the patch since the
refactoring of for_each_reflog_ent()'s error checking is quite trivial
and IMHO actually results in more readable code.

I'm just asking because I'm not particularly inclined to do it first
and get rejected _again_.

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Johannes Schindelin]

Hi,

On Tue, 20 Jan 2009, Thomas Rast wrote:

> Johannes Schindelin wrote:
> > > Junio C Hamano <gitster@pobox.com> writes:
> > > > You can have quite a many reflog entries, but you typically won't recall
> > > > which branch you were on after switching branches for more than several
> > > > times.
> > 
> > This, together with a removal of the hard-coded limit of 16 could be 
> > squashed with this patch:
> 
> You know, I'm quite puzzled as to why we had working code that could
> read the reflog backwards earlier in this thread, but it got shot down
> solely based on impact and line counts, and you now have to jump
> through hoops to work around the lack of this exact functionality.

Okay, I should have told you what my two main concerns with the patch 
were.

1) it introduces a lot of code, with a lot of possibility for bugs to hide 
   (and I found it not simple enough to slap my head and say "of course, 
   this is obvious" as I did with Junio's code (except the modulo thing 
   which I had to thing about for half a minute)).

2) on Windows, mmap() is really implemented as xmalloc() && fread().  So 
   all the shortcomings of what Junio said about my array approach would 
   hold true for your approach, too.

Ciao,
Dscho

Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 1193 bytes --]

Johannes Schindelin wrote:
> 1) it introduces a lot of code, with a lot of possibility for bugs to hide 

And I can understand in the case of read_ref_at() since there the
translation is really nontrivial (though I'd readily put the blame on
the optimized-to-death original code ;-).  But in the case of
for_each_reflog_ent(), it should be really straightforward.

> 2) on Windows, mmap() is really implemented as xmalloc() && fread().  So 
>    all the shortcomings of what Junio said about my array approach would 
>    hold true for your approach, too.

But the @{} syntax _already_ uses mmap via read_ref_at().  And both
uses of for_each_reflog_ent() I'm aware of, the existing git log -g
and the present @{-N} syntax, have to read to the end anyway because
they're mostly interested in the newer stuff.

So while the mmap() might occasionally grab a few more MB of RAM than
would actually be required with simple line-based input, Windows
has to read the whole reflog no matter what.

(Well, unless we make a for_each_reflog_ent_backward() that can jump
in somewhere near the end and start parsing lines backwards.)

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH] interpret_nth_last_branch(): plug small memleak

[Junio C Hamano]

The error return path leaked both cb.buf[] strbuf array itself, and the
strings contained in its elements.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_name.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index f54b6cb..4c0370b 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -754,7 +754,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 int interpret_nth_last_branch(const char *name, struct strbuf *buf)
 {
 	long nth;
-	int i;
+	int i, retval;
 	struct grab_nth_branch_switch_cbdata cb;
 	const char *brace;
 	char *num_end;
@@ -774,17 +774,21 @@ int interpret_nth_last_branch(const char *name, struct strbuf *buf)
 	for (i = 0; i < nth; i++)
 		strbuf_init(&cb.buf[i], 20);
 	cb.cnt = 0;
+	retval = 0;
 	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
 	if (cb.cnt < nth)
-		return 0;
+		goto release_return;
 	i = cb.cnt % nth;
 	strbuf_reset(buf);
 	strbuf_add(buf, cb.buf[i].buf, cb.buf[i].len);
+	retval = brace-name+1;
+
+release_return:
 	for (i = 0; i < nth; i++)
 		strbuf_release(&cb.buf[i]);
 	free(cb.buf);
 
-	return brace-name+1;
+	return retval;
 }
 
 /*
-- 
1.6.1.267.g11c6e

[PATCH] Introduce for_each_recent_reflog_ent().

[Junio C Hamano]

This can be used to scan only the last few kilobytes of a reflog, as a
cheap optimization when the data you are looking for is likely to be
found near the end of it.  The caller is expected to fall back to the
full scan if that is not the case.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 refs.c      |   17 ++++++++++++++++-
 refs.h      |    1 +
 sha1_name.c |    8 +++++++-
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 33ced65..024211d 100644
--- a/refs.c
+++ b/refs.c
@@ -1453,7 +1453,7 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *
 	return 1;
 }
 
-int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
+int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long ofs, void *cb_data)
 {
 	const char *logfile;
 	FILE *logfp;
@@ -1464,6 +1464,16 @@ int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
 	logfp = fopen(logfile, "r");
 	if (!logfp)
 		return -1;
+
+	if (ofs) {
+		struct stat statbuf;
+		if (fstat(fileno(logfp), &statbuf) ||
+		    statbuf.st_size < ofs ||
+		    fseek(logfp, -ofs, SEEK_END) ||
+		    fgets(buf, sizeof(buf), logfp))
+			return -1;
+	}
+
 	while (fgets(buf, sizeof(buf), logfp)) {
 		unsigned char osha1[20], nsha1[20];
 		char *email_end, *message;
@@ -1497,6 +1507,11 @@ int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
 	return ret;
 }
 
+int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
+{
+	return for_each_recent_reflog_ent(ref, fn, 0, cb_data);
+}
+
 static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)
 {
 	DIR *dir = opendir(git_path("logs/%s", base));
diff --git a/refs.h b/refs.h
index 06ad260..3bb529d 100644
--- a/refs.h
+++ b/refs.h
@@ -60,6 +60,7 @@ extern int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned
 /* iterate over reflog entries */
 typedef int each_reflog_ent_fn(unsigned char *osha1, unsigned char *nsha1, const char *, unsigned long, int, const char *, void *);
 int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data);
+int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long, void *cb_data);
 
 /*
  * Calls the specified function for each reflog file until it returns nonzero,
diff --git a/sha1_name.c b/sha1_name.c
index 4c0370b..38c9f1b 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -775,7 +775,13 @@ int interpret_nth_last_branch(const char *name, struct strbuf *buf)
 		strbuf_init(&cb.buf[i], 20);
 	cb.cnt = 0;
 	retval = 0;
-	for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+	for_each_recent_reflog_ent("HEAD", grab_nth_branch_switch, 40960, &cb);
+	if (cb.cnt < nth) {
+		cb.cnt = 0;
+		for (i = 0; i < nth; i++)
+			strbuf_release(&cb.buf[i]);
+		for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+	}
 	if (cb.cnt < nth)
 		goto release_return;
 	i = cb.cnt % nth;
-- 
1.6.1.267.g11c6e

Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> This, together with a removal of the hard-coded limit of 16 could be 
> squashed with this patch:

Yeah, I think that makes very much sense, not because 16 is too small, but
because it does not make sense to keep track of all 16 when you asked for
the last (or the second from the last) event.

Thanks for a free sanity ;-)

Re: [TOY PATCH] git-resurrect: find traces of a branch name and resurrect it

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 591 bytes --]

[Sorry for missing this message, it seems KMail4 does have some rather
annoying filtering bugs...]

Johannes Schindelin wrote:
> On Sun, 18 Jan 2009, Thomas Rast wrote:
> 
> >  Makefile         |    1 +
> >  git-resurrect.sh |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> Maybe have it in contrib/ instead?

It was really intended as a toy patch, but if people find it useful
(Boyd?) I can add the rest of the options so that all searches can be
chosen independently, and shape it as a "real" contrib patch.

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Introduce for_each_recent_reflog_ent().

[Johannes Schindelin]

Hi,

On Mon, 19 Jan 2009, Junio C Hamano wrote:

> This can be used to scan only the last few kilobytes of a reflog, as a 
> cheap optimization when the data you are looking for is likely to be 
> found near the end of it.  The caller is expected to fall back to the 
> full scan if that is not the case.

FWIW I really like it, as it works around mmap() nicely.

Ciao,
Dscho

Re: [PATCH] interpret_nth_last_branch(): plug small memleak

[Johannes Schindelin]

Hi,

On Mon, 19 Jan 2009, Junio C Hamano wrote:

> The error return path leaked both cb.buf[] strbuf array itself, and the 
> strings contained in its elements.

Oops.  Thanks,
Dscho

Re: [TOY PATCH] git-resurrect: find traces of a branch name and resurrect it

[Boyd Stephen Smith Jr.]

[-- Attachment #1: Type: text/plain, Size: 603 bytes --]

On Tuesday 2009 January 20 03:01:50 Thomas Rast wrote:
>It was really intended as a toy patch, but if people find it useful
>(Boyd?) I can add the rest of the options so that all searches can be
>chosen independently, and shape it as a "real" contrib patch.

I'll test it out later today and get back to you.

[OT]
I actually prefer Stephen; My father is Boyd.
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [TOY PATCH] git-resurrect: find traces of a branch name and resurrect it

[Boyd Stephen Smith Jr.]

[-- Attachment #1: Type: text/plain, Size: 961 bytes --]

On Tuesday 2009 January 20 10:57:17 Boyd Stephen Smith Jr. wrote:
>On Tuesday 2009 January 20 03:01:50 Thomas Rast wrote:
>>It was really intended as a toy patch, but if people find it useful
>>(Boyd?) I can add the rest of the options so that all searches can be
>>chosen independently, and shape it as a "real" contrib patch.
>
>I'll test it out later today and get back to you.

In my particular case, it wasn't useful without the -m option, but I 
understand why it is not the default.

I think it could be quite nice; "undelete"-type commands are generally 
well-received by users and when run against reflogs alone, that's what the 
command is.

It's useful enough to me that I'd love to see it mainlined.
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[VALGRIND PATCH for nth_last patch series] Fix invalid memory access

[Johannes Schindelin]

Please squash in.
---

	On Mon, 19 Jan 2009, Johannes Schindelin wrote:

	> @@ -720,7 +718,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
	>  	if (target[len] == '\n' && !strncmp(match, target, len))
	>  		return 0;

	This code is still not valid, as target[len] can be well after the 
	NUL marker.

	Found by valgrind.

 sha1_name.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 4d10705..803f9d2 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -735,7 +735,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 	if ((target = strstr(match, " to ")) != NULL) {
 		len = target - match;
 		target += 4;
-		if (target[len] == '\n' && !strncmp(match, target, len))
+		if (len == strlen(target) && !strncmp(match, target, len))
 			return 0;
 	}
 	else
-- 
1.6.1.243.g6c8bb35

Re: [VALGRIND PATCH for nth_last patch series] Fix invalid memory access

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>  sha1_name.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/sha1_name.c b/sha1_name.c
> index 4d10705..803f9d2 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -735,7 +735,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
>  	if ((target = strstr(match, " to ")) != NULL) {
>  		len = target - match;
>  		target += 4;
> -		if (target[len] == '\n' && !strncmp(match, target, len))
> +		if (len == strlen(target) && !strncmp(match, target, len))
>  			return 0;
>  	}
>  	else

Actually, I think this patch to a884d0c (sha1_name: tweak @{-N} lookup,
2009-01-17) would make more sense.

-- >8 --
Subject: [PATCH] Simplify parsing branch switching events in reflog

We only accept "checkout: moving from A to B" newer style reflog entries,
in order to pick up A.  There is no point computing where B begins at
after running strstr to locate " to ", nor adding 4 and then subtracting 4
from the same pointer.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_name.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 38c9f1b..7d95bbb 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -723,17 +723,13 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 
 	if (!prefixcmp(message, "checkout: moving from ")) {
 		match = message + strlen("checkout: moving from ");
-		if ((target = strstr(match, " to ")) != NULL)
-			target += 4;
+		target = strstr(match, " to ");
 	}
 
 	if (!match || !target)
 		return 0;
 
-	len = target - match - 4;
-	if (target[len] == '\n' && !strncmp(match, target, len))
-		return 0;
-
+	len = target - match;
 	nth = cb->cnt++ % cb->alloc;
 	strbuf_reset(&cb->buf[nth]);
 	strbuf_add(&cb->buf[nth], match, len);
-- 
1.6.1.281.g16db

Re: [VALGRIND PATCH for nth_last patch series] Fix invalid memory access

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 1045 bytes --]

Junio C Hamano wrote:
> We only accept "checkout: moving from A to B" newer style reflog entries,
> in order to pick up A.  There is no point computing where B begins at
> after running strstr to locate " to ", nor adding 4 and then subtracting 4
> from the same pointer.
[...]
> -	len = target - match - 4;
> -	if (target[len] == '\n' && !strncmp(match, target, len))
> -		return 0;
> -
> +	len = target - match;

Actually the point of that exercise was to ignore branch (non)switches
of the form

  checkout: moving from A to A

I originally thought that this would be desirable behaviour, but now
that it causes so much trouble, I'm not that sure any more.  I still
think it would be more intuitive to not count them as switches (after
all git-checkout says 'Already on "$branch"'), but OTOH 'cd .; cd -'
also stays in the same directory.

Thanks for all the work you (both) are doing on this.  I hope to find
the time to read the current state of the series tonight.

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [VALGRIND PATCH for nth_last patch series] Fix invalid memory access

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> Actually the point of that exercise was to ignore branch (non)switches
> of the form
>
>   checkout: moving from A to A

Ahhh, Ok, that is what I missed.

> I originally thought that this would be desirable behaviour, but now
> that it causes so much trouble, I'm not that sure any more.  I still
> think it would be more intuitive to not count them as switches (after
> all git-checkout says 'Already on "$branch"'), but OTOH 'cd .; cd -'
> also stays in the same directory.

An entry of the form "from A to A" is made only when you explicitly ask to
checkout the current branch by name (i.e. "git checkout" without any
parameter won't add such an entry to the reflog), so I tend to agree with
"cd" that the users may find it more natural if we counted them.

Having said all that, I think Dscho's one had an off-by-one (but it is
getting late and it may be I who has one).

When parsing "checkout: moving from master to side\n", match points at
"master to...", target points at "side\n", and len is 6 (length of
"master").  We want to see if target is "master\n" and ignore such an
entry, so we should be checking if target is one longer than len.

 sha1_name.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git c/sha1_name.c w/sha1_name.c
index 38c9f1b..9aed8ae 100644
--- c/sha1_name.c
+++ w/sha1_name.c
@@ -731,7 +731,10 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 		return 0;
 
 	len = target - match - 4;
-	if (target[len] == '\n' && !strncmp(match, target, len))
+	if (len + 1 == strlen(target) &&
+	    target[len] == '\n' &&
+	    !memcmp(target, match, len))
+		/* switching same branch "from A to A\n" */
 		return 0;
 
 	nth = cb->cnt++ % cb->alloc;

Re: [VALGRIND PATCH for nth_last patch series] Fix invalid memory access

[Johannes Schindelin]

Hi,

On Wed, 21 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >  sha1_name.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/sha1_name.c b/sha1_name.c
> > index 4d10705..803f9d2 100644
> > --- a/sha1_name.c
> > +++ b/sha1_name.c
> > @@ -735,7 +735,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
> >  	if ((target = strstr(match, " to ")) != NULL) {
> >  		len = target - match;
> >  		target += 4;
> > -		if (target[len] == '\n' && !strncmp(match, target, len))
> > +		if (len == strlen(target) && !strncmp(match, target, len))
> >  			return 0;
> >  	}
> >  	else
> 
> Actually, I think this patch to a884d0c (sha1_name: tweak @{-N} lookup,
> 2009-01-17) would make more sense.
> 
> -- >8 --
> Subject: [PATCH] Simplify parsing branch switching events in reflog
> 
> We only accept "checkout: moving from A to B" newer style reflog entries,
> in order to pick up A.  There is no point computing where B begins at
> after running strstr to locate " to ", nor adding 4 and then subtracting 4
> from the same pointer.

Yeah, you're right.

Ciao,
Dscho

Re: [VALGRIND PATCH for nth_last patch series] Fix invalid memory access

[Johannes Schindelin]

Hi,

On Wed, 21 Jan 2009, Junio C Hamano wrote:

> Having said all that, I think Dscho's one had an off-by-one (but it is
> getting late and it may be I who has one).

Yep, I missed the "\n" at the end.

Ciao,
Dscho

[PATCH] contrib git-resurrect: find traces of a branch name and resurrect it

[Thomas Rast]

Add a tool 'git-resurrect.sh <branch>' that tries to find traces of
the <branch> in the HEAD reflog and, optionally, all merge commits in
the repository.  It can then resurrect the branch, pointing it at the
most recent of all candidate commits found.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Boyd Stephen Smith Jr. wrote:
> I think it could be quite nice; "undelete"-type commands are generally 
> well-received by users and when run against reflogs alone, that's what the 
> command is.
> 
> It's useful enough to me that I'd love to see it mainlined.

So here's a version for contrib with more options and some other
tweaks.

I removed the ability to "batch resurrect" with several <name>
arguments since that would have conflicted with -b <newname>, but
otherwise the features are the same.

> In my particular case, it wasn't useful without the -m option, but I 
> understand why it is not the default.

Aside from the obvious speed reasons, I don't really want to teach
people that commits "know" the branch they were on.  It is a pure
coincidence if you can resurrect a topic branch from merge messages;
an equivalent merge could have gone through as a fast-forward, and
you'd never know.



 contrib/git-resurrect.sh |  140 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 140 insertions(+), 0 deletions(-)
 create mode 100755 contrib/git-resurrect.sh

diff --git a/contrib/git-resurrect.sh b/contrib/git-resurrect.sh
new file mode 100755
index 0000000..29bf723
--- /dev/null
+++ b/contrib/git-resurrect.sh
@@ -0,0 +1,140 @@
+#!/bin/sh
+
+USAGE="[-h] [-r] [-m] [-t] [-n] [-b <newname>] <name>"
+LONG_USAGE="git-resurrect attempts to find traces of a branch tip
+called <name>, and tries to resurrect it.  Currently, the reflog is
+searched for checkout messages, and with -r also merge messages.  With
+-m and -t, the history of all refs is scanned for Merge <name> into
+other/Merge <other> into <name> (respectively) commit subjects, which
+is rather slow but allows you to resurrect other people's topic
+branches."
+
+OPTIONS_SPEC="\
+git resurrect [-h] [-r] [-m] [-t] [-n] [-b <newname>] <name>
+--
+b,branch=            save branch as <newname> instead of <name>
+H,try-hard           same as -r -m -t
+r,reflog-merges      scan for merges recorded in reflog
+m,merges             scan for merges into other branches (slow)
+t,merge-targets      scan for merges of other branches into <name>
+n,dry-run            don't recreate the branch"
+
+. git-sh-setup
+cd_to_toplevel
+
+search_reflog () {
+        sed -n 's~^\([^ ]*\) .*\tcheckout: moving from '"$1"' .*~\1~p' \
+                < .git/logs/HEAD
+}
+
+search_reflog_merges () {
+        sed -n 's~^[^ ]* \([^ ]*\) .*\tmerge '"$1"':~\1~p' \
+                < .git/logs/HEAD
+}
+
+search_merges () {
+	git rev-list --pretty=tformat:"%h %p:%s" --all |
+	grep "Merge branch.*'$branch'.*into" |
+	while read sha rest; do
+		parents="$(echo "$rest" | cut -d: -f1)"
+		case "$parents" in
+		    *' '*' '*)
+			warn "$branch took part in octopus merge $sha"
+			warn "check manually!"
+			;;
+		    *' '*)
+			echo "$parents" | cut -d' ' -f2
+			;;
+		esac
+	done
+}
+
+search_merge_targets () {
+	git rev-list --pretty=tformat:"%h %s" --all |
+	grep "Merge branch '[^']*' into $branch$" |
+	cut -d' ' -f1
+}
+
+dry_run=
+scan_reflog_merges=
+scan_merges=
+scan_merge_targets=
+new_name=
+
+while test "$#" != 0; do
+	case "$1" in
+	    -b|--branch)
+		shift
+		new_name="$1"
+		;;
+	    -n|--dry-run)
+		dry_run=t
+		;;
+	    -m|--merges)
+		scan_merges=t
+		;;
+	    -r|--reflog_merges)
+		scan_reflog_merges=t
+		;;
+	    -t|--merge-targets)
+		scan_merge_targets=t
+		;;
+	    -H|--try-hard)
+		scan_reflog_merges=t
+		scan_merges=t
+		scan_merge_targets=t
+		;;
+	    --)
+		shift
+		break
+		;;
+	    *)
+		usage
+		;;
+	esac
+	shift
+done
+
+test "$#" = 1 || usage
+
+branch="$1"
+test -z "$new_name" && new_name="$branch"q
+
+candidates="$(search_reflog $1)"
+if test ! -z "$scan_reflog_merges"; then
+	candidates="$candidates $(search_reflog_merges $1)"
+fi
+if test ! -z "$scan_merges"; then
+	candidates="$candidates $(search_merges $1)"
+fi
+if test ! -z "$scan_merge_targets"; then
+	candidates="$candidates $(search_merge_targets $1)"
+fi
+
+candidates="$(git rev-parse $candidates | sort -u)"
+
+if test -z "$candidates"; then
+	hint=
+	test "z$scan_merges$scan_reflog_merges$scan_merge_targets" != "zttt" \
+		&& hint="(maybe try again with -H)"
+	die "no candidates for $branch found" $hint
+fi
+
+echo "** Candidates for $branch **"
+for cmt in $candidates; do
+	git --no-pager log --pretty=oneline --abbrev-commit -1 $cmt
+done
+
+newest="$(git rev-list -1 $candidates)"
+if test ! -z "$dry_run"; then
+	printf "Most recent: "
+	git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+elif ! git rev-parse --verify --quiet $new_name >/dev/null; then
+	printf "** Restoring $new_name to "
+	git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+	git branch $new_name $newest
+else
+	printf "Most recent: "
+	git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+	echo "** $new_name already exists, doing nothing"
+fi
-- 
1.6.1.447.gbdf1d

Re: [PATCH] contrib git-resurrect: find traces of a branch name and resurrect it

[Boyd Stephen Smith Jr.]

[-- Attachment #1: Type: text/plain, Size: 3475 bytes --]

On Friday 2009 January 23 14:03:05 Thomas Rast wrote:
>Boyd Stephen Smith Jr. wrote:
>> I think it could be quite nice; "undelete"-type commands are generally
>> well-received by users and when run against reflogs alone, that's what the
>> command is.
>>
>> It's useful enough to me that I'd love to see it mainlined.
>
>So here's a version for contrib with more options and some other
>tweaks.

I wanted/needed the ability to ignore reflogs entirely.  Use went something 
like this:
1. resurrect branch from origin/pu
2. add patches, mail to list
3. # wait 24 hours
4. pull, see from logs that branch was modified, but not just my changes (or 
without all of my changes).
5. delete local branch
6. Try to resurrect branch from origin/pu, get local version I just deleted.
7. delete reflog for that branch
8. Try to resurrect branch from origin/pu, get local version I merged into 
master at some point.
9. Add new option.

So, I added a couple of options locally: --only-merges, so it would only look 
at the first line of commit logs, ignoring my local reflogs entirely; 
and --revisions, to specify arguments to pass to rev-list so it wouldn't even 
see my local merges (I passed 'origin/pu origin/next').

Yeah, my usage might be abusage, but it worked for me. :)

Would you object to a patch adding a --reflog option and allowing each of the 
scan options to be negated?

>I removed the ability to "batch resurrect" with several <name>
>arguments since that would have conflicted with -b <newname>, but
>otherwise the features are the same.

In my local version, which I was going to try and clean up over the weekend, I 
was going to support both, by borrowing refspec syntax from fetch/push.  
Specifically.  Resurrecting 'js/notes' as 'pu/js/notes' would look like:
git-resurrect -H js/notes:pu/js/notes

Would you object to a patch that dropped -b in favor of the refspec syntax?

>> In my particular case, it wasn't useful without the -m option, but I
>> understand why it is not the default.
>
>Aside from the obvious speed reasons, I don't really want to teach
>people that commits "know" the branch they were on.  It is a pure
>coincidence if you can resurrect a topic branch from merge messages;
>an equivalent merge could have gone through as a fast-forward, and
>you'd never know.

Yeah, agreed.  I made this more clear in my local version by changing the 
documentation from "scan for merges" to "scan first line of commit messages 
for possible merges".  It's more wordy, but it make it clear that it is 
dependent on the message, and it's not tracked outside of that.

I also tend to merge topic branches with --no-ff so that I do get the merge 
message, so it has a better chance of working against my repository.  (I also 
enjoy octopus merging when possible so the history indicates the patch sets 
are separable, but maybe I'm just a little "touched" and haven't been bitten 
by by an octopus yet.[1])

Not directly related to any issue you bring up:

There seems to be some needless redundancy between USAGE and OPTIONS_SPEC.

Would you object to a patch that used $USAGE inside OPTIONS_SPEC?
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

[1] I hear they are even more feral than penguins.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [VALGRIND PATCH for nth_last patch series] Fix invalid memory access

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]

Thomas Rast wrote:
> Thanks for all the work you (both) are doing on this.  I hope to find
> the time to read the current state of the series tonight.

I was off by half a week :-(

The debugging 'git log --graph ...' that I forgot about is still in,
sorry.  I'll follow up with a small patch.  Other than that, I didn't
notice anything, though admittedly it wouldn't have been very hard to
sneak something past me.

By the way, you (Junio) remarked earlier in the thread that we could
forbid any use of 'git checkout -' except in literally that command
(i.e., no options or paths).  I'm midly opposed to it because I can
see myself saying 'git checkout -b sidebranch -' (and @{} is somewhat
awkward to type on Swiss keyboards).  Admittedly 'git checkout - --
file' looks rather confusing.

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH next] t1505: remove debugging cruft

[Thomas Rast]

Remove a call to git-log that I introduced for debugging and that
accidentally made it into d18ba22 (sha1_name: support @{-N} syntax in
get_sha1(), 2009-01-17).

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 t/t1505-rev-parse-last.sh |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh
index c745ec4..d709ecf 100755
--- a/t/t1505-rev-parse-last.sh
+++ b/t/t1505-rev-parse-last.sh
@@ -32,8 +32,6 @@ test_expect_success 'setup' '
 #
 # and 'side' should be the last branch
 
-git log --graph --all --pretty=oneline --decorate
-
 test_rev_equivalent () {
 
 	git rev-parse "$1" > expect &&
-- 
1.6.1.468.g15c0

Re: [PATCH next] t1505: remove debugging cruft

[Junio C Hamano]

Thanks.

Re: [PATCH] contrib git-resurrect: find traces of a branch name and resurrect it

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 1817 bytes --]

Hi Stephen,

Sorry for the long delay.  I'm going to roll a v2 with two small
fixes.  After that it's all yours ;-)

Boyd Stephen Smith Jr. wrote:
[...]
> 6. Try to resurrect branch from origin/pu, get local version I just deleted.
> 7. delete reflog for that branch
> 8. Try to resurrect branch from origin/pu, get local version I merged into 
> master at some point.
> 9. Add new option.
> 
> So, I added a couple of options locally: --only-merges, so it would only look 
> at the first line of commit logs, ignoring my local reflogs entirely; 
> and --revisions, to specify arguments to pass to rev-list so it wouldn't even 
> see my local merges (I passed 'origin/pu origin/next').
> 
> Yeah, my usage might be abusage, but it worked for me. :)

I'm fine with adding such an option, but I still wonder what was wrong
with the original scheme of asking 'git rev-list -1' for the newest
commit.  I thought rev-list always listed by date, so that command
should always pick the newest candidate commit from all candidates
selected.  Do you have an example where that breaks?  Or did you just
have a use-case in which you wanted something other than the newest
candidate?

> In my local version, which I was going to try and clean up over the weekend, I 
> was going to support both, by borrowing refspec syntax from fetch/push.  
> Specifically.  Resurrecting 'js/notes' as 'pu/js/notes' would look like:
> git-resurrect -H js/notes:pu/js/notes
> 
> Would you object to a patch that dropped -b in favor of the refspec syntax?

No, that would be fine by me.

> There seems to be some needless redundancy between USAGE and OPTIONS_SPEC.
> 
> Would you object to a patch that used $USAGE inside OPTIONS_SPEC?

Also a good idea.

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH v2] contrib git-resurrect: find traces of a branch name and resurrect it

[Thomas Rast]

Add a tool 'git-resurrect.sh <branch>' that tries to find traces of
the <branch> in the HEAD reflog and, optionally, all merge commits in
the repository.  It can then resurrect the branch, pointing it at the
most recent of all candidate commits found.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Fixed the -h to upper-case in the short options summaries, and removed
a stray 'q' in the default assignment of new_name.


 contrib/git-resurrect.sh |  140 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 140 insertions(+), 0 deletions(-)
 create mode 100755 contrib/git-resurrect.sh

diff --git a/contrib/git-resurrect.sh b/contrib/git-resurrect.sh
new file mode 100755
index 0000000..3c1c946
--- /dev/null
+++ b/contrib/git-resurrect.sh
@@ -0,0 +1,140 @@
+#!/bin/sh
+
+USAGE="[-H] [-r] [-m] [-t] [-n] [-b <newname>] <name>"
+LONG_USAGE="git-resurrect attempts to find traces of a branch tip
+called <name>, and tries to resurrect it.  Currently, the reflog is
+searched for checkout messages, and with -r also merge messages.  With
+-m and -t, the history of all refs is scanned for Merge <name> into
+other/Merge <other> into <name> (respectively) commit subjects, which
+is rather slow but allows you to resurrect other people's topic
+branches."
+
+OPTIONS_SPEC="\
+git resurrect [-H] [-r] [-m] [-t] [-n] [-b <newname>] <name>
+--
+b,branch=            save branch as <newname> instead of <name>
+H,try-hard           same as -r -m -t
+r,reflog-merges      scan for merges recorded in reflog
+m,merges             scan for merges into other branches (slow)
+t,merge-targets      scan for merges of other branches into <name>
+n,dry-run            don't recreate the branch"
+
+. git-sh-setup
+cd_to_toplevel
+
+search_reflog () {
+        sed -n 's~^\([^ ]*\) .*\tcheckout: moving from '"$1"' .*~\1~p' \
+                < .git/logs/HEAD
+}
+
+search_reflog_merges () {
+        sed -n 's~^[^ ]* \([^ ]*\) .*\tmerge '"$1"':~\1~p' \
+                < .git/logs/HEAD
+}
+
+search_merges () {
+	git rev-list --pretty=tformat:"%h %p:%s" --all |
+	grep "Merge branch.*'$branch'.*into" |
+	while read sha rest; do
+		parents="$(echo "$rest" | cut -d: -f1)"
+		case "$parents" in
+		    *' '*' '*)
+			warn "$branch took part in octopus merge $sha"
+			warn "check manually!"
+			;;
+		    *' '*)
+			echo "$parents" | cut -d' ' -f2
+			;;
+		esac
+	done
+}
+
+search_merge_targets () {
+	git rev-list --pretty=tformat:"%h %s" --all |
+	grep "Merge branch '[^']*' into $branch$" |
+	cut -d' ' -f1
+}
+
+dry_run=
+scan_reflog_merges=
+scan_merges=
+scan_merge_targets=
+new_name=
+
+while test "$#" != 0; do
+	case "$1" in
+	    -b|--branch)
+		shift
+		new_name="$1"
+		;;
+	    -n|--dry-run)
+		dry_run=t
+		;;
+	    -m|--merges)
+		scan_merges=t
+		;;
+	    -r|--reflog_merges)
+		scan_reflog_merges=t
+		;;
+	    -t|--merge-targets)
+		scan_merge_targets=t
+		;;
+	    -H|--try-hard)
+		scan_reflog_merges=t
+		scan_merges=t
+		scan_merge_targets=t
+		;;
+	    --)
+		shift
+		break
+		;;
+	    *)
+		usage
+		;;
+	esac
+	shift
+done
+
+test "$#" = 1 || usage
+
+branch="$1"
+test -z "$new_name" && new_name="$branch"
+
+candidates="$(search_reflog $1)"
+if test ! -z "$scan_reflog_merges"; then
+	candidates="$candidates $(search_reflog_merges $1)"
+fi
+if test ! -z "$scan_merges"; then
+	candidates="$candidates $(search_merges $1)"
+fi
+if test ! -z "$scan_merge_targets"; then
+	candidates="$candidates $(search_merge_targets $1)"
+fi
+
+candidates="$(git rev-parse $candidates | sort -u)"
+
+if test -z "$candidates"; then
+	hint=
+	test "z$scan_merges$scan_reflog_merges$scan_merge_targets" != "zttt" \
+		&& hint="(maybe try again with -H)"
+	die "no candidates for $branch found" $hint
+fi
+
+echo "** Candidates for $branch **"
+for cmt in $candidates; do
+	git --no-pager log --pretty=oneline --abbrev-commit -1 $cmt
+done
+
+newest="$(git rev-list -1 $candidates)"
+if test ! -z "$dry_run"; then
+	printf "Most recent: "
+	git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+elif ! git rev-parse --verify --quiet $new_name >/dev/null; then
+	printf "** Restoring $new_name to "
+	git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+	git branch $new_name $newest
+else
+	printf "Most recent: "
+	git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+	echo "** $new_name already exists, doing nothing"
+fi
-- 
1.6.1.469.g6f3d5

Re: [PATCH v2] contrib git-resurrect: find traces of a branch name and resurrect it

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> Add a tool 'git-resurrect.sh <branch>' that tries to find traces of
> the <branch> in the HEAD reflog and, optionally, all merge commits in
> the repository.  It can then resurrect the branch, pointing it at the
> most recent of all candidate commits found.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>
> Fixed the -h to upper-case in the short options summaries, and removed
> a stray 'q' in the default assignment of new_name.

I hate to paint bikeshed, but -H "try-hard" looks somewhat unusual doesn't
it?  It sounds more like --all (find from all possible sources).

> +. git-sh-setup
> +cd_to_toplevel

Why?

> +search_reflog () {
> +        sed -n 's~^\([^ ]*\) .*\tcheckout: moving from '"$1"' .*~\1~p' \
> +                < .git/logs/HEAD
> +}

Once you used ". git-sh-setup", use "$GIT_DIR/logs/HEAD".  That way, you
can work in a bare repository (and you do not have to cd_to_toplevel,
either, I think).

Oh, don't forget to skip this step if the reflog does not exist.

> +search_reflog_merges () {
> +        sed -n 's~^[^ ]* \([^ ]*\) .*\tmerge '"$1"':~\1~p' \
> +                < .git/logs/HEAD
> +}

The two commits both point at the HEAD that merges the other branch into,
so this finds a merge commit that has the tip of target branch as its
second parent.  Is that really what you want?

> +search_merges () {
> +	git rev-list --pretty=tformat:"%h %p:%s" --all |
> +	grep "Merge branch.*'$branch'.*into" |

"git merge tr/topic~4" can say "Merge branch 'tr/topic' (early part)".
Also merge into 'master' won't have "into ...".

> +	while read sha rest; do
> +		parents="$(echo "$rest" | cut -d: -f1)"
> +		case "$parents" in
> +		    *' '*' '*)
> +			warn "$branch took part in octopus merge $sha"
> +			warn "check manually!"
> +			;;
> +		    *' '*)
> +			echo "$parents" | cut -d' ' -f2
> +			;;
> +		esac
> +	done

Reading everything down to the root commit sounds like fun.  rev-list
gives you the output from newer to older so you may want to break out once
you have found enough candidates.

Anyway, if I were doing this script, I'd write this part like this without
a shell loop:

        _x40="[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]"
        _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"

	git rev-list --all --grep="Merge branch '$1'" \
        	--pretty=tformat:"%H %P %s" |
	sed -ne "s/^$_x40 $_x40 \($_x40\) Merge .*/\1/p"

Re: [PATCH v2] contrib git-resurrect: find traces of a branch name and resurrect it

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 327 bytes --]

Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> 
> > Add a tool 'git-resurrect.sh <branch>' that tries to find traces of

Thanks for your review.  I've been busy and thus out of the loop all
week, but I'll try and make an improved version Soon(tm).

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH v3] contrib git-resurrect: find traces of a branch name and resurrect it

[Thomas Rast]

Add a tool 'git-resurrect.sh <branch>' that tries to find traces of
the <branch> in the HEAD reflog and, optionally, all merge commits in
the repository.  It can then resurrect the branch, pointing it at the
most recent of all candidate commits found.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Junio C Hamano wrote:
> I hate to paint bikeshed, but -H "try-hard" looks somewhat unusual doesn't
> it?  It sounds more like --all (find from all possible sources).

Why not.  I had '-h' but then found out the hard way that it's
reserved...

> > +search_reflog_merges () {
> > +        sed -n 's~^[^ ]* \([^ ]*\) .*\tmerge '"$1"':~\1~p' \
> > +                < .git/logs/HEAD
> > +}
> 
> The two commits both point at the HEAD that merges the other branch into,
> so this finds a merge commit that has the tip of target branch as its
> second parent.  Is that really what you want?

Good point.  Furthermore the sed expression was broken, it would not
remove the remainder of the line.  Sadly it's not possible to insert
the reflog message and sha1 via --pretty=format, so I now use
rev-parse.

> Reading everything down to the root commit sounds like fun.  rev-list
> gives you the output from newer to older so you may want to break out once
> you have found enough candidates.
> 
> Anyway, if I were doing this script, I'd write this part like this without
> a shell loop:
> 
>         _x40="[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]"
>         _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
> 
> 	git rev-list --all --grep="Merge branch '$1'" \
>         	--pretty=tformat:"%H %P %s" |
> 	sed -ne "s/^$_x40 $_x40 \($_x40\) Merge .*/\1/p"

Nice trick.  The same also works for scan_merge_targets() and gives it
a nice speed boost too.  Unfortunately my sed-fu is not good enough to
figure out how to only print the first line (for resurrections from
pu, we expect there to be a single match).  All uses of 'q' I could
come up with resulted in an early exit independent of the
substitutions.  Appending '| head -n 1' does not seem to make any
difference either.

I also added the relative committer time to the candidate list, and
made it sort according to time; it seems somewhat more readable now.


 contrib/git-resurrect.sh |  172 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 172 insertions(+), 0 deletions(-)
 create mode 100755 contrib/git-resurrect.sh

diff --git a/contrib/git-resurrect.sh b/contrib/git-resurrect.sh
new file mode 100755
index 0000000..3a040ae
--- /dev/null
+++ b/contrib/git-resurrect.sh
@@ -0,0 +1,172 @@
+#!/bin/sh
+
+USAGE="[-a] [-r] [-m] [-t] [-n] [-b <newname>] <name>"
+LONG_USAGE="git-resurrect attempts to find traces of a branch tip
+called <name>, and tries to resurrect it.  Currently, the reflog is
+searched for checkout messages, and with -r also merge messages.  With
+-m and -t, the history of all refs is scanned for Merge <name> into
+other/Merge <other> into <name> (respectively) commit subjects, which
+is rather slow but allows you to resurrect other people's topic
+branches."
+
+OPTIONS_SPEC="\
+git resurrect $USAGE
+--
+b,branch=            save branch as <newname> instead of <name>
+a,all                same as -l -r -m -t
+l,reflog             scan reflog for checkouts (enabled by default)
+r,reflog-merges      scan for merges recorded in reflog
+m,merges             scan for merges into other branches (slow)
+t,merge-targets      scan for merges of other branches into <name>
+n,dry-run            don't recreate the branch"
+
+. git-sh-setup
+
+search_reflog () {
+        sed -ne 's~^\([^ ]*\) .*\tcheckout: moving from '"$1"' .*~\1~p' \
+                < "$GIT_DIR"/logs/HEAD
+}
+
+search_reflog_merges () {
+	git rev-parse $(
+		sed -ne 's~^[^ ]* \([^ ]*\) .*\tmerge '"$1"':.*~\1^2~p' \
+			< "$GIT_DIR"/logs/HEAD
+	)
+}
+
+_x40="[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]"
+_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+
+search_merges () {
+        git rev-list --all --grep="Merge branch '$1'" \
+                --pretty=tformat:"%P %s" |
+        sed -ne "s/^$_x40 \($_x40\) Merge .*/\1/p"
+}
+
+search_merge_targets () {
+	git rev-list --all --grep="Merge branch '[^']*' into $branch\$" \
+		--pretty=tformat:"%H %s" --all |
+	sed -ne "s/^\($_x40\) Merge .*/\1/p"
+}
+
+dry_run=
+scan_reflog=t
+scan_reflog_merges=
+scan_merges=
+scan_merge_targets=
+new_name=
+
+while test "$#" != 0; do
+	case "$1" in
+	    -b|--branch)
+		shift
+		new_name="$1"
+		;;
+	    -n|--dry-run)
+		dry_run=t
+		;;
+	    --no-dry-run)
+		dry_run=
+		;;
+	    -m|--merges)
+		scan_merges=t
+		;;
+	    --no-merges)
+		scan_merges=
+		;;
+	    -l|--reflog)
+		scan_reflog=t
+		;;
+	    --no-reflog)
+		scan_reflog=
+		;;
+	    -r|--reflog_merges)
+		scan_reflog_merges=t
+		;;
+	    --no-reflog_merges)
+		scan_reflog_merges=
+		;;
+	    -t|--merge-targets)
+		scan_merge_targets=t
+		;;
+	    --no-merge-targets)
+		scan_merge_targets=
+		;;
+	    -a|--all)
+		scan_reflog=t
+		scan_reflog_merges=t
+		scan_merges=t
+		scan_merge_targets=t
+		;;
+	    --)
+		shift
+		break
+		;;
+	    *)
+		usage
+		;;
+	esac
+	shift
+done
+
+test "$#" = 1 || usage
+
+all_strategies="$scan_reflog$scan_reflog_merges$scan_merges$scan_merge_targets"
+if test -z "$all_strategies"; then
+	die "must enable at least one of -lrmt"
+fi
+
+branch="$1"
+test -z "$new_name" && new_name="$branch"
+
+if test ! -z "$scan_reflog"; then
+	if test -r "$GIT_DIR"/logs/HEAD; then
+		candidates="$(search_reflog $branch)"
+	else
+		die 'reflog scanning requested, but' \
+			'$GIT_DIR/logs/HEAD not readable'
+	fi
+fi
+if test ! -z "$scan_reflog_merges"; then
+	if test -r "$GIT_DIR"/logs/HEAD; then
+		candidates="$candidates $(search_reflog_merges $branch)"
+	else
+		die 'reflog scanning requested, but' \
+			'$GIT_DIR/logs/HEAD not readable'
+	fi
+fi
+if test ! -z "$scan_merges"; then
+	candidates="$candidates $(search_merges $branch)"
+fi
+if test ! -z "$scan_merge_targets"; then
+	candidates="$candidates $(search_merge_targets $branch)"
+fi
+
+candidates="$(git rev-parse $candidates | sort -u)"
+
+if test -z "$candidates"; then
+	hint=
+	test "z$all_strategies" != "ztttt" \
+		&& hint=" (maybe try again with -a)"
+	die "no candidates for $branch found$hint"
+fi
+
+echo "** Candidates for $branch **"
+for cmt in $candidates; do
+	git --no-pager log --pretty=tformat:"%ct:%h [%cr] %s" --abbrev-commit -1 $cmt
+done \
+| sort -n | cut -d: -f2-
+
+newest="$(git rev-list -1 $candidates)"
+if test ! -z "$dry_run"; then
+	printf "** Most recent: "
+	git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+elif ! git rev-parse --verify --quiet $new_name >/dev/null; then
+	printf "** Restoring $new_name to "
+	git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+	git branch $new_name $newest
+else
+	printf "Most recent: "
+	git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+	echo "** $new_name already exists, doing nothing"
+fi
-- 
1.6.1.2.495.gb8db2

Re: [PATCH v3] contrib git-resurrect: find traces of a branch name and resurrect it

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

>> Reading everything down to the root commit sounds like fun.  rev-list
>> gives you the output from newer to older so you may want to break out once
>> you have found enough candidates.
>> 
>> Anyway, if I were doing this script, I'd write this part like this without
>> a shell loop:
>> 
>>         _x40="[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]"
>>         _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
>> 
>> 	git rev-list --all --grep="Merge branch '$1'" \
>>         	--pretty=tformat:"%H %P %s" |
>> 	sed -ne "s/^$_x40 $_x40 \($_x40\) Merge .*/\1/p"
>
> Nice trick.  The same also works for scan_merge_targets() and gives it
> a nice speed boost too.  Unfortunately my sed-fu is not good enough to
> figure out how to only print the first line (for resurrections from
> pu, we expect there to be a single match).

Do you mean something like this?

	sed -n -e "/^$_x40 $_x40 \($_x40\) Merge .*/ {
		s//\1/p
                q
        }"

[PATCH v4] contrib git-resurrect: find traces of a branch name and resurrect it

[Thomas Rast]

Add a tool 'git-resurrect.sh <branch>' that tries to find traces of
the <branch> in the HEAD reflog and, optionally, all merge commits in
the repository.  It can then resurrect the branch, pointing it at the
most recent of all candidate commits found.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Junio C Hamano wrote:
> Do you mean something like this?
> 
> 	sed -n -e "/^$_x40 $_x40 \($_x40\) Merge .*/ {
> 		s//\1/p
>                 q
>         }"

Yep, precisely.  Thanks!  This indeed gives it a nice speed boost if
all you want is a topic "resurrection" from pu.



 contrib/git-resurrect.sh |  180 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 180 insertions(+), 0 deletions(-)
 create mode 100755 contrib/git-resurrect.sh

diff --git a/contrib/git-resurrect.sh b/contrib/git-resurrect.sh
new file mode 100755
index 0000000..c364dda
--- /dev/null
+++ b/contrib/git-resurrect.sh
@@ -0,0 +1,180 @@
+#!/bin/sh
+
+USAGE="[-a] [-r] [-m] [-t] [-n] [-b <newname>] <name>"
+LONG_USAGE="git-resurrect attempts to find traces of a branch tip
+called <name>, and tries to resurrect it.  Currently, the reflog is
+searched for checkout messages, and with -r also merge messages.  With
+-m and -t, the history of all refs is scanned for Merge <name> into
+other/Merge <other> into <name> (respectively) commit subjects, which
+is rather slow but allows you to resurrect other people's topic
+branches."
+
+OPTIONS_SPEC="\
+git resurrect $USAGE
+--
+b,branch=            save branch as <newname> instead of <name>
+a,all                same as -l -r -m -t
+k,keep-going         full rev-list scan (instead of first match)
+l,reflog             scan reflog for checkouts (enabled by default)
+r,reflog-merges      scan for merges recorded in reflog
+m,merges             scan for merges into other branches (slow)
+t,merge-targets      scan for merges of other branches into <name>
+n,dry-run            don't recreate the branch"
+
+. git-sh-setup
+
+search_reflog () {
+        sed -ne 's~^\([^ ]*\) .*\tcheckout: moving from '"$1"' .*~\1~p' \
+                < "$GIT_DIR"/logs/HEAD
+}
+
+search_reflog_merges () {
+	git rev-parse $(
+		sed -ne 's~^[^ ]* \([^ ]*\) .*\tmerge '"$1"':.*~\1^2~p' \
+			< "$GIT_DIR"/logs/HEAD
+	)
+}
+
+_x40="[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]"
+_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+
+search_merges () {
+        git rev-list --all --grep="Merge branch '$1'" \
+                --pretty=tformat:"%P %s" |
+        sed -ne "/^$_x40 \($_x40\) Merge .*/ {s//\1/p;$early_exit}"
+}
+
+search_merge_targets () {
+	git rev-list --all --grep="Merge branch '[^']*' into $branch\$" \
+		--pretty=tformat:"%H %s" --all |
+	sed -ne "/^\($_x40\) Merge .*/ {s//\1/p;$early_exit} "
+}
+
+dry_run=
+early_exit=q
+scan_reflog=t
+scan_reflog_merges=
+scan_merges=
+scan_merge_targets=
+new_name=
+
+while test "$#" != 0; do
+	case "$1" in
+	    -b|--branch)
+		shift
+		new_name="$1"
+		;;
+	    -n|--dry-run)
+		dry_run=t
+		;;
+	    --no-dry-run)
+		dry_run=
+		;;
+	    -k|--keep-going)
+		early_exit=
+		;;
+	    --no-keep-going)
+		early_exit=q
+		;;
+	    -m|--merges)
+		scan_merges=t
+		;;
+	    --no-merges)
+		scan_merges=
+		;;
+	    -l|--reflog)
+		scan_reflog=t
+		;;
+	    --no-reflog)
+		scan_reflog=
+		;;
+	    -r|--reflog_merges)
+		scan_reflog_merges=t
+		;;
+	    --no-reflog_merges)
+		scan_reflog_merges=
+		;;
+	    -t|--merge-targets)
+		scan_merge_targets=t
+		;;
+	    --no-merge-targets)
+		scan_merge_targets=
+		;;
+	    -a|--all)
+		scan_reflog=t
+		scan_reflog_merges=t
+		scan_merges=t
+		scan_merge_targets=t
+		;;
+	    --)
+		shift
+		break
+		;;
+	    *)
+		usage
+		;;
+	esac
+	shift
+done
+
+test "$#" = 1 || usage
+
+all_strategies="$scan_reflog$scan_reflog_merges$scan_merges$scan_merge_targets"
+if test -z "$all_strategies"; then
+	die "must enable at least one of -lrmt"
+fi
+
+branch="$1"
+test -z "$new_name" && new_name="$branch"
+
+if test ! -z "$scan_reflog"; then
+	if test -r "$GIT_DIR"/logs/HEAD; then
+		candidates="$(search_reflog $branch)"
+	else
+		die 'reflog scanning requested, but' \
+			'$GIT_DIR/logs/HEAD not readable'
+	fi
+fi
+if test ! -z "$scan_reflog_merges"; then
+	if test -r "$GIT_DIR"/logs/HEAD; then
+		candidates="$candidates $(search_reflog_merges $branch)"
+	else
+		die 'reflog scanning requested, but' \
+			'$GIT_DIR/logs/HEAD not readable'
+	fi
+fi
+if test ! -z "$scan_merges"; then
+	candidates="$candidates $(search_merges $branch)"
+fi
+if test ! -z "$scan_merge_targets"; then
+	candidates="$candidates $(search_merge_targets $branch)"
+fi
+
+candidates="$(git rev-parse $candidates | sort -u)"
+
+if test -z "$candidates"; then
+	hint=
+	test "z$all_strategies" != "ztttt" \
+		&& hint=" (maybe try again with -a)"
+	die "no candidates for $branch found$hint"
+fi
+
+echo "** Candidates for $branch **"
+for cmt in $candidates; do
+	git --no-pager log --pretty=tformat:"%ct:%h [%cr] %s" --abbrev-commit -1 $cmt
+done \
+| sort -n | cut -d: -f2-
+
+newest="$(git rev-list -1 $candidates)"
+if test ! -z "$dry_run"; then
+	printf "** Most recent: "
+	git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+elif ! git rev-parse --verify --quiet $new_name >/dev/null; then
+	printf "** Restoring $new_name to "
+	git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+	git branch $new_name $newest
+else
+	printf "Most recent: "
+	git --no-pager log -1 --pretty=tformat:"%h %s" $newest
+	echo "** $new_name already exists, doing nothing"
+fi
-- 
1.6.1.2.530.gdaa1c

Re: [PATCH v4] contrib git-resurrect: find traces of a branch name and resurrect it

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> Junio C Hamano wrote:
>> Do you mean something like this?
>> 
>> 	sed -n -e "/^$_x40 $_x40 \($_x40\) Merge .*/ {
>> 		s//\1/p
>>                 q
>>         }"
>
> Yep, precisely.  Thanks!  This indeed gives it a nice speed boost if
> all you want is a topic "resurrection" from pu.
> ...
> +search_merges () {
> +        git rev-list --all --grep="Merge branch '$1'" \
> +                --pretty=tformat:"%P %s" |
> +        sed -ne "/^$_x40 \($_x40\) Merge .*/ {s//\1/p;$early_exit}"
> +}

Will apply, but just to let you know, I wrote my example on separate lines
(and with separate -n and -e options for that matter) for a reason.  I
recall some implementation of sed (perhaps older BSDs, but don't quote me
on that) did not understanding semicolon with close brace on the same
line.  It may not be a problem in practice these days, but I do not have
access to many different platforms to check as I used to.