| Message ID | pull.679.git.git.1576861788.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | rebase: make the default backend configurable | expand |
Hi Elijah, Le 20/12/2019 à 18:09, Elijah Newren via GitGitGadget a écrit : > This series does a lot of work around making the default rebase backend > configurable: > > * provide configurability of handling empty commits and iron out > differences between backends > * increase/fix capabilities of the merge/interactive backend to make it > workable as the default > * document the remaining differences in backends more thoroughly > * add an --am option for explicitly requesting the am-backend > * extend merge/interactive backend testing to put it on par with the am > backend > * add a 'rebase.backend' config option and: > * switch the rebase.backend default value from 'am' to 'merge'. > > Areas I'd like reviewers to focus, in priority order: > > * Patch 15 (Are we ready to switch the default backend? Or should we leave > this patch out for now? Junio suggested we may be ready or at least are > close[1]) > * Patch 1 (Does my empty handling make sense? Do others agree I fixed > --keep-empty, or do they view it as breaking it?) > * Patch 8 (Do the updates to the documentation of behavioral differences > make sense? Is it too long?) * Patch 11 (okay to change the > git-completion shell prompt slightly, especially in light of patches > 15 & 16? We did a prompt change previously when we merged the merge > backend with the interactive one, so I assume so, but just want to > make sure people have a chance to chime in.) > > > > If it's too soon to switch from the 'am' to 'merge' backend, we can just > drop the last patch and I'll resubmit it later. I generated this series > mostly through switching the default first and then watching what broke, but > moved the patch to the end to make it easy to drop. > > Briefly, reasons for switching the default backend boil down to the fact > that the am-backend drops information and thus limits what it can do. This > manifests in different ways: > > * lack of tree information that would allow us to warn users that new files > in old directories might want to move along with the other files that > were renamed with those directories[1] > * incorrect application of patches in the presence of non-unique context > lines[2], which could be avoided with access to the original files > involved. > * less information available to annotate conflict markers (since am creates > fake ancestors and commits on top of them, and doesn't have access to the > original commits) > > [1] https://lore.kernel.org/git/xmqqa78d2qmk.fsf@gitster-ct.c.googlers.com/ > [2] https://lore.kernel.org/git/xmqqh8jeh1id.fsf@gitster-ct.c.googlers.com/ > [3] > https://lore.kernel.org/git/CABPp-BGiu2nVMQY_t-rnFR5GQUz_ipyEE8oDocKeO+h+t4Mn4A@mail.gmail.com/ > > Elijah Newren (15): > rebase: extend the options for handling of empty commits > t3406: simplify an already simple test > rebase, sequencer: remove the broken GIT_QUIET handling > rebase: make sure to pass along the quiet flag to the sequencer > rebase: fix handling of restrict_revision > t3432: make these tests work with either am or merge backends > rebase: allow more types of rebases to fast-forward > git-rebase.txt: add more details about behavioral differences of > backends > rebase: move incompatibility checks between backend options a bit > earlier > rebase: add an --am option > contrib: change the prompt for am-based rebases > rebase tests: mark tests specific to the am-backend with --am > rebase tests: repeat some tests using the merge backend instead of am > rebase: make the backend configurable via config setting > rebase: change the default backend from "am" to "merge" > > Documentation/config/rebase.txt | 8 ++ > Documentation/git-rebase.txt | 150 ++++++++++++++++---- > builtin/rebase.c | 181 +++++++++++++++++++----- > contrib/completion/git-prompt.sh | 2 +- > rebase-interactive.c | 4 +- > rebase-interactive.h | 2 +- > sequencer.c | 80 ++++++++--- > sequencer.h | 6 +- > t/t3400-rebase.sh | 36 ++++- > t/t3401-rebase-and-am-rename.sh | 4 +- > t/t3404-rebase-interactive.sh | 2 +- > t/t3406-rebase-message.sh | 19 ++- > t/t3407-rebase-abort.sh | 6 +- > t/t3420-rebase-autostash.sh | 2 +- > t/t3421-rebase-topology-linear.sh | 4 +- > t/t3424-rebase-empty.sh | 89 ++++++++++++ > t/t3425-rebase-topology-merges.sh | 8 +- > t/t3427-rebase-subtree.sh | 16 ++- > t/t3432-rebase-fast-forward.sh | 59 ++++---- > t/t3433-rebase-options-compatibility.sh | 13 +- > t/t5407-post-rewrite-hook.sh | 12 +- > t/t5520-pull.sh | 27 +++- > t/t6047-diff3-conflict-markers.sh | 13 +- > t/t7512-status-help.sh | 12 +- > t/t9106-git-svn-commit-diff-clobber.sh | 3 +- > t/t9903-bash-prompt.sh | 6 +- > 26 files changed, 582 insertions(+), 182 deletions(-) > create mode 100755 t/t3424-rebase-empty.sh > > > base-commit: 12029dc57db23baef008e77db1909367599210ee > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-679%2Fnewren%2Frebase-fixes-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-679/newren/rebase-fixes-v1 > Pull-Request: https://github.com/git/git/pull/679 > BTW, I have investigated on the performance regression when split-index is enabled, I’ll give my conclusions next week. Cheers, Alban
Hi Alban, On Fri, Dec 20, 2019 at 10:53 AM Alban Gruin <alban.gruin@gmail.com> wrote: > > Hi Elijah, > > Le 20/12/2019 à 18:09, Elijah Newren via GitGitGadget a écrit : > > This series does a lot of work around making the default rebase backend > > configurable: > > > > BTW, I have investigated on the performance regression when split-index > is enabled, I’ll give my conclusions next week. > > Cheers, > Alban Nice!
This series does a lot of work around making the default rebase backend configurable: * provide configurability of handling empty commits and iron out differences between backends * increase/fix capabilities of the merge/interactive backend to make it workable as the default * document the remaining differences in backends more thoroughly * add an --am option for explicitly requesting the am-backend * extend merge/interactive backend testing to put it on par with the am backend * add a 'rebase.backend' config option and: * switch the rebase.backend default value from 'am' to 'merge'. Areas I'd like reviewers to focus, in priority order: * Patch 15 (Are we ready to switch the default backend? Or should we leave this patch out for now? Junio suggested we may be ready or at least are close[1]) * Patch 1 (Does my empty handling make sense? Do others agree I fixed --keep-empty, or do they view it as breaking it?) * Patch 8 (Do the updates to the documentation of behavioral differences make sense? Is it too long?) * Patch 11 (okay to change the git-completion shell prompt slightly, especially in light of patches 15 & 16? We did a prompt change previously when we merged the merge backend with the interactive one, so I assume so, but just want to make sure people have a chance to chime in.) If it's too soon to switch from the 'am' to 'merge' backend, we can just drop the last patch and I'll resubmit it later. I generated this series mostly through switching the default first and then watching what broke, but moved the patch to the end to make it easy to drop. Briefly, reasons for switching the default backend boil down to the fact that the am-backend drops information and thus limits what it can do. This manifests in different ways: * lack of tree information that would allow us to warn users that new files in old directories might want to move along with the other files that were renamed with those directories[1] * incorrect application of patches in the presence of non-unique context lines[2], which could be avoided with access to the original files involved. * less information available to annotate conflict markers (since am creates fake ancestors and commits on top of them, and doesn't have access to the original commits) [1] https://lore.kernel.org/git/xmqqa78d2qmk.fsf@gitster-ct.c.googlers.com/ [2] https://lore.kernel.org/git/xmqqh8jeh1id.fsf@gitster-ct.c.googlers.com/ [3] https://lore.kernel.org/git/CABPp-BGiu2nVMQY_t-rnFR5GQUz_ipyEE8oDocKeO+h+t4Mn4A@mail.gmail.com/ Elijah Newren (15): rebase: extend the options for handling of empty commits t3406: simplify an already simple test rebase, sequencer: remove the broken GIT_QUIET handling rebase: make sure to pass along the quiet flag to the sequencer rebase: fix handling of restrict_revision t3432: make these tests work with either am or merge backends rebase: allow more types of rebases to fast-forward git-rebase.txt: add more details about behavioral differences of backends rebase: move incompatibility checks between backend options a bit earlier rebase: add an --am option contrib: change the prompt for am-based rebases rebase tests: mark tests specific to the am-backend with --am rebase tests: repeat some tests using the merge backend instead of am rebase: make the backend configurable via config setting rebase: change the default backend from "am" to "merge" Documentation/config/rebase.txt | 8 ++ Documentation/git-rebase.txt | 150 ++++++++++++++++---- builtin/rebase.c | 181 +++++++++++++++++++----- contrib/completion/git-prompt.sh | 2 +- rebase-interactive.c | 4 +- rebase-interactive.h | 2 +- sequencer.c | 80 ++++++++--- sequencer.h | 6 +- t/t3400-rebase.sh | 36 ++++- t/t3401-rebase-and-am-rename.sh | 4 +- t/t3404-rebase-interactive.sh | 2 +- t/t3406-rebase-message.sh | 19 ++- t/t3407-rebase-abort.sh | 6 +- t/t3420-rebase-autostash.sh | 2 +- t/t3421-rebase-topology-linear.sh | 4 +- t/t3424-rebase-empty.sh | 89 ++++++++++++ t/t3425-rebase-topology-merges.sh | 8 +- t/t3427-rebase-subtree.sh | 16 ++- t/t3432-rebase-fast-forward.sh | 59 ++++---- t/t3433-rebase-options-compatibility.sh | 13 +- t/t5407-post-rewrite-hook.sh | 12 +- t/t5520-pull.sh | 27 +++- t/t6047-diff3-conflict-markers.sh | 13 +- t/t7512-status-help.sh | 12 +- t/t9106-git-svn-commit-diff-clobber.sh | 3 +- t/t9903-bash-prompt.sh | 6 +- 26 files changed, 582 insertions(+), 182 deletions(-) create mode 100755 t/t3424-rebase-empty.sh base-commit: 12029dc57db23baef008e77db1909367599210ee Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-679%2Fnewren%2Frebase-fixes-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-679/newren/rebase-fixes-v1 Pull-Request: https://github.com/git/git/pull/679