Mercurial clobber for changing branch

[even]

Right now, each time we want to change branch, we clobber the whole repository. That is not needed...

We might prefer to use a simpler an quicker sequence if the branch is not correct like : hg update --clean --rev branchName

Whatever, the person doing the compilation might clobber himself or delete symbols if needed but forcing it is not a very good approach.

I'll try to issue a patch about that as quickly as possible (in my situation, that clobber might cause me 1 hour and a half building time on windows... it is not really affordable).

[even]

First version of the patch. Tested on win32.

[marcusl]

Hm. I understand you concern. However, I'm a bit wary with leaving stuff in there as well. Also, the current buildbot tests fail if the sourcestep does not clobber when the branch change. Perhaps that is unwarranted, but I didn't want to change the tests behaviour when I wrote the inrepo-test for Mercurial, so I added the clobber check to the source step.

Is the problem that the clone takes time, or the subsequent build, or both?

If only the former, perhaps solving #412 would help? IIUC, that would make test pass, while leave the local .hg dir intact, thus avoiding the expensive re-clone.

If the latter, I would want the patch to make the behaviour switchable, as I like my buildsteps to do the right thing, always.

What do you think?

[even]

About the time problematics, I think that the first problem, checkout time, is the worst of them. We are whatsoever deleting objects when changing branches, at least for each important builds.

And yes, solving #412 would be another way to solve that issue, at least if the purge extension is present. But I would like to point out that mercurial seems to handle very well its files and there should be no problem in updating from one branch to another without clobbering. But if, in a specific scenario of an user, there is a risk, he might ask for clobber himself knowing of his situation. I don't understand why we would want to impose the clobbering to the user here.

Maybe we can also add an option for that like "clobberOnBranchChange" that might defaults to False but might be needed in very specific scenario.

[marcusl]

Ok. I'm basically fine with changing it, but this causes a regression in the tests. (buildbot.test.test_vc.MercurialInRepo?.testBranch fails)

If we still want to do this, we need to change the test. For that, we need some input from some of the more senior developers who wrote the thing in the first place (i.e. Brian or Dustin).

We could also, instead of the overly brutal clobber, just delete everything except the .hg dir, then update again. That would be a middle-ground, and a good fallback for #412.

I've made a branch from the main repo (djmitche/master) and added your patch (slightly modified as we've added clobber on dirname-branching) here:

 http://github.com/marcusl/buildbot/tree/bug462

We can't merge to main until the tests run cleanly. I'll ask the mail-list for opinions here.

[marcusl]

FYI, previous discussion(s):

 http://thread.gmane.org/gmane.comp.python.buildbot.devel/3899

[even]

Thanks for the quick feedback (as always, but I never took the time to say how much I appreciate that !).

I wrote a modification of my previous patch to add the clobberOnBranchChange argument. Since it's a bit late now, I'll try it only tomorrow. If that works I will put the patch here and look the test suite to see how we can fix those.

I'll maybe take some time to solve #412 too since it looks promising. If I get #412 ready and tests running, we might think about having clobberOnBranchChange defaulting to True instead of False.

We're also going to have to fix documentation and advise people to activate purge extension and/or setting clobberOnBranchChange to False if they have big repositories.

[even]

I just thought about it. The clobber is necessary on dirname branching. clobberOnBranchChange is a possibility only if using inrepos branches, just as the purge extension. We need to clobber to get rid of the whole folder if using dirname branching.

That's probably for that case that clobbering was requested to change branch on previous implementations.

I'll try to do a nice patch tomorrow.

[even]

Removing the review flag, it's far from correct right now. There's quite a lot of work to do for this bug.

[marcusl]

I didn't touch clobber on dirname branching. That should still work correct, as before.

It should be trivial to change the clobber assignment on inrep-branch-change (that I commmented out in the  commit) to use a 'clobberOnBranchChange' flag, once that is present in the mercurial slave command.

If you're able, you could create your own fork off my github repo on github and work there. Collaboration is a bit easier and simplifies for me or Dustin to merge the change into mainstream once everyone is satisfied. (If you're reasonably comfortable with git, that is.)

For #412, I'd like the fallback as well. I could help out with that. And yes, we should fix the documentation.

[even]

Patch bug462 branch to add clobberOnBranchChange

[even]

Here is the argument we talked about. I think this works. I don't know what addFactoryArguments do so I didn't add it there but it's probably something that had to be added.

I forked your buildbot repository and added the patch in it. It it there:  http://github.com/even/buildbot/tree/master. Hope it is helping.

[marcusl]

Ok. Looks good. I'm still waiting to hear from Brian on the clobber thing, but this is a good workaround.

I've updated folks on the mail-list, to see what the 'addFactoryArguments' is about, but beyond that, I don't think anything is holding this back. (If you'd like to join that discussion, you're welcome.)

Thanks for the nice work and the patches. :)

[even]

As you saw, since I changed my mind and get the clobber as default behavior, testes are running smoothly now.

I would be happy to join the discussion since I would like to know what is that 'addFactoryArguments' thing to know if I have to update my patch to add my argument in it too. And for later changes as well maybe... The method is not documented and I totally miss its purpose...

[marcusl]

Just pop in. The address is buildbot-devel@…. I use gmane.org to read it, but just subscribing is ok too.

[even]

I think review here is correct to.

[even]

I fixed the call to addFactoryArguments in my branch based on Dustin explanation on the ML.

Now everything should be correct here.

[marcusl]

Pushed to master.

See  b94b919dbeb25335affeb4ef32eb345574f4c20b and four earlier.