GNOME Bugzilla – Bug 560246
Support more exotic SVN layouts without misusing module attribute
Last modified: 2008-11-17 16:16:18 UTC
This idea/patch was spawned from work on DVCS mirror integration and a bug we have hit (the vala-trunk definition causes the mirror patch to try and checkout a git repo from git-mirror.gnome.org/git/vala/trunk.....) Sometimes module is set to $module/trunk because of (what i consider) a bug. In attempting to fix this, i found i would cause a regression in other areas. It seems we currently misuse the module="" attribute to support non-gnome-like SVN layouts. If i'm grepping correctly we fudge portions of the href into module in around 200 cases.... It would be better if we were stricter about this, as it enables nice things like the mirror integration to be smoother. I want to improve our support for non standard repo layouts in a more generic way without overloading the module attribute. One way to do this is template urls. I'm proposing these would be set on the <repository> tag alongside the trunk-path type attributes. The default case would be (the kind of layout we optimise for is): http://svn.gnome.org/svn/jhbuild/trunk We could describe that as: trunk_template = "%(href)s/%(module)s/%(trunk_path)s" branches_template = "%(href)s/%(module)s/%(branches_path)s/%(branch)s" tags_template = "%(href)s/%(module)s/%(tags_path)s/%(tag)s" (These definitions are built in and for standard layouts don't appear) There are also urls like: http://svn.foo.org/trunk/modulename We could just have: trunk_template = "%(href)s/%(trunk_path)s/%(module)s Google code has: http://modulename.googlecode.com/svn/trunk so trunk_template = "http://%(module)s.googlecode.com/svn/%(trunk_path)s" I like this one too: trunk_template = "http://%(module)s.svn.sf.net/svnroot/%(module)s/%(trunk_path)s" We now have just one repository definition for all googlecode repositories, and one repository entry for all sourceforge.net projects. Joy :-) I'll attach a proof of concept patch, but i wanted to get some feedback on the idea before i start hacking on modulesets. One option I have at this point is just to drop trunk-path, branches-path and tags-path... Any thoughts?
Created attachment 122366 [details] [review] Proof of concept Initial brain dump. 1. Should I drop the {trunk,branches,tags}-path attributes? 2. I have a patch to simplify the svn.py stuff some, BUT it can't land until (or rather unless) we clean up the use of module="" in svn.py
Created attachment 122367 [details] [review] One googlecode to rule them all (Should warn I haven't tested this yet, will post updates as I get time)
Created attachment 122400 [details] [review] Working version Fix typos and move href out of the template. Allow href to contain %(module)s type stuff too.
Great work; trunk-path and friends can be removed. Btw I am reminded of another problem now, could you have a look at bug 548151 ?
Created attachment 122401 [details] [review] One googlecode to rule them all (update to match last changes to svn.py)
Created attachment 122403 [details] [review] Drop trunk-path and friends
Created attachment 122404 [details] [review] Simplify the templates now trunk-path and friends are gone
Created attachment 122405 [details] [review] Add tag support
Created attachment 122409 [details] [review] svn.py simplifications Am I OK to commit the first 3 patches? This final patch is a change i'd like to make, but it causes a regression when the module attribute is overloaded to manipulate the href. I would like to apply when (if?) i can remove all cases where module is overloaded. If we can't change the behaviour of module then i'll submit a 2nd version which falls back to the current behaviour for modules with a '/' in the module atrribute (and clean up as many existing cases of that as possible).
Commited patch for *-template and svn tag support.
It seems my svn (1.5.1) is unable to understand paths with ../ This is the output trying to update all *-webkit module *** Checking out yelp-webkit *** [1/1] svn switch --accept postpone http://svn.gnome.org/svn/yelp/branches/../branches/webkit svn: URL 'http://svn.gnome.org/svn/yelp/branches/../branches/webkit' contains a '..' element *** error during stage checkout of yelp-webkit: ########## *** Checking out epiphany-webkit *** [1/1] svn switch --accept postpone http://svn.gnome.org/svn/epiphany/branches/../trunk svn: URL 'http://svn.gnome.org/svn/epiphany/branches/../trunk' contains a '..' element *** error during stage checkout of epiphany-webkit: ##########
That would fix it. 2008-11-14 Frederic Peters <fpeters@0d.be> * jhbuild/versioncontrol/svn.py: reformat module HREF to resolve .. before handing it to Subversion. (closes: #560246#c11)
John, I am still not sure about it but revision="../trunk" doesn't look right, could revision defaults to "../trunk", and those who would want it to be "" just set an empty revision="" attribute?
Created attachment 122851 [details] [review] Simplify svn.py Simplify the svn path generating code. This now includes a workaround for people misusing the module attribute.
Looks good; please commit. My latest comment (#13) can be addressed later.
Created attachment 122852 [details] [review] Fix the '..' cases Shouldn't need to use ../trunk. If no revision is specified, it will use trunk by itself. Don't need to use ../branches/branchname.. The url you just made was svn.gnome.org/svn/foo/branches/../branches/branchname
Created attachment 122854 [details] [review] Make checkmodulesets pass (for gnome-2.24 and gnome2.26)
Created attachment 122856 [details] [review] Use revision arg instead of misusing module arg
All good to commit.
All are committed. Closing this bug as framework is landed. Will post more cleanup separately.