Hi Eddy, thanks for the fast reply. :)
On Fri, Nov 21, 2008 at 07:03:05PM +0200, Eddy Petrișor wrote: > Since this bug proposes to add branches support to svn-buildpackage, I > guess is reasonable to prove the functionality by adding it to the > svn-buildpackage in a branch and at the end show us that svn-buildpackage > can be built from that branch. Sounds reasonable. > What do you say? Do you have an account on alioth? If you do, you should > request to be added to the collab-maint group. I have: jhr-guest. I requested to be added. Thanks for the hint to collab-maint! > The URI should be (if you have an account on alioth) > > svn+ssh://svn.debian.org/svn/collab-maint/deb-maint/svn-buildpackage/trunk/ I already did a checkout (without +ssh yet) to see the latest changes. > I've looked hastily over the patch and I have a question. > > *Functionally* wouldn't only this part (and the code for the added > option) be enough? > > - $trunkdir = "$package/trunk"; > + # NB: no :) > + $workingdir="$package/trunk"; > + $workingdir="$package/branches/$opt_branch" if ($opt_branch); > withecho "svn up"; Well, I'm not sure atm what changes could be reverted without loss of functionality. FWIW, don't you think the code is getting unreadable if variables are heavily misleading? "trunk" has a certain meaning and any variable including "trunk" should referr to trunk, shouldn't it? :) > I know we're talking about a branch, and you'd generally make a new one, or > make > another checkout, but is expected that if I have trunk and then svn > switch to a branch > that someone created based on the old trunk, my copy should work, shouldn't > it? Absolutely. > If there are overrides or debian/svn-deblayout exists, then the > branches-enabled svn-bp > wouldn't work, would it, since it will have references to obsolete > layout info (trunkUrl or trunkDir) right? > > trunkUrl seems reasonble to fail, but what about trunkDir? > > Still, now, that I've written these ideas, it seems like there > wouldn't be a reason to > override trunkDir and should be generated each time... TBH, I don't have an idea how to solve that appropiately atm. > This isn't technically correct, although svn-bp tags something which might > be the trunk (see #478697), it certainly doesn't tag the working directory > either. True; didn't think about it. > I guess the most sane thing to do it would be to target fixing #478697, too, > while you're adding the branch feature, too. Yes, makes sense. As a matter of fact, I was reading that bug report before I began changing the code. Don't know why I didn't address that problem, too. > Also, although "trunk" might be hardcoded throughout svn-bp's code, I feel > uneasy about hardcoding "branches" since it would farther us from a possible > fix for "#365116 -- svn-inject should allow an arbitrary layout". Well, that makes it more difficult -- or at least it means a larger rewrite of some code, doesn't it? Again, thanks for your answer. Let's see how to rock that beast... ;-) Cheers, Hauke
signature.asc
Description: Digital signature
