On 06/19/2014 02:39 PM, Fabian Ruch wrote:
> Hi Michael,
>
> thanks for your reply.
>
> On 06/19/2014 01:35 PM, Michael Haggerty wrote:
>> On 06/18/2014 02:10 PM, Fabian Ruch wrote:
>>> `rebase` supports the option `--root` both with and without `--onto`.
>>> The case where `--onto` is not specified is handled by creating a
>>> sentinel commit and squashing the root commit into it. The sentinel
>>> commit refers to the empty tree and does not have a log message
>>> associated with it. Its purpose is that `rebase` can rely on having a
>>> rebase base even without `--onto`.
>>>
>>> The combination of `--root` and no `--onto` implies an interactive
>>> rebase. When `--preserve-merges` is not specified on the `rebase`
>>> command line, `rebase--interactive` uses `--cherry-pick` with
>>> git-rev-list to put the initial to-do list together. If the root commit
>>> is empty, it is treated as a cherry-pick of the sentinel commit and
>>> omitted from the todo-list. This is unexpected because the user does not
>>> know of the sentinel commit.
>>
>> I see that your new tests below both use --keep-empty. Without
>> --keep-empty, I would have expected empty commits to be discarded by
>> design. If that is the case, then there is only a bug if --keep-empty
>> is used, and I think you should mention that option earlier in this
>> description.
>
> Now that you mention it, --keep-empty is crucial for this to be a bug
> (except for the case where the branch consists solely of empty commits).
> I intended to use --keep-empty merely as a pedagogic tool so nobody
> would get confused about what is on the to-do list.
>
>> Also, I think this bug strikes if *any* of the commits to be rebased is
>> empty, not only the first commit.
>
> Ah, I really did not deduce that all empty commits would disappear with
> --root and --keep-empty. Thanks.
>
>>> Add a test case. Create an empty root commit, run `rebase --root` and
>>> check that it is still there. If the branch consists of the root commit
>>> only, the bug described above causes the resulting history to consist of
>>> the sentinel commit only. If the root commit has children, the resulting
>>> history contains neither the root nor the sentinel commit. This
>>> behaviour is the same with `--keep-empty`.
>>>
>>> Signed-off-by: Fabian Ruch <bafain@xxxxxxxxx>
>>> ---
>>>
>>> Notes:
>>> Hi,
>>>
>>> This is not a fix yet.
>>
>> It is actually OK to add failing tests to the test suite, but they must
>> be added with 'test_expect_failure' instead of 'test_expect_success'.
>> Though of course it is preferred if the new test is followed by a commit
>> that fixes it :-)
>
> I did not plan to have this accepted but to amend the patch with a fix
> later on. Also, I hoped the ready-to-apply tests would give someone else
> a smoother start when taking over and compensate for a possibly
> incomprehensible problem description.
>
>>> We are currently special casing in `do_pick` and whether the current
>>> head is the sentinel commit is not a special case that would fit into
>>> `do_pick`'s interface description. What if we added the feature of
>>> creating root commits to `do_pick`, using `commit-tree` just like when
>>> creating the sentinel commit? We would have to add another special case
>>> (`test -z "$onto"`) to where the to-do list is put together in
>>> `rebase--interactive`. An empty `$onto` would imply
>>>
>>> git rev-list $orig_head
>>>
>>> to form the to-do list. The rebase comment in the commit message editor
>>> would have to become something similar to
>>>
>>> Rebase $shortrevisions as new history
>>>
>>> , which might be even less confusing than mentioning the hash of the
>>> sentinel commit.
>>
>> Since you are working on a hammer, I'm tempted to see this problem as a
>> nail. Would it make it easier to encode the special behavior into the
>> todo list itself?:
>>
>> pick --orphan 0cf23b1 New initial commit
>> pick 144a852 Second commit
>> pick 255f8de Third commit
>
> While I agree to enable pick to create orphan commits, I don't think a
> user option --orphan is of much help. Firstly, does --orphan make sense
> for any commit but the first one on the to-do list? Secondly, does
> --orphan make sense when we are rebasing onto another branch? The second
> point is related to the first in the sense that "pick --orphan" would be
> used on a commit that is understood to have a parent.
--orphan as a user option would only really make sense if we get around
to supporting interactive rebase of arbitrary DAGs.
Perhaps a more practical problem with --orphan is that it makes it
harder for the user to change the order of the first two commits.
Another possible construct would be a separate "orphan" command:
orphan
pick 0cf23b1 New initial commit
pick 144a852 Second commit
pick 255f8de Third commit
But these are just wild ideas. I haven't thought enough about the
problem to advocate anything.
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html