[PATCH RFC 0/4] rebase -i: Add --refs option to rewrite heads within branch

[PATCH RFC 0/4] rebase -i: Add --refs option to rewrite heads within branch

[Greg Price]

This RFC series adds a "ref" command to rebase -i, and an option
--refs to generate "ref" commands in the original TODO file.  This
makes it easy to rebase a branch together with intermediate markers,
or a series of branches.

For example, in this situation:

      part1 part2 topic
        |     |     |
        v     v     v
  A--*--*--*--*--*--*
   \
    B <--master

we may want to rebase 'topic' onto the new master, rewriting the
intermediate branches 'part1', 'part2' to the corresponding new
commits.  This can be done with a sequence of "git rebase --onto"
commands, but it's tricky to get right.

With this series, the command
  $ git rebase -i --refs master topic
suffices to produce this result:

        part1 part2 topic
  A       |     |     |
   \      v     v     v
    B--*--*--*--*--*--*
    ^
    |
    master


These patches work for me.  Before I recommend the last two patches
for merge, they'll need the following additional work:
 - interoperate with rebase --abort
 - add documentation
 - probably add tests


The series begins with two patches that are independently useful, and
that I believe are ready for merge:
  pretty: Add %D for script-friendly decoration
  log --decorate=full: drop the "tag: " prefix
They make the functionality of 'git log --decorate' available in an
output format optimized for machine parsing.  I expect other people
will find this useful for other scripts.

Greg

[PATCH 1/4] pretty: Add %D for script-friendly decoration

[Greg Price]

When in a script or porcelain one wants to identify what refs point to
which commits in a series, the functionality of 'git log --decorate'
is extremely useful.  This is available with the %d format code in a
form optimized for humans, but for scripts a more raw format is better.
Make such a format available through a new format code %D.

Signed-off-by: Greg Price <price@ksplice.com>
---
 Documentation/pretty-formats.txt |    1 +
 pretty.c                         |   33 +++++++++++++++++++++++++--------
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 53a9168..b6b840e 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -119,6 +119,7 @@ The placeholders are:
 - '%ct': committer date, UNIX timestamp
 - '%ci': committer date, ISO 8601 format
 - '%d': ref names, like the --decorate option of linkgit:git-log[1]
+- '%D': full ref names, like the --decorate=full option of linkgit:git-log[1]
 - '%e': encoding
 - '%s': subject
 - '%f': sanitized subject line, suitable for a filename
diff --git a/pretty.c b/pretty.c
index 8f5bd1a..18ce2ff 100644
--- a/pretty.c
+++ b/pretty.c
@@ -582,21 +582,35 @@ static void parse_commit_message(struct format_commit_context *c)
 	c->commit_message_parsed = 1;
 }
 
-static void format_decoration(struct strbuf *sb, const struct commit *commit)
+
+static void format_decoration(struct strbuf *sb, const struct commit *commit,
+			      int decoration_style, const char *affixes[3])
 {
 	struct name_decoration *d;
-	const char *prefix = " (";
+	const char *affix = affixes[0];
 
-	load_ref_decorations(DECORATE_SHORT_REFS);
+	load_ref_decorations(decoration_style);
 	d = lookup_decoration(&name_decoration, &commit->object);
 	while (d) {
-		strbuf_addstr(sb, prefix);
-		prefix = ", ";
+		strbuf_addstr(sb, affix);
+		affix = affixes[1];
 		strbuf_addstr(sb, d->name);
 		d = d->next;
 	}
-	if (prefix[0] == ',')
-		strbuf_addch(sb, ')');
+	if (affix == affixes[1])
+		strbuf_addstr(sb, affixes[2]);
+}
+
+static void format_decoration_short(struct strbuf *sb, const struct commit *commit)
+{
+	const char *affixes[3] = {" (", ", ", ")"};
+	format_decoration(sb, commit, DECORATE_SHORT_REFS, affixes);
+}
+
+static void format_decoration_full(struct strbuf *sb, const struct commit *commit)
+{
+	const char *affixes[3] = {"", " ", ""};
+	format_decoration(sb, commit, DECORATE_FULL_REFS, affixes);
 }
 
 static void strbuf_wrap(struct strbuf *sb, size_t pos,
@@ -756,7 +770,10 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 		                 : '>');
 		return 1;
 	case 'd':
