{"thread":{"id":"14955","subject":"[BUG?] rebase -i -p leaves index changed","startedAt":"2008-08-12T09:16:39Z","lastAt":"2008-08-13T21:41:24Z","messageCount":10,"participants":["Thomas Rast","Stephan Beyer"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"86918","messageId":"200808121116.41535.trast@student.ethz.ch","threadId":"14955","inReplyTo":null,"subject":"[BUG?] rebase -i -p leaves index changed","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-08-12T09:16:39Z","receivedAt":"2008-08-12T09:16:39Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Hi,\n\n'rebase -i -p' appears to be a bit confused about what it is doing.\nTry a history like this:\n\n  O -- A -- M -- B\n   \\       /\n    \\- C -/\n\nbuilt by:\n\n  git init\n  touch foo\n  git add foo\n  git ci -m initial\n  git tag root\n  git co -b side\n  echo blah > other\n  git add other\n  git ci -m other\n  git co master\n  echo a >> foo\n  git add foo\n  git ci -m a\n  git add foo\n  git merge side\n  echo b >> foo\n  git add foo\n  git ci -m 'b: depends on side'\n\nFirst try this:\n\n  GIT_EDITOR=true git rebase -i -p root\n  git diff --cached\n\nresulting in\n\n  diff --git a/foo b/foo\n  index 422c2b7..e69de29 100644\n  --- a/foo\n  +++ b/foo\n  @@ -1,2 +0,0 @@\n  -a\n  -b\n  diff --git a/other b/other\n  deleted file mode 100644\n  index 907b308..0000000\n  --- a/other\n  +++ /dev/null\n  @@ -1 +0,0 @@\n  -blah\n\nSecond, 'edit' is also a bit suspicious:\n\n  git reset --hard\n  GIT_EDITOR='perl -i -pe \"s/pick 096/edit 096/\"' git rebase -i -p root\n\nDespite claiming \"Stopped at 096[...] a\", a quick 'git show' tells us\nthat it is actually stuck at 'initial'.  (At least 'git rebase\n--continue' then ends up with the same result as the first test.)\n\nGranted, I'm not entirely sure what it _should_ do.  In my use case\n(relating to the filter-branch topic), C was a commit from elsewhere\nthat B depended on.  So I kind of hoped 'rebase -i -p' would let me\nedit A, then rebuild M and B on top, while leaving C alone.\n\n- Thomas\n\n-- \nThomas Rast\ntrast@student.ethz.ch\n\n\n\n\n\n"},{"id":"87034","messageId":"8bbecf58e7fba14497720afe5cdaf52fec6bcb40.1218621695.git.trast@student.ethz.ch","threadId":"14955","inReplyTo":"200808121116.41535.trast@student.ethz.ch","subject":"[PATCH 1/2] rebase -i -p: handle index and workdir correctly","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-08-13T10:04:16Z","receivedAt":"2008-08-13T10:04:16Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"'git rebase -i -p' forgot to update the index and working directory\nduring fast forwards.  Fix this.  Restores plain 'git rebase -i -p' to\nbe a no-op.\n\nAlso, it attempted to do a fast forward even if it was instructed not\nto commit (via -n).  Fall back to the cherry-pick code path and let\nthat handle the issue for us.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n git-rebase--interactive.sh    |   14 ++++++++++++--\n t/t3404-rebase-interactive.sh |    6 ++++++\n 2 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 4e334ba..51a6147 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -145,7 +145,16 @@ pick_one () {\n }\n \n pick_one_preserving_merges () {\n-\tcase \"$1\" in -n) sha1=$2 ;; *) sha1=$1 ;; esac\n+\tfast_forward=t\n+\tcase \"$1\" in\n+\t-n)\n+\t\tfast_forward=f\n+\t\tsha1=$2\n+\t\t;;\n+\t*)\n+\t\tsha1=$1\n+\t\t;;\n+\tesac\n \tsha1=$(git rev-parse $sha1)\n \n \tif test -f \"$DOTEST\"/current-commit\n@@ -181,7 +190,8 @@ pick_one_preserving_merges () {\n \tcase $fast_forward in\n \tt)\n \t\toutput warn \"Fast forward to $sha1\"\n-\t\ttest $preserve = f || echo $sha1 > \"$REWRITTEN\"/$sha1\n+\t\toutput git reset --hard $sha1 ||\n+\t\t\tdie \"Cannot fast forward to $sha1\"\n \t\t;;\n \tf)\n \t\ttest \"a$1\" = a-n && die \"Refusing to squash a merge: $sha1\"\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex ffe3dd9..4d62b9a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -202,6 +202,9 @@ test_expect_success 'retain authorship when squashing' '\n test_expect_success '-p handles \"no changes\" gracefully' '\n \tHEAD=$(git rev-parse HEAD) &&\n \tgit rebase -i -p HEAD^ &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD -- &&\n \ttest $HEAD = $(git rev-parse HEAD)\n '\n \n@@ -235,6 +238,9 @@ test_expect_success 'preserve merges with -p' '\n \tgit checkout -b to-be-rebased &&\n \ttest_tick &&\n \tgit rebase -i -p --onto branch1 master &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD -- &&\n \ttest $(git rev-parse HEAD~6) = $(git rev-parse branch1) &&\n \ttest $(git rev-parse HEAD~4^2) = $(git rev-parse to-be-preserved) &&\n \ttest $(git rev-parse HEAD^^2^) = $(git rev-parse HEAD^^^) &&\n-- \n1.6.0.rc2.38.g33ece.dirty\n"},{"id":"87033","messageId":"65932ae2b77a1249da300e384704394d65696caf.1218621695.git.trast@student.ethz.ch","threadId":"14955","inReplyTo":"8bbecf58e7fba14497720afe5cdaf52fec6bcb40.1218621695.git.trast@student.ethz.ch","subject":"[PATCH 2/2] rebase -i -p: fix parent rewriting","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-08-13T10:04:17Z","receivedAt":"2008-08-13T10:04:17Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The existing parent rewriting did not handle the case where a previous\ncommit was amended (via edit or squash).  Fix by always putting the\nnew sha1 of the last commit into the $REWRITTEN map.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n git-rebase--interactive.sh    |    6 ++----\n t/t3404-rebase-interactive.sh |   12 ++++++++++++\n 2 files changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 51a6147..929d681 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -165,15 +165,14 @@ pick_one_preserving_merges () {\n \t\tdie \"Cannot write current commit's replacement sha1\"\n \tfi\n \n+\techo $sha1 > \"$DOTEST\"/current-commit\n+\n \t# rewrite parents; if none were rewritten, we can fast-forward.\n-\tfast_forward=t\n-\tpreserve=t\n \tnew_parents=\n \tfor p in $(git rev-list --parents -1 $sha1 | cut -d' ' -f2-)\n \tdo\n \t\tif test -f \"$REWRITTEN\"/$p\n \t\tthen\n-\t\t\tpreserve=f\n \t\t\tnew_p=$(cat \"$REWRITTEN\"/$p)\n \t\t\ttest $p != $new_p && fast_forward=f\n \t\t\tcase \"$new_parents\" in\n@@ -201,7 +200,6 @@ pick_one_preserving_merges () {\n \t\toutput git checkout $first_parent 2> /dev/null ||\n \t\t\tdie \"Cannot move HEAD to $first_parent\"\n \n-\t\techo $sha1 > \"$DOTEST\"/current-commit\n \t\tcase \"$new_parents\" in\n \t\t' '*' '*)\n \t\t\t# redo merge\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 4d62b9a..5aa487a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -250,6 +250,18 @@ test_expect_success 'preserve merges with -p' '\n \ttest $(git show HEAD:unrelated-file) = 1\n '\n \n+test_expect_success 'edit ancestor with -p' '\n+\tFAKE_LINES=\"1 edit 2 3 4\" git rebase -i -p HEAD~3 &&\n+\techo 2 > unrelated-file &&\n+\ttest_tick &&\n+\tgit commit -m L2-modified --amend unrelated-file &&\n+\tgit rebase --continue &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD -- &&\n+\ttest $(git show HEAD:unrelated-file) = 2\n+'\n+\n test_expect_success '--continue tries to commit' '\n \ttest_tick &&\n \ttest_must_fail git rebase -i --onto new-branch1 HEAD^ &&\n-- \n1.6.0.rc2.38.g33ece.dirty\n"},{"id":"87035","messageId":"200808131207.31616.trast@student.ethz.ch","threadId":"14955","inReplyTo":"8bbecf58e7fba14497720afe5cdaf52fec6bcb40.1218621695.git.trast@student.ethz.ch","subject":"Re: [PATCH 1/2] rebase -i -p: handle index and workdir correctly","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-08-13T10:07:29Z","receivedAt":"2008-08-13T10:07:29Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Thomas Rast wrote:\n> -\t\ttest $preserve = f || echo $sha1 > \"$REWRITTEN\"/$sha1\n> +\t\toutput git reset --hard $sha1 ||\n> +\t\t\tdie \"Cannot fast forward to $sha1\"\n\nDoh.  I obviously shouldn't try to split patches before lunch.\n\nPlease disregard, I'll fix and resend when I get back.\n\n-- \nThomas Rast\ntrast@student.ethz.ch\n\n"},{"id":"87044","messageId":"23b4780a3a7bbe06f4157b6843d1d9dab26d7087.1218628444.git.trast@student.ethz.ch","threadId":"14955","inReplyTo":"200808131207.31616.trast@student.ethz.ch","subject":"[PATCH v2 1/2] rebase -i -p: handle index and workdir correctly","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-08-13T11:56:59Z","receivedAt":"2008-08-13T11:56:59Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"'git rebase -i -p' forgot to update the index and working directory\nduring fast forwards.  Fix this.  Makes 'GIT_EDITOR=true rebase -i -p\n<ancestor>' a no-op again.\n\nAlso, it attempted to do a fast forward even if it was instructed not\nto commit (via -n).  Fall back to the cherry-pick code path and let\nthat handle the issue for us.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n git-rebase--interactive.sh    |   13 ++++++++++++-\n t/t3404-rebase-interactive.sh |    6 ++++++\n 2 files changed, 18 insertions(+), 1 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 4e334ba..1dc24b1 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -145,7 +145,16 @@ pick_one () {\n }\n \n pick_one_preserving_merges () {\n-\tcase \"$1\" in -n) sha1=$2 ;; *) sha1=$1 ;; esac\n+\tfast_forward=t\n+\tcase \"$1\" in\n+\t-n)\n+\t\tfast_forward=f\n+\t\tsha1=$2\n+\t\t;;\n+\t*)\n+\t\tsha1=$1\n+\t\t;;\n+\tesac\n \tsha1=$(git rev-parse $sha1)\n \n \tif test -f \"$DOTEST\"/current-commit\n@@ -182,6 +191,8 @@ pick_one_preserving_merges () {\n \tt)\n \t\toutput warn \"Fast forward to $sha1\"\n \t\ttest $preserve = f || echo $sha1 > \"$REWRITTEN\"/$sha1\n+\t\toutput git reset --hard $sha1 ||\n+\t\t\tdie \"Cannot fast forward to $sha1\"\n \t\t;;\n \tf)\n \t\ttest \"a$1\" = a-n && die \"Refusing to squash a merge: $sha1\"\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex ffe3dd9..4d62b9a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -202,6 +202,9 @@ test_expect_success 'retain authorship when squashing' '\n test_expect_success '-p handles \"no changes\" gracefully' '\n \tHEAD=$(git rev-parse HEAD) &&\n \tgit rebase -i -p HEAD^ &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD -- &&\n \ttest $HEAD = $(git rev-parse HEAD)\n '\n \n@@ -235,6 +238,9 @@ test_expect_success 'preserve merges with -p' '\n \tgit checkout -b to-be-rebased &&\n \ttest_tick &&\n \tgit rebase -i -p --onto branch1 master &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD -- &&\n \ttest $(git rev-parse HEAD~6) = $(git rev-parse branch1) &&\n \ttest $(git rev-parse HEAD~4^2) = $(git rev-parse to-be-preserved) &&\n \ttest $(git rev-parse HEAD^^2^) = $(git rev-parse HEAD^^^) &&\n-- \n1.6.0.rc2.36.g234a\n"},{"id":"87043","messageId":"9cc595258568081dd3f773ec859d552d5d892990.1218628445.git.trast@student.ethz.ch","threadId":"14955","inReplyTo":"23b4780a3a7bbe06f4157b6843d1d9dab26d7087.1218628444.git.trast@student.ethz.ch","subject":"[PATCH v2 2/2] rebase -i -p: fix parent rewriting","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-08-13T11:57:00Z","receivedAt":"2008-08-13T11:57:00Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The existing parent rewriting did not handle the case where a previous\ncommit was amended (via edit or squash).  Fix by always putting the\nnew sha1 of the last commit into the $REWRITTEN map.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n git-rebase--interactive.sh    |    7 ++-----\n t/t3404-rebase-interactive.sh |   12 ++++++++++++\n 2 files changed, 14 insertions(+), 5 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 1dc24b1..929d681 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -165,15 +165,14 @@ pick_one_preserving_merges () {\n \t\tdie \"Cannot write current commit's replacement sha1\"\n \tfi\n \n+\techo $sha1 > \"$DOTEST\"/current-commit\n+\n \t# rewrite parents; if none were rewritten, we can fast-forward.\n-\tfast_forward=t\n-\tpreserve=t\n \tnew_parents=\n \tfor p in $(git rev-list --parents -1 $sha1 | cut -d' ' -f2-)\n \tdo\n \t\tif test -f \"$REWRITTEN\"/$p\n \t\tthen\n-\t\t\tpreserve=f\n \t\t\tnew_p=$(cat \"$REWRITTEN\"/$p)\n \t\t\ttest $p != $new_p && fast_forward=f\n \t\t\tcase \"$new_parents\" in\n@@ -190,7 +189,6 @@ pick_one_preserving_merges () {\n \tcase $fast_forward in\n \tt)\n \t\toutput warn \"Fast forward to $sha1\"\n-\t\ttest $preserve = f || echo $sha1 > \"$REWRITTEN\"/$sha1\n \t\toutput git reset --hard $sha1 ||\n \t\t\tdie \"Cannot fast forward to $sha1\"\n \t\t;;\n@@ -202,7 +200,6 @@ pick_one_preserving_merges () {\n \t\toutput git checkout $first_parent 2> /dev/null ||\n \t\t\tdie \"Cannot move HEAD to $first_parent\"\n \n-\t\techo $sha1 > \"$DOTEST\"/current-commit\n \t\tcase \"$new_parents\" in\n \t\t' '*' '*)\n \t\t\t# redo merge\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 4d62b9a..5aa487a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -250,6 +250,18 @@ test_expect_success 'preserve merges with -p' '\n \ttest $(git show HEAD:unrelated-file) = 1\n '\n \n+test_expect_success 'edit ancestor with -p' '\n+\tFAKE_LINES=\"1 edit 2 3 4\" git rebase -i -p HEAD~3 &&\n+\techo 2 > unrelated-file &&\n+\ttest_tick &&\n+\tgit commit -m L2-modified --amend unrelated-file &&\n+\tgit rebase --continue &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD -- &&\n+\ttest $(git show HEAD:unrelated-file) = 2\n+'\n+\n test_expect_success '--continue tries to commit' '\n \ttest_tick &&\n \ttest_must_fail git rebase -i --onto new-branch1 HEAD^ &&\n-- \n1.6.0.rc2.36.g234a\n"},{"id":"87103","messageId":"20080813205103.GB10758@leksak.fem-net","threadId":"14955","inReplyTo":"200808121116.41535.trast@student.ethz.ch","subject":"Re: [BUG?] rebase -i -p leaves index changed","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-13T20:51:03Z","receivedAt":"2008-08-13T20:51:03Z","isPatch":false,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nThomas Rast wrote:\n> 'rebase -i -p' appears to be a bit confused about what it is doing.\n> Try a history like this:\n> \n>   O -- A -- M -- B\n>    \\       /\n>     \\- C -/\n> \n> built by:\n> \n>   git init\n>   touch foo\n>   git add foo\n>   git ci -m initial\n>   git tag root\n>   git co -b side\n\nPuh, it is nice that you provide a sequence how you generated your test\ncase, but \"ci\" and \"co\" are no git commands.  Ok, because of the\nsequence it was clear to me that you mean \"commit\" and \"checkout\",\nbut I couldn't just c&p it into my xterm :(\n;-)\n\n> Second, 'edit' is also a bit suspicious:\n> \n>   git reset --hard\n>   GIT_EDITOR='perl -i -pe \"s/pick 096/edit 096/\"' git rebase -i -p root\n\nThis perl expression does nothing if your commit is not prefixed with 096 ;)\nBut I tested by changing the line of commit \"a\" to \"edit\".\n\n> Despite claiming \"Stopped at 096[...] a\", a quick 'git show' tells us\n> that it is actually stuck at 'initial'.  (At least 'git rebase\n> --continue' then ends up with the same result as the first test.)\n\nRegarding your [BUG?]: this is a bug, definitely.\n\n> Granted, I'm not entirely sure what it _should_ do.  In my use case\n> (relating to the filter-branch topic), C was a commit from elsewhere\n> that B depended on.  So I kind of hoped 'rebase -i -p' would let me\n> edit A, then rebuild M and B on top, while leaving C alone.\n\nI think your hope is right.  So this is also a bug.\n\nI tested this with my sequencer-based rebase -i -p first and it worked\nas you hope. I tested it with your v2 patches and it works, too. ;)\nSo I'm having a deeper look at your patches in some minutes...\n\nRegards,\n  Stephan\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"87105","messageId":"20080813205806.GC10758@leksak.fem-net","threadId":"14955","inReplyTo":"23b4780a3a7bbe06f4157b6843d1d9dab26d7087.1218628444.git.trast@student.ethz.ch","subject":"Re: [PATCH v2 1/2] rebase -i -p: handle index and workdir correctly","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-13T20:58:06Z","receivedAt":"2008-08-13T20:58:06Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nThomas Rast wrote:\n> Also, it attempted to do a fast forward even if it was instructed not\n> to commit (via -n).  Fall back to the cherry-pick code path and let\n> that handle the issue for us.\n> \n> Signed-off-by: Thomas Rast <trast@student.ethz.ch>\n> ---\n>  git-rebase--interactive.sh    |   13 ++++++++++++-\n>  t/t3404-rebase-interactive.sh |    6 ++++++\n>  2 files changed, 18 insertions(+), 1 deletions(-)\n> \n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index 4e334ba..1dc24b1 100755\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -145,7 +145,16 @@ pick_one () {\n>  }\n>  \n>  pick_one_preserving_merges () {\n> -\tcase \"$1\" in -n) sha1=$2 ;; *) sha1=$1 ;; esac\n> +\tfast_forward=t\n> +\tcase \"$1\" in\n> +\t-n)\n> +\t\tfast_forward=f\n> +\t\tsha1=$2\n\nThe intention of setting fast_forward=f is clear, but it is overwritten\nlater on line 169 that is not shown in this patch:\n\n168:\t# rewrite parents; if none were rewritten, we can fast-forward.\n169:\tfast_forward=t\n\nClearly, this is due to your patch split, because after the second\npatch, this is fixed.\n\nThe rest of this patch seems to be ok to me.\n\nRegards.\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"87110","messageId":"f3d190b3dde04470c0f431d93cf6be6e3990c510.1218663527.git.trast@student.ethz.ch","threadId":"14955","inReplyTo":"20080813205806.GC10758@leksak.fem-net","subject":"[PATCH v3 1/2] rebase -i -p: handle index and workdir correctly","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-08-13T21:41:23Z","receivedAt":"2008-08-13T21:41:23Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"'git rebase -i -p' forgot to update the index and working directory\nduring fast forwards.  Fix this.  Makes 'GIT_EDITOR=true rebase -i -p\n<ancestor>' a no-op again.\n\nAlso, it attempted to do a fast forward even if it was instructed not\nto commit (via -n).  Fall back to the cherry-pick code path and let\nthat handle the issue for us.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n\nStephan Beyer <s-beyer@gmx.net> wrote:\n>\n> The intention of setting fast_forward=f is clear, but it is overwritten\n> later on line 169 that is not shown in this patch:\n[...]\n> Clearly, this is due to your patch split, because after the second\n> patch, this is fixed.\n\nIndeed, thanks for catching this.\n\n git-rebase--interactive.sh    |   14 ++++++++++++--\n t/t3404-rebase-interactive.sh |    6 ++++++\n 2 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 4e334ba..58126bd 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -145,7 +145,16 @@ pick_one () {\n }\n \n pick_one_preserving_merges () {\n-\tcase \"$1\" in -n) sha1=$2 ;; *) sha1=$1 ;; esac\n+\tfast_forward=t\n+\tcase \"$1\" in\n+\t-n)\n+\t\tfast_forward=f\n+\t\tsha1=$2\n+\t\t;;\n+\t*)\n+\t\tsha1=$1\n+\t\t;;\n+\tesac\n \tsha1=$(git rev-parse $sha1)\n \n \tif test -f \"$DOTEST\"/current-commit\n@@ -157,7 +166,6 @@ pick_one_preserving_merges () {\n \tfi\n \n \t# rewrite parents; if none were rewritten, we can fast-forward.\n-\tfast_forward=t\n \tpreserve=t\n \tnew_parents=\n \tfor p in $(git rev-list --parents -1 $sha1 | cut -d' ' -f2-)\n@@ -182,6 +190,8 @@ pick_one_preserving_merges () {\n \tt)\n \t\toutput warn \"Fast forward to $sha1\"\n \t\ttest $preserve = f || echo $sha1 > \"$REWRITTEN\"/$sha1\n+\t\toutput git reset --hard $sha1 ||\n+\t\t\tdie \"Cannot fast forward to $sha1\"\n \t\t;;\n \tf)\n \t\ttest \"a$1\" = a-n && die \"Refusing to squash a merge: $sha1\"\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex ffe3dd9..4d62b9a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -202,6 +202,9 @@ test_expect_success 'retain authorship when squashing' '\n test_expect_success '-p handles \"no changes\" gracefully' '\n \tHEAD=$(git rev-parse HEAD) &&\n \tgit rebase -i -p HEAD^ &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD -- &&\n \ttest $HEAD = $(git rev-parse HEAD)\n '\n \n@@ -235,6 +238,9 @@ test_expect_success 'preserve merges with -p' '\n \tgit checkout -b to-be-rebased &&\n \ttest_tick &&\n \tgit rebase -i -p --onto branch1 master &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD -- &&\n \ttest $(git rev-parse HEAD~6) = $(git rev-parse branch1) &&\n \ttest $(git rev-parse HEAD~4^2) = $(git rev-parse to-be-preserved) &&\n \ttest $(git rev-parse HEAD^^2^) = $(git rev-parse HEAD^^^) &&\n-- \n1.6.0.rc2.69.g29dd9\n"},{"id":"87111","messageId":"e345bea8d069f267772d323ca5f76182083f2f6b.1218663527.git.trast@student.ethz.ch","threadId":"14955","inReplyTo":"f3d190b3dde04470c0f431d93cf6be6e3990c510.1218663527.git.trast@student.ethz.ch","subject":"[PATCH v3 2/2] rebase -i -p: fix parent rewriting","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-08-13T21:41:24Z","receivedAt":"2008-08-13T21:41:24Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The existing parent rewriting did not handle the case where a previous\ncommit was amended (via edit or squash).  Fix by always putting the\nnew sha1 of the last commit into the $REWRITTEN map.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n git-rebase--interactive.sh    |    6 ++----\n t/t3404-rebase-interactive.sh |   12 ++++++++++++\n 2 files changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 58126bd..929d681 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -165,14 +165,14 @@ pick_one_preserving_merges () {\n \t\tdie \"Cannot write current commit's replacement sha1\"\n \tfi\n \n+\techo $sha1 > \"$DOTEST\"/current-commit\n+\n \t# rewrite parents; if none were rewritten, we can fast-forward.\n-\tpreserve=t\n \tnew_parents=\n \tfor p in $(git rev-list --parents -1 $sha1 | cut -d' ' -f2-)\n \tdo\n \t\tif test -f \"$REWRITTEN\"/$p\n \t\tthen\n-\t\t\tpreserve=f\n \t\t\tnew_p=$(cat \"$REWRITTEN\"/$p)\n \t\t\ttest $p != $new_p && fast_forward=f\n \t\t\tcase \"$new_parents\" in\n@@ -189,7 +189,6 @@ pick_one_preserving_merges () {\n \tcase $fast_forward in\n \tt)\n \t\toutput warn \"Fast forward to $sha1\"\n-\t\ttest $preserve = f || echo $sha1 > \"$REWRITTEN\"/$sha1\n \t\toutput git reset --hard $sha1 ||\n \t\t\tdie \"Cannot fast forward to $sha1\"\n \t\t;;\n@@ -201,7 +200,6 @@ pick_one_preserving_merges () {\n \t\toutput git checkout $first_parent 2> /dev/null ||\n \t\t\tdie \"Cannot move HEAD to $first_parent\"\n \n-\t\techo $sha1 > \"$DOTEST\"/current-commit\n \t\tcase \"$new_parents\" in\n \t\t' '*' '*)\n \t\t\t# redo merge\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 4d62b9a..5aa487a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -250,6 +250,18 @@ test_expect_success 'preserve merges with -p' '\n \ttest $(git show HEAD:unrelated-file) = 1\n '\n \n+test_expect_success 'edit ancestor with -p' '\n+\tFAKE_LINES=\"1 edit 2 3 4\" git rebase -i -p HEAD~3 &&\n+\techo 2 > unrelated-file &&\n+\ttest_tick &&\n+\tgit commit -m L2-modified --amend unrelated-file &&\n+\tgit rebase --continue &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD -- &&\n+\ttest $(git show HEAD:unrelated-file) = 2\n+'\n+\n test_expect_success '--continue tries to commit' '\n \ttest_tick &&\n \ttest_must_fail git rebase -i --onto new-branch1 HEAD^ &&\n-- \n1.6.0.rc2.69.g29dd9\n"}]}