[PATCH 3 of 8] Add filesystem path to dirstate.statwalk return value

Paul Moore p.f.moore at gmail.com
Thu May 1 15:37:35 CDT 2008


2008/5/1 Matt Mackall <mpm at selenic.com>:
>
>  >                      for f, src, st in findfiles(f):
>  > -                        yield src, f, st
>  > +                        if src == 'm':
>  > +                            yield src, f, f, st
>  > +                        else:
>  > +                            yield src, f, fspath(f), st
>
>  findfiles presumable returns things -only- in the case on the
>  filesystem, so fspath here is redundant. In fact, we'd actually need a
>  dirstatepath(f) here, because various things will be expecting to get
>  filenames as they are in the dirstate/manifest.

Ack. Certainly the fspath is redundant. Whether you need
dirstatepath() I'm not so sure. In theory, yes, I guess so, but in
practice, nothing is exhibiting a bug thus far that is caused by that.

One proviso here - I'm concentrating solely on fixing identified bugs.
Once we get into what is theoretically required, you get into a whole
other mess, as the assumptions in the current code simply aren't
consistent. For example, comparing filenames using == is wrong on a
case insensitive system, but the code is riddled with filename
comparisons using ==. "Fixing" these is both unnecessary and wasteful
unless they are actually causing issues. (I'm struggling here to
maintain my "practicality is better than purity" stance here, as part
of me really wants to "do it right", but the cost is just too high,
and anyway I'm nowhere near familiar enough with the code to risk
trying...)

Paul.


More information about the Mercurial-devel mailing list