[PATCH 00 of 13] Cleanup of the purge extension

Alexis S. L. Carvalho alexis at cecm.usp.br
Mon Mar 26 19:48:19 CDT 2007


Thus spake Emanuele Aina:
> Alexis S. L. Carvalho abbozzò:
> >But I think I'm leaning towards something like trying to lstat every
> >missing file.  If we find any of them, abort (unconditionally).
> >Otherwise, abort if --force was not specified.  Something like
> >this (completely untested):
> >
> >found = []
> >for f in missing:
> >    try:
> >        os.lstat(f)
> >    except OSError, inst:
> >        if inst.errno != errno.ENOENT:
> >            raise
> 
> Maybe we could just ignore every possible error: after all, our problem
>  is with files that still exist. :)
> 
> Then it could be simplyfied to:
> 
> found = [f for f in missing if os.path.exist(f)]

Sure - but use the util.lexists function that I've (finally) just added
to crew, since we may have some dangling symlinks there.

> >        continue
> >    found.append(f)
> >
> >if found:
> >    if not ui.quiet:
> >        ui.warn(_("The following tracked files weren't listed by the "
> >	          "filesystem, but could still be found:\n"))
> >        for f in found:
> >            ui.warn("%s\n" % f)
> >	ui.warn(_("Is this a case-insensitive filesystem?\n"))
> 
> What about using "util.checkfolding(repo.path)" to avoid the question?

OK...

> >    raise util.Abort(_("purging on name mangling filesystems is not "
> >                       "yet fully supported"))
> >
> >if missing and not force:
> >    raise util.Abort(_("some nice message that mentions missing files "
> >                       "and --force"))
> 
> I'd rather check this before stat'ing the missing files.

To avoid the extra stats?  /me shrugs.  By the time this runs we'll have
stat'ed the whole repo, so it shouldn't make much difference.  I
suggested doing it after the stats just to keep the --force message from
being displayed before we're sure it'll make a difference.

But if you prefer it coming before the stats, go for it...

> >I think that should be safe enough - we really want to abort in the
> >"if found:" case; OTOH the second conditional is probably not strictly
> >necessary.
> 
> My try:
> 
> if missing and not force:
>     raise util.Abort(_("There are missing files in the working dir and "
>                        "purge still has problems with them due to name "
>                        "mangling filesystems.\n"
>                        "Use --force if you know what you are doing"))
> 
> found = [f for f in missing if os.path.exist(f)]

                                 util.lexists(f)

> if found:
>     if not ui.quiet:
>         ui.warn(_("The following tracked files weren't listed by the "
>                   "filesystem, but could still be found:\n"))
>         for f in found:
>             ui.warn("%s\n" % f)
>         if util.checkfolding(repo.path):
>             ui.warn(_("This probably due to a case-insensitive "
>                       "filesystem\n"))

                          missing verb.

>     raise util.Abort(_("purging on name mangling filesystems is not "
>                        "yet fully supported"))

Looks OK.  Can you send the patch?

Thanks

Alexis


More information about the Mercurial-devel mailing list