Re: [PATCH/RFC] fast-import: disallow empty branches as parents

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


+Shawn Pearce

On Thu, Jun 21, 2012 at 1:34 AM, Dmitry Ivankov <divanorama@xxxxxxxxx> wrote:
> Combinations of "reset", "commit" with "from" and/or "merge" commands
> may make fast-import to produce bad objects (null_sha1 parents) or
> accept bad inputs (ones asking for empty branches as parents).

I was trying to split the patch into small an obvious cases and found out that
there are 4 ways to make or refer to "empty" branches:
1. reset branch_name
2. reset branch_name \n from `0{40}`
3. refer to it as `0{40}`
4. commit branch_name for a new branch name. It's empty up to the
'commit' command end.
Note: I'll use 4 to denote a reference to a branch name from the
'commit' command to this very branch name.

And these branches can be used in 'from' and 'merge' commands.

In 'from' command:
- 1,2,3 are allowed in 'from', no bug of writing 0{40} parent here as
'from' parent is checked against null_sha1
- 4 is not allowed with message "Can't create a branch from itself"

In 'merge' command:
- 1,2 are allowed in 'merge' and lead to 0{40} being actually written
as a commit parent - BUG
- 3 is not allowed with message "Not a valid commit"
- 4 without 'from' is allowed, parent is previous branch tip. May be 0{40} - BUG
- 4 with 'from' is allowed, 'merge' parent is 'from' parent - probably
a bug, also may lead to 0{40} being written - BUG

BUG stuff obviously needs to be fixed, at least we can just skip 0{40}
parents in 'merge' command.

Then difference in 3 between 'from' and 'merge' should be dealt with.
'merge' behaviour  comes from
    tags/v1.5.0.4~21^2 2f6dc35d2ad0b.. 5 Mar 2007 fast-import: Fail if
a non-existant commit is used for merge
since then 'merge sha1' just looks up commit with this sha1. But
'from' command has it's story too - it was
a 'new_branch' long ago (up to tags/v1.5.0-rc4~14^2~64) and since the
very beginning (tags/v1.5.0-rc4~14^2~76)
is had a special case to allow 0{40} as a parent. So, should we just
allow 'merge 0{40}'?

And finally 4 in 'merge' should probably always refer to the previous
tip to cause less surprise (it was the largest
part of my patch, though it turns out not the most interesting one).

P.S. At first I thought that any reference to an empty branch in
'merge' of 'from' should be rejected, but given that we
allow sha1 0{40} to be used in these and in 2, I guess we should keep
it allowed.
--
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


[Newbies FAQ]     [Linux Kernel Development]     [Free Online Dating]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Free Online Dating]     [Photo]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]     [Linux Resources]

Add to Google