Omit or disallow merge parents to be ancestors

edre commented 5 months ago

Description

It doesn't make logical sense to me to have a commit with two parents that have an ancestral relationship. The ancestor is redundant and should probably be omitted.

I experienced this when I had two local branches from upstream and a local merge of them. One of the branches got accepted upstream. When I rebased, my second branch got rebased onto my first, and my local merge got rebased onto both of them, meaning that it had two parents that were ancestor and descendant. I expected the ancestral commit to be omitted from the merge during the rebase.

I checked with git and hg; neither of them allow merging with an ancestral commit.

Steps to Reproduce the Problem

jj git init .
echo a > a; jj commit -m  'one a'
echo bb > b; jj commit -m 'two b'
jj new @- @--

Expected Behavior

Merge commit does not have a parent that is an ancestor of another parent, either by error or omission.

Actual Behavior

New commit has both parents.

Specifications

Platform: linux musl
Version: jj 0.16.0-2dcdc7fb3f20f262a73f52019f5a7b7e48fe069d

martinvonz commented 5 months ago

We actually did that simplification until recently: commits https://github.com/martinvonz/jj/commit/3f1d75f518600c8c4ba4c65de2c126b95cdac7a9, https://github.com/martinvonz/jj/commit/a89884733360f1e758816603ff3f93ab7a81c34a, and https://github.com/martinvonz/jj/commit/e14ee8b563422e3f2d87bcbcdc4b8cd998785822. As I mentioned in the second of those commits, this was a bit of an experiment and we might roll it back. So thanks for your input.

I feel pretty strongly that auto-rebasing should not simplify ancestor merges, however (i.e., I don't think we should roll back the first of those commits).

yuja commented 5 months ago

(I'm not against changing the rebase command behavior or adding --simplify-merge option, but)

jj new @- @--

I don't think redundant parents should be silently omitted here because it would be confusing if jj new x y didn't create a merge. It may error out, but I personally like the current behavior. It's simple and consistent. git can create a such merge with --no-ff.

edre commented 5 months ago

I guess I don't have a strong opinion about the default rebase behavior, but I'd like a flag or a config option to turn on simplification.

For jj new I agree that redundant parents shouldn't be silently omitted, but I can't see a use case where someone would do it on purpose so an error could be more suitable. Maybe not if y'all feel this would be against the jj ethos of not failing commands.

martinvonz / jj