[PATCH 1 of 2] subrepo: check that subrepo paths are not existing files

[Jordi Gutiérrez Hermoso]

On Wed, 2014-09-10 at 01:31 +0200, Mads Kiilerich wrote:
> On 09/09/2014 07:56 PM, Jordi Gutiérrez Hermoso wrote:
> > # HG changeset patch
> > # User Jordi Gutiérrez Hermoso <jordigh at octave.org>
> > # Date 1410279390 14400
> > #      Tue Sep 09 12:16:30 2014 -0400
> > # Node ID 7611a7833657301d9a881095828d5e75558d82f9
> > # Parent  897041f6b025778193c6da5b9795da09a91c866e
> > subrepo: check that subrepo paths are not existing files
> >
> > This check has to happen early one, as the .hgsub file is being
> > parsed.
> 
> Why?

Because why not catch an error as early as possible?

Currently these problems are caught later after hgsub has been parsed
and when attempting to create a subrepo Python object. This is too
late, and is the core problem in bug #4363.

> > This should provide an early error message.
> 
> Could we please have a test to show it?

Sure, I'll write one.

> > diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> > --- a/mercurial/subrepo.py
> > +++ b/mercurial/subrepo.py
> > @@ -126,6 +126,10 @@ def state(ctx, ui):
> >   
> >       state = {}
> >       for path, src in p[''].items():
> > +
> 
> Why add an extra empty line here? It is stupid to spend time
> complaining about it but it also doesn't work to add an empty after
> every for loop.

It made sense to me, but I don't really care, I'll get rid of it.

> > +        if os.path.isfile(path):
> 
> Depending on what problem this patch is solving, shouldn't it also 
> consider directories and symlinks and other file types?

Those other problems are already caught by pathauditor in the other
patch.

I'll explain that in the commit message.

- Jordi G. H.