patch, 6 partsrebase -i: Add the "ref" command

This series adds a "ref" command to rebase -i, and options --rewrite-{refs,heads,tags} 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 --rewrite-heads master topic
suffices to produce this result:

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

I originally submitted a version of this series nearly a year and a half ago [1]. This version has been rebased to the new release 1.7.6 (Martin von Zweigbergk's cleanups gave me quite some conflicts to resolve, but are nevertheless much appreciated!) and the interface has been adjusted for the comments of Junio and others [2]. The general command is --rewrite-refs=PATTERN; --rewrite-heads and --rewrite-tags are provided as shortcuts, the former being equivalent to the --refs of my original proposal.

 [1]  http://www.amailbox.net/mailarchive/git/2009/12/22/18967
 [2]  http://www.amailbox.net/mailarchive/git/2009/12/22/18972

I have no excuse for the year and a half of delay in submitting the revised version, but I believe that late is better than never. =)

The series contains two preparatory patches that are independently useful:
  pretty: Add %D for script-friendly decoration
  for-each-ref: --stdin to match specified refs against pattern
Each one exposes a bit more knowledge of the repository in a
machine-readable form convenient for scripting.  I hope other people
may find them useful in writing other scripts.

Greg

---
Greg Price (6):
  rebase -i: Add the "ref" command
  pretty: Add %D for script-friendly decoration
  for-each-ref: --stdin to match specified refs against pattern
  rebase: --rewrite-{refs,heads,tags} to pull refs along with branch
  t/lib-rebase.sh: pass through ref commands
  rebase --rewrite-refs: tests

 Documentation/git-rebase.txt     |   15 ++++++++
 Documentation/pretty-formats.txt |    1 +
 builtin/for-each-ref.c           |   22 ++++++++++-
 git-rebase--interactive.sh       |   37 ++++++++++++++++++-
 git-rebase.sh                    |   28 ++++++++++++++
 pretty.c                         |   33 +++++++++++++----
 t/lib-rebase.sh                  |    2 +-
 t/t3420-rebase-ref.sh            |   75 ++++++++++++++++++++++++++++++++++++++
 8 files changed, 201 insertions(+), 12 deletions(-)
 create mode 100644 t/t3420-rebase-ref.sh

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

Signed-off-by: Greg Price <price@mit.edu>
---
 Documentation/git-rebase.txt |    4 ++++
 git-rebase--interactive.sh   |   12 ++++++++++++
 git-rebase.sh                |   11 +++++++++++
 3 files changed, 27 insertions(+), 0 deletions(-)

Greg Price <price@MIT.EDU> writes:

(Style) Extra SP between ">>" and "$state_dir/oldrefs"

(Style) I think almost everybody else spells out "test". Also please break line before the while, like this:

	test -n "$oldrefs" &&
	echo "$oldrefs" |
	while read sha1 ref
        do
        	...

Who is giving you "(null)"???

Thanks for the review.

On Mon, Jun 27, 2011 at 11:46:52AM -0700, Junio C Hamano wrote:

Hmm -- it looks like the prevalent style in the codebase is actually to include the space:

greg@gouda:~/w/git$ git grep -c '>>[^ ]' v1.7.6 git-*.sh v1.7.6:git-bisect.sh:3 v1.7.6:git-instaweb.sh:2 v1.7.6:git-rebase--interactive.sh:2 v1.7.6:git-stash.sh:1 greg@gouda:~/w/git$ git grep -c '>> ' v1.7.6 git-*.sh v1.7.6:git-am.sh:1 v1.7.6:git-filter-branch.sh:1 v1.7.6:git-instaweb.sh:8 v1.7.6:git-rebase--interactive.sh:10 v1.7.6:git-rebase--merge.sh:1

and in particular in git-rebase--interactive.sh. But I could do it either way.

Sure, done.

I am, myself -- it's what the 'ref' implementation in git-rebase--interactive.sh uses to indicate that a ref had not existed and should be deleted on abort.

+       ref)
+               mark_action_done
+               refname=$sha1
+               sha1=$(git rev-parse --quiet --verify "$refname" \
+                       || echo "(null)")
+               if ! grep -Fq " $refname" "$state_dir"/oldrefs 2>/dev/null
+               then
+                       echo "$sha1 $refname" >> "$state_dir"/oldrefs
+               fi

I could change it to something like "-". It needs to be something that the 'read' builtin, as used at the top of the loop, treats as a word.

