Technical question about dirstate.statwalk

Paul Moore p.f.moore at gmail.com
Tue May 20 05:41:34 CDT 2008


2008/5/20 Benoit Boissinot <bboissin at gmail.com>:
> I think there is a bug and it should test if nf is in dc instead,
> while looking at the code I've (and Adrian) found a couple more bugs.

Thanks for checking this.

> Can someone review the following ?
>
> And it would be great to have more tests for this.
>

> diff -r 170818dad56b mercurial/dirstate.py
> --- a/mercurial/dirstate.py     Mon May 19 10:23:47 2008 +0200
> +++ b/mercurial/dirstate.py     Tue May 20 11:55:42 2008 +0200
> @@ -435,7 +435,7 @@
>         '''
>
>         def fwarn(f, msg):
> -            self._ui.warn('%s: %s\n' % (self.pathto(ff), msg))
> +            self._ui.warn('%s: %s\n' % (self.pathto(f), msg))
>             return False

That definitely looks like a bug.

>         badfn = fwarn
>         if hasattr(match, 'bad'):
> @@ -543,7 +543,7 @@
>                     if inst.errno != errno.ENOENT:
>                         fwarn(ff, inst.strerror)
>                     elif badfn(ff, inst.strerror) and imatch(nf):
> -                        yield 'f', ff, None
> +                        yield 'f', nf, None

Not sure about this - ff is what the user entered, nf is the
"normpath" corrected version. Either is plausible here. I don't think
it's a clear bug, but it's probably a little bettter the way you've
changed it.

>                 continue
>             if s_isdir(st.st_mode):
>                 if not dirignore(nf):
> @@ -556,7 +556,7 @@
>                 if match(nf):
>                     if supported(ff, st.st_mode, verbose=True):
>                         yield 'f', nf, st
> -                    elif ff in dc:
> +                    elif nf in dc:
>                         yield 'm', nf, st

That's the one I noticed, so obviously I agree here :-)

I agree with you that additional tests would be very useful.
Unfortunately, while I have the test suite working on Windows, I'm not
entirely happy with it - at the moment, I get 18 failed tests with the
latest testing patch series plus current crew (no local changes). I
can only assume that these are "meant" to fail on Windows, as I've no
way of knowing if they are genuine bugs. If someone says that the
Windows tests should pass 100%, then fair enough but if so, then crew
is pretty broken at the moment :-( (Before my last pull, it was only 8
tests failing, so the failing tests aren't even stable :-()

I'd love it if the test suite was migrated to a unittest-based
portable set of tests, but that's a pretty huge job, and not one I
expect to be able to tackle soon, so I just have to live with things
as they are at the moment...

Paul.


More information about the Mercurial-devel mailing list