Hi Sergey, On Wed, 14 Mar 2018, Sergey Organov wrote: > Igor Djordjevic <igor.d.djordjevic@xxxxxxxxx> writes: > > > On 07/03/2018 08:26, Johannes Schindelin wrote: > > [...] > > >> Second side note: if we can fast-forward, currently we prefer that, > >> and I think we should keep that behavior with -R, too. > > > > I agree. > > I'm admittedly somewhat lost in the discussion, ... and cutting so much context does not help... The "fast-forward" here refers to the thing `git rebase` does unless you call it with `--force-rebase`: if we are about to `pick` a commit, and HEAD already points to its parent commit, we do not evey cherry-pick: we just fast-forward to the original commit. Likewise, `merge -C <commit>` will simply fast-forward to the specified commit if HEAD is pointing to its first parent and the specified merge heads are also identical to the respective original parent commits. What I said with my comment was that `merge -R -C <commit>` will behave in exactly the same way. > but are you talking fast-forward on _rebasing_ existing merge? Where > would it go in any of the suggested algorithms of rebasing and why? And this is something I did not talk about yet, but it does come up in practice: the main use case of rebasing branches is to follow an "upstream", of course. And guess what? From time to time, some of the branches get merged upstream (or, in Git's own source code, applied). In this case, the Git garden shears *do* skip the merge (as the new merge head is already an ancestor of HEAD). In --recreate-merges, it *will* create a new merge commit, though, which is a bit annoying. The best way to handle this would *probably* look similar to how "empty" commits (i.e. commits whose patch is empty) are handled by rebase. But it is not yet clear to me how that would look in practice, as `pick <commit>` is a single operation that can be easily commented out in the todo list depending on `--allow-empty`, while the `merge -C <commit>` command is not the entire operation, as there may be `pick` commands in the merge head that could potentially be skipped due to merge conflicts with already-applied versions of the same patches. If this sounds unclear to you, please do ask for clarification. Although this is currently not my highest priority in the `sequencer-shears` branch thicket (where `--recreate-merges` is the first part). > I readily see how it can break merges. E.g., any "git merge --ff-only > --no-ff" merge will magically disappear. Did you mean `git merge --no-ff`? Combining `--ff-only` with `--no-ff` does not make sense. > So, even if somehow supported, fast-forward should not be performed by > default during _rebasing_ of a merge. That statement is too general to be correct. *If* we detect that the original merge could have been fast-forwarded instead (and it is very easy to detect that in --make-list), we would have to handle that similar to afore-mentioned empty commits in conjunction with `--allow-empty`. If the original merge could not have been fast-forwarded, but during the rebase it *can*, we should skip it, as the reason for the merge commit is clearly no longer there. This, by the way, is an insight you can really only win by using --recreate-merges (or the Git garden shears). And using it a *lot*. Because otherwise, you will not even guess correctly what will, and what won't, come up in practice. > >> If the user wants to force a new merge, they simply remove that -R > >> flag. > > Alternatively, they'd replace 'pick' with 'merge', as they already do > for other actions. "A plurality is not to be posited without necessity". No, no, no and no again! We learned how *wrong* the `pick` approach was with --preserve-merges! Have you *ever* used --preserve-merges in any real way? If so, you *have* encountered the problems, and probably directed several lengthy curses my way for that lousy design. A pick is a pick is a pick. Of a single patch with metadata such as the author, commit message and the date. A merge is not a patch. While a pick introduces a specific change on top of a single revision, the changes introduced by a merge are ideally already there. It is conceptually *very* different. Sure, a merge commit sometimes needs to introduce extra changes on top of the merged changes, sure, that happens (and is called "evil merge" in Git parlance). Those *additional* changes should be kept as minimal as possible, in particular there should only be changes *necessitated* by the reconciled changes. So no, a `pick` is not a `merge`. Not at all. > Please, _please_, don't use 'merge' command to 'pick' merge commits! > It's utterly confusing! Please, please, please *work* with branch thickets for a while. And you will see that it makes a *huge* difference! For example, within a block of `pick` lines, it is relatively safe to reorder. You see more or less what changes interact with one another. Be *very* careful when reordering `merge` lines, in particular when you have a real-world branch thicket, not just a toy example. Do *not* use `pick` for merges. Not. Ever. By the way, let this here discussion serve as Yet Another Example why it is important to not only consider new features theoretically. Only in practice do you find out where your theory sounded too good to be actually true. > Thinking about it I've got an idea that what we actually need is > --no-flatten flag that, when used alone, will just tell "git rebase" to > stop flattening history, and which will be implicitly imposed by > --recreate-merges (and --preserve-merges). ... and this flag would only make sense if every `git rebase` involved a todo list. Which it does not. Ciao, Johannes