Greg

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@mit.edu>
---
 Documentation/pretty-formats.txt |    1 +
 pretty.c                         |   33 +++++++++++++++++++++++++--------
 2 files changed, 26 insertions(+), 8 deletions(-)

Greg Price <price@MIT.EDU> writes:

Nice and cute abstraction. I like this part of the series.

The new option --rewrite-refs causes the TODO file to contain a "ref" command for each appropriate ref pointing to a selected commit, other than the one we are already rebasing. The argument to --rewrite-refs is a ref pattern of the same type accepted by for-each-ref: a pattern matches a ref name if it matches exactly, matches exactly up to a slash, or matches according to fnmatch(3). The options --rewrite-heads and --rewrite-tags are supplied as shortcuts.

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 --rewrite-heads 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@mit.edu>
---
 Documentation/git-rebase.txt |   13 ++++++++++++-
 git-rebase--interactive.sh   |   25 ++++++++++++++++++++++++-
 git-rebase.sh                |   17 +++++++++++++++++
 3 files changed, 53 insertions(+), 2 deletions(-)

On 11/18/2009 06:51 PM, Greg Price wrote:

I think you meant to spell that "rewrite_refs=" instead of "rebase_refs=".

Then it matches the rest of the commit.

Phil

On Mon, Jun 27, 2011 at 12:11:39PM -0400, Phil Hord wrote:

Ha, so I did.  Fixed.  Thanks!

Greg

The pattern syntax of for-each-ref is handy for users, but a bit subtle: a pattern matches a ref if it matches as an fnmatch pattern, or literally, or literally against a prefix of the refname up to a slash. Expose the for-each-ref pattern-matcher for scripts to use, so that they can provide a consistent user interface without duplicating the implementation.

Signed-off-by: Greg Price <price@mit.edu>
---
 builtin/for-each-ref.c |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

Signed-off-by: Greg Price <price@mit.edu>
---
 t/lib-rebase.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Signed-off-by: Greg Price <price@mit.edu>
---
 t/t3420-rebase-ref.sh |   75 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 75 insertions(+), 0 deletions(-)
 create mode 100644 t/t3420-rebase-ref.sh

On 01/24/2010 09:28 PM, Greg Price wrote:

I think there is a missing "&&" at the end of "test_commit D". But it's possible I just don't understand the test machinery enough to know this is normal. Can you explain to me the difference in that case?

Phil

On Mon, Jun 27, 2011 at 12:07:10PM -0400, Phil Hord wrote:

Nope, that's just a mistake -- it means that if "test_commit D" were to somehow fail, we'd move on as if nothing was wrong. Fixed, thanks.

Greg

Greg Price <price@MIT.EDU> writes:

Forgot the executable bit?

On Mon, Jun 27, 2011 at 02:02:36PM -0700, Junio C Hamano wrote:

Yup, thanks, fixed.

Greg

Greg Price <greg@quora.com> writes:

Thanks for a re-roll.

I notice that this does not address the "side branch" issue raised during the original discussion. I do agree with Michael

  http://thread.gmane.org/gmane.comp.version-control.git/135601/focus=135617

that having some commits on these part$N side branches is far more common use case that would benefit from a "rewrite together" feature like this, than moving part$N side branches that just mark points in the topic without doing anything on their own and makes me doubt if doing only the parts that can sanely done within the limitation of the current rebase-i implementation like this series does adds much value to the system [*1*].

It would be nice to have a clear definition of what _should_ happen in this case, and a test that makes sure that that is the behaviour we get.

Starting from this topology

          1   2   topic
  A---X---Y---Z---W
   \
    B

where the change going from A to B is an equilvalent to the change going from Y to Z, a rebase of A..W would reproduce this topology:

          1   2   topic
  A---X---Y---Z---W
   \
    B---X'--Y'--W'
	    1'  topic'

What should heppen to ref2? Should it be deleted? Should it point at Y'?

[Footnote]

*1* I suspect that dealing with side branches would require a much richer implementation of the sequencer machinery that lets you go back to a previous state, which we do not have right now.

While I think that it makes your series much less interesting than the series could be that not being able to rewrite side branches, I do not think it is reasonable to expect it be done within the current rebase-i implementation/limitation.

With a richer sequencer, when you want to rebuild 'topic' along with 'side' in this picture:

            D side
           /
  A---B---C---E---F topic
   \
    X

