* [PATCH 0/5] rebase: improve the keep-empty
@ 2013-05-28 13:29 Felipe Contreras
2013-05-28 13:29 ` [PATCH 1/5] rebase: split the cherry-pick stuff Felipe Contreras
` (6 more replies)
0 siblings, 7 replies; 18+ messages in thread
From: Felipe Contreras @ 2013-05-28 13:29 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk, Felipe Contreras
Hi,
I've been analyzing 'git rebase' and found that the --keep-empty option
triggers a very very different behavior. Here's a bunch of patches that make it
behave like the 'am' does does for the most part.
There's only a few minor changes, after which it might be possible to replace
the whole 'am' mode to use cherr-pick instead.
Felipe Contreras (5):
rebase: split the cherry-pick stuff
rebase: fix 'cherry' mode storage
rebase: fix sequence continuation
rebase: fix abort of cherry mode
rebase: fix cherry-pick invocations
.gitignore | 1 +
Makefile | 1 +
git-rebase--am.sh | 65 ++++++++++++++++++++++-----------------------------
git-rebase--cherry.sh | 55 +++++++++++++++++++++++++++++++++++++++++++
git-rebase.sh | 8 +++++++
5 files changed, 93 insertions(+), 37 deletions(-)
create mode 100644 git-rebase--cherry.sh
--
1.8.3.rc3.312.g47657de
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/5] rebase: split the cherry-pick stuff
2013-05-28 13:29 [PATCH 0/5] rebase: improve the keep-empty Felipe Contreras
@ 2013-05-28 13:29 ` Felipe Contreras
2013-05-28 22:24 ` Junio C Hamano
2013-05-28 13:29 ` [PATCH 2/5] rebase: fix 'cherry' mode storage Felipe Contreras
` (5 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Felipe Contreras @ 2013-05-28 13:29 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk, Felipe Contreras
They do something completely different from 'git am', it belongs in a
different file.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
.gitignore | 1 +
Makefile | 1 +
git-rebase--am.sh | 65 ++++++++++++++++++++++-----------------------------
git-rebase--cherry.sh | 34 +++++++++++++++++++++++++++
git-rebase.sh | 4 ++++
5 files changed, 68 insertions(+), 37 deletions(-)
create mode 100644 git-rebase--cherry.sh
diff --git a/.gitignore b/.gitignore
index 6669bf0..284fc8f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -113,6 +113,7 @@
/git-read-tree
/git-rebase
/git-rebase--am
+/git-rebase--cherry
/git-rebase--interactive
/git-rebase--merge
/git-receive-pack
diff --git a/Makefile b/Makefile
index 0f931a2..a3cd4bc 100644
--- a/Makefile
+++ b/Makefile
@@ -469,6 +469,7 @@ SCRIPT_SH += git-web--browse.sh
SCRIPT_LIB += git-mergetool--lib
SCRIPT_LIB += git-parse-remote
SCRIPT_LIB += git-rebase--am
+SCRIPT_LIB += git-rebase--cherry
SCRIPT_LIB += git-rebase--interactive
SCRIPT_LIB += git-rebase--merge
SCRIPT_LIB += git-sh-setup
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index f84854f..ee1b1b9 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -19,52 +19,43 @@ esac
test -n "$rebase_root" && root_flag=--root
ret=0
-if test -n "$keep_empty"
-then
- # we have to do this the hard way. git format-patch completely squashes
- # empty commits and even if it didn't the format doesn't really lend
- # itself well to recording empty patches. fortunately, cherry-pick
- # makes this easy
- git cherry-pick --allow-empty "$revisions"
- ret=$?
-else
- rm -f "$GIT_DIR/rebased-patches"
-
- git format-patch -k --stdout --full-index --ignore-if-in-upstream \
- --src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
- $root_flag "$revisions" >"$GIT_DIR/rebased-patches"
- ret=$?
- if test 0 != $ret
- then
- rm -f "$GIT_DIR/rebased-patches"
- case "$head_name" in
- refs/heads/*)
- git checkout -q "$head_name"
- ;;
- *)
- git checkout -q "$orig_head"
- ;;
- esac
+rm -f "$GIT_DIR/rebased-patches"
- cat >&2 <<-EOF
+git format-patch -k --stdout --full-index --ignore-if-in-upstream \
+ --src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
+ $root_flag "$revisions" >"$GIT_DIR/rebased-patches"
+ret=$?
- git encountered an error while preparing the patches to replay
- these revisions:
+if test 0 != $ret
+then
+ rm -f "$GIT_DIR/rebased-patches"
+ case "$head_name" in
+ refs/heads/*)
+ git checkout -q "$head_name"
+ ;;
+ *)
+ git checkout -q "$orig_head"
+ ;;
+ esac
- $revisions
+ cat >&2 <<-EOF
- As a result, git cannot rebase them.
- EOF
- exit $?
- fi
+ git encountered an error while preparing the patches to replay
+ these revisions:
- git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" <"$GIT_DIR/rebased-patches"
- ret=$?
+ $revisions
- rm -f "$GIT_DIR/rebased-patches"
+ As a result, git cannot rebase them.
+ EOF
+ exit $?
fi
+git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" <"$GIT_DIR/rebased-patches"
+ret=$?
+
+rm -f "$GIT_DIR/rebased-patches"
+
if test 0 != $ret
then
test -d "$state_dir" && write_basic_state
diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh
new file mode 100644
index 0000000..cbf80f9
--- /dev/null
+++ b/git-rebase--cherry.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Junio C Hamano.
+#
+
+case "$action" in
+continue)
+ git am --resolved --resolvemsg="$resolvemsg" &&
+ move_to_original_branch
+ exit
+ ;;
+skip)
+ git am --skip --resolvemsg="$resolvemsg" &&
+ move_to_original_branch
+ exit
+ ;;
+esac
+
+test -n "$rebase_root" && root_flag=--root
+
+# we have to do this the hard way. git format-patch completely squashes
+# empty commits and even if it didn't the format doesn't really lend
+# itself well to recording empty patches. fortunately, cherry-pick
+# makes this easy
+git cherry-pick --allow-empty "$revisions"
+ret=$?
+
+if test 0 != $ret
+then
+ test -d "$state_dir" && write_basic_state
+ exit $ret
+fi
+
+move_to_original_branch
diff --git a/git-rebase.sh b/git-rebase.sh
index 2c692c3..2754985 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -379,6 +379,10 @@ elif test -n "$do_merge"
then
type=merge
state_dir="$merge_dir"
+elif test -n "$keep_empty"
+then
+ type=cherry
+ state_dir="$apply_dir"
else
type=am
state_dir="$apply_dir"
--
1.8.3.rc3.312.g47657de
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/5] rebase: fix 'cherry' mode storage
2013-05-28 13:29 [PATCH 0/5] rebase: improve the keep-empty Felipe Contreras
2013-05-28 13:29 ` [PATCH 1/5] rebase: split the cherry-pick stuff Felipe Contreras
@ 2013-05-28 13:29 ` Felipe Contreras
2013-05-28 22:29 ` Junio C Hamano
2013-05-28 13:29 ` [PATCH 3/5] rebase: fix sequence continuation Felipe Contreras
` (4 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Felipe Contreras @ 2013-05-28 13:29 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk, Felipe Contreras
We don't use the 'rebase-apply'.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
git-rebase--cherry.sh | 4 ++++
git-rebase.sh | 5 ++++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh
index cbf80f9..ab1f8b7 100644
--- a/git-rebase--cherry.sh
+++ b/git-rebase--cherry.sh
@@ -18,6 +18,9 @@ esac
test -n "$rebase_root" && root_flag=--root
+mkdir "$state_dir" || die "Could not create temporary $state_dir"
+: > "$state_dir"/cherry || die "Could not mark as cherry"
+
# we have to do this the hard way. git format-patch completely squashes
# empty commits and even if it didn't the format doesn't really lend
# itself well to recording empty patches. fortunately, cherry-pick
@@ -32,3 +35,4 @@ then
fi
move_to_original_branch
+rm -rf "$state_dir"
diff --git a/git-rebase.sh b/git-rebase.sh
index 2754985..b7759d5 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -174,6 +174,9 @@ then
then
type=interactive
interactive_rebase=explicit
+ elif test -f "$merge_dir"/cherry
+ then
+ type=cherry
else
type=merge
fi
@@ -382,7 +385,7 @@ then
elif test -n "$keep_empty"
then
type=cherry
- state_dir="$apply_dir"
+ state_dir="$merge_dir"
else
type=am
state_dir="$apply_dir"
--
1.8.3.rc3.312.g47657de
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/5] rebase: fix sequence continuation
2013-05-28 13:29 [PATCH 0/5] rebase: improve the keep-empty Felipe Contreras
2013-05-28 13:29 ` [PATCH 1/5] rebase: split the cherry-pick stuff Felipe Contreras
2013-05-28 13:29 ` [PATCH 2/5] rebase: fix 'cherry' mode storage Felipe Contreras
@ 2013-05-28 13:29 ` Felipe Contreras
2013-05-28 22:31 ` Junio C Hamano
2013-05-28 13:29 ` [PATCH 4/5] rebase: fix abort of cherry mode Felipe Contreras
` (3 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Felipe Contreras @ 2013-05-28 13:29 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk, Felipe Contreras
We are not in am mode.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
git-rebase--cherry.sh | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh
index ab1f8b7..ca78b1b 100644
--- a/git-rebase--cherry.sh
+++ b/git-rebase--cherry.sh
@@ -5,13 +5,15 @@
case "$action" in
continue)
- git am --resolved --resolvemsg="$resolvemsg" &&
- move_to_original_branch
+ git cherry-pick --continue &&
+ move_to_original_branch &&
+ rm -rf "$state_dir"
exit
;;
skip)
- git am --skip --resolvemsg="$resolvemsg" &&
- move_to_original_branch
+ git cherry-pick --skip &&
+ move_to_original_branch &&
+ rm -rf "$state_dir"
exit
;;
esac
--
1.8.3.rc3.312.g47657de
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/5] rebase: fix abort of cherry mode
2013-05-28 13:29 [PATCH 0/5] rebase: improve the keep-empty Felipe Contreras
` (2 preceding siblings ...)
2013-05-28 13:29 ` [PATCH 3/5] rebase: fix sequence continuation Felipe Contreras
@ 2013-05-28 13:29 ` Felipe Contreras
2013-05-28 13:29 ` [PATCH 5/5] rebase: fix cherry-pick invocations Felipe Contreras
` (2 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Felipe Contreras @ 2013-05-28 13:29 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk, Felipe Contreras
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
git-rebase.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/git-rebase.sh b/git-rebase.sh
index b7759d5..48bd1b8 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -335,6 +335,7 @@ skip)
run_specific_rebase
;;
abort)
+ test "$type" == "cherry" && git cherry-pick --abort
git rerere clear
read_basic_state
case "$head_name" in
--
1.8.3.rc3.312.g47657de
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/5] rebase: fix cherry-pick invocations
2013-05-28 13:29 [PATCH 0/5] rebase: improve the keep-empty Felipe Contreras
` (3 preceding siblings ...)
2013-05-28 13:29 ` [PATCH 4/5] rebase: fix abort of cherry mode Felipe Contreras
@ 2013-05-28 13:29 ` Felipe Contreras
2013-05-28 22:46 ` Junio C Hamano
2013-05-28 13:33 ` [PATCH 0/5] rebase: improve the keep-empty Felipe Contreras
2013-05-28 17:17 ` Martin von Zweigbergk
6 siblings, 1 reply; 18+ messages in thread
From: Felipe Contreras @ 2013-05-28 13:29 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk, Felipe Contreras
So that all the tests pass.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
git-rebase--cherry.sh | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh
index ca78b1b..c3a2ac9 100644
--- a/git-rebase--cherry.sh
+++ b/git-rebase--cherry.sh
@@ -23,11 +23,26 @@ test -n "$rebase_root" && root_flag=--root
mkdir "$state_dir" || die "Could not create temporary $state_dir"
: > "$state_dir"/cherry || die "Could not mark as cherry"
+if test -n "$rebase_root"
+then
+ revisions="$onto...$orig_head"
+else
+ revisions="$upstream...$orig_head"
+fi
+
# we have to do this the hard way. git format-patch completely squashes
# empty commits and even if it didn't the format doesn't really lend
# itself well to recording empty patches. fortunately, cherry-pick
# makes this easy
-git cherry-pick --allow-empty "$revisions"
+if test -n "$keep_empty"
+then
+ extra="--allow-empty"
+else
+ extra="--skip-empty --cherry-pick"
+fi
+test -n "$GIT_QUIET" && extra="$extra -q"
+test -z "$force_rebase" && extra="$extra --ff"
+git cherry-pick --no-merges --right-only --topo-order --do-walk --copy-notes $extra "$revisions"
ret=$?
if test 0 != $ret
--
1.8.3.rc3.312.g47657de
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 0/5] rebase: improve the keep-empty
2013-05-28 13:29 [PATCH 0/5] rebase: improve the keep-empty Felipe Contreras
` (4 preceding siblings ...)
2013-05-28 13:29 ` [PATCH 5/5] rebase: fix cherry-pick invocations Felipe Contreras
@ 2013-05-28 13:33 ` Felipe Contreras
2013-05-28 17:17 ` Martin von Zweigbergk
6 siblings, 0 replies; 18+ messages in thread
From: Felipe Contreras @ 2013-05-28 13:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk, Felipe Contreras
On Tue, May 28, 2013 at 8:29 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Hi,
>
> I've been analyzing 'git rebase' and found that the --keep-empty option
> triggers a very very different behavior. Here's a bunch of patches that make it
> behave like the 'am' does does for the most part.
>
> There's only a few minor changes, after which it might be possible to replace
> the whole 'am' mode to use cherr-pick instead.
>
> Felipe Contreras (5):
> rebase: split the cherry-pick stuff
> rebase: fix 'cherry' mode storage
> rebase: fix sequence continuation
> rebase: fix abort of cherry mode
> rebase: fix cherry-pick invocations
To see the issue, simply enable keep_mode by default, and run the tests:
Test Summary Report
-------------------
./t3401-rebase-partial.sh (Wstat: 256
Tests: 7 Failed: 4)
Failed tests: 3, 5-7
Non-zero exit status: 1
./t3403-rebase-skip.sh (Wstat: 256
Tests: 10 Failed: 3)
Failed tests: 4-5, 9
Non-zero exit status: 1
./t3406-rebase-message.sh (Wstat: 256
Tests: 6 Failed: 3)
Failed tests: 3-5
Non-zero exit status: 1
./t3400-rebase.sh (Wstat: 256
Tests: 27 Failed: 10)
Failed tests: 13-17, 22-25, 27
Non-zero exit status: 1
./t3407-rebase-abort.sh (Wstat: 256
Tests: 11 Failed: 5)
Failed tests: 2-6
Non-zero exit status: 1
./t3412-rebase-root.sh (Wstat: 256
Tests: 31 Failed: 17)
Failed tests: 4, 6-8, 10-12, 14-17, 23, 25-26, 28-29
31
Non-zero exit status: 1
./t3417-rebase-whitespace-fix.sh (Wstat: 256
Tests: 4 Failed: 4)
Failed tests: 1-4
Non-zero exit status: 1
./t3419-rebase-patch-id.sh (Wstat: 256
Tests: 9 Failed: 1)
Failed test: 4
Non-zero exit status: 1
./t3418-rebase-continue.sh (Wstat: 256
Tests: 6 Failed: 2)
Failed tests: 3, 5
Non-zero exit status: 1
./t5407-post-rewrite-hook.sh (Wstat: 256
Tests: 13 Failed: 4)
Failed tests: 4-6, 8
Non-zero exit status: 1
./t5520-pull.sh (Wstat: 256
Tests: 21 Failed: 8)
Failed tests: 12-15, 18-21
Non-zero exit status: 1
./t7512-status-help.sh (Wstat: 256
Tests: 35 Failed: 3)
Failed tests: 5-6, 28
Non-zero exit status: 1
./t9106-git-svn-commit-diff-clobber.sh (Wstat: 256
Tests: 10 Failed: 3)
Failed tests: 6, 9-10
Non-zero exit status: 1
./t3404-rebase-interactive.sh (Wstat: 256
Tests: 71 Failed: 2)
Failed tests: 50, 69
Non-zero exit status: 1
./t9137-git-svn-dcommit-clobber-series.sh (Wstat: 256
Tests: 5 Failed: 1)
Failed test: 4
Non-zero exit status: 1
./t9140-git-svn-reset.sh (Wstat: 256
Tests: 6 Failed: 1)
Failed test: 6
Non-zero exit status: 1
./t9903-bash-prompt.sh (Wstat: 256
Tests: 51 Failed: 20)
Failed tests: 27-34, 36-39, 41-42, 45-49, 51
Non-zero exit status: 1
./t9164-git-svn-dcommit-concurrent.sh (Wstat: 256
Tests: 8 Failed: 4)
Failed tests: 4-6, 8
Non-zero exit status: 1
Files=629, Tests=10036, 308 wallclock secs ( 7.60 usr 1.39 sys +
556.35 cusr 382.46 csys = 947.80 CPU)
Result: FAIL
--
Felipe Contreras
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/5] rebase: improve the keep-empty
2013-05-28 13:29 [PATCH 0/5] rebase: improve the keep-empty Felipe Contreras
` (5 preceding siblings ...)
2013-05-28 13:33 ` [PATCH 0/5] rebase: improve the keep-empty Felipe Contreras
@ 2013-05-28 17:17 ` Martin von Zweigbergk
2013-05-29 3:01 ` Felipe Contreras
6 siblings, 1 reply; 18+ messages in thread
From: Martin von Zweigbergk @ 2013-05-28 17:17 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano
Hi,
I think I have some patches at home that instead teach 'git am' the
--keep-empty flag. Does that make sense? It's been a while since I
looked at it, but I'll try to take a look tonight (PST).
Martin
On Tue, May 28, 2013 at 6:29 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Hi,
>
> I've been analyzing 'git rebase' and found that the --keep-empty option
> triggers a very very different behavior. Here's a bunch of patches that make it
> behave like the 'am' does does for the most part.
>
> There's only a few minor changes, after which it might be possible to replace
> the whole 'am' mode to use cherr-pick instead.
>
> Felipe Contreras (5):
> rebase: split the cherry-pick stuff
> rebase: fix 'cherry' mode storage
> rebase: fix sequence continuation
> rebase: fix abort of cherry mode
> rebase: fix cherry-pick invocations
>
> .gitignore | 1 +
> Makefile | 1 +
> git-rebase--am.sh | 65 ++++++++++++++++++++++-----------------------------
> git-rebase--cherry.sh | 55 +++++++++++++++++++++++++++++++++++++++++++
> git-rebase.sh | 8 +++++++
> 5 files changed, 93 insertions(+), 37 deletions(-)
> create mode 100644 git-rebase--cherry.sh
>
> --
> 1.8.3.rc3.312.g47657de
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] rebase: split the cherry-pick stuff
2013-05-28 13:29 ` [PATCH 1/5] rebase: split the cherry-pick stuff Felipe Contreras
@ 2013-05-28 22:24 ` Junio C Hamano
0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2013-05-28 22:24 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Martin von Zweigbergk
Felipe Contreras <felipe.contreras@gmail.com> writes:
> They do something completely different from 'git am', it belongs in a
> different file.
I would prefer to see it called --cherry-pick, not --cherry, as they
are different commands (the latter may be useful when deciding which
one to use the former).
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> .gitignore | 1 +
> Makefile | 1 +
> git-rebase--am.sh | 65 ++++++++++++++++++++++-----------------------------
> git-rebase--cherry.sh | 34 +++++++++++++++++++++++++++
> git-rebase.sh | 4 ++++
> 5 files changed, 68 insertions(+), 37 deletions(-)
> create mode 100644 git-rebase--cherry.sh
>
> diff --git a/.gitignore b/.gitignore
> index 6669bf0..284fc8f 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -113,6 +113,7 @@
> /git-read-tree
> /git-rebase
> /git-rebase--am
> +/git-rebase--cherry
> /git-rebase--interactive
> /git-rebase--merge
> /git-receive-pack
> diff --git a/Makefile b/Makefile
> index 0f931a2..a3cd4bc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -469,6 +469,7 @@ SCRIPT_SH += git-web--browse.sh
> SCRIPT_LIB += git-mergetool--lib
> SCRIPT_LIB += git-parse-remote
> SCRIPT_LIB += git-rebase--am
> +SCRIPT_LIB += git-rebase--cherry
> SCRIPT_LIB += git-rebase--interactive
> SCRIPT_LIB += git-rebase--merge
> SCRIPT_LIB += git-sh-setup
> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
> index f84854f..ee1b1b9 100644
> --- a/git-rebase--am.sh
> +++ b/git-rebase--am.sh
> @@ -19,52 +19,43 @@ esac
> test -n "$rebase_root" && root_flag=--root
>
> ret=0
> +rm -f "$GIT_DIR/rebased-patches"
>
> - cat >&2 <<-EOF
> +git format-patch -k --stdout --full-index --ignore-if-in-upstream \
> + --src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
> + $root_flag "$revisions" >"$GIT_DIR/rebased-patches"
> +ret=$?
>
> +if test 0 != $ret
> +then
> + rm -f "$GIT_DIR/rebased-patches"
> + case "$head_name" in
> + refs/heads/*)
> + git checkout -q "$head_name"
> + ;;
> + *)
> + git checkout -q "$orig_head"
> + ;;
> + esac
>
> - $revisions
> + cat >&2 <<-EOF
>
> + git encountered an error while preparing the patches to replay
> + these revisions:
>
> + $revisions
>
> + As a result, git cannot rebase them.
> + EOF
> + exit $?
> fi
>
> +git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" <"$GIT_DIR/rebased-patches"
> +ret=$?
> +
> +rm -f "$GIT_DIR/rebased-patches"
> +
> if test 0 != $ret
> then
> test -d "$state_dir" && write_basic_state
OK, this part seems a straightforward reindentation of "else" side.
> diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh
> new file mode 100644
> index 0000000..cbf80f9
> --- /dev/null
> +++ b/git-rebase--cherry.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2010 Junio C Hamano.
> +#
> +
> +case "$action" in
> +continue)
> + git am --resolved --resolvemsg="$resolvemsg" &&
> + move_to_original_branch
> + exit
> + ;;
> +skip)
> + git am --skip --resolvemsg="$resolvemsg" &&
> + move_to_original_branch
> + exit
> + ;;
> +esac
> +
> +test -n "$rebase_root" && root_flag=--root
Hmm, do we want to duplicate this part between the two? Perhaps it
will be unified in a later patch---I'll read on.
> +# we have to do this the hard way. git format-patch completely squashes
> +# empty commits and even if it didn't the format doesn't really lend
> +# itself well to recording empty patches. fortunately, cherry-pick
> +# makes this easy
> +git cherry-pick --allow-empty "$revisions"
> +ret=$?
> +
> +if test 0 != $ret
> +then
> + test -d "$state_dir" && write_basic_state
> + exit $ret
> +fi
> +
> +move_to_original_branch
OK. This is the "then" side of the original, with clean-up phase
copied.
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 2c692c3..2754985 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -379,6 +379,10 @@ elif test -n "$do_merge"
> then
> type=merge
> state_dir="$merge_dir"
> +elif test -n "$keep_empty"
> +then
> + type=cherry
> + state_dir="$apply_dir"
> else
> type=am
> state_dir="$apply_dir"
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] rebase: fix 'cherry' mode storage
2013-05-28 13:29 ` [PATCH 2/5] rebase: fix 'cherry' mode storage Felipe Contreras
@ 2013-05-28 22:29 ` Junio C Hamano
2013-05-29 3:03 ` Felipe Contreras
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2013-05-28 22:29 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Martin von Zweigbergk
Felipe Contreras <felipe.contreras@gmail.com> writes:
> We don't use the 'rebase-apply'.
s/.$/; we will use rebase-merge instead./ I think.
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> git-rebase--cherry.sh | 4 ++++
> git-rebase.sh | 5 ++++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh
> index cbf80f9..ab1f8b7 100644
> --- a/git-rebase--cherry.sh
> +++ b/git-rebase--cherry.sh
> @@ -18,6 +18,9 @@ esac
>
> test -n "$rebase_root" && root_flag=--root
>
> +mkdir "$state_dir" || die "Could not create temporary $state_dir"
> +: > "$state_dir"/cherry || die "Could not mark as cherry"
Style:
>"$state_dir/cherry-pick"
I am not sure if the user _cares_ that internally we use cherry-pick
when s/he asks us to do a keep-empty, and I suspect "mark as cherry"
incomprehensible. I do not even know what is going on at this point
(yet).
I _suspect_ that you are using cherry-pick when --keep-empty is given,
so it might even make sense to talk in the end-users' terms,
i.e. call this "$state_dir/keep-empty" (I am not sure if calling
this split half "git-rebase--keep-empty.sh" makes sense, yet).
> +
> # we have to do this the hard way. git format-patch completely squashes
> # empty commits and even if it didn't the format doesn't really lend
> # itself well to recording empty patches. fortunately, cherry-pick
> @@ -32,3 +35,4 @@ then
> fi
>
> move_to_original_branch
> +rm -rf "$state_dir"
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 2754985..b7759d5 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -174,6 +174,9 @@ then
> then
> type=interactive
> interactive_rebase=explicit
> + elif test -f "$merge_dir"/cherry
> + then
> + type=cherry
> else
> type=merge
> fi
> @@ -382,7 +385,7 @@ then
> elif test -n "$keep_empty"
> then
> type=cherry
> - state_dir="$apply_dir"
> + state_dir="$merge_dir"
> else
> type=am
> state_dir="$apply_dir"
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/5] rebase: fix sequence continuation
2013-05-28 13:29 ` [PATCH 3/5] rebase: fix sequence continuation Felipe Contreras
@ 2013-05-28 22:31 ` Junio C Hamano
2013-05-29 5:53 ` Felipe Contreras
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2013-05-28 22:31 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Martin von Zweigbergk
Felipe Contreras <felipe.contreras@gmail.com> writes:
> We are not in am mode.
That may make sense, but shouldn't this part be like so from the
very beginning? In other words, this looks like an "oops, 1/5 was
buggy and this is a hotfix".
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> git-rebase--cherry.sh | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh
> index ab1f8b7..ca78b1b 100644
> --- a/git-rebase--cherry.sh
> +++ b/git-rebase--cherry.sh
> @@ -5,13 +5,15 @@
>
> case "$action" in
> continue)
> - git am --resolved --resolvemsg="$resolvemsg" &&
> - move_to_original_branch
> + git cherry-pick --continue &&
> + move_to_original_branch &&
> + rm -rf "$state_dir"
> exit
> ;;
> skip)
> - git am --skip --resolvemsg="$resolvemsg" &&
> - move_to_original_branch
> + git cherry-pick --skip &&
> + move_to_original_branch &&
> + rm -rf "$state_dir"
> exit
> ;;
> esac
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] rebase: fix cherry-pick invocations
2013-05-28 13:29 ` [PATCH 5/5] rebase: fix cherry-pick invocations Felipe Contreras
@ 2013-05-28 22:46 ` Junio C Hamano
2013-05-28 22:51 ` Junio C Hamano
2013-05-29 5:55 ` Felipe Contreras
0 siblings, 2 replies; 18+ messages in thread
From: Junio C Hamano @ 2013-05-28 22:46 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Martin von Zweigbergk
Felipe Contreras <felipe.contreras@gmail.com> writes:
> So that all the tests pass.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> git-rebase--cherry.sh | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh
> index ca78b1b..c3a2ac9 100644
> --- a/git-rebase--cherry.sh
> +++ b/git-rebase--cherry.sh
> @@ -23,11 +23,26 @@ test -n "$rebase_root" && root_flag=--root
> mkdir "$state_dir" || die "Could not create temporary $state_dir"
> : > "$state_dir"/cherry || die "Could not mark as cherry"
>
> +if test -n "$rebase_root"
> +then
> + revisions="$onto...$orig_head"
> +else
> + revisions="$upstream...$orig_head"
> +fi
"So that all the tests pass" needs a bit more explanation to say for
cherry-pick codepath why and how two-dot range fails and why and how
three-dot variant with --right-only fixes it. What are the problematic
cases?
> # we have to do this the hard way. git format-patch completely squashes
> # empty commits and even if it didn't the format doesn't really lend
> # itself well to recording empty patches. fortunately, cherry-pick
> # makes this easy
> -git cherry-pick --allow-empty "$revisions"
> +if test -n "$keep_empty"
> +then
> + extra="--allow-empty"
> +else
> + extra="--skip-empty --cherry-pick"
> +fi
> +test -n "$GIT_QUIET" && extra="$extra -q"
> +test -z "$force_rebase" && extra="$extra --ff"
> +git cherry-pick --no-merges --right-only --topo-order --do-walk --copy-notes $extra "$revisions"
> ret=$?
>
> if test 0 != $ret
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] rebase: fix cherry-pick invocations
2013-05-28 22:46 ` Junio C Hamano
@ 2013-05-28 22:51 ` Junio C Hamano
2013-05-29 5:57 ` Felipe Contreras
2013-05-29 5:55 ` Felipe Contreras
1 sibling, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2013-05-28 22:51 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Martin von Zweigbergk
Junio C Hamano <gitster@pobox.com> writes:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> So that all the tests pass.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>> git-rebase--cherry.sh | 17 ++++++++++++++++-
>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh
>> index ca78b1b..c3a2ac9 100644
>> --- a/git-rebase--cherry.sh
>> +++ b/git-rebase--cherry.sh
>> @@ -23,11 +23,26 @@ test -n "$rebase_root" && root_flag=--root
>> mkdir "$state_dir" || die "Could not create temporary $state_dir"
>> : > "$state_dir"/cherry || die "Could not mark as cherry"
>>
>> +if test -n "$rebase_root"
>> +then
>> + revisions="$onto...$orig_head"
>> +else
>> + revisions="$upstream...$orig_head"
>> +fi
>
> "So that all the tests pass" needs a bit more explanation to say for
> cherry-pick codepath why and how two-dot range fails and why and how
> three-dot variant with --right-only fixes it. What are the problematic
> cases?
Yikes, sorry, this was me being slow. Walking A...B range with
right-only and --cherry applied will filter the duplicates, which is
wat you want, I think, and walking A..B range will not do the
filtering for you.
>> # we have to do this the hard way. git format-patch completely squashes
>> # empty commits and even if it didn't the format doesn't really lend
>> # itself well to recording empty patches. fortunately, cherry-pick
>> # makes this easy
>> -git cherry-pick --allow-empty "$revisions"
>> +if test -n "$keep_empty"
>> +then
>> + extra="--allow-empty"
>> +else
>> + extra="--skip-empty --cherry-pick"
>> +fi
>> +test -n "$GIT_QUIET" && extra="$extra -q"
>> +test -z "$force_rebase" && extra="$extra --ff"
>> +git cherry-pick --no-merges --right-only --topo-order --do-walk --copy-notes $extra "$revisions"
>> ret=$?
>>
>> if test 0 != $ret
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/5] rebase: improve the keep-empty
2013-05-28 17:17 ` Martin von Zweigbergk
@ 2013-05-29 3:01 ` Felipe Contreras
0 siblings, 0 replies; 18+ messages in thread
From: Felipe Contreras @ 2013-05-29 3:01 UTC (permalink / raw)
To: Martin von Zweigbergk, Felipe Contreras; +Cc: git, Junio C Hamano
Martin von Zweigbergk wrote:
> I think I have some patches at home that instead teach 'git am' the
> --keep-empty flag. Does that make sense? It's been a while since I
> looked at it, but I'll try to take a look tonight (PST).
I think it does make sense. But I still would prefer 'git rebase' to rely on
'git cherry-pick' primariliy.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] rebase: fix 'cherry' mode storage
2013-05-28 22:29 ` Junio C Hamano
@ 2013-05-29 3:03 ` Felipe Contreras
0 siblings, 0 replies; 18+ messages in thread
From: Felipe Contreras @ 2013-05-29 3:03 UTC (permalink / raw)
To: Junio C Hamano, Felipe Contreras; +Cc: git, Martin von Zweigbergk
Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
> > We don't use the 'rebase-apply'.
>
> s/.$/; we will use rebase-merge instead./ I think.
We could use 'rebase-apply' or any directory, but currently we don't use any,
and 'rebase-apply' is for 'git am'.
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > ---
> > git-rebase--cherry.sh | 4 ++++
> > git-rebase.sh | 5 ++++-
> > 2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh
> > index cbf80f9..ab1f8b7 100644
> > --- a/git-rebase--cherry.sh
> > +++ b/git-rebase--cherry.sh
> > @@ -18,6 +18,9 @@ esac
> >
> > test -n "$rebase_root" && root_flag=--root
> >
> > +mkdir "$state_dir" || die "Could not create temporary $state_dir"
> > +: > "$state_dir"/cherry || die "Could not mark as cherry"
>
> Style:
>
> >"$state_dir/cherry-pick"
Fix that in git-rebase--interactive.sh then, where this code came from.
> I am not sure if the user _cares_ that internally we use cherry-pick
> when s/he asks us to do a keep-empty, and I suspect "mark as cherry"
> incomprehensible. I do not even know what is going on at this point
> (yet).
It's the same we do in git-rebase--interactive.sh.
> I _suspect_ that you are using cherry-pick when --keep-empty is given,
> so it might even make sense to talk in the end-users' terms,
> i.e. call this "$state_dir/keep-empty" (I am not sure if calling
> this split half "git-rebase--keep-empty.sh" makes sense, yet).
If git-rebase--cherry(-pick)?.sh does the same as 'git-rebase--am.sh', we can
replace it, and make yet another step of replacing scripts (git am) with C code
(git cherry-pick). I thought I explained that somewhow, maybe I didn't.
It still makes sense to fix the broken --keep-empty behavior.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/5] rebase: fix sequence continuation
2013-05-28 22:31 ` Junio C Hamano
@ 2013-05-29 5:53 ` Felipe Contreras
0 siblings, 0 replies; 18+ messages in thread
From: Felipe Contreras @ 2013-05-29 5:53 UTC (permalink / raw)
To: Junio C Hamano, Felipe Contreras; +Cc: git, Martin von Zweigbergk
Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
> > We are not in am mode.
>
> That may make sense, but shouldn't this part be like so from the
> very beginning? In other words, this looks like an "oops, 1/5 was
> buggy and this is a hotfix".
How can 1/5 be introducing a bug? It's merely splitting the code without
introducing *ANY* functional changes.
The bug is already there in 'master'.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] rebase: fix cherry-pick invocations
2013-05-28 22:46 ` Junio C Hamano
2013-05-28 22:51 ` Junio C Hamano
@ 2013-05-29 5:55 ` Felipe Contreras
1 sibling, 0 replies; 18+ messages in thread
From: Felipe Contreras @ 2013-05-29 5:55 UTC (permalink / raw)
To: Junio C Hamano, Felipe Contreras; +Cc: git, Martin von Zweigbergk
Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
> > So that all the tests pass.
> >
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > ---
> > git-rebase--cherry.sh | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh
> > index ca78b1b..c3a2ac9 100644
> > --- a/git-rebase--cherry.sh
> > +++ b/git-rebase--cherry.sh
> > @@ -23,11 +23,26 @@ test -n "$rebase_root" && root_flag=--root
> > mkdir "$state_dir" || die "Could not create temporary $state_dir"
> > : > "$state_dir"/cherry || die "Could not mark as cherry"
> >
> > +if test -n "$rebase_root"
> > +then
> > + revisions="$onto...$orig_head"
> > +else
> > + revisions="$upstream...$orig_head"
> > +fi
>
> "So that all the tests pass" needs a bit more explanation to say for
> cherry-pick codepath why and how two-dot range fails and why and how
> three-dot variant with --right-only fixes it. What are the problematic
> cases?
There's too many failures to count. We are blindingly applying a series of
commits on top of another without checking if they are merges or if they
already exist in the destination branch.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] rebase: fix cherry-pick invocations
2013-05-28 22:51 ` Junio C Hamano
@ 2013-05-29 5:57 ` Felipe Contreras
0 siblings, 0 replies; 18+ messages in thread
From: Felipe Contreras @ 2013-05-29 5:57 UTC (permalink / raw)
To: Junio C Hamano, Felipe Contreras; +Cc: git, Martin von Zweigbergk
Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Felipe Contreras <felipe.contreras@gmail.com> writes:
> >
> >> So that all the tests pass.
> >>
> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> >> ---
> >> git-rebase--cherry.sh | 17 ++++++++++++++++-
> >> 1 file changed, 16 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh
> >> index ca78b1b..c3a2ac9 100644
> >> --- a/git-rebase--cherry.sh
> >> +++ b/git-rebase--cherry.sh
> >> @@ -23,11 +23,26 @@ test -n "$rebase_root" && root_flag=--root
> >> mkdir "$state_dir" || die "Could not create temporary $state_dir"
> >> : > "$state_dir"/cherry || die "Could not mark as cherry"
> >>
> >> +if test -n "$rebase_root"
> >> +then
> >> + revisions="$onto...$orig_head"
> >> +else
> >> + revisions="$upstream...$orig_head"
> >> +fi
> >
> > "So that all the tests pass" needs a bit more explanation to say for
> > cherry-pick codepath why and how two-dot range fails and why and how
> > three-dot variant with --right-only fixes it. What are the problematic
> > cases?
>
> Yikes, sorry, this was me being slow. Walking A...B range with
> right-only and --cherry applied will filter the duplicates, which is
> wat you want, I think, and walking A..B range will not do the
> filtering for you.
That's right.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2013-05-29 6:00 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-28 13:29 [PATCH 0/5] rebase: improve the keep-empty Felipe Contreras
2013-05-28 13:29 ` [PATCH 1/5] rebase: split the cherry-pick stuff Felipe Contreras
2013-05-28 22:24 ` Junio C Hamano
2013-05-28 13:29 ` [PATCH 2/5] rebase: fix 'cherry' mode storage Felipe Contreras
2013-05-28 22:29 ` Junio C Hamano
2013-05-29 3:03 ` Felipe Contreras
2013-05-28 13:29 ` [PATCH 3/5] rebase: fix sequence continuation Felipe Contreras
2013-05-28 22:31 ` Junio C Hamano
2013-05-29 5:53 ` Felipe Contreras
2013-05-28 13:29 ` [PATCH 4/5] rebase: fix abort of cherry mode Felipe Contreras
2013-05-28 13:29 ` [PATCH 5/5] rebase: fix cherry-pick invocations Felipe Contreras
2013-05-28 22:46 ` Junio C Hamano
2013-05-28 22:51 ` Junio C Hamano
2013-05-29 5:57 ` Felipe Contreras
2013-05-29 5:55 ` Felipe Contreras
2013-05-28 13:33 ` [PATCH 0/5] rebase: improve the keep-empty Felipe Contreras
2013-05-28 17:17 ` Martin von Zweigbergk
2013-05-29 3:01 ` Felipe Contreras
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.