mercurial checkouts should separate clone and update

[dsallings]

The current vc full model uses clone -r to get a tree to a particular version. This is incorrect in the case where the target version is in a named branch. The repository history will be truncated at the correct location, but the automatic checkout will be on the default branch (as of 0.9.5).

The ideal behavior is to clone *without* a checkout, and then update to the correct revision after the clone.

The attached patch is how we're doing this. You could possibly do both (clone without checkout to a revision and then update to that revision), but I think that the difference will generally be negligible.

[dsallings]

separate the clone and update in mercurial

[dustin]

I'm not very familiar with mercurial, but it looks like this patch is removing a big chunk of "fallback" functionality for older versions of hg. I'd like to hear from some other mercurial users before adopting this patch.

[Pike]

Ben, can you check?

[even]

That patch may fail if self.argsbranch? does not exists when calling doVCFull. You should change your if to match : if self.argsrevision?:

updatecmd.extend(['--rev', self.argsrevision?])

elif self.argsbranch?:

updatecmd.extend(['--rev', self.argsbranch?])

else

updatecmd.extend(['--rev', 'default'])

[even]

Hmm, it seems that doing

if self.args['branch']:
  #...

is not good because branch does not exists every time. We can have errors because it does not exists. Maybe testing

if branch in self.args and self.args['branch']:
  #...

will do the trick.

[marcusl]

self.argsbranch? throws KeyError:s all over the place when I tried to apply this.

Shouldn't you use 'branch' in self.args or self.args.get('branch') instead? (Or am I just new to python?)

This was with Python 2.5.2 on Vista, btw.

[even]

Yes, you're right. That piece of code is not complete. I tried it too and and it was not good enough. That is why I posted another comment with a correction for that bug. The complete test sequence should be :

if self.args['revision']:
  updatecmd.extend(['--rev', self.args['revision']])
elif 'branch' in self.args and self.args['branch']:
  updatecmd.extend(['--rev', self.args['branch']])
else:
  updatecmd.extend(['--rev', 'default'])

That code is tested and in production for building instantbird's nightlies ( http://www.instantbird.com/) and it seems to work well. I'm waiting for this bug to be fixed before updating to a new version of buildbot since it is somehow critical for me (cloning anew the whole repository is taking hours on windows, so I can't accept useless ones) and that I don't want to produce a personal version of buildbot at each new version.

[marcusl]

Non-throwing key-in-dict checks for Mercurial + use 'default' as branch if none set

[marcusl]

Heh. I didn't see your first post before writing the first thing, then I didn't seed your second before creating a patch for it. :)

As you can see I used get(key, default) instead of an extra if-clause. Does that matter?

[marcusl]

Replying to marcusl:

As you can see I used get(key, default) instead of an extra if-clause. Does that matter?

Right. It might matter if the key is in there, and the value is actually the 'None' object.

[even]

Yep. I think we can find a better way of writing that. What do you think of that :

if self.args['revision']:
  updatecmd.extend(['--rev', self.args['revision']])
else:
  updatecmd.extend(['--rev', self.args.get('branch', 'default')])

It looks like it is the same as my previous proposal but a lot shorter.

[even]

Sry, I did'nt see you write just this in your patcH. It's perfect to me. I think you can ask for review.

[marcusl]

Adding review keyword

[dustin]

added to my branch.