[PATCH] merge: detect conflict between added file and directory (issue29)

[Evgeniy Makeev]

On 10/10/12 3:16 PM, "Matt Mackall" <mpm at selenic.com> wrote:

>On Tue, 2012-10-09 at 14:21 -0700, Evgeniy Makeev wrote:
>> # HG changeset patch
>> # User Evgeniy Makeev <evgeniym at fb.com>
>> # Date 1349739050 25200
>> # Node ID 072243d45be07be438736f125baa6a2381dd53c4
>> # Parent  3c775c5a6c03ed010ce2b85dd59968c63afc77b6
>> merge: detect conflict between added file and directory (issue29)
>> 
>> The fix checks for conflicts between files being added by merge/update
>>and
>> local directories during manifest processing phase. If there is
>> a conflict, user will be prompted to either abort the operation or skip
>> adding the conflicting files if the conflicting local directory is not
>> empty. If the conflicting local directory is empty, user will be
>>prompted
>> either to remove the empty directory or abort.
>
>I think this is a good start, but needs a lot of work still.
>
>First, this appears to only check for collisions in one direction. What
>about going the other way?

There is another patch checking the other direction collisions, I emailed
it yesterday as well (header below).
Did you have chance to look at it? Should I combine both patches in one?

# HG changeset patch
# User Evgeniy Makeev <evgeniym at fb.com>
# Date 1349754802 25200
# Node ID 227411b21482b7ee80a85628989957716eb3659b
# Parent  072243d45be07be438736f125baa6a2381dd53c4
merge: detect conflict between path of added file and a local file
(issue29)

The fix checks for conflicts between local files and directories being
merged/added, if there is a conflict, user is prompted to abbort or
skip merging/adding all files wich have the conflicting directories
in their paths. The check is done by building a directory set off all
partial paths of added files.

diff -r 072243d45be0 -r 227411b21482 mercurial/merge.py
...

>
>> +                if os.path.isdir(dirpath):
>
>Poking at the filesystem with an extra syscall for each new remote file
>is not a good idea. Especially on systems like OS X and Windows which
>have really bad file lookup performance and syscall overhead.

Would satisfying 'if f in p1.dirs()' condition first instead if
os.path.isdir be better? There won't be a syscall in non collision cases,
but there will be an overhead of building all dirs set.

>
>> +                        if not repo.ui.promptchoice(
>> +                            _("cannot merge file %s, existing
>>directory %s is"
>> +                              " in the way\nwould you like to (A)bort
>>operation"
>> +                              " or (S)kip writing the file? [a]")
>> +                              % (f, dirpath), (_("&Abort"),
>>_("&Skip")), 0):
>
>Here's what an existing prompt looks like, spot the style differences:
>
>                if repo.ui.promptchoice(
>                    _(" local changed %s which remote deleted\n"
>                      "use (c)hanged version or (d)elete?") % f,
>                    (_("&Changed"), _("&Delete")), 0):
>
>Also, your "(s)kip" here actually means "resolve the conflict by keeping
>the directory and deleting the file", which we should be more explicit
>about. Really, we should give an option to keep either the directory or
>the file, like this:
>
> local file foo conflicts with remote directory foo/ with 2 files
> keep (f)ile or keep (d)irectory?
>
>Further, there's no attempt here to consult the ancestor to see where
>the conflict came from. In some cases, this may actually be a
>non-conflict.
>
>Lastly, there's the issue of case-collision: if the file is foo and the
>directory is Foo/, we'll still get the same nasty abort on OS X or
>Windows.
>
>-- 
>Mathematics is the supreme nostalgia of our time.
>
>

Thanks,
Evgeniy