-		format_decoration(sb, commit);
+		format_decoration_short(sb, commit);
+		return 1;
+	case 'D':
+		format_decoration_full(sb, commit);
 		return 1;
 	case 'g':		/* reflog info */
 		switch(placeholder[1]) {
-- 
1.6.6.rc1.9.g2ad41.dirty

[PATCH 2/4] log --decorate=full: drop the "tag: " prefix

[Greg Price]

The "tag: " prefix complicates machine parsing of decorations, so we
drop it from the output formats intended to be parsed by machine,
namely --decorate=full and the %D format code.

The prefix is helpful for a human reader to see that the ref is an
(annotated) tag, especially since we omit the "refs/tags/" prefix in
the default output of "git log --decorate".  In a script, however, it
is easy to use "git cat-file -t" to distinguish annotated tags from
commits when the distinction is relevant.

Signed-off-by: Greg Price <price@ksplice.com>
---
 log-tree.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 0fdf159..5eb6b00 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -22,17 +22,18 @@ static void add_name_decoration(const char *prefix, const char *name, struct obj
 
 static int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
 {
+	int short_refs = !!(!cb_data || *(int *)cb_data == DECORATE_SHORT_REFS);
 	struct object *obj = parse_object(sha1);
 	if (!obj)
 		return 0;
-	if (!cb_data || *(int *)cb_data == DECORATE_SHORT_REFS)
+	if (short_refs)
 		refname = prettify_refname(refname);
 	add_name_decoration("", refname, obj);
 	while (obj->type == OBJ_TAG) {
 		obj = ((struct tag *)obj)->tagged;
 		if (!obj)
 			break;
-		add_name_decoration("tag: ", refname, obj);
+		add_name_decoration(short_refs ? "tag: " : "", refname, obj);
 	}
 	return 0;
 }
-- 
1.6.6.rc1.9.g2ad41.dirty

[PATCH RFC 3/4] rebase -i: Add the "ref" command

[Greg Price]

This is useful for, e.g., rewriting a branch that has ancestor
branches along the way.

Signed-off-by: Greg Price <price@ksplice.com>
---
 git-rebase--interactive.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0bd3bf7..25ac3e3 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -367,6 +367,10 @@ do_next () {
 		warn
 		exit 0
 		;;
+	ref)
+		mark_action_done
+		git update-ref $sha1 HEAD # $sha1 is actually a refname
+		;;
 	squash|s)
 		comment_for_reflog squash
 
-- 
1.6.6.rc1.9.g2ad41.dirty

[PATCH RFC 4/4] rebase -i: add --refs option to rewrite heads within branch

[Greg Price]

The new option --refs causes the TODO file to contain a "ref" command
for each head pointing to a selected commit, other than the one we are
already rebasing.  The effect of this is that when a branch contains
intermediate branches, like so:

      part1 part2 topic
        |     |     |
        v     v     v
  A--*--*--*--*--*--*
   \
    B <--master

a single command like "git rebase -i --refs master topic" suffices to
rewrite all the heads that are part of the topic, like so:

        part1 part2 topic
  A       |     |     |
   \      v     v     v
    B--*--*--*--*--*--*
    ^
    |
    master

Signed-off-by: Greg Price <price@ksplice.com>
---
 git-rebase--interactive.sh |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 25ac3e3..cccb031 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -18,6 +18,7 @@ git-rebase [-i] (--continue | --abort | --skip)
  Available options are
 v,verbose          display a diffstat of what changed upstream
 onto=              rebase onto given branch instead of upstream
+refs               rewrite intermediate heads on branch
 p,preserve-merges  try to recreate merges instead of ignoring them
 s,strategy=        use the given merge strategy
 m,merge            always used (no-op)
@@ -42,6 +43,7 @@ REWRITTEN="$DOTEST"/rewritten
 DROPPED="$DOTEST"/dropped
 PRESERVE_MERGES=
 STRATEGY=
+REWRITE_REFS=
 ONTO=
 VERBOSE=
 OK_TO_SKIP_PRE_REBASE=
@@ -578,6 +580,9 @@ first and then run 'git rebase --continue' again."
 
 		output git reset --hard && do_rest
 		;;
+	--refs)
+		REWRITE_REFS=t
+		;;
 	-s)
 		case "$#,$1" in
 		*,*=*)
