GNOME Bugzilla – Bug 573049
Factorize repository root finding code
Last modified: 2009-03-01 23:41:08 UTC
There's code duplication in vc plugins. The code that tries to find the repository root is duplicated in a lot of files and is almost always identical. The attached patch tries to consolidate this into a single implementation in _vc.py The patch should be a no-op Please review Tested with git+hg (dual repository) and svn The diffstat is nice: _vc.py | 13 +++++++++++++ bzr.py | 8 ++------ darcs.py | 8 ++------ git.py | 19 +++++++++---------- mercurial.py | 9 +-------- monotone.py | 11 ++--------- tla.py | 8 ++------ 7 files changed, 31 insertions(+), 45 deletions(-) CVS & svn not touched as they didn't have the same code cut'n'pasted
Created attachment 129447 [details] [review] factorize it
Created attachment 129601 [details] [review] factorize it (fixed) This one fix a bug in vc/git.py constructor, _tree_cache was not initialized properly
Bug #556404 contains a large patch which, among other things, factorises out the repository root finding and also makes the root finding work on windows (see the find_vcs_root function). It would probably make sense to apply that part of the windows patch separately, but I'm not really familiar enough with the VC-related code to be sure that it's all correct. Maybe you could try and cherry-pick the useful parts of the windows patch (attachment #127707 [details]) into your patch here?
When reading your comment I told myself that I've seen code in anyvc that would be even better than mine. Then I read #127707, and saw something very similar to what's in anyvc (no hard coded '/' search) So I think this is all good, I'll extract the relevant bits from #127707, and apply them after my factorization, as follow-ups, as some of those hunks would change behavior (something that I tried to avoid, when factorizing)
Committed as #1189 to #1192 - Factorization (noop) - Make cvs & svn OS-independant - detect cvs & svn repo by their directories, avoid files named .svn or CVS - Make find_repo_root() OS-independant Parts of this are taken from patch #127707 of bug #556404