| Message ID | pull.195.v2.git.1630497435.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Drop support for git rebase --preserve-merges | expand |
On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote: > In 427c3bd28ab (rebase: deprecate --preserve-merges, 2019-03-11) (which was > included in v2.22.0), we officially deprecated the --preserve-merges > backend. Over two years later, it is time to drop that backend, and here is > a patch series that does just that. > > Changes since v1: > > * Rebased onto v2.33.0 I'm very much in favor if this series. I independently came up with pretty much the same at the beginning of last month before finding your v1 in the list archive. The comments I left inline are based on the diff between our two versions, i.e. I found some spots you missed (and your version has spots I missed). You're also leaving behind a comment in builtin/rebase.c referring to PRESERVE_MERGES. Perhaps we should just turn that into an "else if" while we're at it (the "ignore-space-change" argument won't need wrapping anymore then): builtin/rebase.c- } else { builtin/rebase.c: /* REBASE_MERGE and PRESERVE_MERGES */ builtin/rebase.c- if (ignore_whitespace) { builtin/rebase.c- string_list_append(&strategy_options, builtin/rebase.c- "ignore-space-change"); builtin/rebase.c- } builtin/rebase.c- } I do find the left-behind "enum rebase_type" rather unpleasant, i.e. we have a REBASE_UNSPECIFIED during the initial parse_options() phase, and after that just REBASE_{APPLY,MERGE}, but some of those codepaths use switch(), some just check on or the other, and it's not immediately obvious where we are in the control flow. Ideally we'd take care of parsing out the option(s) early, and could just have an "int rebase_apply" in the struct to clearly indicate the rarer cases where we take the REBASE_APPLY path. But I wasn't able to make that work with some quick hacking, and in any case that can be left as a cleanup for later. Ideally with the code comments I left addressed, but either way the correctness of the end result isn't affected in terms of what the program ends up doing: Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > In 427c3bd28ab (rebase: deprecate --preserve-merges, 2019-03-11) (which was > included in v2.22.0), we officially deprecated the --preserve-merges > backend. Over two years later, it is time to drop that backend, and here is > a patch series that does just that. A good goal. There is no remaining use case where (a fictitious and properly working version of) "--preserve-merges" option cannot be replaced by "--rebase-merges", is it? I somehow had a feeling that the other Johannes (sorry if it weren't you, j6t) had cases that the former worked better, but perhaps I am mis-remembering things. Thanks.
Hi Ævar, On Wed, 1 Sep 2021, Ævar Arnfjörð Bjarmason wrote: > > On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote: > > > In 427c3bd28ab (rebase: deprecate --preserve-merges, 2019-03-11) (which was > > included in v2.22.0), we officially deprecated the --preserve-merges > > backend. Over two years later, it is time to drop that backend, and here is > > a patch series that does just that. > > > > Changes since v1: > > > > * Rebased onto v2.33.0 > > I'm very much in favor if this series. > > I independently came up with pretty much the same at the beginning of > last month before finding your v1 in the list archive. The comments I > left inline are based on the diff between our two versions, i.e. I found > some spots you missed (and your version has spots I missed). > > You're also leaving behind a comment in builtin/rebase.c referring to > PRESERVE_MERGES. Perhaps we should just turn that into an "else if" > while we're at it (the "ignore-space-change" argument won't need > wrapping anymore then): > > builtin/rebase.c- } else { > builtin/rebase.c: /* REBASE_MERGE and PRESERVE_MERGES */ > builtin/rebase.c- if (ignore_whitespace) { > builtin/rebase.c- string_list_append(&strategy_options, > builtin/rebase.c- "ignore-space-change"); > builtin/rebase.c- } > builtin/rebase.c- } While it would be technically correct to turn this into an `else if`, by all other standards it would be incorrect. As I commented on your earlier comment: just because it uses less lines does not make the intention clearer. In this instance, I am actually quite certain that it dilutes the intention. The `if (options.type == REBASE_APPLY)` clearly indicates a top-level decision on the rebase backend, and an `else if (ignore_whitespace)` would disrupt that idea to be about distinguishing between completely unrelated concerns. In other words: while I accept that your taste would prefer the suggested change, my taste prefers the opposite, and since I am the owner of this patch series contribution, I go with my taste. > I do find the left-behind "enum rebase_type" rather unpleasant, i.e. we > have a REBASE_UNSPECIFIED during the initial parse_options() phase, and > after that just REBASE_{APPLY,MERGE}, but some of those codepaths use > switch(), some just check on or the other, and it's not immediately > obvious where we are in the control flow. Ideally we'd take care of > parsing out the option(s) early, and could just have an "int > rebase_apply" in the struct to clearly indicate the rarer cases where we > take the REBASE_APPLY path. Thank you for offering your opinion. This encourages me to offer my (differing) opinion to keep the `enum rebase_type` to clarify that we have multiple rebase backends, and even leave Git's source code open to future contributions of other rebase backends. Ciao, Dscho
Hi Junio, On Wed, 1 Sep 2021, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > > > In 427c3bd28ab (rebase: deprecate --preserve-merges, 2019-03-11) (which was > > included in v2.22.0), we officially deprecated the --preserve-merges > > backend. Over two years later, it is time to drop that backend, and here is > > a patch series that does just that. > > A good goal. There is no remaining use case where (a fictitious and > properly working version of) "--preserve-merges" option cannot be > replaced by "--rebase-merges", is it? I somehow had a feeling that > the other Johannes (sorry if it weren't you, j6t) had cases that the > former worked better, but perhaps I am mis-remembering things. I think that I managed to address whatever concerns there were about the `--rebase-merges` backend in the meantime. To be honest, I developed one (minor) concern in the meantime... Should we maybe try to be nicer to our users and keep handling the `--preserve-merges` option by explicitly erroring out with the suggestion to use `--rebase-merges` instead? Not everybody reads release notes, after all. In fact, it is my experience that preciously few users have the time to even skim release notes... Ciao, Dscho
On Thu, Sep 02 2021, Johannes Schindelin wrote: > Hi Ævar, > > On Wed, 1 Sep 2021, Ævar Arnfjörð Bjarmason wrote: > >> >> On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote: >> >> > In 427c3bd28ab (rebase: deprecate --preserve-merges, 2019-03-11) (which was >> > included in v2.22.0), we officially deprecated the --preserve-merges >> > backend. Over two years later, it is time to drop that backend, and here is >> > a patch series that does just that. >> > >> > Changes since v1: >> > >> > * Rebased onto v2.33.0 >> >> I'm very much in favor if this series. >> >> I independently came up with pretty much the same at the beginning of >> last month before finding your v1 in the list archive. The comments I >> left inline are based on the diff between our two versions, i.e. I found >> some spots you missed (and your version has spots I missed). >> >> You're also leaving behind a comment in builtin/rebase.c referring to >> PRESERVE_MERGES. Perhaps we should just turn that into an "else if" >> while we're at it (the "ignore-space-change" argument won't need >> wrapping anymore then): >> >> builtin/rebase.c- } else { >> builtin/rebase.c: /* REBASE_MERGE and PRESERVE_MERGES */ >> builtin/rebase.c- if (ignore_whitespace) { >> builtin/rebase.c- string_list_append(&strategy_options, >> builtin/rebase.c- "ignore-space-change"); >> builtin/rebase.c- } >> builtin/rebase.c- } > > While it would be technically correct to turn this into an `else if`, by > all other standards it would be incorrect. As I commented on your earlier > comment: just because it uses less lines does not make the intention > clearer. In this instance, I am actually quite certain that it dilutes the > intention. The `if (options.type == REBASE_APPLY)` clearly indicates a > top-level decision on the rebase backend, and an `else if > (ignore_whitespace)` would disrupt that idea to be about distinguishing > between completely unrelated concerns. > > In other words: while I accept that your taste would prefer the suggested > change, my taste prefers the opposite, and since I am the owner of this > patch series contribution, I go with my taste. Sounds good. >> I do find the left-behind "enum rebase_type" rather unpleasant, i.e. we >> have a REBASE_UNSPECIFIED during the initial parse_options() phase, and >> after that just REBASE_{APPLY,MERGE}, but some of those codepaths use >> switch(), some just check on or the other, and it's not immediately >> obvious where we are in the control flow. Ideally we'd take care of >> parsing out the option(s) early, and could just have an "int >> rebase_apply" in the struct to clearly indicate the rarer cases where we >> take the REBASE_APPLY path. > > Thank you for offering your opinion. > > This encourages me to offer my (differing) opinion to keep the `enum > rebase_type` to clarify that we have multiple rebase backends, and even > leave Git's source code open to future contributions of other rebase > backends. Just to clarify this isn't a comment describing the suggested is_merge() changes I had elsewhere, or that we should get rid of "enum rebase_type". But rather that if we were to split up the "we haven't decided on the type yet" from "we know the rebase type, let's act on it" which only happens around the option/config/directory introspection etc. from the rest of the code, we could stick on the "real" rebase types in the enum. We could then have just two enum arms for switches (less verbosity) and no "default: BUG(...)" case. Such a change should make it easier for contributors to add new backends, as they'd get the compiler helping them to see where they'd need to add their code. Now you need to hunt around for various is_merge, "opts.type == " etc, switches with "default" etc. comparisons. Of course such a change might not be worth it, i.e. maybe it's too painful to untangle REBASE_UNSPECIFIED from the existing enum. I just wanted to clarify what it was that I was suggesting here.
Am 02.09.21 um 16:18 schrieb Johannes Schindelin: > On Wed, 1 Sep 2021, Junio C Hamano wrote: >> A good goal. There is no remaining use case where (a fictitious and >> properly working version of) "--preserve-merges" option cannot be >> replaced by "--rebase-merges", is it? I somehow had a feeling that >> the other Johannes (sorry if it weren't you, j6t) had cases that the >> former worked better, but perhaps I am mis-remembering things. > > I think that I managed to address whatever concerns there were about the > `--rebase-merges` backend in the meantime. That was either my suggestion/desire to make no-rebase-cousins the default. That has been settled. Or my wish not to redo the merge, but to replay the first-parent difference. The idea never got traction, and I've long since abandoned my implementation of it. > To be honest, I developed one (minor) concern in the meantime... Should we > maybe try to be nicer to our users and keep handling the > `--preserve-merges` option by explicitly erroring out with the suggestion > to use `--rebase-merges` instead? Not everybody reads release notes, after > all. In fact, it is my experience that preciously few users have the time > to even skim release notes... A valid concern, I would think. -- Hannes
Hi Johannes, Le 01/09/2021 à 13:57, Johannes Schindelin via GitGitGadget a écrit : > In 427c3bd28ab (rebase: deprecate --preserve-merges, 2019-03-11) (which was > included in v2.22.0), we officially deprecated the --preserve-merges > backend. Over two years later, it is time to drop that backend, and here is > a patch series that does just that. > > Changes since v1: > > * Rebased onto v2.33.0 > > Johannes Schindelin (7): > t5520: do not use `pull.rebase=preserve` > remote: warn about unhandled branch.<name>.rebase values > tests: stop testing `git rebase --preserve-merges` > pull: remove support for `--rebase=preserve` > rebase: drop support for `--preserve-merges` > git-svn: drop support for `--preserve-merges` > rebase: drop the internal `rebase--interactive` command > This is good. preserve-merge is the only user of `rebase--interactive'. In builtin/rebase.c, it is the only producer of the following actions: - ACTION_SHORTEN_OIDS - ACTION_EXPAND_OIDS - ACTION_CHECK_TODO_LIST - ACTION_REARRANGE_SQUASH - ACTION_ADD_EXEC Which in turn, are the only reason for these functions to exist: - transform_todo_file() - check_todo_list_from_file() (from the sequencer) - rearrange_squash_in_todo_file() - add_exec_commands() This is from a cursory glance, it may go a bit deeper. Should we remove these as well? Alban
On Wed, Sep 01 2021, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > >> In 427c3bd28ab (rebase: deprecate --preserve-merges, 2019-03-11) (which was >> included in v2.22.0), we officially deprecated the --preserve-merges >> backend. Over two years later, it is time to drop that backend, and here is >> a patch series that does just that. > > A good goal. There is no remaining use case where (a fictitious and > properly working version of) "--preserve-merges" option cannot be > replaced by "--rebase-merges", is it? I somehow had a feeling that > the other Johannes (sorry if it weren't you, j6t) had cases that the > former worked better, but perhaps I am mis-remembering things. Fair enough. To be clear I think this series is fine as-is, we've just usually done "now that this function is dead, rm it" as part of the series that makes it dead, so I figured fixups/squashes to change those parts would be welcome & integrated, likewise Alban Gruin's suggestions in <62fbd389-28f5-76e5-d3f3-5510415a7bf5@gmail.com>. But the git-sh-i18n.sh change and/or his suggestions can be done after this lands...
Hi Alban and Johannes On 04/09/2021 23:30, Alban Gruin wrote: > Hi Johannes, > > Le 01/09/2021 à 13:57, Johannes Schindelin via GitGitGadget a écrit : >> In 427c3bd28ab (rebase: deprecate --preserve-merges, 2019-03-11) (which was >> included in v2.22.0), we officially deprecated the --preserve-merges >> backend. Over two years later, it is time to drop that backend, and here is >> a patch series that does just that. >> >> Changes since v1: >> >> * Rebased onto v2.33.0 >> >> Johannes Schindelin (7): >> t5520: do not use `pull.rebase=preserve` >> remote: warn about unhandled branch.<name>.rebase values >> tests: stop testing `git rebase --preserve-merges` >> pull: remove support for `--rebase=preserve` >> rebase: drop support for `--preserve-merges` >> git-svn: drop support for `--preserve-merges` >> rebase: drop the internal `rebase--interactive` command >> > > This is good. I agree > preserve-merge is the only user of `rebase--interactive'. In > builtin/rebase.c, it is the only producer of the following actions: > > - ACTION_SHORTEN_OIDS > - ACTION_EXPAND_OIDS > - ACTION_CHECK_TODO_LIST > - ACTION_REARRANGE_SQUASH > - ACTION_ADD_EXEC > > Which in turn, are the only reason for these functions to exist: > > - transform_todo_file() > - check_todo_list_from_file() (from the sequencer) > - rearrange_squash_in_todo_file() > - add_exec_commands() > > This is from a cursory glance, it may go a bit deeper. I think patch 7 addresses most of that apart from the removal of the enum members and check_todo_list_from_file() Best Wishes Phillip > > Should we remove these as well? > > Alban >
Hi Alban, On Sun, 5 Sep 2021, Alban Gruin wrote: > Le 01/09/2021 à 13:57, Johannes Schindelin via GitGitGadget a écrit : > > In 427c3bd28ab (rebase: deprecate --preserve-merges, 2019-03-11) (which was > > included in v2.22.0), we officially deprecated the --preserve-merges > > backend. Over two years later, it is time to drop that backend, and here is > > a patch series that does just that. > > > > Changes since v1: > > > > * Rebased onto v2.33.0 > > > > Johannes Schindelin (7): > > t5520: do not use `pull.rebase=preserve` > > remote: warn about unhandled branch.<name>.rebase values > > tests: stop testing `git rebase --preserve-merges` > > pull: remove support for `--rebase=preserve` > > rebase: drop support for `--preserve-merges` > > git-svn: drop support for `--preserve-merges` > > rebase: drop the internal `rebase--interactive` command > > > > This is good. Thanks! > preserve-merge is the only user of `rebase--interactive'. In > builtin/rebase.c, it is the only producer of the following actions: > > - ACTION_SHORTEN_OIDS > - ACTION_EXPAND_OIDS > - ACTION_CHECK_TODO_LIST > - ACTION_REARRANGE_SQUASH > - ACTION_ADD_EXEC Makes sense! > Which in turn, are the only reason for these functions to exist: > > - transform_todo_file() My patch series already removed this. > - check_todo_list_from_file() (from the sequencer) Good point! > - rearrange_squash_in_todo_file() This was already removed by my patch series. > - add_exec_commands() Actually, this function is still needed, but its scope can be reduced. Thank you! Dscho
Hi Hannes, On Thu, 2 Sep 2021, Johannes Sixt wrote: > Am 02.09.21 um 16:18 schrieb Johannes Schindelin: > > On Wed, 1 Sep 2021, Junio C Hamano wrote: > >> A good goal. There is no remaining use case where (a fictitious and > >> properly working version of) "--preserve-merges" option cannot be > >> replaced by "--rebase-merges", is it? I somehow had a feeling that > >> the other Johannes (sorry if it weren't you, j6t) had cases that the > >> former worked better, but perhaps I am mis-remembering things. > > > > I think that I managed to address whatever concerns there were about the > > `--rebase-merges` backend in the meantime. > > That was either my suggestion/desire to make no-rebase-cousins the > default. That has been settled. > > Or my wish not to redo the merge, but to replay the first-parent > difference. The idea never got traction, and I've long since abandoned > my implementation of it. Thank you for clarifying. Yes, I remember how that idea came up, and I even tried that strategy for a couple of merging rebases of Git for Windows' branch thicket. Sadly, it did not work half as well as I had hoped. The best idea I had back then still is in want of being implemented: sort of a "four-way merge". It is basically the same as a three-way merge, but allows for the pre-images to differ in the context (and yes, this cannot be represented using the current conflict markers). Definitely not trivial. > > To be honest, I developed one (minor) concern in the meantime... Should we > > maybe try to be nicer to our users and keep handling the > > `--preserve-merges` option by explicitly erroring out with the suggestion > > to use `--rebase-merges` instead? Not everybody reads release notes, after > > all. In fact, it is my experience that preciously few users have the time > > to even skim release notes... > > A valid concern, I would think. Good. Since you concur with my hunch, I implemented that change. Thank you for reviewing, Dscho
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > On Wed, Sep 01 2021, Junio C Hamano wrote: > >> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> >> writes: >> >>> In 427c3bd28ab (rebase: deprecate --preserve-merges, 2019-03-11) (which was >>> included in v2.22.0), we officially deprecated the --preserve-merges >>> backend. Over two years later, it is time to drop that backend, and here is >>> a patch series that does just that. >> >> A good goal. There is no remaining use case where (a fictitious and >> properly working version of) "--preserve-merges" option cannot be >> replaced by "--rebase-merges", is it? I somehow had a feeling that >> the other Johannes (sorry if it weren't you, j6t) had cases that the >> former worked better, but perhaps I am mis-remembering things. > > Fair enough. To be clear I think this series is fine as-is, we've just > usually done "now that this function is dead, rm it" as part of the > series that makes it dead, so I figured fixups/squashes to change those > parts would be welcome & integrated, likewise Alban Gruin's suggestions > in <62fbd389-28f5-76e5-d3f3-5510415a7bf5@gmail.com>. > > But the git-sh-i18n.sh change and/or his suggestions can be done after > this lands... I have this funny feeling that the "Fair enough" is thrown at a comment that you didn't intend to ;-)
On Tue, Sep 07 2021, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> On Wed, Sep 01 2021, Junio C Hamano wrote: >> >>> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> >>> writes: >>> >>>> In 427c3bd28ab (rebase: deprecate --preserve-merges, 2019-03-11) (which was >>>> included in v2.22.0), we officially deprecated the --preserve-merges >>>> backend. Over two years later, it is time to drop that backend, and here is >>>> a patch series that does just that. >>> >>> A good goal. There is no remaining use case where (a fictitious and >>> properly working version of) "--preserve-merges" option cannot be >>> replaced by "--rebase-merges", is it? I somehow had a feeling that >>> the other Johannes (sorry if it weren't you, j6t) had cases that the >>> former worked better, but perhaps I am mis-remembering things. >> >> Fair enough. To be clear I think this series is fine as-is, we've just >> usually done "now that this function is dead, rm it" as part of the >> series that makes it dead, so I figured fixups/squashes to change those >> parts would be welcome & integrated, likewise Alban Gruin's suggestions >> in <62fbd389-28f5-76e5-d3f3-5510415a7bf5@gmail.com>. >> >> But the git-sh-i18n.sh change and/or his suggestions can be done after >> this lands... > > I have this funny feeling that the "Fair enough" is thrown at a > comment that you didn't intend to ;-) I think I meant to reply to https://lore.kernel.org/git/xmqqlf4aejko.fsf@gitster.g/; I don't know how I got them mixed up, sorry about that.
On Tue, Sep 7, 2021 at 11:51 AM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > > Hi Hannes, > > On Thu, 2 Sep 2021, Johannes Sixt wrote: > > > Am 02.09.21 um 16:18 schrieb Johannes Schindelin: > > > On Wed, 1 Sep 2021, Junio C Hamano wrote: > > >> A good goal. There is no remaining use case where (a fictitious and > > >> properly working version of) "--preserve-merges" option cannot be > > >> replaced by "--rebase-merges", is it? I somehow had a feeling that > > >> the other Johannes (sorry if it weren't you, j6t) had cases that the > > >> former worked better, but perhaps I am mis-remembering things. > > > > > > I think that I managed to address whatever concerns there were about the > > > `--rebase-merges` backend in the meantime. > > > > That was either my suggestion/desire to make no-rebase-cousins the > > default. That has been settled. > > > > Or my wish not to redo the merge, but to replay the first-parent > > difference. The idea never got traction, and I've long since abandoned > > my implementation of it. > > Thank you for clarifying. > > Yes, I remember how that idea came up, and I even tried that strategy for > a couple of merging rebases of Git for Windows' branch thicket. Sadly, it > did not work half as well as I had hoped. > > The best idea I had back then still is in want of being implemented: sort > of a "four-way merge". It is basically the same as a three-way merge, but > allows for the pre-images to differ in the context (and yes, this cannot > be represented using the current conflict markers). Definitely not > trivial. merge-ort opens a new possibility (since it does merges without touching the index or working tree): Take the merge commit, M, that you are trying to transplant. Hold on to it for a minute. Do what rebase-merges does now; namely, do a simple merge of the desired new branches that otherwise ignores M to get your new merge commit N. Hang on to N too for a minute. Now use merge-ort to auto-remerge M (much like AUTO_MERGE or --remerge-diff does) to get a new merge commit that we'll call pre-M. If M was a clean merge that the user didn't amend, then pre-M will match M. If M wasn't a clean merge or was amended, then pre-M will otherwise differ from M by not including any manual changes the user made when they originally created M -- such as removing conflict markers, fixing semantic conflicts, evil changes, etc. Now we've got three merge commits: pre-M, M, and N. (Technically, pre-M and N might be toplevel trees rather than full commits, but whatever.) The difference between pre-M and M represent the manual work the user did in order to create M. Now, do a three-way (non-recursive) merge of those commits, to get the rebased result, R. This operation has the affect of applying the changes from pre-M to M on top of N. There's obviously some edge cases (e.g. nested conflict markers), but I think they're better than the edge cases presented by the alternatives: * the first-parent difference idea silently discards intermediate changes from reapplying other patches (especially if other patches are added or dropped), which to me feels _very_ dangerous * the current rebase-merges idea silently discards manual user changes within the original merge commit (i.e. it hopes that there is no difference between pre-M and M), which can also be lossy * I don't think this idea drops any data, but it does run the risk of conflicts that are difficult to understand. But I suspect less so than your five-way merge would entail. If the difficulty of conflicts in this scheme is too high, we could do a few things like providing multiple versions (e.g. if either pre-M:file or N:file had conflicts, or maybe if R:file has nested conflicts, then place both R:file and N:file in the working tree somewhere) or pointing at special commands that help users figure out what went on (e.g. 'git log -1 --remerge-diff M -- file').
Hi Elijah, On Tue, 7 Sep 2021, Elijah Newren wrote: > On Tue, Sep 7, 2021 at 11:51 AM Johannes Schindelin > <Johannes.Schindelin@gmx.de> wrote: > > > > On Thu, 2 Sep 2021, Johannes Sixt wrote: > > > > > Am 02.09.21 um 16:18 schrieb Johannes Schindelin: > > > > On Wed, 1 Sep 2021, Junio C Hamano wrote: > > > >> A good goal. There is no remaining use case where (a fictitious and > > > >> properly working version of) "--preserve-merges" option cannot be > > > >> replaced by "--rebase-merges", is it? I somehow had a feeling that > > > >> the other Johannes (sorry if it weren't you, j6t) had cases that the > > > >> former worked better, but perhaps I am mis-remembering things. > > > > > > > > I think that I managed to address whatever concerns there were about the > > > > `--rebase-merges` backend in the meantime. > > > > > > That was either my suggestion/desire to make no-rebase-cousins the > > > default. That has been settled. > > > > > > Or my wish not to redo the merge, but to replay the first-parent > > > difference. The idea never got traction, and I've long since abandoned > > > my implementation of it. > > > > Thank you for clarifying. > > > > Yes, I remember how that idea came up, and I even tried that strategy for > > a couple of merging rebases of Git for Windows' branch thicket. Sadly, it > > did not work half as well as I had hoped. > > > > The best idea I had back then still is in want of being implemented: sort > > of a "four-way merge". It is basically the same as a three-way merge, but > > allows for the pre-images to differ in the context (and yes, this cannot > > be represented using the current conflict markers). Definitely not > > trivial. > > merge-ort opens a new possibility (since it does merges without > touching the index or working tree): Take the merge commit, M, that > you are trying to transplant. Hold on to it for a minute. Do what > rebase-merges does now; namely, do a simple merge of the desired new > branches that otherwise ignores M to get your new merge commit N. > Hang on to N too for a minute. Now use merge-ort to auto-remerge M > (much like AUTO_MERGE or --remerge-diff does) to get a new merge > commit that we'll call pre-M. If M was a clean merge that the user > didn't amend, then pre-M will match M. If M wasn't a clean merge or > was amended, then pre-M will otherwise differ from M by not including > any manual changes the user made when they originally created M -- > such as removing conflict markers, fixing semantic conflicts, evil > changes, etc. > > Now we've got three merge commits: pre-M, M, and N. (Technically, > pre-M and N might be toplevel trees rather than full commits, but > whatever.) The difference between pre-M and M represent the manual > work the user did in order to create M. Now, do a three-way > (non-recursive) merge of those commits, to get the rebased result, R. > This operation has the affect of applying the changes from pre-M to M > on top of N. > > There's obviously some edge cases (e.g. nested conflict markers), but > I think they're better than the edge cases presented by the > alternatives: > * the first-parent difference idea silently discards intermediate > changes from reapplying other patches (especially if other patches are > added or dropped), which to me feels _very_ dangerous > * the current rebase-merges idea silently discards manual user > changes within the original merge commit (i.e. it hopes that there is > no difference between pre-M and M), which can also be lossy > * I don't think this idea drops any data, but it does run the risk > of conflicts that are difficult to understand. But I suspect less so > than your five-way merge would entail. > > If the difficulty of conflicts in this scheme is too high, we could do > a few things like providing multiple versions (e.g. if either > pre-M:file or N:file had conflicts, or maybe if R:file has nested > conflicts, then place both R:file and N:file in the working tree > somewhere) or pointing at special commands that help users figure out > what went on (e.g. 'git log -1 --remerge-diff M -- file'). While I agree that `merge-ort` makes a lot of things much better, I think in this context we need to keep in mind that those nested merge conflicts can really hurt. In my tests (I tried to implement a strategy where a 3-way merge is done with M and N^, using the parent commits of M as merge parents successively, see https://lore.kernel.org/git/nycvar.QRO.7.76.6.1804130002090.65@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz/ for the nitty gritty), I ran into _nasty_ nested merge conflicts, even with trivial examples. And I came to the conviction that treating the merge conflict markers as Just Another Line was the main culprit. I wish I had the time to try out your proposed strategy with the conconcted example I presented in that mail I linked above. Because now I am curious what it would do... Ciao, Dscho
On Fri, Sep 10, 2021 at 5:08 AM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > > Hi Elijah, > > On Tue, 7 Sep 2021, Elijah Newren wrote: > > > On Tue, Sep 7, 2021 at 11:51 AM Johannes Schindelin > > <Johannes.Schindelin@gmx.de> wrote: > > > ... > > > Thank you for clarifying. > > > > > > Yes, I remember how that idea came up, and I even tried that strategy for > > > a couple of merging rebases of Git for Windows' branch thicket. Sadly, it > > > did not work half as well as I had hoped. > > > > > > The best idea I had back then still is in want of being implemented: sort > > > of a "four-way merge". It is basically the same as a three-way merge, but > > > allows for the pre-images to differ in the context (and yes, this cannot > > > be represented using the current conflict markers). Definitely not > > > trivial. > > > > merge-ort opens a new possibility (since it does merges without > > touching the index or working tree): Take the merge commit, M, that > > you are trying to transplant. Hold on to it for a minute. Do what > > rebase-merges does now; namely, do a simple merge of the desired new > > branches that otherwise ignores M to get your new merge commit N. > > Hang on to N too for a minute. Now use merge-ort to auto-remerge M > > (much like AUTO_MERGE or --remerge-diff does) to get a new merge > > commit that we'll call pre-M. If M was a clean merge that the user > > didn't amend, then pre-M will match M. If M wasn't a clean merge or > > was amended, then pre-M will otherwise differ from M by not including > > any manual changes the user made when they originally created M -- > > such as removing conflict markers, fixing semantic conflicts, evil > > changes, etc. > > > > Now we've got three merge commits: pre-M, M, and N. (Technically, > > pre-M and N might be toplevel trees rather than full commits, but > > whatever.) The difference between pre-M and M represent the manual > > work the user did in order to create M. Now, do a three-way > > (non-recursive) merge of those commits, to get the rebased result, R. > > This operation has the affect of applying the changes from pre-M to M > > on top of N. > > > > There's obviously some edge cases (e.g. nested conflict markers), but > > I think they're better than the edge cases presented by the > > alternatives: > > * the first-parent difference idea silently discards intermediate > > changes from reapplying other patches (especially if other patches are > > added or dropped), which to me feels _very_ dangerous > > * the current rebase-merges idea silently discards manual user > > changes within the original merge commit (i.e. it hopes that there is > > no difference between pre-M and M), which can also be lossy > > * I don't think this idea drops any data, but it does run the risk > > of conflicts that are difficult to understand. But I suspect less so > > than your five-way merge would entail. > > > > If the difficulty of conflicts in this scheme is too high, we could do > > a few things like providing multiple versions (e.g. if either > > pre-M:file or N:file had conflicts, or maybe if R:file has nested > > conflicts, then place both R:file and N:file in the working tree > > somewhere) or pointing at special commands that help users figure out > > what went on (e.g. 'git log -1 --remerge-diff M -- file'). > > While I agree that `merge-ort` makes a lot of things much better, I think > in this context we need to keep in mind that those nested merge conflicts > can really hurt. Yes, and I'm not sure you fully appreciate how much either. Your discussion in the other thread of nested conflicts suggests you may not be aware of a few facts; for regular merges (not talking about rebase-merges yet): * Merges can have nested conflicts DESPITE having unique merge bases and NOT using merge.conflictstyle=diff3 * With unique merge bases (i.e. not a "recursive" merge), a merge will have at most two layers of conflicts * non-unique merge-bases and merge.conflictstyle=diff3 make nested conflicts much more likely * There is no limit on the number of nestings of conflict markers for a merge Now, in terms of rebase-merges: You noted in the other thread the possibility of being unable to differentiate conflict markers because they had the same length. However, increasing the length on the second merge by one or two characters is not sufficient in general, because that might just make you match the length of one of the nested conflicts from the other merge. You need to programmatically determine the sizes of the conflict markers in the first merge, and then adjust based on it for your other merge(s). I know this will sound like I'm scaring you off of my idea even further, but I actually think despite the above that my ideas for rebase-merges may have merit. I just want people to actually understand the edge and corner cases. From my reading of the previous threads on rebasing merges, I'm concerned that there is no discussion of arbitrarily nested conflict markers and a complete omission of any considerations for path-based rather than content-based conflicts. > In my tests (I tried to implement a strategy where a 3-way merge is done > with M and N^, using the parent commits of M as merge parents > successively, see > https://lore.kernel.org/git/nycvar.QRO.7.76.6.1804130002090.65@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz/ > for the nitty gritty), I ran into _nasty_ nested merge conflicts, even > with trivial examples. And I came to the conviction that treating the > merge conflict markers as Just Another Line was the main culprit. I agree that we should not treat merge conflict markers as Just Another Line. There are other issues I can mention besides the above here, but I'm getting kind of long already. However, I think a big part of the problem you ran into was that the previous suggestions only work nicely in _really_ simple cases. In particular, although I like Phillip's suggested sequence of merges[1] a lot more than the other suggestions in that thread or from prior threads, his suggestion is going to be prone to unnecessary nested conflict markers, as you found in your testcase. [1] https://lore.kernel.org/git/6c8749ca-ec5d-b4b7-f1a0-50d9ad2949a5@talktalk.net/ > I wish I had the time to try out your proposed strategy with the > conconcted example I presented in that mail I linked above. Because now I > am curious what it would do... For this simple testcase, as best I understood it (you didn't quite describe it fully so I had to take a guess or two), it provides a simpler conflict than either of the two you showed you got (from different merge orderings of Phillip's suggestion). However, let me double check that I understood your testcase correctly; please correct any errors in my understanding. I believe the commit graph you were describing was this: * rebase-of-original-merge |\ | * rebase-of-local-add-caller-of-core * | rebase-of-local-rename-to-hi |/ * upstream | | | | * original-merge | |\ | | * local-add-caller-of-core | |/ |/| | * local-rename-to-hi |/ * initial-version Further, the versions of main.c in each of those are as follows: ==> initial-version: int core(void) { printf("Hello, world!\n"); } ==> local-rename-to-hi: int hi(void) { printf("Hello, world!\n"); } ==> local-add-caller-of-core: int core(void) { printf("Hello, world!\n"); } /* caller */ void caller(void) { core(); } ==> what an automatic merge of the two local-* branches would give: int hi(void) { printf("Hello, world!\n"); } /* caller */ void caller(void) { core(); } ==> original-merge (amended from above by s/core/hi/ in caller()): int hi(void) { printf("Hello, world!\n"); } /* caller */ void caller(void) { hi(); } ==> upstream: int greeting(void) { printf("Hello, world!\n"); } /* main event loop */ void event_loop(void) { /* TODO: place holder for now */ } ==> rebase-of-local-rename-to-hi (kept 'hi' over 'greeting'): int hi(void) { printf("Hello, world!\n"); } /* main event loop */ void event_loop(void) { /* TODO: place holder for now */ } ==> rebase-of-local-add-caller-of-core (kept both new functions, updated caller): int greeting(void) { printf("Hello, world!\n"); } /* main event loop */ void event_loop(void) { /* TODO: place holder for now */ } /* caller */ void caller(void) { greeting(); } If I've understood that all correctly, then my idea will give you the following conflict to resolve: ==> rebase-of-original-merge, before conflict resolution: int hi(void) { printf("Hello, world!\n"); } /* main event loop */ void event_loop(void) { /* TODO: place holder for now */ } /* caller */ void caller(void) { <<<<<<< HEAD greeting(); ||||||| auto-remerge of original-merge core(); ======= hi(); >>>>>>> original-merge }
Hi Elijah, On Fri, 10 Sep 2021, Elijah Newren wrote: > On Fri, Sep 10, 2021 at 5:08 AM Johannes Schindelin > <Johannes.Schindelin@gmx.de> wrote: > > > > On Tue, 7 Sep 2021, Elijah Newren wrote: > > > > > On Tue, Sep 7, 2021 at 11:51 AM Johannes Schindelin > > > <Johannes.Schindelin@gmx.de> wrote: > > > > > > [... talk about re-resolving merge conflicts when recreating merges > > > in a `git rebase --rebase-merges` run ...] > ... > > > > While I agree that `merge-ort` makes a lot of things much better, I think > > in this context we need to keep in mind that those nested merge conflicts > > can really hurt. > > Yes, and I'm not sure you fully appreciate how much either. *grin* > [... a lot of insight that I totally agree with ...] > > I believe the commit graph you were describing was this: > > * rebase-of-original-merge > |\ > | * rebase-of-local-add-caller-of-core > * | rebase-of-local-rename-to-hi > |/ > * upstream > | > | > | > | * original-merge > | |\ > | | * local-add-caller-of-core > | |/ > |/| > | * local-rename-to-hi > |/ > * initial-version > > > Further, the versions of main.c in each of those are as follows: > > ==> initial-version: > int core(void) { > printf("Hello, world!\n"); > } > > > ==> local-rename-to-hi: > int hi(void) { > printf("Hello, world!\n"); > } > > > ==> local-add-caller-of-core: > int core(void) { > printf("Hello, world!\n"); > } > /* caller */ > void caller(void) { > core(); > } > > > ==> what an automatic merge of the two local-* branches would give: > int hi(void) { > printf("Hello, world!\n"); > } > /* caller */ > void caller(void) { > core(); > } > > > ==> original-merge (amended from above by s/core/hi/ in caller()): > int hi(void) { > printf("Hello, world!\n"); > } > /* caller */ > void caller(void) { > hi(); > } > > > ==> upstream: > int greeting(void) { > printf("Hello, world!\n"); > } > /* main event loop */ > void event_loop(void) { > /* TODO: place holder for now */ > } > > > ==> rebase-of-local-rename-to-hi (kept 'hi' over 'greeting'): > int hi(void) { > printf("Hello, world!\n"); > } > /* main event loop */ > void event_loop(void) { > /* TODO: place holder for now */ > } > > > ==> rebase-of-local-add-caller-of-core (kept both new functions, > updated caller): > int greeting(void) { > printf("Hello, world!\n"); > } > /* main event loop */ > void event_loop(void) { > /* TODO: place holder for now */ > } > /* caller */ > void caller(void) { > greeting(); > } That all matches my recollection of what I wanted to drive at. > If I've understood that all correctly, then my idea will give you the > following conflict to resolve: > > ==> rebase-of-original-merge, before conflict resolution: > int hi(void) { > printf("Hello, world!\n"); > } > /* main event loop */ > void event_loop(void) { > /* TODO: place holder for now */ > } > /* caller */ > void caller(void) { > <<<<<<< HEAD > greeting(); > ||||||| auto-remerge of original-merge > core(); > ======= > hi(); > >>>>>>> original-merge > } That looks very intriguing! I would _love_ to play with this a bit, and I think you provided enough guidance to get going. I am currently preparing to go mostly offline for the second half of September, read: I won't be able to play with this before October. But I am definitely interested, this sounds very exciting. Ciao, Dscho
On Mon, Sep 13, 2021 at 4:24 AM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > > Hi Elijah, > > On Fri, 10 Sep 2021, Elijah Newren wrote: > > > On Fri, Sep 10, 2021 at 5:08 AM Johannes Schindelin > > <Johannes.Schindelin@gmx.de> wrote: > > > > > > On Tue, 7 Sep 2021, Elijah Newren wrote: > > > [...snip...] > > If I've understood that all correctly, then my idea will give you the > > following conflict to resolve: > > > > ==> rebase-of-original-merge, before conflict resolution: > > int hi(void) { > > printf("Hello, world!\n"); > > } > > /* main event loop */ > > void event_loop(void) { > > /* TODO: place holder for now */ > > } > > /* caller */ > > void caller(void) { > > <<<<<<< HEAD > > greeting(); > > ||||||| auto-remerge of original-merge > > core(); > > ======= > > hi(); > > >>>>>>> original-merge > > } > > That looks very intriguing! I would _love_ to play with this a bit, and I > think you provided enough guidance to get going. I am currently preparing > to go mostly offline for the second half of September, read: I won't be > able to play with this before October. But I am definitely interested, > this sounds very exciting. If you start working on it, let me know. I was thinking of playing with it, but don't know exactly when I'll get time to do so; very unlikely before October, and reasonably likely not even before the end of the year. While I've provided the high level details in this thread which are good enough to handle the simple cases, I think that the interesting bits are the non-simple cases. I have not thought all of them through, but I'll include below some notes of mine that might be helpful if you get to it first. Note that I focus below on the non-simple cases, and discuss content-based conflicts before covering path-based ones: * We're doing a three way merge of merges: pre-M, M, and N to get R; M is the original merge, pre-M is (automatic) remerge of M, and N is automatic merge of rebased parents of M. * Note that N is what current rebase-merges uses, so we have all information from that merge and can provide it to the user when or if it is helpful. * Both pre-M and N may themselves have conflicts. * We need to programmatically handle conflict marker length when pre-M and/or N have nested conflicts. (must modify merge routines to return the maximal conflict marker depth used) * Special case that pre-M matches N (per hunk): If both pre-M and N have conflict markers, but they happen to match, then we know to take the version from M and the result IS clean (at least for that hunk). So, you can still get a clean merge even if there are conflicts in both pre-M and N. * Special case that pre-M matches M (per hunk): Usually in the three-way merge of "Base, Left, Right => Result", if Base matches either side then you get a clean merge. However, if pre-M matches M but N has conflicts, the result is NOT clean. Another way to look at this is that conflict markers are special and should be treated differently than other lines. (And path-based conflicts probably need special handling too, as discussed below.) * In the case of complicated conflicts, consider providing user with both R:resulting-file and N:resulting-file (and point them at `git log -1 --remerge-diff M [-- resulting-file]`) * Having either binary files or path-based conflicts (e.g. modify/delete, file vs. directory vs. submodule, switch to symlink vs. switch to executable, rename/add, rename/rename -- either 1to2 or 2to1, directory rename detection, etc.) in either pre-M or N -- or both -- are going to need special care. * One example of path-based conflicts: Let's say pre-M had no conflict at path P, and that pre-M:P and M:P matched. Let's say that N:P had a modify/delete conflict. Note that for modify/delete conflicts we tend to print a message to the console and leave the modified version of the file in the working tree. Here, despite the fact that pre-M:P and M:P matched, we cannot just take the modified file from N at P as the merge result. The modify/delete conflict should persist and the user given an opportunity to resolve it. Representing the modify/delete might be an interesting question, though since... * If both pre-M and N have conflicts, then pre-M would have had up to three versions of file in the index at higher stages, N would have had up to three versions of file in the index at higher stages, and M would have one. We cannot represent all 7 versions of the file in the index at the end, which means conflict representation might be tricky. content-based conflicts are easier to handle here than path-based ones, since content-based conflicts can just do similar to what rename/rename(2to1) conflicts do today: just stick the result of the preliminary three-way merges into the index. path-based conflicts get more interesting because you can't do a preliminary merge of binary files or a preliminary merge of a modify/delete, etc. * If both pre-M and N have path-based conflicts, but of different types, how exactly do we mention that to the user? Just list all the types? (This probably qualifies as a case of a "complicated" conflict where we want to (also?) provide the user with N:resulting-file instead of (just?) R:resulting-file.) We may also need to modify merge machinery to return types of conflicts per-path, an extension of what my "merge-into" series (not yet submitted) provides.
In 427c3bd28ab (rebase: deprecate --preserve-merges, 2019-03-11) (which was included in v2.22.0), we officially deprecated the --preserve-merges backend. Over two years later, it is time to drop that backend, and here is a patch series that does just that. Changes since v1: * Rebased onto v2.33.0 Johannes Schindelin (7): t5520: do not use `pull.rebase=preserve` remote: warn about unhandled branch.<name>.rebase values tests: stop testing `git rebase --preserve-merges` pull: remove support for `--rebase=preserve` rebase: drop support for `--preserve-merges` git-svn: drop support for `--preserve-merges` rebase: drop the internal `rebase--interactive` command .github/workflows/main.yml | 1 - .gitignore | 1 - Documentation/config/branch.txt | 4 - Documentation/config/pull.txt | 4 - Documentation/git-pull.txt | 6 +- Documentation/git-rebase.txt | 51 - Documentation/git-svn.txt | 1 - Makefile | 2 - builtin/pull.c | 9 +- builtin/rebase.c | 318 +------ builtin/remote.c | 3 + contrib/completion/git-completion.bash | 2 +- git-rebase--preserve-merges.sh | 1057 --------------------- git-svn.perl | 1 - git.c | 1 - rebase.c | 2 - rebase.h | 1 - t/t3404-rebase-interactive.sh | 76 -- t/t3408-rebase-multi-line.sh | 10 - t/t3409-rebase-preserve-merges.sh | 130 --- t/t3410-rebase-preserve-dropped-merges.sh | 90 -- t/t3411-rebase-preserve-around-merges.sh | 80 -- t/t3412-rebase-root.sh | 37 - t/t3414-rebase-preserve-onto.sh | 85 -- t/t3418-rebase-continue.sh | 15 - t/t3421-rebase-topology-linear.sh | 19 - t/t3422-rebase-incompatible-options.sh | 11 - t/t3425-rebase-topology-merges.sh | 151 --- t/t3427-rebase-subtree.sh | 19 - t/t5520-pull.sh | 24 +- t/t7505-prepare-commit-msg-hook.sh | 1 - t/t7517-per-repo-email.sh | 13 - t/test-lib.sh | 4 - 33 files changed, 17 insertions(+), 2212 deletions(-) delete mode 100644 git-rebase--preserve-merges.sh delete mode 100755 t/t3409-rebase-preserve-merges.sh delete mode 100755 t/t3410-rebase-preserve-dropped-merges.sh delete mode 100755 t/t3411-rebase-preserve-around-merges.sh delete mode 100755 t/t3414-rebase-preserve-onto.sh base-commit: 225bc32a989d7a22fa6addafd4ce7dcd04675dbf Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-195%2Fdscho%2Fdrop-rebase-p-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-195/dscho/drop-rebase-p-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/195 Range-diff vs v1: 1: 662e16dbc12 ! 1: 8da29d539fc t5520: do not use `pull.rebase=preserve` @@ t/t5520-pull.sh: test_expect_success '--rebase=false create a new merge commit' - test_config pull.rebase preserve && + test_config pull.rebase merges && git pull --rebase=true . copy && - test "$(git rev-parse HEAD^^)" = "$(git rev-parse copy)" && - test file3 = "$(git show HEAD:file3.t)" + test_cmp_rev HEAD^^ copy && + echo file3 >expect && @@ t/t5520-pull.sh: test_expect_success '--rebase=invalid fails' ' - ! git pull --rebase=invalid . copy + test_must_fail git pull --rebase=invalid . copy ' -test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-merge' ' @@ t/t5520-pull.sh: test_expect_success '--rebase=invalid fails' ' - test_config pull.rebase preserve && + test_config pull.rebase merges && git pull --rebase . copy && - test "$(git rev-parse HEAD^^)" = "$(git rev-parse copy)" && - test file3 = "$(git show HEAD:file3.t)" + test_cmp_rev HEAD^^ copy && + echo file3 >expect && 2: fb531064b35 < -: ----------- remote: warn about unhandled branch.<name>.rebase values -: ----------- > 2: acda0f59947 remote: warn about unhandled branch.<name>.rebase values 3: b614336f3df ! 3: cdb9fae4b93 tests: stop testing `git rebase --preserve-merges` @@ Commit message Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> - ## azure-pipelines.yml ## -@@ azure-pipelines.yml: jobs: - HOME: $(Build.SourcesDirectory) - MSYSTEM: MINGW64 - NO_SVN_TESTS: 1 -- GIT_TEST_SKIP_REBASE_P: 1 - - powershell: | - if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") { - cmd /c rmdir "$(Build.SourcesDirectory)\test-cache" -@@ azure-pipelines.yml: jobs: - HOME: $(Build.SourcesDirectory) - MSYSTEM: MINGW64 - NO_SVN_TESTS: 1 -- GIT_TEST_SKIP_REBASE_P: 1 - - powershell: | - if ("$GITFILESHAREPWD" -ne "" -and "$GITFILESHAREPWD" -ne "`$`(gitfileshare.pwd)") { - cmd /c rmdir "$(Build.SourcesDirectory)\test-cache" + ## .github/workflows/main.yml ## +@@ .github/workflows/main.yml: jobs: + shell: bash + env: + NO_SVN_TESTS: 1 +- GIT_TEST_SKIP_REBASE_P: 1 + run: ci/run-test-slice.sh ${{matrix.nr}} 10 + - name: ci/print-test-failures.sh + if: failure() ## t/t3404-rebase-interactive.sh ## @@ t/t3404-rebase-interactive.sh: test_expect_success 'retain authorship when squashing' ' @@ t/t3404-rebase-interactive.sh: test_expect_success 'retain authorship when squas -' - -test_expect_success REBASE_P 'preserve merges with -p' ' -- git checkout -b to-be-preserved master^ && +- git checkout -b to-be-preserved primary^ && - : > unrelated-file && - git add unrelated-file && - test_tick && - git commit -m "unrelated" && -- git checkout -b another-branch master && +- git checkout -b another-branch primary && - echo B > file1 && - test_tick && - git commit -m J file1 && @@ t/t3404-rebase-interactive.sh: test_expect_success 'retain authorship when squas - git commit -m M file1 && - git checkout -b to-be-rebased && - test_tick && -- git rebase -i -p --onto branch1 master && +- git rebase -i -p --onto branch1 primary && - git update-index --refresh && - git diff-files --quiet && - git diff-index --quiet --cached HEAD -- && -- test $(git rev-parse HEAD~6) = $(git rev-parse branch1) && -- test $(git rev-parse HEAD~4^2) = $(git rev-parse to-be-preserved) && -- test $(git rev-parse HEAD^^2^) = $(git rev-parse HEAD^^^) && +- test_cmp_rev HEAD~6 branch1 && +- test_cmp_rev HEAD~4^2 to-be-preserved && +- test_cmp_rev HEAD^^2^ HEAD^^^ && - test $(git show HEAD~5:file1) = B && - test $(git show HEAD~3:file1) = C && - test $(git show HEAD:file1) = E && @@ t/t3409-rebase-preserve-merges.sh (deleted) - -Run "git rebase -p" and check that merges are properly carried along -' +-GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main +-export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +- -. ./test-lib.sh - -if ! test_have_prereq REBASE_P; then @@ t/t3409-rebase-preserve-merges.sh (deleted) - -# Clone 2 (conflicting merge): -# --# A1--A2--B3 <-- origin/master +-# A1--A2--B3 <-- origin/main -# \ \ -# B1------M <-- topic -# \ @@ t/t3409-rebase-preserve-merges.sh (deleted) -# -# Clone 3 (no-ff merge): -# --# A1--A2--B3 <-- origin/master +-# A1--A2--B3 <-- origin/main -# \ -# B1------M <-- topic -# \ / @@ t/t3409-rebase-preserve-merges.sh (deleted) - echo Second > B && - git add B && - git commit -m "Add B1" && -- git checkout -f master && +- git checkout -f main && - echo Third >> A && - git commit -a -m "Modify A2" && - echo Fifth > B && @@ t/t3409-rebase-preserve-merges.sh (deleted) - ( - cd clone2 && - git checkout -b topic origin/topic && -- test_must_fail git merge origin/master && +- test_must_fail git merge origin/main && - echo Resolved >B && - git add B && -- git commit -m "Merge origin/master into topic" +- git commit -m "Merge origin/main into topic" - ) && - - git clone ./. clone3 && @@ t/t3412-rebase-root.sh: test_expect_success 'pre-rebase got correct input (4)' ' -test_expect_success REBASE_P 'rebase -i -p with linear history' ' - git checkout -b work5 other && -- git rebase -i -p --root --onto master && +- git rebase -i -p --root --onto main && - git log --pretty=tformat:"%s" > rebased5 && - test_cmp expect rebased5 -' @@ t/t3412-rebase-root.sh: commit work6~4 -test_expect_success REBASE_P 'rebase -i -p with merge' ' - git checkout -b work6 other && -- git rebase -i -p --root --onto master && +- git rebase -i -p --root --onto main && - log_with_names work6 > rebased6 && - test_cmp expect-side rebased6 -' @@ t/t3412-rebase-root.sh: commit work7~5 -test_expect_success REBASE_P 'rebase -i -p with two roots' ' - git checkout -b work7 other && -- git rebase -i -p --root --onto master && +- git rebase -i -p --root --onto main && - log_with_names work7 > rebased7 && - test_cmp expect-third rebased7 -' @@ t/t3412-rebase-root.sh: commit conflict3~6 -test_expect_success REBASE_P 'rebase -i -p --root with conflict (first part)' ' - git checkout -b conflict3 other && -- test_must_fail git rebase -i -p --root --onto master && +- test_must_fail git rebase -i -p --root --onto main && - git ls-files -u | grep "B$" -' - @@ t/t3418-rebase-continue.sh: test_rerere_autoupdate ## t/t3421-rebase-topology-linear.sh ## @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { - test_run_rebase success '' + test_run_rebase success --apply test_run_rebase success -m test_run_rebase success -i -test_have_prereq !REBASE_P || test_run_rebase success -p @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { test_expect_success 'setup branches and remote tracking' ' git tag -l >tags && @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { - test_run_rebase success '' + test_run_rebase success --apply test_run_rebase success -m test_run_rebase success -i -test_have_prereq !REBASE_P || test_run_rebase success -p test_run_rebase () { result=$1 -@@ t/t3421-rebase-topology-linear.sh: test_run_rebase success '' +@@ t/t3421-rebase-topology-linear.sh: test_run_rebase success --apply test_run_rebase success --fork-point test_run_rebase success -m test_run_rebase success -i @@ t/t3421-rebase-topology-linear.sh: test_run_rebase success '' test_run_rebase () { result=$1 -@@ t/t3421-rebase-topology-linear.sh: test_run_rebase success '' +@@ t/t3421-rebase-topology-linear.sh: test_run_rebase success --apply test_run_rebase success --fork-point test_run_rebase success -m test_run_rebase success -i @@ t/t3421-rebase-topology-linear.sh: test_run_rebase success '' test_run_rebase () { result=$1 -@@ t/t3421-rebase-topology-linear.sh: test_run_rebase success '' +@@ t/t3421-rebase-topology-linear.sh: test_run_rebase success --apply test_run_rebase success --fork-point test_run_rebase success -m test_run_rebase success -i @@ t/t3421-rebase-topology-linear.sh: test_run_rebase success '' # f # / @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { - test_run_rebase success '' + test_run_rebase success --apply test_run_rebase success -m test_run_rebase success -i -test_have_prereq !REBASE_P || test_run_rebase success -p @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { test_run_rebase () { result=$1 @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { - test_run_rebase success '' + test_run_rebase success --apply test_run_rebase success -m test_run_rebase success -i -test_have_prereq !REBASE_P || test_run_rebase success -p @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { test_run_rebase () { result=$1 @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { - test_run_rebase success '' + test_run_rebase success --apply test_run_rebase success -m test_run_rebase success -i -test_have_prereq !REBASE_P || test_run_rebase success -p @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { test_run_rebase () { result=$1 @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { - test_run_rebase success '' + test_run_rebase success --apply test_run_rebase success -m test_run_rebase success -i -test_have_prereq !REBASE_P || test_run_rebase success -p @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { # a---b---c---j! # \ @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { - test_run_rebase success '' + test_run_rebase failure --apply test_run_rebase success -m test_run_rebase success -i --test_have_prereq !REBASE_P || test_run_rebase success -p +-test_have_prereq !REBASE_P || test_run_rebase failure -p test_run_rebase () { result=$1 @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { - test_run_rebase success '' + } test_run_rebase success -m test_run_rebase success -i --test_have_prereq !REBASE_P || test_run_rebase failure -p +-test_have_prereq !REBASE_P || test_run_rebase success -p test_run_rebase () { result=$1 @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { - test_run_rebase success '' + } test_run_rebase success -m test_run_rebase success -i --test_have_prereq !REBASE_P || test_run_rebase failure -p +-test_have_prereq !REBASE_P || test_run_rebase success -p test_run_rebase success --rebase-merges # m @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { - test_run_rebase success '' + test_run_rebase success --apply test_run_rebase success -m test_run_rebase success -i -test_have_prereq !REBASE_P || test_run_rebase success -p @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { test_run_rebase () { result=$1 @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { - test_run_rebase success '' + test_run_rebase success --apply test_run_rebase success -m test_run_rebase success -i -test_have_prereq !REBASE_P || test_run_rebase failure -p @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { test_run_rebase () { result=$1 @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { - test_run_rebase success '' + test_run_rebase success --apply test_run_rebase success -m test_run_rebase success -i -test_have_prereq !REBASE_P || test_run_rebase success -p @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { test_run_rebase () { result=$1 @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { - test_run_rebase success '' + test_run_rebase success --apply test_run_rebase success -m test_run_rebase success -i -test_have_prereq !REBASE_P || test_run_rebase failure -p @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { test_run_rebase () { result=$1 @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { - test_run_rebase success '' + test_run_rebase success --apply test_run_rebase success -m test_run_rebase success -i -test_have_prereq !REBASE_P || test_run_rebase failure -p @@ t/t3421-rebase-topology-linear.sh: test_run_rebase () { test_done ## t/t3422-rebase-incompatible-options.sh ## -@@ t/t3422-rebase-incompatible-options.sh: test_rebase_am_only --ignore-whitespace - test_rebase_am_only --committer-date-is-author-date +@@ t/t3422-rebase-incompatible-options.sh: test_rebase_am_only () { + test_rebase_am_only --whitespace=fix test_rebase_am_only -C4 -test_expect_success REBASE_P '--preserve-merges incompatible with --signoff' ' @@ t/t3422-rebase-incompatible-options.sh: test_rebase_am_only --ignore-whitespace test_done ## t/t3425-rebase-topology-merges.sh ## -@@ t/t3425-rebase-topology-merges.sh: test_run_rebase success 'd n o e' '' +@@ t/t3425-rebase-topology-merges.sh: test_run_rebase success 'd n o e' --apply test_run_rebase success 'd n o e' -m test_run_rebase success 'd n o e' -i @@ t/t3427-rebase-subtree.sh: test_expect_success 'setup' ' git commit -m "Empty commit" --allow-empty ' --# FAILURE: Does not preserve master4. +-# FAILURE: Does not preserve topic_4. -test_expect_failure REBASE_P 'Rebase -Xsubtree --preserve-merges --onto commit' ' - reset_rebase && - git checkout -b rebase-preserve-merges to-rebase && -- git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master && -- verbose test "$(commit_message HEAD~)" = "master4" && -- verbose test "$(commit_message HEAD)" = "files_subtree/master5" +- git rebase -Xsubtree=files_subtree --preserve-merges --onto files-main main && +- verbose test "$(commit_message HEAD~)" = "topic_4" && +- verbose test "$(commit_message HEAD)" = "files_subtree/topic_5" -' - --# FAILURE: Does not preserve master4. +-# FAILURE: Does not preserve topic_4. -test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit' ' - reset_rebase && - git checkout -b rebase-keep-empty to-rebase && -- git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master && -- verbose test "$(commit_message HEAD~2)" = "master4" && -- verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && +- git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-main main && +- verbose test "$(commit_message HEAD~2)" = "topic_4" && +- verbose test "$(commit_message HEAD~)" = "files_subtree/topic_5" && - verbose test "$(commit_message HEAD)" = "Empty commit" -' - - test_expect_success 'Rebase -Xsubtree --keep-empty --onto commit' ' + test_expect_success 'Rebase -Xsubtree --empty=ask --onto commit' ' reset_rebase && git checkout -b rebase-onto to-rebase && ## t/t5520-pull.sh ## @@ t/t5520-pull.sh: test_expect_success 'pull.rebase=1 is treated as true and flattens keep-merge' ' - test file3 = "$(git show HEAD:file3.t)" + test_cmp expect actual ' -test_expect_success REBASE_P \ @@ t/t5520-pull.sh: test_expect_success 'pull.rebase=1 is treated as true and flatt - git reset --hard before-preserve-rebase && - test_config pull.rebase preserve && - git pull . copy && -- test "$(git rev-parse HEAD^^)" = "$(git rev-parse copy)" && -- test "$(git rev-parse HEAD^2)" = "$(git rev-parse keep-merge)" +- test_cmp_rev HEAD^^ copy && +- test_cmp_rev HEAD^2 keep-merge -' - test_expect_success 'pull.rebase=interactive' ' write_script "$TRASH_DIRECTORY/fake-editor" <<-\EOF && echo I was here >fake.out && @@ t/t5520-pull.sh: test_expect_success '--rebase=true rebases and flattens keep-merge' ' - test file3 = "$(git show HEAD:file3.t)" + test_cmp expect actual ' -test_expect_success REBASE_P \ @@ t/t5520-pull.sh: test_expect_success '--rebase=true rebases and flattens keep-me - git reset --hard before-preserve-rebase && - test_config pull.rebase true && - git pull --rebase=preserve . copy && -- test "$(git rev-parse HEAD^^)" = "$(git rev-parse copy)" && -- test "$(git rev-parse HEAD^2)" = "$(git rev-parse keep-merge)" +- test_cmp_rev HEAD^^ copy && +- test_cmp_rev HEAD^2 keep-merge -' - test_expect_success '--rebase=invalid fails' ' git reset --hard before-preserve-rebase && - ! git pull --rebase=invalid . copy + test_must_fail git pull --rebase=invalid . copy ## t/t7505-prepare-commit-msg-hook.sh ## @@ t/t7505-prepare-commit-msg-hook.sh: test_rebase () { @@ t/t7505-prepare-commit-msg-hook.sh: test_rebase () { -test_have_prereq !REBASE_P || test_rebase success -p test_expect_success 'with hook (cherry-pick)' ' - test_when_finished "git checkout -f master" && + test_when_finished "git checkout -f main" && ## t/t7517-per-repo-email.sh ## @@ t/t7517-per-repo-email.sh: test_expect_success 'noop interactive rebase does not care about ident' ' @@ t/t7517-per-repo-email.sh: test_expect_success 'noop interactive rebase does not -test_expect_success REBASE_P \ - 'fast-forward rebase does not care about ident (preserve)' ' - git checkout -B tmp side-without-commit && -- git rebase -p master +- git rebase -p main -' - -test_expect_success REBASE_P \ - 'non-fast-forward rebase refuses to write commits (preserve)' ' - test_when_finished "git rebase --abort || true" && - git checkout -B tmp side-with-commit && -- test_must_fail git rebase -p master +- test_must_fail git rebase -p main -' - test_expect_success 'author.name overrides user.name' ' @@ t/t7517-per-repo-email.sh: test_expect_success 'noop interactive rebase does not test_config user.email user@example.com && ## t/test-lib.sh ## -@@ t/test-lib.sh: test_lazy_prereq CURL ' - test_lazy_prereq SHA1 ' - test $(git hash-object /dev/null) = e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 +@@ t/test-lib.sh: test_lazy_prereq SHA1 ' + esac ' -- + -test_lazy_prereq REBASE_P ' - test -z "$GIT_TEST_SKIP_REBASE_P" -' +- + # Ensure that no test accidentally triggers a Git command + # that runs the actual maintenance scheduler, affecting a user's + # system permanently. 4: 0c8bfe5d18d ! 4: b493046134d pull: remove support for `--rebase=preserve` @@ Commit message Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> ## Documentation/config/branch.txt ## -@@ Documentation/config/branch.txt: When `merges`, pass the `--rebase-merges` option to 'git rebase' +@@ Documentation/config/branch.txt: When `merges` (or just 'm'), pass the `--rebase-merges` option to 'git rebase' so that the local merge commits are included in the rebase (see linkgit:git-rebase[1] for details). + --When `preserve` (deprecated in favor of `merges`), also pass +-When `preserve` (or just 'p', deprecated in favor of `merges`), also pass -`--preserve-merges` along to 'git rebase' so that locally committed merge -commits will not be flattened by running 'git pull'. -+ - When the value is `interactive`, the rebase is run in interactive mode. + When the value is `interactive` (or just 'i'), the rebase is run in interactive + mode. + - *NOTE*: this is a possibly dangerous operation; do *not* use ## Documentation/config/pull.txt ## -@@ Documentation/config/pull.txt: When `merges`, pass the `--rebase-merges` option to 'git rebase' +@@ Documentation/config/pull.txt: When `merges` (or just 'm'), pass the `--rebase-merges` option to 'git rebase' so that the local merge commits are included in the rebase (see linkgit:git-rebase[1] for details). + --When `preserve` (deprecated in favor of `merges`), also pass +-When `preserve` (or just 'p', deprecated in favor of `merges`), also pass -`--preserve-merges` along to 'git rebase' so that locally committed merge -commits will not be flattened by running 'git pull'. -+ - When the value is `interactive`, the rebase is run in interactive mode. + When the value is `interactive` (or just 'i'), the rebase is run in interactive + mode. + - *NOTE*: this is a possibly dangerous operation; do *not* use ## Documentation/git-pull.txt ## @@ Documentation/git-pull.txt: Options related to merging @@ Documentation/git-pull.txt: When set to `merges`, rebase using `git rebase --reb -`--preserve-merges` option passed to `git rebase` so that locally created -merge commits will not be flattened. -+ - When false, merge the current branch into the upstream branch. + When false, merge the upstream branch into the current branch. + When `interactive`, enable the interactive mode of rebase. ## builtin/pull.c ## -@@ builtin/pull.c: enum rebase_type { - REBASE_INVALID = -1, - REBASE_FALSE = 0, - REBASE_TRUE, -- REBASE_PRESERVE, - REBASE_MERGES, - REBASE_INTERACTIVE - }; -@@ builtin/pull.c: enum rebase_type { +@@ /** * Parses the value of --rebase. If value is a false value, returns * REBASE_FALSE. If value is a true value, returns REBASE_TRUE. If value is @@ builtin/pull.c: enum rebase_type { */ static enum rebase_type parse_config_rebase(const char *key, const char *value, int fatal) -@@ builtin/pull.c: static enum rebase_type parse_config_rebase(const char *key, const char *value, - return REBASE_FALSE; - else if (v > 0) - return REBASE_TRUE; -- else if (!strcmp(value, "preserve") || !strcmp(value, "p")) -- return REBASE_PRESERVE; - else if (!strcmp(value, "merges") || !strcmp(value, "m")) - return REBASE_MERGES; - else if (!strcmp(value, "interactive") || !strcmp(value, "i")) @@ builtin/pull.c: static struct option pull_options[] = { /* Options passed to git-merge or git-rebase */ OPT_GROUP(N_("Options related to merging")), - { OPTION_CALLBACK, 'r', "rebase", &opt_rebase, -- "(false|true|merges|preserve|interactive)", -+ "(false|true|merges|interactive)", - N_("incorporate changes by rebasing rather than merging"), - PARSE_OPT_OPTARG, parse_opt_rebase }, + OPT_CALLBACK_F('r', "rebase", &opt_rebase, +- "(false|true|merges|preserve|interactive)", ++ "(false|true|merges|interactive)", + N_("incorporate changes by rebasing rather than merging"), + PARSE_OPT_OPTARG, parse_opt_rebase), OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL, -@@ builtin/pull.c: static int run_rebase(const struct object_id *curr_head, +@@ builtin/pull.c: static int run_rebase(const struct object_id *newbase, /* Options passed to git-rebase */ if (opt_rebase == REBASE_MERGES) - argv_array_push(&args, "--rebase-merges"); + strvec_push(&args, "--rebase-merges"); - else if (opt_rebase == REBASE_PRESERVE) -- argv_array_push(&args, "--preserve-merges"); +- strvec_push(&args, "--preserve-merges"); else if (opt_rebase == REBASE_INTERACTIVE) - argv_array_push(&args, "--interactive"); + strvec_push(&args, "--interactive"); if (opt_diffstat) ## contrib/completion/git-completion.bash ## @@ contrib/completion/git-completion.bash: __git_complete_config_variable_value () return ;; remote.pushdefault) + + ## rebase.c ## +@@ rebase.c: enum rebase_type rebase_parse_value(const char *value) + return REBASE_FALSE; + else if (v > 0) + return REBASE_TRUE; +- else if (!strcmp(value, "preserve") || !strcmp(value, "p")) +- return REBASE_PRESERVE; + else if (!strcmp(value, "merges") || !strcmp(value, "m")) + return REBASE_MERGES; + else if (!strcmp(value, "interactive") || !strcmp(value, "i")) + + ## rebase.h ## +@@ rebase.h: enum rebase_type { + REBASE_INVALID = -1, + REBASE_FALSE = 0, + REBASE_TRUE, +- REBASE_PRESERVE, + REBASE_MERGES, + REBASE_INTERACTIVE + }; 5: 14e242b3cf9 ! 5: eb738b1bf05 rebase: drop support for `--preserve-merges` @@ Documentation/git-rebase.txt: i.e. commits that would be excluded by linkgit:git onto `<upstream>` (or `<onto>`, if specified). + -The `--rebase-merges` mode is similar in spirit to the deprecated --`--preserve-merges`, but in contrast to that option works well in interactive --rebases: commits can be reordered, inserted and dropped at will. +-`--preserve-merges` but works with interactive rebases, +-where commits can be reordered, inserted and dropped at will. -+ It is currently only possible to recreate the merge commits using the `recursive` merge strategy; Different merge strategies can be used only via @@ Documentation/git-rebase.txt: are incompatible with the following options: - * --preserve-merges * --interactive * --exec - * --keep-empty + * --no-keep-empty @@ Documentation/git-rebase.txt: are incompatible with the following options: In addition, the following pairs of options are incompatible: @@ Documentation/git-rebase.txt: are incompatible with the following options: - * --preserve-merges and --interactive - * --preserve-merges and --signoff - * --preserve-merges and --rebase-merges +- * --preserve-merges and --empty= +- * --preserve-merges and --ignore-whitespace +- * --preserve-merges and --committer-date-is-author-date +- * --preserve-merges and --ignore-date * --keep-base and --onto * --keep-base and --root - -@@ Documentation/git-rebase.txt: merge tlsv1.3 - merge cmake - ------------ + * --fork-point and --root +@@ Documentation/git-rebase.txt: CONFIGURATION + include::config/rebase.txt[] + include::config/sequencer.txt[] -BUGS ----- @@ Documentation/git-rebase.txt: merge tlsv1.3 Part of the linkgit:git[1] suite ## Makefile ## -@@ Makefile: SCRIPT_SH += git-web--browse.sh +@@ Makefile: SCRIPT_SH += git-submodule.sh + SCRIPT_SH += git-web--browse.sh SCRIPT_LIB += git-mergetool--lib - SCRIPT_LIB += git-parse-remote -SCRIPT_LIB += git-rebase--preserve-merges - SCRIPT_LIB += git-sh-setup SCRIPT_LIB += git-sh-i18n + SCRIPT_LIB += git-sh-setup @@ Makefile: XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --language=Perl \ + --keyword=__ --keyword=N__ --keyword="__n:1,2" LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H) LOCALIZED_SH = $(SCRIPT_SH) - LOCALIZED_SH += git-parse-remote.sh -LOCALIZED_SH += git-rebase--preserve-merges.sh LOCALIZED_SH += git-sh-setup.sh LOCALIZED_PERL = $(SCRIPT_PERL) ## builtin/rebase.c ## -@@ builtin/rebase.c: enum rebase_type { +@@ builtin/rebase.c: static GIT_PATH_FUNC(merge_dir, "rebase-merge") + enum rebase_type { REBASE_UNSPECIFIED = -1, - REBASE_AM, - REBASE_MERGE, -- REBASE_INTERACTIVE, + REBASE_APPLY, +- REBASE_MERGE, - REBASE_PRESERVE_MERGES -+ REBASE_INTERACTIVE ++ REBASE_MERGE }; - struct rebase_options { + enum empty_type { @@ builtin/rebase.c: int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) - static int is_interactive(struct rebase_options *opts) + static int is_merge(struct rebase_options *opts) { -- return opts->type == REBASE_INTERACTIVE || +- return opts->type == REBASE_MERGE || - opts->type == REBASE_PRESERVE_MERGES; -+ return opts->type == REBASE_INTERACTIVE; ++ return opts->type == REBASE_MERGE; } - static void imply_interactive(struct rebase_options *opts, const char *option) -@@ builtin/rebase.c: static void imply_interactive(struct rebase_options *opts, const char *option) - die(_("%s requires an interactive rebase"), option); + static void imply_merge(struct rebase_options *opts, const char *option) +@@ builtin/rebase.c: static void imply_merge(struct rebase_options *opts, const char *option) + die(_("%s requires the merge backend"), option); break; - case REBASE_INTERACTIVE: + case REBASE_MERGE: - case REBASE_PRESERVE_MERGES: break; - case REBASE_MERGE: - /* we now implement --merge via --interactive */ + default: + opts->type = REBASE_MERGE; /* implied */ @@ builtin/rebase.c: static struct commit *peel_committish(const char *name) return (struct commit *)peel_to_type(name, 0, obj, OBJ_COMMIT); } @@ builtin/rebase.c: static struct commit *peel_committish(const char *name) - } -} - - #define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION" - - #define RESET_HEAD_DETACH (1<<0) + static int move_to_original_branch(struct rebase_options *opts) + { + struct strbuf orig_head_reflog = STRBUF_INIT, head_reflog = STRBUF_INIT; @@ builtin/rebase.c: static int run_am(struct rebase_options *opts) static int run_specific_rebase(struct rebase_options *opts, enum action action) @@ builtin/rebase.c: static int run_am(struct rebase_options *opts) int status; - const char *backend, *backend_func; - if (opts->type == REBASE_INTERACTIVE) { - /* Run builtin interactive rebase */ + if (opts->type == REBASE_MERGE) { + /* Run sequencer-based rebase */ @@ builtin/rebase.c: static int run_specific_rebase(struct rebase_options *opts, enum action action) } - status = run_rebase_interactive(opts, action); + status = run_sequencer_rebase(opts, action); - goto finished_rebase; - } - -- if (opts->type == REBASE_AM) { -+ } else if (opts->type == REBASE_AM) +- if (opts->type == REBASE_APPLY) { ++ } else if (opts->type == REBASE_APPLY) status = run_am(opts); - goto finished_rebase; - } @@ builtin/rebase.c: static int run_specific_rebase(struct rebase_options *opts, en - add_var(&script_snippet, "revisions", opts->revisions); - add_var(&script_snippet, "restrict_revision", opts->restrict_revision ? - oid_to_hex(&opts->restrict_revision->object.oid) : NULL); -- add_var(&script_snippet, "GIT_QUIET", -- opts->flags & REBASE_NO_QUIET ? "" : "t"); -- sq_quote_argv_pretty(&buf, opts->git_am_opts.argv); +- sq_quote_argv_pretty(&buf, opts->git_am_opts.v); - add_var(&script_snippet, "git_am_opt", buf.buf); - strbuf_release(&buf); - add_var(&script_snippet, "verbose", @@ builtin/rebase.c: static int run_specific_rebase(struct rebase_options *opts, en - add_var(&script_snippet, "git_format_patch_opt", - opts->git_format_patch_opt.buf); - -- if (is_interactive(opts) && +- if (is_merge(opts) && - !(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) { - strbuf_addstr(&script_snippet, - "GIT_SEQUENCE_EDITOR=:; export GIT_SEQUENCE_EDITOR; "); @@ builtin/rebase.c: static int run_specific_rebase(struct rebase_options *opts, en -finished_rebase: if (opts->dont_finish_rebase) ; /* do nothing */ - else if (opts->type == REBASE_INTERACTIVE) + else if (opts->type == REBASE_MERGE) @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix) N_("let the user edit the list of commits to rebase"), PARSE_OPT_NOARG | PARSE_OPT_NONEG, - parse_opt_interactive }, + parse_opt_interactive), - OPT_SET_INT_F('p', "preserve-merges", &options.type, - N_("(DEPRECATED) try to recreate merges instead of " - "ignoring them"), - REBASE_PRESERVE_MERGES, PARSE_OPT_HIDDEN), OPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate), - OPT_BOOL('k', "keep-empty", &options.keep_empty, - N_("preserve empty commits during rebase")), + OPT_CALLBACK_F(0, "empty", &options, "{drop,keep,ask}", + N_("how to handle commits that become empty"), @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix) strbuf_reset(&buf); strbuf_addf(&buf, "%s/rewritten", merge_dir()); @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix if (options.onto_name) die(_("cannot combine '--keep-base' with '--onto'")); @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix) + case REBASE_APPLY: die(_("--strategy requires --merge or --interactive")); case REBASE_MERGE: - case REBASE_INTERACTIVE: - case REBASE_PRESERVE_MERGES: /* compatible */ break; case REBASE_UNSPECIFIED: @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix) + switch (options.type) { case REBASE_MERGE: - case REBASE_INTERACTIVE: - case REBASE_PRESERVE_MERGES: options.state_dir = merge_dir(); break; - case REBASE_AM: + case REBASE_APPLY: @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix) - } + options.reschedule_failed_exec = reschedule_failed_exec; if (options.signoff) { - if (options.type == REBASE_PRESERVE_MERGES) - die("cannot combine '--signoff' with " - "'--preserve-merges'"); - argv_array_push(&options.git_am_opts, "--signoff"); + strvec_push(&options.git_am_opts, "--signoff"); options.flags |= REBASE_FORCE; } @@ git-rebase--preserve-merges.sh (deleted) - fi -} - --# Put the last action marked done at the beginning of the todo list --# again. If there has not been an action marked done yet, leave the list of --# items on the todo list unchanged. --reschedule_last_action () { -- tail -n 1 "$done" | cat - "$todo" >"$todo".new -- sed -e \$d <"$done" >"$done".new -- mv -f "$todo".new "$todo" -- mv -f "$done".new "$done" --} -- -append_todo_help () { - gettext " -Commands: 6: b7ba83969da = 6: a987e9439af git-svn: drop support for `--preserve-merges` 7: 634e4141e97 ! 7: 4492cca369c rebase: drop the internal `rebase--interactive` command @@ builtin/rebase.c: static const char *action_names[] = { "undefined", static int edit_todo_file(unsigned flags) { const char *todo_file = rebase_path_todo(); -@@ builtin/rebase.c: static int run_rebase_interactive(struct rebase_options *opts, +@@ builtin/rebase.c: static int run_sequencer_rebase(struct rebase_options *opts, break; } @@ builtin/rebase.c: static int run_rebase_interactive(struct rebase_options *opts, default: BUG("invalid command '%d'", command); } -@@ builtin/rebase.c: static int run_rebase_interactive(struct rebase_options *opts, - return ret; +@@ builtin/rebase.c: static int parse_opt_keep_empty(const struct option *opt, const char *arg, + return 0; } -static const char * const builtin_rebase_interactive_usage[] = { @@ builtin/rebase.c: static int run_rebase_interactive(struct rebase_options *opts, -int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) -{ - struct rebase_options opts = REBASE_OPTIONS_INIT; -- struct object_id squash_onto = null_oid; +- struct object_id squash_onto = *null_oid(); - enum action command = ACTION_NONE; - struct option options[] = { - OPT_NEGBIT(0, "ff", &opts.flags, N_("allow fast-forward"), - REBASE_FORCE), -- OPT_BOOL(0, "keep-empty", &opts.keep_empty, N_("keep empty commits")), -- OPT_BOOL(0, "allow-empty-message", &opts.allow_empty_message, -- N_("allow commits with empty messages")), +- OPT_CALLBACK_F('k', "keep-empty", &options, NULL, +- N_("keep commits which start empty"), +- PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, +- parse_opt_keep_empty), +- OPT_BOOL_F(0, "allow-empty-message", &opts.allow_empty_message, +- N_("allow commits with empty messages"), +- PARSE_OPT_HIDDEN), - OPT_BOOL(0, "rebase-merges", &opts.rebase_merges, N_("rebase merge commits")), - OPT_BOOL(0, "rebase-cousins", &opts.rebase_cousins, - N_("keep original branch points of cousins")), @@ builtin/rebase.c: static int run_rebase_interactive(struct rebase_options *opts, - warning(_("--[no-]rebase-cousins has no effect without " - "--rebase-merges")); - -- return !!run_rebase_interactive(&opts, command); +- return !!run_sequencer_rebase(&opts, command); -} - - static int is_interactive(struct rebase_options *opts) + static int is_merge(struct rebase_options *opts) { - return opts->type == REBASE_INTERACTIVE; + return opts->type == REBASE_MERGE; ## git.c ## @@ git.c: static struct cmd_struct commands[] = { 8: 45fee72059d < -: ----------- remote: no longer claim that branch.*.rebase=preserve is a thing