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

Matt Mackall mpm at selenic.com
Wed Oct 10 17:16:07 CDT 2012


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?

> +                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.

> +                        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.




More information about the Mercurial-devel mailing list