Re: [PATCH 3/6] rebase -i --root: let the sequencer handle even the initial part
Hi Stefan, On Sat, 28 Apr 2018, Stefan Beller wrote: > On Fri, Apr 27, 2018 at 3:31 PM, Johannes Schindelin > wrote: > > In this developer's earlier attempt to accelerate interactive rebases by > > converting large parts from Unix shell script into portable, performant > > C, the --root handling was specifically excluded (to simplify the task a > > little bit; it still took over a year to get that reduced set of patches > > into Git proper). > > > > This patch ties up that loose end: now only --preserve-merges uses the > > slow Unix shell script implementation to perform the interactive rebase. > > > > As the rebase--helper reports progress to stderr (unlike the scripted > > interactive rebase, which reports it to stdout, of all places), we have > > to adjust a couple of tests that did not expect that for `git rebase -i > > --root`. > > > > This patch fixes -- at long last! -- the really old bug reported in > > 6a6bc5bdc4d (add tests for rebasing root, 2013-06-06) that rebasing with > > --root *always* rewrote the root commit, even if there were no changes. > > > > The bug still persists in --preserve-merges mode, of course, but that > > mode will be deprecated as soon as the new --rebase-merges mode > > stabilizes, anyway. > > > > Signed-off-by: Johannes Schindelin > > --- > > git-rebase--interactive.sh| 4 +++- > > t/t3404-rebase-interactive.sh | 19 +-- > > t/t3421-rebase-topology-linear.sh | 6 +++--- > > 3 files changed, 19 insertions(+), 10 deletions(-) > > > > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > > index cbf44f86482..2f4941d0fc9 100644 > > --- a/git-rebase--interactive.sh > > +++ b/git-rebase--interactive.sh > > @@ -894,6 +894,8 @@ init_revisions_and_shortrevisions () { > > else > > revisions=$onto...$orig_head > > shortrevisions=$shorthead > > + test -z "$squash_onto" || > > + echo "$squash_onto" >"$state_dir"/squash-onto > > fi > > } > > > > @@ -948,7 +950,7 @@ EOF > > die "Could not skip unnecessary pick commands" > > > > checkout_onto > > - if test -z "$rebase_root" && test ! -d "$rewritten" > > + if test ! -d "$rewritten" > > I have the impression this is the line that is really well > explained in the commit message ("migrate to rebase > helper even when there is $rebase_root set") > > The rest of the patch is covered as "a couple of places > where we adjust stdout to stderr"? Correct. Thanks for reviewing! Dscho
Re: [PATCH 3/6] rebase -i --root: let the sequencer handle even the initial part
On Fri, Apr 27, 2018 at 3:31 PM, Johannes Schindelin wrote: > In this developer's earlier attempt to accelerate interactive rebases by > converting large parts from Unix shell script into portable, performant > C, the --root handling was specifically excluded (to simplify the task a > little bit; it still took over a year to get that reduced set of patches > into Git proper). > > This patch ties up that loose end: now only --preserve-merges uses the > slow Unix shell script implementation to perform the interactive rebase. > > As the rebase--helper reports progress to stderr (unlike the scripted > interactive rebase, which reports it to stdout, of all places), we have > to adjust a couple of tests that did not expect that for `git rebase -i > --root`. > > This patch fixes -- at long last! -- the really old bug reported in > 6a6bc5bdc4d (add tests for rebasing root, 2013-06-06) that rebasing with > --root *always* rewrote the root commit, even if there were no changes. > > The bug still persists in --preserve-merges mode, of course, but that > mode will be deprecated as soon as the new --rebase-merges mode > stabilizes, anyway. > > Signed-off-by: Johannes Schindelin > --- > git-rebase--interactive.sh| 4 +++- > t/t3404-rebase-interactive.sh | 19 +-- > t/t3421-rebase-topology-linear.sh | 6 +++--- > 3 files changed, 19 insertions(+), 10 deletions(-) > > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > index cbf44f86482..2f4941d0fc9 100644 > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh > @@ -894,6 +894,8 @@ init_revisions_and_shortrevisions () { > else > revisions=$onto...$orig_head > shortrevisions=$shorthead > + test -z "$squash_onto" || > + echo "$squash_onto" >"$state_dir"/squash-onto > fi > } > > @@ -948,7 +950,7 @@ EOF > die "Could not skip unnecessary pick commands" > > checkout_onto > - if test -z "$rebase_root" && test ! -d "$rewritten" > + if test ! -d "$rewritten" I have the impression this is the line that is really well explained in the commit message ("migrate to rebase helper even when there is $rebase_root set") The rest of the patch is covered as "a couple of places where we adjust stdout to stderr"? Makes sense, Stefan
[PATCH 3/6] rebase -i --root: let the sequencer handle even the initial part
In this developer's earlier attempt to accelerate interactive rebases by converting large parts from Unix shell script into portable, performant C, the --root handling was specifically excluded (to simplify the task a little bit; it still took over a year to get that reduced set of patches into Git proper). This patch ties up that loose end: now only --preserve-merges uses the slow Unix shell script implementation to perform the interactive rebase. As the rebase--helper reports progress to stderr (unlike the scripted interactive rebase, which reports it to stdout, of all places), we have to adjust a couple of tests that did not expect that for `git rebase -i --root`. This patch fixes -- at long last! -- the really old bug reported in 6a6bc5bdc4d (add tests for rebasing root, 2013-06-06) that rebasing with --root *always* rewrote the root commit, even if there were no changes. The bug still persists in --preserve-merges mode, of course, but that mode will be deprecated as soon as the new --rebase-merges mode stabilizes, anyway. Signed-off-by: Johannes Schindelin --- git-rebase--interactive.sh| 4 +++- t/t3404-rebase-interactive.sh | 19 +-- t/t3421-rebase-topology-linear.sh | 6 +++--- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index cbf44f86482..2f4941d0fc9 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -894,6 +894,8 @@ init_revisions_and_shortrevisions () { else revisions=$onto...$orig_head shortrevisions=$shorthead + test -z "$squash_onto" || + echo "$squash_onto" >"$state_dir"/squash-onto fi } @@ -948,7 +950,7 @@ EOF die "Could not skip unnecessary pick commands" checkout_onto - if test -z "$rebase_root" && test ! -d "$rewritten" + if test ! -d "$rewritten" then require_clean_work_tree "rebase" exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \ diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 59c766540e5..c65826ddace 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1204,10 +1204,6 @@ test_expect_success 'drop' ' test A = $(git cat-file commit HEAD^^ | sed -ne \$p) ' -cat >expectexpect actual.2 && + cr_to_nl actual && test_i18ncmp expect actual && test D = $(git cat-file commit HEAD | sed -ne \$p) ' diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh index e7438ad06ac..99b2aac9219 100755 --- a/t/t3421-rebase-topology-linear.sh +++ b/t/t3421-rebase-topology-linear.sh @@ -328,9 +328,9 @@ test_run_rebase () { test_cmp_rev c HEAD " } -test_run_rebase failure '' -test_run_rebase failure -m -test_run_rebase failure -i +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i test_run_rebase failure -p test_run_rebase () { -- 2.17.0.windows.1.33.gfcbb1fa0445