All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC/PATCH] rebase -i: add run command to launch a shell command
@ 2010-07-28 13:29 Matthieu Moy
  2010-07-28 14:12 ` Santi Béjar
                   ` (2 more replies)
  0 siblings, 3 replies; 33+ messages in thread
From: Matthieu Moy @ 2010-07-28 13:29 UTC (permalink / raw)
  To: git; +Cc: Matthieu Moy

The typical usage pattern would be to run a test (or simply a compilation
command) at given points in history.

The shell command is ran, and the rebase is stopped when the command
fails, to give the user an opportunity to fix the problem before
continuing with "git rebase --continue".

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---

The name of the command may be subject to discussions. I've chosen
"run", but maybe "shell" would be OK too. In both cases, it doesn't
allow the one-letter version since both "r" and "s" are already used.

 Documentation/git-rebase.txt  |   18 ++++++++++++++++++
 git-rebase--interactive.sh    |   15 +++++++++++++++
 t/lib-rebase.sh               |    2 ++
 t/t3404-rebase-interactive.sh |   31 +++++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index be23ad2..72e3152 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -459,6 +459,24 @@ sure that the current HEAD is "B", and call
 $ git rebase -i -p --onto Q O
 -----------------------------
 
+Reordering and editing commits usually creates untested intermediate
+steps.  You may want to check that your history editting did not break
+anything by running a test, or at least recompiling at intermediate
+points in history by using the "run" command.  You may do so by
+creating a todo list like this one:
+
+-------------------------------------------
+pick deadbee The oneline of this commit
+run make
+pick c0ffeee The oneline of the next commit
+run make test
+...
+-------------------------------------------
+
+The interactive rebase will stop when a command fails (i.e. exists
+with non-0 status) to give you an opportunity to fix the problem.  You
+can continue with `git rebase --continue`.
+
 
 SPLITTING COMMITS
 -----------------
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b94c2a0..ab8bae0 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -537,6 +537,20 @@ do_next () {
 		esac
 		record_in_rewritten $sha1
 		;;
+	run)
+		read -r command rest < "$TODO"
+		mark_action_done
+		printf 'Running command: %s\n' "$rest"
+		if ! eval "$rest"
+		then
+			warn "Command $rest failed"
+			warn "You can fix the problem, and then run"
+			warn
+			warn "	git rebase --continue"
+			warn
+			exit 0
+		fi
+		;;
 	*)
 		warn "Unknown command: $command $sha1 $rest"
 		if git rev-parse --verify -q "$sha1" >/dev/null
@@ -957,6 +971,7 @@ first and then run 'git rebase --continue' again."
 #  e, edit = use commit, but stop for amending
 #  s, squash = use commit, but meld into previous commit
 #  f, fixup = like "squash", but discard this commit's log message
+#  run <command> = Run a shell command, and stop if it fails
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6aefe27..81dd17b 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -47,6 +47,8 @@ for line in $FAKE_LINES; do
 	case $line in
 	squash|fixup|edit|reword)
 		action="$line";;
+	run*)
+		echo "$line" | sed 's/_/ /g' >> "$1";;
 	"#")
 		echo '# comment' >> "$1";;
 	">")
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 9f03ce6..274a767 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -64,6 +64,37 @@ test_expect_success 'setup' '
 	done
 '
 
+test_expect_success 'rebase -i with the run command' '
+	git checkout master
+	FAKE_LINES="1 run_touch_touch-one 2 run_touch_touch-two run_false run_touch_touch-three 3 4
+		run_touch_\"touch-file__name_with_spaces\" 5" \
+		git rebase -i A &&
+	if ! [ -f touch-one ]; then
+		echo file touch-one not created
+		exit 1
+	fi &&
+	if ! [ -f touch-two ]; then
+		echo file touch-two not created
+		exit 1
+	fi &&
+	if [ -f touch-three ]; then
+		echo "file touch-three already created. Should have stopped before"
+		exit 1
+	fi &&
+	test $(git rev-parse C) = $(git rev-parse HEAD) &&
+	git rebase --continue &&
+	if ! [ -f touch-three ]; then
+		echo "file touch-three not created"
+		exit 1
+	fi &&
+	if ! [ -f "touch-file  name with spaces" ]; then
+		echo "file \"touch-file  name with spaces\" not created"
+		exit 1
+	fi &&
+	test $(git rev-parse master) = $(git rev-parse HEAD) &&
+	rm -f touch-*
+'
+
 test_expect_success 'no changes are a nop' '
 	git checkout branch2 &&
 	git rebase -i F &&
-- 
1.7.2.21.ge9796

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-28 13:29 [RFC/PATCH] rebase -i: add run command to launch a shell command Matthieu Moy
@ 2010-07-28 14:12 ` Santi Béjar
  2010-07-28 14:26   ` Matthieu Moy
  2010-07-30 14:51 ` Marc Branchaud
  2010-07-31 14:40 ` Jared Hance
  2 siblings, 1 reply; 33+ messages in thread
From: Santi Béjar @ 2010-07-28 14:12 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git

On Wed, Jul 28, 2010 at 3:29 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> The typical usage pattern would be to run a test (or simply a compilation
> command) at given points in history.
>
> The shell command is ran, and the rebase is stopped when the command
> fails, to give the user an opportunity to fix the problem before
> continuing with "git rebase --continue".
>

I think this is a useful addition, but I would find it more useful if
I could run a command (make test) on top of all commits of a patch
series, like:

$ git run HEAD^4.. command arguments