on top of X, I would imagine that your rebase-i insn sheet would say something like this:

    detach at X
    replay B
    replay C
    replay E
    replay F
    update ref "topic" with HEAD
    detach at the rewritten C
    replay D
    update ref "side" with HEAD

On Mon, Jun 27, 2011 at 11:36 AM, Junio C Hamano <gitster@pobox.com> wrote:

I would also like to support that use case. In my personal experience, the case where the part$N are ancestors of the topic has actually been the more common case; typically it's that part1 is some topic, and then part2 is a further topic that depends on the changes in part1, so naturally it goes directly on top of it. So I'd be pleased to get the functionality of the present series in, even before supporting the more general case.

I agree with your footnote -- the more general case will require a more powerful sequencer to support properly. And now I see that Ramkumar Ramachandra is making progress on such a thing right now! That's great news -- this is a project that has been attempted at least four times, by five people (including me), in the last three years. I hope to see this round make it in -- I was actually thinking about returning to the problem after seeing this series through, but I would be glad to see Ram beat me to it.

When the more powerful sequencer comes along, it may be necessary to rewrite some of this code in C (IIUC how the new sequencer will work), but that shouldn't be hard, and is probably easier starting from a shell version than from scratch. Other than that, this code should go in smoothly as the main ingredient, other than the sequencer itself, needed for the general "rewrite side branches" feature. With that and because this series is independently useful, my suggestion would be to merge it when it's ready in itself, without waiting for the sequencer.

I would have it point at Y'. That's what would happen if we just said "git rebase B ref2" (or threw in --rewrite-heads to pull along ref1), and I think it makes the most sense to be consistent.

This consistency also means we can think of the reset that rebase performs when it finishes as equivalent to an implicit "ref refs/heads/topic" command at the end of the TODO file, which I wouldn't point out in explaining to a beginner but I think is a nice property for "ref" to have.

I'll add a test for this scenario.

Greg

On Tue, Jun 28, 2011 at 06:47:58AM -0400, Greg Price wrote:

[Hello, Ram!]

Hmm, on further reading I'm not sure the sequencer Ram aims to build this summer actually extends to the "mark" and "reset" commands (or the concepts of "the rewritten <commit>" and "detach" in the pseudo-TODO notation of your footnote) that would be required to implement this broader rewrite-side-branches feature. It looks like the focus may be on taking the existing features of rebase and bringing them into C.

This sounds great too -- it should make rebase a lot faster -- but it will leave open the pet project I've thought about returning to, of making "rebase -i -p" work correctly even when the user wants to rearrange the commits rather than just s/pick/edit/ etc. That will require implementing a richer sequencer very like the one required for a general rewrite-side-branches feature.

Ram, have I correctly read your plans?  For context, see Junio's
remarks upthread about one thing a richer sequencer could be used for:
  http://thread.gmane.org/gmane.comp.version-control.git/176339/focus=176359

Greg

Hi Greg!

Greg Price writes:

You're more than welcome to join in -- all the work is public :) The latest iteration of the sequencer is in 'pu', and I'm currently busy rolling out a nicer version.

Yes, you're absolutely right. Bringing the existing features to C is quite a challenging task in itself, and is bound to keep me busy for the rest of the summer. I don't plan to implement the features your series introduces yet, but it's certainly a great long-term goal -- I think the current series has all the right ingredients; it just requires a lot of work before it can start providing more features.

Right. Since all this will take time, it'll probably be a good idea to get your series merged in now.

-- Ram

Heya,

On Tue, Jun 28, 2011 at 12:47, Greg Price <price@mit.edu> wrote:

FWIW, I'm having the exact same need while working on the remote-hg series (which depends on my remote-helper follow-up series, which depends on fast-export-fixes, which depends on my original remote-helper series, which depends on peffs initial patches; luckily the latter two have been merged to next :P). I'd really like having this available, especially the rebase -i support.

On Wed, Aug 03, 2011 at 03:32:41PM +0200, Sverre Rabbelier wrote:

Thanks for the encouragement and the poke. Work is busy, but I've put together a new version of the series addressing everyone's comments from last time. I'll give it the finishing touches and try to send it out soon.

Greg

Have you ran all tests with the changes in this series, especially the ones in t34xx series?

On Mon, Jun 27, 2011 at 2:23 PM, Junio C Hamano <gitster@pobox.com> wrote:

Err, now that would be smart, now wouldn't it. =)

In fact, I had a stupid bug that this caught. Fixed, will include in next reroll (much sooner than this reroll was =P). All tests now pass.

Greg