[PATCH 2 of 2 stable] scmutil: record all symlinks to a repository in walkrepos

Matt Mackall mpm at selenic.com
Fri Dec 12 14:52:27 CST 2014


On Thu, 2014-12-11 at 10:47 -0500, Enrique A. Tobis wrote:
> # HG changeset patch
> # User Enrique A. Tobis <enrique at tobis.com.ar>
> # Date 1418275100 18000
> #      Thu Dec 11 00:18:20 2014 -0500
> # Branch stable
> # Node ID a2351ea044f117402b17809050bdc5dc2de29ff4
> # Parent  c67b563426866e1c717c6a25b0056bb51cb155e5
> scmutil: record all symlinks to a repository in walkrepos
> 
> In walkrepos, list all paths that lead to a repository when symlinks are
> followed, while still avoiding infinite loops.

This patch is too complex and includes unrelated changes that interfere
with its readability. For instance:

> -    def errhandler(err):
> -        if err.filename == path:
> -            raise err

..we used to have this error handling code that was presumably there for
a reason. You removed it with no explanation.

Even if there is a good reason to remove it, it doesn't belong in this
patch.

Relatedly, you've replaced os.walk without implementing its error
handling.

If you really need to rewrite os.walk (I'm not convinced) to get the
behavior you want, you should do your patches like this:

- cleanups
- replace os.walk, no functional changes
- behavior change

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list