@@ -705,11 +710,14 @@ first and then run 'git rebase --continue' again."
 			REVISIONS=$ONTO...$HEAD
 			SHORTREVISIONS=$SHORTHEAD
 		fi
-		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
-			--abbrev=7 --reverse --left-right --topo-order \
+		git rev-list $MERGES_OPTION --abbrev-commit --abbrev=7 \
+			--reverse --left-right --topo-order \
+			--pretty=format:"$(printf '%%m%%h %%s\n%%m%%D')" \
 			$REVISIONS | \
 			sed -n "s/^>//p" | while read shortsha1 rest
 		do
+			read refs
+
 			if test t != "$PRESERVE_MERGES"
 			then
 				echo "pick $shortsha1 $rest" >> "$TODO"
@@ -734,6 +742,16 @@ first and then run 'git rebase --continue' again."
 					echo "pick $shortsha1 $rest" >> "$TODO"
 				fi
 			fi
+
+			if test t = "$REWRITE_REFS"
+			then
+				for ref in $refs
+				do
+					test ${ref#refs/heads/} != $ref &&
+					test $ref != $(cat "$DOTEST"/head-name) &&
+					echo "ref $ref" >> "$TODO"
+				done
+			fi
 		done
 
 		# Watch for commits that been dropped by --cherry-pick
-- 
1.6.6.rc1.9.g2ad41.dirty

Re: [PATCH RFC 4/4] rebase -i: add --refs option to rewrite heads within branch

[Junio C Hamano]

Greg Price <price@ksplice.com> writes:

> The new option --refs causes the TODO file to contain a "ref" command
> for each head pointing to a selected commit, other than the one we are
> already rebasing.  The effect of this is that when a branch contains
> intermediate branches, like so:
>
>       part1 part2 topic
>         |     |     |
>         v     v     v
>   A--*--*--*--*--*--*
>    \
>     B <--master
>
> a single command like "git rebase -i --refs master topic" suffices to
> rewrite all the heads that are part of the topic, like so:
>
>         part1 part2 topic
>   A       |     |     |
>    \      v     v     v
>     B--*--*--*--*--*--*
>     ^
>     |
>     master
>
> Signed-off-by: Greg Price <price@ksplice.com>
> ---

Two comments and a half.

 - As decoration is a fairly expensive operation (which is the reason why
   loading_ref_decorations() is called lazily by format_decoration() in
   the first place, especially in repositories with tons of refs), you
   shouldn't give --format=%D to rev-list when the new feature is not
   asked for.

 - This seems to rewrite only branch heads; don't you want to allow users
   to rewrite lightweight tags and possibly annotated ones as well, by
   perhaps giving "--rewrite-refs=refs/heads/" or "--rewrite-refs=refs/"
   to limit what parts of the ref namespace to consider rewriting?

 - Otherwise the option should not be called "refs" but be named using the
   word "branch" to clarify that it affects _only_ branches.

Obviously the series also needs tests.

I also have to wonder if this feature should also handle a case like this:

                  side
                  |
                  V
                  *
                 /
        part1   *    topic
          |    /      |
          v   /       v
    A--*--*--*--*--*--*
     \
      B <--master

===>

                     side
                     |
                     V
                     *
                    /
           part1   *    topic
     A       |    /      |
      \      v   /       v
       B--*--*--*--*--*--*
       ^
       |
       master

especially if it were to be specific to branch management.

On the other hand, if the "partN" markers in your example workflow are
primarily meant to be used to mark places on a branch (as opposed to
arbitrary branch tips that independent development starting from them can
further continue), it would make a lot more sense to use lightweight or
annotated tags for them, and instead of "--refs" that rewrites only other
branch tips, it might make a lot more sense to have "--rewrite-tags" that
rewrites tags that point at the commits that are rewritten, without
touching any branch tip.

Re: [PATCH RFC 4/4] rebase -i: add --refs option to rewrite heads within branch

[Greg Price]

On Tue, Dec 22, 2009 at 6:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
>  - As decoration is a fairly expensive operation (which is the reason why
>   loading_ref_decorations() is called lazily by format_decoration() in
>   the first place, especially in repositories with tons of refs), you
>   shouldn't give --format=%D to rev-list when the new feature is not
>   asked for.

OK, will do.


>  - This seems to rewrite only branch heads; don't you want to allow users
>   to rewrite lightweight tags and possibly annotated ones as well, by
>   perhaps giving "--rewrite-refs=refs/heads/" or "--rewrite-refs=refs/"
>   to limit what parts of the ref namespace to consider rewriting?

Sure.  I specifically left out tags because I generally think of a tag
as something immutable that it would not make sense to rewrite.  But
people use Git in different ways and it makes sense to give the option
of rewriting tags as well as heads.

I do worry that passing --rewrite-refs=refs/ will set up remote refs
for rewriting, which is likely to be confusing if the user does not
notice them and remove them from the TODO.  Perhaps it makes sense to
accept forms like "--rewrite-refs=refs/heads/,refs/tags/" or
"--rewrite-refs=refs/heads/ --rewrite-refs=refs/tags/".  Is there a
Git convention for accepting a sequence of arguments like this to an
option -- one of these, or something else?


> On the other hand, if the "partN" markers in your example workflow are
> primarily meant to be used to mark places on a branch (as opposed to
> arbitrary branch tips that independent development starting from them can
> further continue), it would make a lot more sense to use lightweight or
> annotated tags for them, and instead of "--refs" that rewrites only other
> branch tips, it might make a lot more sense to have "--rewrite-tags" that
> rewrites tags that point at the commits that are rewritten, without
> touching any branch tip.

I think of them as a topic branch developing one feature, then another
branch developing a related follow-on feature, etc.  I would also feel
odd rewriting tags as a routine operation, or calling a ref a tag when
I expect to rewrite it.  So I do think they're best recorded as branch
tips rather than tags.


> Obviously the series also needs tests.

Yes.


> I also have to wonder if this feature should also handle a case like this:
>
>                  side
>                  |
>                  V
>                  *
>                 /
>        part1   *    topic
>          |    /      |
>          v   /       v
>    A--*--*--*--*--*--*
>     \
>      B <--master
>
> ===>
>
>                     side
>                     |
>                     V
>                     *
>                    /
>           part1   *    topic
>     A       |    /      |
>      \      v   /       v
>       B--*--*--*--*--*--*
>       ^ [
>       |
>       master
>
> especially if it were to be specific to branch management.

Huh, that's an interesting idea.  I hadn't thought of that.  This
feature could be nice.  But I am not sure what it would look like.
How might the user indicate that they want both "side" and "topic" to
be rebased?  I suppose we could extend the familiar command line
   git rebase <upstream> [<branch>]
to the form
   git rebase <upstream> [...<branches>...]
so that your example would be
   $ git rebase -i --rewrite-heads master topic side
If we choose this approach, it might even be independent of
--rewrite-refs, though the implementation would presumably rely on the
"ref" command.  Was this interface what you were thinking, or do you
have another idea?

Greg

Re: [PATCH RFC 4/4] rebase -i: add --refs option to rewrite heads within branch

[Michael J Gruber]

Greg Price venit, vidit, dixit 23.12.2009 08:03:
> On Tue, Dec 22, 2009 at 6:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>  - As decoration is a fairly expensive operation (which is the reason why
>>   loading_ref_decorations() is called lazily by format_decoration() in
>>   the first place, especially in repositories with tons of refs), you
>>   shouldn't give --format=%D to rev-list when the new feature is not
>>   asked for.
> 
> OK, will do.
> 
> 
>>  - This seems to rewrite only branch heads; don't you want to allow users
>>   to rewrite lightweight tags and possibly annotated ones as well, by
>>   perhaps giving "--rewrite-refs=refs/heads/" or "--rewrite-refs=refs/"
>>   to limit what parts of the ref namespace to consider rewriting?
> 
> Sure.  I specifically left out tags because I generally think of a tag
> as something immutable that it would not make sense to rewrite.  But
> people use Git in different ways and it makes sense to give the option
> of rewriting tags as well as heads.
> 
> I do worry that passing --rewrite-refs=refs/ will set up remote refs
> for rewriting, which is likely to be confusing if the user does not
> notice them and remove them from the TODO.  Perhaps it makes sense to
> accept forms like "--rewrite-refs=refs/heads/,refs/tags/" or
> "--rewrite-refs=refs/heads/ --rewrite-refs=refs/tags/".  Is there a
> Git convention for accepting a sequence of arguments like this to an
> option -- one of these, or something else?
> 
> 
>> On the other hand, if the "partN" markers in your example workflow are
>> primarily meant to be used to mark places on a branch (as opposed to
>> arbitrary branch tips that independent development starting from them can
>> further continue), it would make a lot more sense to use lightweight or
>> annotated tags for them, and instead of "--refs" that rewrites only other
>> branch tips, it might make a lot more sense to have "--rewrite-tags" that
>> rewrites tags that point at the commits that are rewritten, without
>> touching any branch tip.
> 
> I think of them as a topic branch developing one feature, then another
> branch developing a related follow-on feature, etc.  I would also feel
> odd rewriting tags as a routine operation, or calling a ref a tag when
> I expect to rewrite it.  So I do think they're best recorded as branch
> tips rather than tags.
> 
> 
>> Obviously the series also needs tests.
> 
> Yes.
> 
> 
>> I also have to wonder if this feature should also handle a case like this:
>>
>>                  side
>>                  |
>>                  V
>>                  *
>>                 /
>>        part1   *    topic
>>          |    /      |
>>          v   /       v
>>    A--*--*--*--*--*--*
>>     \
>>      B <--master
>>
>> ===>
>>
>>                     side
>>                     |
>>                     V
>>                     *
>>                    /
>>           part1   *    topic
>>     A       |    /      |
>>      \      v   /       v
>>       B--*--*--*--*--*--*
>>       ^ [
>>       |
>>       master
>>
>> especially if it were to be specific to branch management.
> 
> Huh, that's an interesting idea.  I hadn't thought of that.  This
> feature could be nice.  But I am not sure what it would look like.
> How might the user indicate that they want both "side" and "topic" to
> be rebased?  I suppose we could extend the familiar command line
>    git rebase <upstream> [<branch>]
> to the form
>    git rebase <upstream> [...<branches>...]
> so that your example would be
>    $ git rebase -i --rewrite-heads master topic side
> If we choose this approach, it might even be independent of
> --rewrite-refs, though the implementation would presumably rely on the
> "ref" command.  Was this interface what you were thinking, or do you
> have another idea?
> 
> Greg

If I may jump in: I imagine this to be the more common use case, i.e.:
You have a part of the DAG which you want to rebase, with all kinds of
refs (branches, tags) pointing to commits in that part of the DAG. If
you rebase that part of the DAG you typically want some refs rewritten
(such as the head of the branch you're rebasing) but maybe not others
(say a release tag or branch). remote refs should never be rebased. So,
one would need an easy way to specify one ref, all or anything in between...

Michael

Re: [PATCH RFC 4/4] rebase -i: add --refs option to rewrite heads within branch

[Greg Price]

On Wed, Dec 23, 2009 at 3:53 AM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> If I may jump in: I imagine this to be the more common use case, i.e.:
> You have a part of the DAG which you want to rebase, with all kinds of
> refs (branches, tags) pointing to commits in that part of the DAG. If
> you rebase that part of the DAG you typically want some refs rewritten
> (such as the head of the branch you're rebasing) but maybe not others
> (say a release tag or branch). remote refs should never be rebased. So,
> one would need an easy way to specify one ref, all or anything in between...

I see the interactive aspect as the backstop that gives the user total
flexibility, whatever command-line interface we choose.  So, for
example, a user might find it easiest to pass --rewrite-heads so that
all heads on the given part of the DAG are inserted as "ref" commands
in the appropriate places, and then go through and remove from the
TODO file any exceptions.

But I agree it is worth making convenient shortcuts by which the user
can specify their intention precisely on the command line itself.  And
in any case, if we are to satisfy this latter use case we need a way
of specifying a part of the DAG to rebase that is not only ancestors
of a single commit.  What do you think of the
"--rewrite-refs=refs/heads/,refs/tags/" and "git rebase -i master
topic side" proposals in my reply to Junio?

Greg

Re: [PATCH RFC 4/4] rebase -i: add --refs option to rewrite heads within branch

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Greg Price venit, vidit, dixit 23.12.2009 08:03:
> ...
>> Huh, that's an interesting idea.  I hadn't thought of that.  This
>> feature could be nice.  But I am not sure what it would look like.
>> How might the user indicate that they want both "side" and "topic" to
>> be rebased?  I suppose we could extend the familiar command line
>>    git rebase <upstream> [<branch>]
>> to the form
>>    git rebase <upstream> [...<branches>...]
>> so that your example would be
>>    $ git rebase -i --rewrite-heads master topic side
>> If we choose this approach, it might even be independent of
>> --rewrite-refs, though the implementation would presumably rely on the
>> "ref" command.  Was this interface what you were thinking, or do you
>> have another idea?
>
> If I may jump in: I imagine this to be the more common use case, i.e.:
> You have a part of the DAG which you want to rebase, with all kinds of
> refs (branches, tags) pointing to commits in that part of the DAG. If
> you rebase that part of the DAG you typically want some refs rewritten
> (such as the head of the branch you're rebasing) but maybe not others
> (say a release tag or branch). remote refs should never be rebased. So,
> one would need an easy way to specify one ref, all or anything in between...

And warn (or perhaps reject without --force) if the sub-DAG you would
rebase contains an already tagged commit, especially if you are treating
tags as immutable, even the ones you haven't published.

I actually don't think "rebasing side branches while rebasing this branch"
was such a good idea---we have filter-branch for that already ;-).

Re: [PATCH RFC 4/4] rebase -i: add --refs option to rewrite heads within branch

[Johannes Schindelin]

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

Hi,

On Wed, 23 Dec 2009, Greg Price wrote:

> On Tue, Dec 22, 2009 at 6:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> >  - This seems to rewrite only branch heads; don't you want to allow 
> > users  to rewrite lightweight tags and possibly annotated ones as 
> > well, by  perhaps giving "--rewrite-refs=refs/heads/" or 
> > "--rewrite-refs=refs/"  to limit what parts of the ref namespace to 
> > consider rewriting?
> 
> Sure.  I specifically left out tags because I generally think of a tag 
> as something immutable that it would not make sense to rewrite.

I do agree: if you plan to rewrite a ref, you _should_ make it a branch 
anyway.

A tag is not meant to be updated easily, in fact, we explicitely lack 
tools to do so (and it is quite hard to get updated tags from a repository 
where the tags changed anyway).  So rewriting tags is something that 
causes only trouble.

Why should rebase -i cause that trouble all of a sudden?

Ciao,
Dscho