(I'm not quite sure about the syntax). Something like "git bisect run"
but for all the commits in the range.

 I know you said "given points in history", maybe each approach is
useful for each use case.

Thanks,
Santi

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-28 14:12 ` Santi Béjar
@ 2010-07-28 14:26   ` Matthieu Moy
  2010-07-30 10:04     ` Matthieu Moy
  0 siblings, 1 reply; 33+ messages in thread
From: Matthieu Moy @ 2010-07-28 14:26 UTC (permalink / raw)
  To: Santi Béjar; +Cc: git

Santi Béjar <santi@agolina.net> writes:

> $ git run HEAD^4.. command arguments
>
> (I'm not quite sure about the syntax). Something like "git bisect run"
> but for all the commits in the range.
>
>  I know you said "given points in history", maybe each approach is
> useful for each use case.

Yes, I think both approaches make sense.

The cool thing with my version is that you're already in an
interactive rebase, which means:

* You can re-order commits, rebase them on upstream branch, and check
  the result in one pass.

* You're ready to ammend commits if they need fixing.

Also, you may not need to re-check everything for each commit. You may
want a todo-list like this

pick deadbee log.c: do something
run make
pick c0ffeee Documentation for something
run make doc

or whatever. Note also that this "git run" can easily be implemented
on top of my patch:

GIT_EDITOR="sed -i 's/^[^#].*/\0\nrun make/'" git rebase -i

we can also imagine a "git rebase -i --run=cmd" that would prepare a
todo-list with "run cmd" after each pick line.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-28 14:26   ` Matthieu Moy
@ 2010-07-30 10:04     ` Matthieu Moy
  0 siblings, 0 replies; 33+ messages in thread
From: Matthieu Moy @ 2010-07-30 10:04 UTC (permalink / raw)
  To: Santi Béjar; +Cc: git

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Santi Béjar <santi@agolina.net> writes:
>
>> $ git run HEAD^4.. command arguments
>>
>> (I'm not quite sure about the syntax). Something like "git bisect run"
>> but for all the commits in the range.
>>
>>  I know you said "given points in history", maybe each approach is
>> useful for each use case.
>
> Yes, I think both approaches make sense.

I started playing with the patch, and I'm already starting to love
it ;-). For example

pick <commit1>
fixup <commit4>
run make
pick <commit2>
pick <commit3>
...

Just does "the right thing": it checks that the fixup doesn't break
the commit (which is really not only a new state, but also a new
patch, so it can really break), but doesn't spend too much time
re-checking <commit2> and <commit3> (which are new states, but much
more trustworthy than the fixup since the patches are really the
same. I may check them later, but don't want to lose time with them
while hacking).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-28 13:29 [RFC/PATCH] rebase -i: add run command to launch a shell command Matthieu Moy
  2010-07-28 14:12 ` Santi Béjar
@ 2010-07-30 14:51 ` Marc Branchaud
  2010-07-30 15:24   ` Matthieu Moy
  2010-07-31 14:40 ` Jared Hance
  2 siblings, 1 reply; 33+ messages in thread
From: Marc Branchaud @ 2010-07-30 14:51 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git

On 10-07-28 09:29 AM, Matthieu Moy wrote:
> The typical usage pattern would be to run a test (or simply a compilation
> command) at given points in history.
> 
> The shell command is ran, and the rebase is stopped when the command
> fails, to give the user an opportunity to fix the problem before
> continuing with "git rebase --continue".
> 
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> 
> The name of the command may be subject to discussions. I've chosen
> "run", but maybe "shell" would be OK too. In both cases, it doesn't
> allow the one-letter version since both "r" and "s" are already used.

"exec" with one-letter "x"?

		M.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-30 14:51 ` Marc Branchaud
@ 2010-07-30 15:24   ` Matthieu Moy
  2010-07-30 18:26     ` Neal Kreitzinger
                       ` (2 more replies)
  0 siblings, 3 replies; 33+ messages in thread
From: Matthieu Moy @ 2010-07-30 15:24 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: git

Marc Branchaud <marcnarc@xiplink.com> writes:

>> The name of the command may be subject to discussions. I've chosen
>> "run", but maybe "shell" would be OK too. In both cases, it doesn't
>> allow the one-letter version since both "r" and "s" are already used.
>
> "exec" with one-letter "x"?

Thanks, that sounds good, yes. Any other thought?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-30 15:24   ` Matthieu Moy
@ 2010-07-30 18:26     ` Neal Kreitzinger
  2010-07-31 13:27       ` Matthieu Moy
  2010-07-31 13:56     ` Ævar Arnfjörð Bjarmason
  2010-07-31 15:28     ` Paolo Bonzini
  2 siblings, 1 reply; 33+ messages in thread
From: Neal Kreitzinger @ 2010-07-30 18:26 UTC (permalink / raw)
  To: git


"Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr> wrote in message 
news:vpqd3u53sd2.fsf@bauges.imag.fr...
> Marc Branchaud <marcnarc@xiplink.com> writes:
>
>>> The name of the command may be subject to discussions. I've chosen
>>> "run", but maybe "shell" would be OK too. In both cases, it doesn't
>>> allow the one-letter version since both "r" and "s" are already used.
>>
>> "exec" with one-letter "x"?
>
> Thanks, that sounds good, yes. Any other thought?

"call" with one-letter "c"?

v/r,
Neal 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-30 18:26     ` Neal Kreitzinger
@ 2010-07-31 13:27       ` Matthieu Moy
  0 siblings, 0 replies; 33+ messages in thread
From: Matthieu Moy @ 2010-07-31 13:27 UTC (permalink / raw)
  To: Neal Kreitzinger; +Cc: git

"Neal Kreitzinger" <neal@rsss.com> writes:

> "Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr> wrote in message 
> news:vpqd3u53sd2.fsf@bauges.imag.fr...
>> Marc Branchaud <marcnarc@xiplink.com> writes:
>>
>>>> The name of the command may be subject to discussions. I've chosen
>>>> "run", but maybe "shell" would be OK too. In both cases, it doesn't
>>>> allow the one-letter version since both "r" and "s" are already used.
>>>
>>> "exec" with one-letter "x"?
>>
>> Thanks, that sounds good, yes. Any other thought?
>
> "call" with one-letter "c"?

"call" would suggest a function or command, but here, you can give
something more elaborate like

  exec cd "some directory"; make

so I prefer the "exec/x" version.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-30 15:24   ` Matthieu Moy
  2010-07-30 18:26     ` Neal Kreitzinger
@ 2010-07-31 13:56     ` Ævar Arnfjörð Bjarmason
  2010-07-31 14:25       ` Miles Bader
                         ` (2 more replies)
  2010-07-31 15:28     ` Paolo Bonzini
  2 siblings, 3 replies; 33+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-07-31 13:56 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Marc Branchaud, git

On Fri, Jul 30, 2010 at 15:24, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Marc Branchaud <marcnarc@xiplink.com> writes:
>
>>> The name of the command may be subject to discussions. I've chosen
>>> "run", but maybe "shell" would be OK too. In both cases, it doesn't
>>> allow the one-letter version since both "r" and "s" are already used.
>>
>> "exec" with one-letter "x"?
>
> Thanks, that sounds good, yes. Any other thought?

I like "exec".

I think the docs need to elaborate on the environment the "exec"
command gets executed in, what's its current working directory for
instance? Wherever I happened to run git-rebase from? the project
root?

your if ! eval .. error message also exits with 0, surely that should
exit with 1?

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-31 13:56     ` Ævar Arnfjörð Bjarmason
@ 2010-07-31 14:25       ` Miles Bader
  2010-08-02 10:02       ` Matthieu Moy
  2010-08-03  8:47       ` Kris Shannon
  2 siblings, 0 replies; 33+ messages in thread
From: Miles Bader @ 2010-07-31 14:25 UTC (permalink / raw)
  To: git

There's also "invoke"

-miles

-- 
Immortality, n.  A toy which people cry for, And on their knees apply for,
      Dispute, contend and lie for, And if allowed Would be right proud
      Eternally to die for.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-28 13:29 [RFC/PATCH] rebase -i: add run command to launch a shell command Matthieu Moy
  2010-07-28 14:12 ` Santi Béjar
  2010-07-30 14:51 ` Marc Branchaud
@ 2010-07-31 14:40 ` Jared Hance
  2 siblings, 0 replies; 33+ messages in thread
From: Jared Hance @ 2010-07-31 14:40 UTC (permalink / raw)
  To: git

On Wed, Jul 28, 2010 at 03:29:44PM +0200, Matthieu Moy wrote:
> +		if ! eval "$rest"
> +		then
> +			warn "Command $rest failed"
> +			warn "You can fix the problem, and then run"
> +			warn
> +			warn "	git rebase --continue"
> +			warn
> +			exit 0
> +		fi

If the command, stored in $rest, is rather long, perhaps "Command
$rest failed" won't be friendly output. Perhaps:

    Command Failure: $rest

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-30 15:24   ` Matthieu Moy
  2010-07-30 18:26     ` Neal Kreitzinger
  2010-07-31 13:56     ` Ævar Arnfjörð Bjarmason
@ 2010-07-31 15:28     ` Paolo Bonzini
  2010-07-31 15:52       ` Ævar Arnfjörð Bjarmason
  2010-07-31 18:54       ` Jared Hance
  2 siblings, 2 replies; 33+ messages in thread
From: Paolo Bonzini @ 2010-07-31 15:28 UTC (permalink / raw)
  To: git; +Cc: Marc Branchaud, git

On 07/30/2010 05:24 PM, Matthieu Moy wrote:
> Marc Branchaud<marcnarc@xiplink.com>  writes:
>
>>> The name of the command may be subject to discussions. I've chosen
>>> "run", but maybe "shell" would be OK too. In both cases, it doesn't
>>> allow the one-letter version since both "r" and "s" are already used.
>>
>> "exec" with one-letter "x"?
>
> Thanks, that sounds good, yes. Any other thought?

I like run, for the short version what about ! (as in vi)?  Maybe with 
an optional space.

Paolo

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-31 15:28     ` Paolo Bonzini
@ 2010-07-31 15:52       ` Ævar Arnfjörð Bjarmason
  2010-07-31 18:54       ` Jared Hance
  1 sibling, 0 replies; 33+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-07-31 15:52 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Matthieu Moy, Marc Branchaud, git

On Sat, Jul 31, 2010 at 15:28, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 07/30/2010 05:24 PM, Matthieu Moy wrote:
>>
>> Marc Branchaud<marcnarc@xiplink.com>  writes:
>>
>>>> The name of the command may be subject to discussions. I've chosen
>>>> "run", but maybe "shell" would be OK too. In both cases, it doesn't
>>>> allow the one-letter version since both "r" and "s" are already used.
>>>
>>> "exec" with one-letter "x"?
>>
>> Thanks, that sounds good, yes. Any other thought?
>
> I like run, for the short version what about ! (as in vi)?  Maybe with an
> optional space.

"run" clashes with the short form of "reword", "exec" also does that,
but the "x" key for that is more obvious. ! is a shift-combo away.

Anyway, </bikeshedding>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-31 15:28     ` Paolo Bonzini
  2010-07-31 15:52       ` Ævar Arnfjörð Bjarmason
@ 2010-07-31 18:54       ` Jared Hance
  1 sibling, 0 replies; 33+ messages in thread
From: Jared Hance @ 2010-07-31 18:54 UTC (permalink / raw)
  To: git

On Sat, Jul 31, 2010 at 05:28:11PM +0200, Paolo Bonzini wrote:
> I like run, for the short version what about ! (as in vi)?  Maybe
> with an optional space.
> 
> Paolo

I disagree. Firstly, "!" is very inconsistent with the current shorthand
forms. I think the "x" makes more since from the point of view that its
very easy to associate with exec (the first syllable is pronounced as
an "x"). "x" is also used by other commands to mean executable (for
example, "chmod +x").

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-31 13:56     ` Ævar Arnfjörð Bjarmason
  2010-07-31 14:25       ` Miles Bader
@ 2010-08-02 10:02       ` Matthieu Moy
  2010-08-02 10:03         ` [PATCH] rebase -i: add exec " Matthieu Moy
                           ` (2 more replies)
  2010-08-03  8:47       ` Kris Shannon
  2 siblings, 3 replies; 33+ messages in thread
From: Matthieu Moy @ 2010-08-02 10:02 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Marc Branchaud, git

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Fri, Jul 30, 2010 at 15:24, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>> Marc Branchaud <marcnarc@xiplink.com> writes:
>>
>>>> The name of the command may be subject to discussions. I've chosen
>>>> "run", but maybe "shell" would be OK too. In both cases, it doesn't
>>>> allow the one-letter version since both "r" and "s" are already used.
>>>
>>> "exec" with one-letter "x"?
>>
>> Thanks, that sounds good, yes. Any other thought?
>
> I like "exec".

Yes, it's the best proposal IMHO. "x" is very often associated to
"execute", and I'd rather keep away from punctuation/shift combo.
There have been discussions in the past about giving "!" a semantics
(like "fixup!" for a variant of "fixup"). Let's keep this as an option
for the future by not using ! now.

> I think the docs need to elaborate on the environment the "exec"
> command gets executed in, what's its current working directory for
> instance? Wherever I happened to run git-rebase from? the project
> root?

That's a good question. My original patch was running the command from
the toplevel, which is the natural way to implement it. I've changed
my mind to execute the command from the place where "git rebase -i"
was started (which means this has to be memorized in a temporary file
to be persistant accross "git rebase --continue"). I think this makes
more sense for the user, and I've actually already been biten by the
old behavior, running "rebase -i" from a doc/ subdirectory, and
wondering why my "exec make" was rebuilding the code itself.

This comes with at least one drawback: if directory from which the
rebase was started didn't exist in the past, then we can't do a simple
"cd" to it. My implementation re-creates the directory temporarily, so
that the command can run, and cleans it up afterwards. The only really
problematic case is when the directory can not be created (like
directory/file conflict). It this case, the command is not ran, and
the script exits.

> your if ! eval .. error message also exits with 0, surely that should
> exit with 1?

I'm now exiting with the same status as the command itself in case of
failure.

New version follows. It should hopefully look more like a real patch
than an RFC now.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH] rebase -i: add exec command to launch a shell command
  2010-08-02 10:02       ` Matthieu Moy
@ 2010-08-02 10:03         ` Matthieu Moy
  2010-08-02 12:30           ` Jared Hance
                             ` (2 more replies)
  2010-08-02 15:04         ` [RFC/PATCH] rebase -i: add run command to launch a shell command Paolo Bonzini
  2010-08-02 21:15         ` Junio C Hamano
  2 siblings, 3 replies; 33+ messages in thread
From: Matthieu Moy @ 2010-08-02 10:03 UTC (permalink / raw)
  To: git, gitster
  Cc: Ævar Arnfjörð Bjarmason, Marc Branchaud, Matthieu Moy

The typical usage pattern would be to run a test (or simply a compilation
command) at given points in history.

The shell command is ran (in the directory where the rebase was started
by the user), and the rebase is stopped when the command fails, to give
the user an opportunity to fix the problem before continuing with "git
rebase --continue".

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/git-rebase.txt  |   26 +++++++++++++
 git-rebase--interactive.sh    |   77 +++++++++++++++++++++++++++++++++++++++
 git-rebase.sh                 |    6 +++
 t/lib-rebase.sh               |    2 +
 t/t3404-rebase-interactive.sh |   79 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 190 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index be23ad2..121f873 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -459,6 +459,32 @@ sure that the current HEAD is "B", and call
 $ git rebase -i -p --onto Q O
 -----------------------------
 
+Reordering and editing commits usually creates untested intermediate
+steps.  You may want to check that your history editting did not break
+anything by running a test, or at least recompiling at intermediate
+points in history by using the "exec" command (shortcut "x").  You may
+do so by creating a todo list like this one:
+
+-------------------------------------------
+pick deadbee Implement feature XXX
+fixup f1a5c00 Fix to feature XXX
+exec make
+pick c0ffeee The oneline of the next commit
+edit deadbab The oneline of the commit after
+exec make; make test
+...
+-------------------------------------------
+
+The interactive rebase will stop when a command fails (i.e. exists
+with non-0 status) to give you an opportunity to fix the problem.  You
+can continue with `git rebase --continue`.
+
+The "exec" command launches the command in a shell (the one specified
+in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
+use usual shell commands like "cd". The commands are ran from the same
+directory as "rebase -i" was started (this directory is temporarily
+re-created if needed).
+
 
 SPLITTING COMMITS
 -----------------
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b94c2a0..98a54e5 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -104,6 +104,10 @@ AMEND="$DOTEST"/amend
 REWRITTEN_LIST="$DOTEST"/rewritten-list
 REWRITTEN_PENDING="$DOTEST"/rewritten-pending
 
+# The "exec" command needs to know from which directory the
+# interactive rebase started.
+START_PREFIX="$DOTEST"/start-prefix
+
 PRESERVE_MERGES=
 STRATEGY=
 ONTO=
@@ -448,6 +452,31 @@ record_in_rewritten() {
 	esac
 }
 
+# Like "mkdir -p", but outputs the largest prefix of "$1" which
+# already exists as a directory
+mkdir_parent_verbose() {
+	dir=$1
+	to_mk=.
+	while ! test -d "$dir"
+	do
+		to_mk=$(basename "$dir")/"$to_mk"
+		dir=$(dirname "$dir")
+	done
+	(cd "$dir" && mkdir -p "$to_mk" && pwd -P)
+}
+
+# Delete directory "$1", and its parent, until encountering a
+# non-empty directory or "$2".
+cleanup_directory() {
+	to_rm="$1"
+	while test -d "$1" && test -d "$2" &&
+		test "$(cd "$to_rm"; pwd -P)" != "$(cd "$2"; pwd -P)" &&
+		rmdir "$to_rm"
+	do
+		to_rm=$(dirname "$to_rm")
+	done
+}
+
 do_next () {
 	rm -f "$MSG" "$AUTHOR_SCRIPT" "$AMEND" || exit
 	read -r command sha1 rest < "$TODO"
@@ -537,6 +566,52 @@ do_next () {
 		esac
 		record_in_rewritten $sha1
 		;;
+	x|"exec")
+		read -r command rest < "$TODO"
+		mark_action_done
+		printf 'Executing: %s\n' "$rest"
+		# "exec" command doesn't take a sha1 in the todo-list.
+		# => can't just use $sha1 here.
+		git rev-parse --verify HEAD > "$DOTEST"/stopped-sha
+		# We run the command from the directory in which "git
+		# rebase -i" was first started.
+		prefix=$(cat "$START_PREFIX")
+		to_rm=
+		old_pwd=$(pwd)
+		cd "$prefix" 2>/dev/null || {
+			# The prefix from which the rebase started
+			# didn't exist in the past.  We create the
+			# directory to let the command run from there,
+			# and remember how to delete it in $to_rm.
+			to_rm=$(mkdir_parent_verbose "$prefix") && cd "$prefix"
+		} || {
+			warn "Cannot create directory $prefix"
+			warn "Aborting execution of command $rest"
+			warn "To continue, run"
+			warn
+			warn "	git rebase --continue"
+			warn
+			exit 1
+		}
+		${SHELL:-@SHELL_PATH@} -c "$rest" # Actual execution
+		status=$?
+		cd "$old_pwd"
+		# Don't leave empty directory around.
+		# Since cleanup_directory never deletes non-empty
+		# directories, this can't lose valuable data.
+		if [ -n "$to_rm" ]; then
+			cleanup_directory "$prefix" "$to_rm"
+		fi
+		if test "$status" -ne 0
+		then
+			warn "Execution failed: $rest"
+			warn "You can fix the problem, and then run"
+			warn
+			warn "	git rebase --continue"
+			warn
+			exit "$status"
+		fi
+		;;
 	*)
 		warn "Unknown command: $command $sha1 $rest"
 		if git rev-parse --verify -q "$sha1" >/dev/null
@@ -851,6 +926,7 @@ first and then run 'git rebase --continue' again."
 		echo $ONTO > "$DOTEST"/onto
 		test -z "$STRATEGY" || echo "$STRATEGY" > "$DOTEST"/strategy
 		test t = "$VERBOSE" && : > "$DOTEST"/verbose
+		echo "$GIT_PREFIX" > "$START_PREFIX"
 		if test t = "$PRESERVE_MERGES"
 		then
 			if test -z "$REBASE_ROOT"
@@ -957,6 +1033,7 @@ first and then run 'git rebase --continue' again."
 #  e, edit = use commit, but stop for amending
 #  s, squash = use commit, but meld into previous commit
 #  f, fixup = like "squash", but discard this commit's log message
+#  x <cmd>, exec <cmd> = Run a shell command <cmd>, and stop if it fails
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/git-rebase.sh b/git-rebase.sh
index ab4afa7..fff512c 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -32,6 +32,12 @@ OPTIONS_SPEC=
 . git-sh-setup
 set_reflog_action rebase
 require_work_tree
+
+# Keep the prefix path in an environment variable so that
+# git-rebase--interactive can find it.
+GIT_PREFIX=$(git rev-parse --show-prefix)
+export GIT_PREFIX
+
 cd_to_toplevel
 
 LF='
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6aefe27..6ccf797 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -47,6 +47,8 @@ for line in $FAKE_LINES; do
 	case $line in
 	squash|fixup|edit|reword)
 		action="$line";;
+	exec*)
+		echo "$line" | sed 's/_/ /g' >> "$1";;
 	"#")
 		echo '# comment' >> "$1";;
 	">")
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 9f03ce6..649a400 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -64,6 +64,85 @@ test_expect_success 'setup' '
 	done
 '
 
+file_must_exist () {
+    if ! [ -f "$1" ]; then
+	echo "file $1 not created."
+	false
+    fi
+}
+
+file_must_not_exist () {
+    if [ -f "$1" ]; then
+	echo "file $1 created while it shouldn't have. $2"
+	false
+    fi
+}
+
+dir_must_exist () {
+    if ! [ -d "$1" ]; then
+	echo "dir $1 does not exist."
+	false
+    fi
+}
+
+dir_must_not_exist () {
+    if [ -d "$1" ]; then
+	echo "dir $1 exists while it shouldn't. $2"
+	false
+    fi
+}
+
+test_expect_success 'rebase -i with the exec command' '
+	git checkout master &&
+	FAKE_LINES="1 exec_touch_touch-one 2 exec_touch_touch-two exec_false exec_touch_touch-three 3 4
+		exec_touch_\"touch-file__name_with_spaces\";_touch_touch-after-semicolon 5" \
+		test_must_fail git rebase -i A &&
+	file_must_exist touch-one &&
+	file_must_exist touch-two &&
+	file_must_not_exist touch-three "(Should have stopped before)" &&
+	test $(git rev-parse C) = $(git rev-parse HEAD) || {
+		echo "Stopped at wrong revision:"
+		echo "($(git describe --tags HEAD) instead of C)"
+		false
+	} &&
+	git rebase --continue &&
+	file_must_exist touch-three &&
+	file_must_exist "touch-file  name with spaces" &&
+	file_must_exist touch-after-semicolon &&
+	test $(git rev-parse master) = $(git rev-parse HEAD) || {
+		echo "Stopped at wrong revision:"
+		echo "($(git describe --tags HEAD) instead of master)"
+		false
+	} &&
+	rm -f touch-*
+'
+
+test_expect_success 'rebase -i with exec in deleted directory' '
+	git checkout master &&
+	rm -f pwd.txt &&
+	mkdir tmpdir &&
+	cd tmpdir &&
+	pwd > ../expect &&
+	FAKE_LINES="1 exec_false_one
+		      exec_pwd>../actual exec_false_two
+		      exec_touch_tmpfile exec_false_three
+		      exec_pwd" \
+		test_must_fail git rebase -i HEAD^ &&
+	cd .. && rm -fr tmpdir &&
+	# Should re-create tmpdir temporarily, and clean it up:
+	test_must_fail git rebase --continue &&
+	test_cmp expect actual &&
+	dir_must_not_exist tmpdir &&
+	# Should not cleanup non-empty directory
+	test_must_fail git rebase --continue &&
+	file_must_exist tmpdir/tmpfile &&
+	rm -fr tmpdir && touch tmpdir &&
+	# Should fail because of D/F conflict
+	test_must_fail git rebase --continue &&
+	git rebase --continue &&
+	rm -fr tmpdir
+'
+
 test_expect_success 'no changes are a nop' '
 	git checkout branch2 &&
 	git rebase -i F &&
-- 
1.7.2.1.10.g5cb67a

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH] rebase -i: add exec command to launch a shell command
  2010-08-02 10:03         ` [PATCH] rebase -i: add exec " Matthieu Moy
@ 2010-08-02 12:30           ` Jared Hance
  2010-08-02 15:51           ` Ævar Arnfjörð Bjarmason
  2010-08-06 21:07           ` Johannes Sixt
  2 siblings, 0 replies; 33+ messages in thread
From: Jared Hance @ 2010-08-02 12:30 UTC (permalink / raw)
  To: git

On Mon, Aug 02, 2010 at 12:03:53PM +0200, Matthieu Moy wrote:
> The shell command is ran (in the directory where the rebase was started
> by the user)

I disagree with how intuitive this behavior is. "rebase" is a
repository level command - It modifies the objects in the repository.
As a side effect, the working tree attached to a (non-bare) repository
is changed, even outside of where the rebase was ran.

Since the rebase affects the entire repository, I think it makes sense
for the commands to be run out of the repository root.

Perhaps a command-line option for toggling this behavior would be a
good compromise.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-08-02 10:02       ` Matthieu Moy
  2010-08-02 10:03         ` [PATCH] rebase -i: add exec " Matthieu Moy
@ 2010-08-02 15:04         ` Paolo Bonzini
  2010-08-02 21:15         ` Junio C Hamano
  2 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2010-08-02 15:04 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Ævar Arnfjörð Bjarmason, Marc Branchaud, git

On 08/02/2010 12:02 PM, Matthieu Moy wrote:
> I think this makes more sense for the user, and I've actually already
> been biten by the old behavior, running "rebase -i" from a doc/
> subdirectory, and wondering why my "exec make" was rebuilding the
> code itself.

I think it's a matter of habits, and I would surely be bitten more by 
the opposite problem: when I'm usually ready to rebase and test I'm 
likely to be in src/ (for packages that have one such directory) or tests/.

cd to the top-level repository is a logical choice since rebase is a 
repository-wide command (even though the particular set of commits might 
touch only a part of it).  It is easier to implement, does not have any 
problem with conflicts or otherwise with deletion, and easier to 
document as well.

If you decide to go with the other choice, however, I would _strongly_ 
suggest failing if the directory not exists.  After all most of the time 
the command ("make" for example) will be pretty unlikely to succeed.

Paolo

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH] rebase -i: add exec command to launch a shell command
  2010-08-02 10:03         ` [PATCH] rebase -i: add exec " Matthieu Moy
  2010-08-02 12:30           ` Jared Hance
@ 2010-08-02 15:51           ` Ævar Arnfjörð Bjarmason
  2010-08-06 21:07           ` Johannes Sixt
  2 siblings, 0 replies; 33+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-02 15:51 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git, gitster, Marc Branchaud

On Mon, Aug 2, 2010 at 10:03, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:

> +use usual shell commands like "cd". The commands are ran from the same

"are run from the same"

> +directory as "rebase -i" was started (this directory is temporarily
> +re-created if needed).

"was started in"

> +file_must_exist () {
> +    if ! [ -f "$1" ]; then
> +       echo "file $1 not created."
> +       false
> +    fi
> +}
> +
> +file_must_not_exist () {
> +    if [ -f "$1" ]; then
> +       echo "file $1 created while it shouldn't have. $2"
> +       false
> +    fi
> +}
> +
> +dir_must_exist () {
> +    if ! [ -d "$1" ]; then
> +       echo "dir $1 does not exist."
> +       false
> +    fi
> +}
> +
> +dir_must_not_exist () {
> +    if [ -d "$1" ]; then
> +       echo "dir $1 exists while it shouldn't. $2"
> +       false
> +    fi
> +}

Leftover debugging code? Looks useful, but probably something we
should have in test-lib.sh as "git_test" as a wrapper for the "test"
built-in.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-08-02 10:02       ` Matthieu Moy
  2010-08-02 10:03         ` [PATCH] rebase -i: add exec " Matthieu Moy
  2010-08-02 15:04         ` [RFC/PATCH] rebase -i: add run command to launch a shell command Paolo Bonzini
@ 2010-08-02 21:15         ` Junio C Hamano
  2010-08-03  6:37           ` Matthieu Moy
  2 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2010-08-02 21:15 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Ævar Arnfjörð Bjarmason, Marc Branchaud, git

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> That's a good question. My original patch was running the command from
> the toplevel, which is the natural way to implement it. I've changed
> my mind to execute the command from the place where "git rebase -i"
> was started (which means this has to be memorized in a temporary file
> to be persistant accross "git rebase --continue"). I think this makes
> more sense for the user, and I've actually already been biten by the
> old behavior, running "rebase -i" from a doc/ subdirectory, and
> wondering why my "exec make" was rebuilding the code itself.
>
> This comes with at least one drawback: if directory from which the
> rebase was started didn't exist in the past, then we can't do a simple
> "cd" to it. My implementation re-creates the directory temporarily, so
> that the command can run, and cleans it up afterwards. The only really
> problematic case is when the directory can not be created (like
> directory/file conflict). It this case, the command is not ran, and
> the script exits.

Sorry to join the discussion after you have already coded it, but I don't
think running the external command at a random subdirectory that the
operation happened to have started makes much sense, as rebase is a
tree-wide operation.  The user if s/he so chooses can chdir down (if the
directory still exists in the revision in question) in the script, but I
think the built-in behaviour should be to just run it from the toplevel.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-08-02 21:15         ` Junio C Hamano
@ 2010-08-03  6:37           ` Matthieu Moy
  0 siblings, 0 replies; 33+ messages in thread
From: Matthieu Moy @ 2010-08-03  6:37 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Ævar Arnfjörð Bjarmason, Marc Branchaud, git

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

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> That's a good question. My original patch was running the command from
>> the toplevel, which is the natural way to implement it. I've changed
>> my mind to execute the command from the place where "git rebase -i"
>> was started (which means this has to be memorized in a temporary file
>> to be persistant accross "git rebase --continue"). I think this makes
>> more sense for the user, and I've actually already been biten by the
>> old behavior, running "rebase -i" from a doc/ subdirectory, and
>> wondering why my "exec make" was rebuilding the code itself.
>>
>> This comes with at least one drawback: if directory from which the
>> rebase was started didn't exist in the past, then we can't do a simple
>> "cd" to it. My implementation re-creates the directory temporarily, so
>> that the command can run, and cleans it up afterwards. The only really
>> problematic case is when the directory can not be created (like
>> directory/file conflict). It this case, the command is not ran, and
>> the script exits.
>
> Sorry to join the discussion after you have already coded it, but I don't
> think running the external command at a random subdirectory that the
> operation happened to have started makes much sense, as rebase is a
> tree-wide operation.  The user if s/he so chooses can chdir down (if the
> directory still exists in the revision in question) in the script, but I
> think the built-in behaviour should be to just run it from the toplevel.

I'm waiting for other people's opinion, since I'm still not 100%
convinced, but this seems to reflect the majority's opinion.

And the simplicity of the code also is an argument: even if the "I'm
too lazy to code it" is not applicable anymore, the simplest version
will also be the simplest to maintain.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-07-31 13:56     ` Ævar Arnfjörð Bjarmason
  2010-07-31 14:25       ` Miles Bader
  2010-08-02 10:02       ` Matthieu Moy
@ 2010-08-03  8:47       ` Kris Shannon
  2010-08-03  9:16         ` Matthieu Moy
  2 siblings, 1 reply; 33+ messages in thread
From: Kris Shannon @ 2010-08-03  8:47 UTC (permalink / raw)
  To: git

On 31 July 2010 23:56, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> On Fri, Jul 30, 2010 at 15:24, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
> > Marc Branchaud <marcnarc@xiplink.com> writes:
> >
> >>> The name of the command may be subject to discussions. I've chosen
> >>> "run", but maybe "shell" would be OK too. In both cases, it doesn't
> >>> allow the one-letter version since both "r" and "s" are already used.
> >>
> >> "exec" with one-letter "x"?
> >
> > Thanks, that sounds good, yes. Any other thought?
>
> I like "exec".
or (t)est.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC/PATCH] rebase -i: add run command to launch a shell command
  2010-08-03  8:47       ` Kris Shannon
@ 2010-08-03  9:16         ` Matthieu Moy
  0 siblings, 0 replies; 33+ messages in thread
From: Matthieu Moy @ 2010-08-03  9:16 UTC (permalink / raw)
  To: Kris Shannon; +Cc: git

Kris Shannon <kris@shannon.id.au> writes:

> On 31 July 2010 23:56, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> On Fri, Jul 30, 2010 at 15:24, Matthieu Moy
>> <Matthieu.Moy@grenoble-inp.fr> wrote:
>> > Marc Branchaud <marcnarc@xiplink.com> writes:
>> >
>> >>> The name of the command may be subject to discussions. I've chosen
>> >>> "run", but maybe "shell" would be OK too. In both cases, it doesn't
>> >>> allow the one-letter version since both "r" and "s" are already used.
>> >>
>> >> "exec" with one-letter "x"?
>> >
>> > Thanks, that sounds good, yes. Any other thought?
>>
>> I like "exec".
> or (t)est.

testing is one possibility, but you may want to do other things, like

pick <commit> ...
exec perl -pi -e 's/foo/bar/g' file.c; git add file.c; git commit --amend

or whatever. "exec" is definitely the best proposal.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH] rebase -i: add exec command to launch a shell command
  2010-08-02 10:03         ` [PATCH] rebase -i: add exec " Matthieu Moy
  2010-08-02 12:30           ` Jared Hance
  2010-08-02 15:51           ` Ævar Arnfjörð Bjarmason
@ 2010-08-06 21:07           ` Johannes Sixt
  2010-08-07  8:48             ` Matthieu Moy
  2 siblings, 1 reply; 33+ messages in thread
From: Johannes Sixt @ 2010-08-06 21:07 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: git, gitster, Ævar Arnfjörð Bjarmason, Marc Branchaud

On Montag, 2. August 2010, Matthieu Moy wrote:
> The typical usage pattern would be to run a test (or simply a compilation
> command) at given points in history.

What happens if the command modifies the worktree and/or the index?

-- Hannes

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH] rebase -i: add exec command to launch a shell command
  2010-08-06 21:07           ` Johannes Sixt
@ 2010-08-07  8:48             ` Matthieu Moy
  2010-08-07  8:56               ` [PATCH 1/2 (new version)] " Matthieu Moy
  2010-08-07  8:56               ` [PATCH 2/2] test-lib: user-friendly alternatives to test [!] [-d|-f] Matthieu Moy
  0 siblings, 2 replies; 33+ messages in thread
From: Matthieu Moy @ 2010-08-07  8:48 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: git, gitster, Ævar Arnfjörð Bjarmason, Marc Branchaud

Johannes Sixt <j6t@kdbg.org> writes:

> On Montag, 2. August 2010, Matthieu Moy wrote:
>> The typical usage pattern would be to run a test (or simply a compilation
>> command) at given points in history.
>
> What happens if the command modifies the worktree and/or the index?

Something terrible :-(. The next patch application fails, and the
patch is more or less silently dropped (I thought it would fail giving
the user an opportunity to fix and re-apply, but it doesn't).

New version checks working tree cleanness right after the command
finishes, and stops cleanly, in time (restart with "git rebase
--continue"). This comes with a new test.

(But the command can possibly create a new commit or amend the
previous one without problem)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 1/2 (new version)] rebase -i: add exec command to launch a shell command
  2010-08-07  8:48             ` Matthieu Moy
@ 2010-08-07  8:56               ` Matthieu Moy
  2010-08-07  8:56               ` [PATCH 2/2] test-lib: user-friendly alternatives to test [!] [-d|-f] Matthieu Moy
  1 sibling, 0 replies; 33+ messages in thread
From: Matthieu Moy @ 2010-08-07  8:56 UTC (permalink / raw)
  To: git, gitster; +Cc: Johannes Sixt, Jacob Helwig, Matthieu Moy

The typical usage pattern would be to run a test (or simply a compilation
command) at given points in history.

The shell command is ran (from the worktree root), and the rebase is
stopped when the command fails, to give the user an opportunity to fix
the problem before continuing with "git rebase --continue".

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Compared to previous version (sorry, I didn't number them) :

* Test the case where the command stops with a dirty tree, and fix it
  (thanks to Johannes Sixt)

* Reword the doc (drop "usual", and replace "shell command" with
  "shell features" to give examples which aren't commands, but yet
  useful)

That should adress everyones's comment.

 Documentation/git-rebase.txt  |   24 ++++++++++++++++++++
 git-rebase--interactive.sh    |   29 ++++++++++++++++++++++++
 t/lib-rebase.sh               |    2 +
 t/t3404-rebase-interactive.sh |   48 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 103 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index be23ad2..9c68b66 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -459,6 +459,30 @@ sure that the current HEAD is "B", and call
 $ git rebase -i -p --onto Q O
 -----------------------------
 
+Reordering and editing commits usually creates untested intermediate
+steps.  You may want to check that your history editing did not break
+anything by running a test, or at least recompiling at intermediate
+points in history by using the "exec" command (shortcut "x").  You may
+do so by creating a todo list like this one:
+
+-------------------------------------------
+pick deadbee Implement feature XXX
+fixup f1a5c00 Fix to feature XXX
+exec make
+pick c0ffeee The oneline of the next commit
+edit deadbab The oneline of the commit after
+exec cd subdir; make test
+...
+-------------------------------------------
+
+The interactive rebase will stop when a command fails (i.e. exits with
+non-0 status) to give you an opportunity to fix the problem. You can
+continue with `git rebase --continue`.
+
+The "exec" command launches the command in a shell (the one specified
+in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
+use shell features (like "cd", ">", ";" ...). The command is run from
+the root of the working tree.
 
 SPLITTING COMMITS
 -----------------
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b94c2a0..6dd5859 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -537,6 +537,34 @@ do_next () {
 		esac
 		record_in_rewritten $sha1
 		;;
+	x|"exec")
+		read -r command rest < "$TODO"
+		mark_action_done
+		printf 'Executing: %s\n' "$rest"
+		# "exec" command doesn't take a sha1 in the todo-list.
+		# => can't just use $sha1 here.
+		git rev-parse --verify HEAD > "$DOTEST"/stopped-sha
+		${SHELL:-@SHELL_PATH@} -c "$rest" # Actual execution
+		status=$?
+		if test "$status" -ne 0
+		then
+			warn "Execution failed: $rest"
+			warn "You can fix the problem, and then run"
+			warn
+			warn "	git rebase --continue"
+			warn
+			exit "$status"
+		fi
+		# Run in subshell because require_clean_work_tree can die.
+		if ! (require_clean_work_tree)
+		then
+			warn "Commit or stash your changes, and then run"
+			warn
+			warn "	git rebase --continue"
+			warn
+			exit 1
+		fi
+		;;
 	*)
 		warn "Unknown command: $command $sha1 $rest"
 		if git rev-parse --verify -q "$sha1" >/dev/null
@@ -957,6 +985,7 @@ first and then run 'git rebase --continue' again."
 #  e, edit = use commit, but stop for amending
 #  s, squash = use commit, but meld into previous commit
 #  f, fixup = like "squash", but discard this commit's log message
+#  x <cmd>, exec <cmd> = Run a shell command <cmd>, and stop if it fails
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6aefe27..6ccf797 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -47,6 +47,8 @@ for line in $FAKE_LINES; do
 	case $line in
 	squash|fixup|edit|reword)
 		action="$line";;
+	exec*)
+		echo "$line" | sed 's/_/ /g' >> "$1";;
 	"#")
 		echo '# comment' >> "$1";;
 	">")
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 9f03ce6..7b0026e 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -64,6 +64,54 @@ test_expect_success 'setup' '
 	done
 '
 
+test_expect_success 'rebase -i with the exec command' '
+	git checkout master &&
+	FAKE_LINES="1 exec_touch_touch-one 2 exec_touch_touch-two exec_false exec_touch_touch-three 3 4
+		exec_touch_\"touch-file__name_with_spaces\";_touch_touch-after-semicolon 5" \
+		test_must_fail git rebase -i A &&
+	test -f touch-one &&
+	test -f touch-two &&
+	! test -f touch-three &&
+	test $(git rev-parse C) = $(git rev-parse HEAD) || {
+		echo "Stopped at wrong revision:"
+		echo "($(git describe --tags HEAD) instead of C)"
+		false
+	} &&
+	git rebase --continue &&
+	test -f touch-three &&
+	test -f "touch-file  name with spaces" &&
+	test -f touch-after-semicolon &&
+	test $(git rev-parse master) = $(git rev-parse HEAD) || {
+		echo "Stopped at wrong revision:"
+		echo "($(git describe --tags HEAD) instead of master)"
+		false
+	} &&
+	rm -f touch-*
+'
+
+test_expect_success 'rebase -i with the exec command runs from tree root' '
+	git checkout master &&
+	mkdir subdir && cd subdir &&
+	FAKE_LINES="1 exec_touch_touch-subdir" \
+		git rebase -i HEAD^ &&
+	cd .. &&
+	test -f touch-subdir &&
+	rm -fr subdir
+'
+
+test_expect_success 'rebase -i with the exec command checks tree cleanness' '
+	git checkout master &&
+	FAKE_LINES="exec_echo_foo_>file1 1" \
+		test_must_fail git rebase -i HEAD^ &&
+	test $(git rev-parse master^) = $(git rev-parse HEAD) || {
+		echo "Stopped at wrong revision:"
+		echo "($(git describe --tags HEAD) instead of master^)"
+		false
+	} &&
+	git reset --hard &&
+	git rebase --continue
+'
+
 test_expect_success 'no changes are a nop' '
 	git checkout branch2 &&
 	git rebase -i F &&
-- 
1.7.2.1.52.g95e25.dirty

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 2/2] test-lib: user-friendly alternatives to test [!] [-d|-f]
  2010-08-07  8:48             ` Matthieu Moy
  2010-08-07  8:56               ` [PATCH 1/2 (new version)] " Matthieu Moy
@ 2010-08-07  8:56               ` Matthieu Moy
  1 sibling, 0 replies; 33+ messages in thread
From: Matthieu Moy @ 2010-08-07  8:56 UTC (permalink / raw)
  To: git, gitster; +Cc: Johannes Sixt, Jacob Helwig, Matthieu Moy

The helper functions are implemented, documented, and used in a few
places to validate them, but not everywhere to avoid useless code churn.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
(this one hasn't been modified since last time)

 t/README                      |    8 ++++++++
 t/t3404-rebase-interactive.sh |   18 +++++++++---------
 t/t3407-rebase-abort.sh       |    6 +++---
 t/test-lib.sh                 |   32 ++++++++++++++++++++++++++++++++
 4 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/t/README b/t/README
index 0d1183c..be760b9 100644
--- a/t/README
+++ b/t/README
@@ -467,6 +467,14 @@ library for your script to use.
    <expected> file.  This behaves like "cmp" but produces more
    helpful output when the test is run with "-v" option.
 
+ - test_file_must_exist <file> [<diagnosis>]
+   test_file_must_not_exist <file> [<diagnosis>]
+   test_dir_must_exist <dir> [<diagnosis>]
+   test_dir_must_not_exist <dir> [<diagnosis>]
+
+   check whether a file/directory exists or doesn't. <diagnosis> will
+   be displayed if the test fails.
+
  - test_when_finished <script>
 
    Prepend <script> to a list of commands to run to clean up
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 7b0026e..aacdc8a 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -69,18 +69,18 @@ test_expect_success 'rebase -i with the exec command' '
 	FAKE_LINES="1 exec_touch_touch-one 2 exec_touch_touch-two exec_false exec_touch_touch-three 3 4
 		exec_touch_\"touch-file__name_with_spaces\";_touch_touch-after-semicolon 5" \
 		test_must_fail git rebase -i A &&
-	test -f touch-one &&
-	test -f touch-two &&
-	! test -f touch-three &&
+	test_file_must_exist touch-one &&
+	test_file_must_exist touch-two &&
+	test_file_must_not_exist touch-three "(Rebase should have stopped before)" &&
 	test $(git rev-parse C) = $(git rev-parse HEAD) || {
 		echo "Stopped at wrong revision:"
 		echo "($(git describe --tags HEAD) instead of C)"
 		false
 	} &&
 	git rebase --continue &&
-	test -f touch-three &&
-	test -f "touch-file  name with spaces" &&
-	test -f touch-after-semicolon &&
+	test_file_must_exist touch-three &&
+	test_file_must_exist "touch-file  name with spaces" &&
+	test_file_must_exist touch-after-semicolon &&
 	test $(git rev-parse master) = $(git rev-parse HEAD) || {
 		echo "Stopped at wrong revision:"
 		echo "($(git describe --tags HEAD) instead of master)"
@@ -95,7 +95,7 @@ test_expect_success 'rebase -i with the exec command runs from tree root' '
 	FAKE_LINES="1 exec_touch_touch-subdir" \
 		git rebase -i HEAD^ &&
 	cd .. &&
-	test -f touch-subdir &&
+	test_file_must_exist touch-subdir &&
 	rm -fr subdir
 '
 
@@ -191,7 +191,7 @@ test_expect_success 'abort' '
 	git rebase --abort &&
 	test $(git rev-parse new-branch1) = $(git rev-parse HEAD) &&
 	test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch1" &&
-	! test -d .git/rebase-merge
+	test_dir_must_not_exist .git/rebase-merge
 '
 
 test_expect_success 'abort with error when new base cannot be checked out' '
@@ -200,7 +200,7 @@ test_expect_success 'abort with error when new base cannot be checked out' '
 	test_must_fail git rebase -i master > output 2>&1 &&
 	grep "Untracked working tree file .file1. would be overwritten" \
 		output &&
-	! test -d .git/rebase-merge &&
+	test_dir_must_not_exist .git/rebase-merge &&
 	git reset --hard HEAD^
 '
 
diff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh
index 2999e78..0ca81fe 100755
--- a/t/t3407-rebase-abort.sh
+++ b/t/t3407-rebase-abort.sh
@@ -38,7 +38,7 @@ testrebase() {
 		# Clean up the state from the previous one
 		git reset --hard pre-rebase &&
 		test_must_fail git rebase$type master &&
-		test -d "$dotest" &&
+		test_dir_must_exist "$dotest" &&
 		git rebase --abort &&
 		test $(git rev-parse to-rebase) = $(git rev-parse pre-rebase) &&
 		test ! -d "$dotest"
@@ -49,7 +49,7 @@ testrebase() {
 		# Clean up the state from the previous one
 		git reset --hard pre-rebase &&
 		test_must_fail git rebase$type master &&
-		test -d "$dotest" &&
+		test_dir_must_exist "$dotest" &&
 		test_must_fail git rebase --skip &&
 		test $(git rev-parse HEAD) = $(git rev-parse master) &&
 		git rebase --abort &&
@@ -62,7 +62,7 @@ testrebase() {
 		# Clean up the state from the previous one
 		git reset --hard pre-rebase &&
 		test_must_fail git rebase$type master &&
-		test -d "$dotest" &&
+		test_dir_must_exist "$dotest" &&
 		echo c > a &&
 		echo d >> a &&
 		git add a &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e8f21d5..694bbe8 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -541,6 +541,38 @@ test_external_without_stderr () {
 	fi
 }
 
+# debugging-friendly alternatives to "test [!] [-f|-d]"
+# The commands test the existence or non-existance of $1. $2 can be
+# given to provide a more precise diagnosis.
+test_file_must_exist () {
+	if ! [ -f "$1" ]; then
+		echo "file $1 doesn't exist. $*"
+		false
+	fi
+}
+
+test_file_must_not_exist () {
+	if [ -f "$1" ]; then
+		echo "file $1 exists. $*"
+		false
+	fi
+}
+
+test_dir_must_exist () {
+	if ! [ -d "$1" ]; then
+		echo "directory $1 doesn't exist. $*"
+		false
+	fi
+}
+
+test_dir_must_not_exist () {
+	if [ -d "$1" ]; then
+		echo "directory $1 exists. $*"
+		false
+	fi
+}
+
+
 # This is not among top-level (test_expect_success | test_expect_failure)
 # but is a prefix that can be used in the test script, like:
 #
-- 
1.7.2.1.52.g95e25.dirty

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH] rebase -i: add exec command to launch a shell command
  2010-08-05 18:37     ` Jacob Helwig
@ 2010-08-05 20:16       ` Junio C Hamano
  0 siblings, 0 replies; 33+ messages in thread
From: Junio C Hamano @ 2010-08-05 20:16 UTC (permalink / raw)
  To: Jacob Helwig; +Cc: Matthieu Moy, Ævar Arnfjörð, git

Jacob Helwig <jacob.helwig@gmail.com> writes:

> On Thu, Aug 5, 2010 at 09:47, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
>> Ęvar Arnfjörš Bjarmason <avarab@gmail.com> writes:
>>
>>>> +in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
>>>> +use usual shell commands like "cd". The command is run from the
>>>
>>> I think that needs a definite article: ".. use the usual ..".
>> ...
> You could probably just drop "usual" entirely: ..., so you can use
> shell commands like "cd".

Sounds sane.  Will do, unless a native speaker stops me from doing so.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH] rebase -i: add exec command to launch a shell command
  2010-08-05 16:47   ` Matthieu Moy
  2010-08-05 18:24     ` Erik Faye-Lund
@ 2010-08-05 18:37     ` Jacob Helwig
  2010-08-05 20:16       ` Junio C Hamano
  1 sibling, 1 reply; 33+ messages in thread
From: Jacob Helwig @ 2010-08-05 18:37 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Ævar Arnfjörð, git, gitster

On Thu, Aug 5, 2010 at 09:47, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
> Ęvar Arnfjörš Bjarmason <avarab@gmail.com> writes:
>
>>> +in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
>>> +use usual shell commands like "cd". The command is run from the
>>
>> I think that needs a definite article: ".. use the usual ..".
>
> I don't think so, especially with a plural "commands" after.
> Googlefight agrees with me ("use the usual commands" => 350 results,
> "use usual commands" => 4900), but that's not a proof. Perhaps a
> native speaker could help?
>

You could probably just drop "usual" entirely: ..., so you can use
shell commands like "cd".

I'm not sure that "usual" really adds anything there, and might
actually be confusing.  Does it mean that "unusual" ones won't work?
What are "unusual" ones?

Possibly make 'like "cd"', parenthetical to further show that it's an
example, and not saying that only commands along the lines of "cd"
will work?  ..., so you can use shell commands (for example: cd).

-Jacob

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH] rebase -i: add exec command to launch a shell command
  2010-08-05 16:47   ` Matthieu Moy
@ 2010-08-05 18:24     ` Erik Faye-Lund
  2010-08-05 18:37     ` Jacob Helwig
  1 sibling, 0 replies; 33+ messages in thread
From: Erik Faye-Lund @ 2010-08-05 18:24 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Ævar Arnfjörð, git, gitster

On Thu, Aug 5, 2010 at 6:47 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Ęvar Arnfjörš Bjarmason <avarab@gmail.com> writes:
>
>>> +in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
>>> +use usual shell commands like "cd". The command is run from the
>>
>> I think that needs a definite article: ".. use the usual ..".
>
> I don't think so, especially with a plural "commands" after.
> Googlefight agrees with me ("use the usual commands" => 350 results,
> "use usual commands" => 4900), but that's not a proof.

Being grammatically correct doesn't automatically make a sentence
good. "Use ususal" is a bit of a tongue-twister, so I'd rewrite that
to "use normal" for that purpose alone.

But I'm not a native English speaker, and the natives might disagree with me.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH] rebase -i: add exec command to launch a shell command
  2010-08-05 13:31 ` Ævar Arnfjörð Bjarmason
@ 2010-08-05 16:47   ` Matthieu Moy
  2010-08-05 18:24     ` Erik Faye-Lund
  2010-08-05 18:37     ` Jacob Helwig
  0 siblings, 2 replies; 33+ messages in thread
From: Matthieu Moy @ 2010-08-05 16:47 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, gitster

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> +in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
>> +use usual shell commands like "cd". The command is run from the
>
> I think that needs a definite article: ".. use the usual ..".

I don't think so, especially with a plural "commands" after.
Googlefight agrees with me ("use the usual commands" => 350 results,
"use usual commands" => 4900), but that's not a proof. Perhaps a
native speaker could help?

> As I pointed out in a previous comment to the series this sort of
> debug code should either be converted to use "test" or we should
> incorporate it into test-lib.sh and use it everywhere.
>
> I somewhat like the latter. It's sometimes hard to see what's going
> wrong with our tests. It'd also translate to TAP subtests.

I'll do both: use test -f in a first patch, and propose alternative in
the second. New patch serie follows.

I don't know TAP and TAP subtests, but if the functions exist, other
patches can be added on top to improve them.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH] rebase -i: add exec command to launch a shell command
  2010-08-05 13:00 [PATCH] rebase -i: add exec " Matthieu Moy
@ 2010-08-05 13:31 ` Ævar Arnfjörð Bjarmason
  2010-08-05 16:47   ` Matthieu Moy
  0 siblings, 1 reply; 33+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-05 13:31 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git, gitster

On Thu, Aug 5, 2010 at 13:00, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> The typical usage pattern would be to run a test (or simply a compilation
> command) at given points in history.
>
> The shell command is ran (from the worktree root), and the rebase is
> stopped when the command fails, to give the user an opportunity to fix
> the problem before continuing with "git rebase --continue".
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>
> So, back to the "run from tree root", but that't now properly
> documented and tested.
>
> One notable difference with my first version is that the command is
> ran in a subshell, defaulting to $SHELL (typically for users like me
> with $SHELL=zsh who may want to take advantage of their shell's
> advanced features)
>
>  Documentation/git-rebase.txt  |   24 +++++++++++++++++++
>  git-rebase--interactive.sh    |   20 ++++++++++++++++
>  t/lib-rebase.sh               |    2 +
>  t/t3404-rebase-interactive.sh |   50 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 96 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index be23ad2..4bd4b66 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -459,6 +459,30 @@ sure that the current HEAD is "B", and call
>  $ git rebase -i -p --onto Q O
>  -----------------------------
>
> +Reordering and editing commits usually creates untested intermediate
> +steps.  You may want to check that your history editing did not break
> +anything by running a test, or at least recompiling at intermediate
> +points in history by using the "exec" command (shortcut "x").  You may
> +do so by creating a todo list like this one:
> +
> +-------------------------------------------
> +pick deadbee Implement feature XXX
> +fixup f1a5c00 Fix to feature XXX
> +exec make
> +pick c0ffeee The oneline of the next commit
> +edit deadbab The oneline of the commit after
> +exec cd subdir; make test
> +...
> +-------------------------------------------
> +
> +The interactive rebase will stop when a command fails (i.e. exits with
> +non-0 status) to give you an opportunity to fix the problem. You can
> +continue with `git rebase --continue`.
> +
> +The "exec" command launches the command in a shell (the one specified
> +in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
> +use usual shell commands like "cd". The command is run from the

I think that needs a definite article: ".. use the usual ..".

> +# debugging-friendly alternatives to "test -f" and "test ! -f"
> +file_must_exist () {
> +    if ! [ -f "$1" ]; then
> +       echo "file $1 not created."
> +       false
> +    fi
> +}
> +
> +file_must_not_exist () {
> +    if [ -f "$1" ]; then
> +       echo "file $1 created while it shouldn't have. $2"
> +       false
> +    fi
> +}

As I pointed out in a previous comment to the series this sort of
debug code should either be converted to use "test" or we should
incorporate it into test-lib.sh and use it everywhere.

I somewhat like the latter. It's sometimes hard to see what's going
wrong with our tests. It'd also translate to TAP subtests.

Otherwise this all looks good. Especially without the fragile mkdir/chdir
part present in a previous submission.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH] rebase -i: add exec command to launch a shell command
@ 2010-08-05 13:00 Matthieu Moy
  2010-08-05 13:31 ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 33+ messages in thread
From: Matthieu Moy @ 2010-08-05 13:00 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy

The typical usage pattern would be to run a test (or simply a compilation
command) at given points in history.

The shell command is ran (from the worktree root), and the rebase is
stopped when the command fails, to give the user an opportunity to fix
the problem before continuing with "git rebase --continue".

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---

So, back to the "run from tree root", but that't now properly
documented and tested.

One notable difference with my first version is that the command is
ran in a subshell, defaulting to $SHELL (typically for users like me
with $SHELL=zsh who may want to take advantage of their shell's
advanced features)

 Documentation/git-rebase.txt  |   24 +++++++++++++++++++
 git-rebase--interactive.sh    |   20 ++++++++++++++++
 t/lib-rebase.sh               |    2 +
 t/t3404-rebase-interactive.sh |   50 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 96 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index be23ad2..4bd4b66 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -459,6 +459,30 @@ sure that the current HEAD is "B", and call
 $ git rebase -i -p --onto Q O
 -----------------------------
 
+Reordering and editing commits usually creates untested intermediate
+steps.  You may want to check that your history editing did not break
+anything by running a test, or at least recompiling at intermediate
+points in history by using the "exec" command (shortcut "x").  You may
+do so by creating a todo list like this one:
+
+-------------------------------------------
+pick deadbee Implement feature XXX
+fixup f1a5c00 Fix to feature XXX
+exec make
+pick c0ffeee The oneline of the next commit
+edit deadbab The oneline of the commit after
+exec cd subdir; make test
+...
+-------------------------------------------
+
+The interactive rebase will stop when a command fails (i.e. exits with
+non-0 status) to give you an opportunity to fix the problem. You can
+continue with `git rebase --continue`.
+
+The "exec" command launches the command in a shell (the one specified
+in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
+use usual shell commands like "cd". The command is run from the
+root of the working tree.
 
 SPLITTING COMMITS
 -----------------
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b94c2a0..33d3087 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -537,6 +537,25 @@ do_next () {
 		esac
 		record_in_rewritten $sha1
 		;;
+	x|"exec")
+		read -r command rest < "$TODO"
+		mark_action_done
+		printf 'Executing: %s\n' "$rest"
+		# "exec" command doesn't take a sha1 in the todo-list.
+		# => can't just use $sha1 here.
+		git rev-parse --verify HEAD > "$DOTEST"/stopped-sha
+		${SHELL:-@SHELL_PATH@} -c "$rest" # Actual execution
+		status=$?
+		if test "$status" -ne 0
+		then
+			warn "Execution failed: $rest"
+			warn "You can fix the problem, and then run"
+			warn
+			warn "	git rebase --continue"
+			warn
+			exit "$status"
+		fi
+		;;
 	*)
 		warn "Unknown command: $command $sha1 $rest"
 		if git rev-parse --verify -q "$sha1" >/dev/null
@@ -957,6 +976,7 @@ first and then run 'git rebase --continue' again."
 #  e, edit = use commit, but stop for amending
 #  s, squash = use commit, but meld into previous commit
 #  f, fixup = like "squash", but discard this commit's log message
+#  x <cmd>, exec <cmd> = Run a shell command <cmd>, and stop if it fails
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6aefe27..6ccf797 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -47,6 +47,8 @@ for line in $FAKE_LINES; do
 	case $line in
 	squash|fixup|edit|reword)
 		action="$line";;
+	exec*)
+		echo "$line" | sed 's/_/ /g' >> "$1";;
 	"#")
 		echo '# comment' >> "$1";;
 	">")
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 9f03ce6..3b07850 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -64,6 +64,56 @@ test_expect_success 'setup' '
 	done
 '
 
+# debugging-friendly alternatives to "test -f" and "test ! -f"
+file_must_exist () {
+    if ! [ -f "$1" ]; then
+	echo "file $1 not created."
+	false
+    fi
+}
+
+file_must_not_exist () {
+    if [ -f "$1" ]; then
+	echo "file $1 created while it shouldn't have. $2"
+	false
+    fi
+}
+
+test_expect_success 'rebase -i with the exec command' '
+	git checkout master &&
+	FAKE_LINES="1 exec_touch_touch-one 2 exec_touch_touch-two exec_false exec_touch_touch-three 3 4
+		exec_touch_\"touch-file__name_with_spaces\";_touch_touch-after-semicolon 5" \
+		test_must_fail git rebase -i A &&
+	file_must_exist touch-one &&
+	file_must_exist touch-two &&
+	file_must_not_exist touch-three "(Should have stopped before)" &&
+	test $(git rev-parse C) = $(git rev-parse HEAD) || {
+		echo "Stopped at wrong revision:"
+		echo "($(git describe --tags HEAD) instead of C)"
+		false
+	} &&
+	git rebase --continue &&
+	file_must_exist touch-three &&
+	file_must_exist "touch-file  name with spaces" &&
+	file_must_exist touch-after-semicolon &&
+	test $(git rev-parse master) = $(git rev-parse HEAD) || {
+		echo "Stopped at wrong revision:"
+		echo "($(git describe --tags HEAD) instead of master)"
+		false
+	} &&
+	rm -f touch-*
+'
+
+test_expect_success 'rebase -i with the exec command runs from tree root' '
+	git checkout master &&
+	mkdir subdir && cd subdir &&
+	FAKE_LINES="1 exec_touch_touch-subdir" \
+		git rebase -i HEAD^ &&
+	cd .. &&
+	file_must_exist touch-subdir &&
+	rm -fr subdir
+'
+
 test_expect_success 'no changes are a nop' '
 	git checkout branch2 &&
 	git rebase -i F &&
-- 
1.7.2.1.30.g18195

^ permalink raw reply related	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2010-08-07 12:13 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-28 13:29 [RFC/PATCH] rebase -i: add run command to launch a shell command Matthieu Moy
2010-07-28 14:12 ` Santi Béjar
2010-07-28 14:26   ` Matthieu Moy
2010-07-30 10:04     ` Matthieu Moy
2010-07-30 14:51 ` Marc Branchaud
2010-07-30 15:24   ` Matthieu Moy
2010-07-30 18:26     ` Neal Kreitzinger
2010-07-31 13:27       ` Matthieu Moy
2010-07-31 13:56     ` Ævar Arnfjörð Bjarmason
2010-07-31 14:25       ` Miles Bader
2010-08-02 10:02       ` Matthieu Moy
2010-08-02 10:03         ` [PATCH] rebase -i: add exec " Matthieu Moy
2010-08-02 12:30           ` Jared Hance
2010-08-02 15:51           ` Ævar Arnfjörð Bjarmason
2010-08-06 21:07           ` Johannes Sixt
2010-08-07  8:48             ` Matthieu Moy
2010-08-07  8:56               ` [PATCH 1/2 (new version)] " Matthieu Moy
2010-08-07  8:56               ` [PATCH 2/2] test-lib: user-friendly alternatives to test [!] [-d|-f] Matthieu Moy
2010-08-02 15:04         ` [RFC/PATCH] rebase -i: add run command to launch a shell command Paolo Bonzini
2010-08-02 21:15         ` Junio C Hamano
2010-08-03  6:37           ` Matthieu Moy
2010-08-03  8:47       ` Kris Shannon
2010-08-03  9:16         ` Matthieu Moy
2010-07-31 15:28     ` Paolo Bonzini
2010-07-31 15:52       ` Ævar Arnfjörð Bjarmason
2010-07-31 18:54       ` Jared Hance
2010-07-31 14:40 ` Jared Hance
2010-08-05 13:00 [PATCH] rebase -i: add exec " Matthieu Moy
2010-08-05 13:31 ` Ævar Arnfjörð Bjarmason
2010-08-05 16:47   ` Matthieu Moy
2010-08-05 18:24     ` Erik Faye-Lund
2010-08-05 18:37     ` Jacob Helwig
2010-08-05 20:16       ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.