"update -r REVNO" support

[Robert Widhopf-Fenk]

On Monday, August 6, 2007 at 15:25:45, Martin Pool wrote:
[...]
> Why are you using getcwd rather tree.branch?

Just cut&paste without looking at the context ...

[...]
> > @@ -1034,7 +1043,8 @@
> >                      note("Tree is up to date at revision %d." %
> >                      (revno,)) return 0
> >              conflicts = tree.update(delta._ChangeReporter(
> > -                                        unversioned_filter=tree.is_ignored))
> > +                                        unversioned_filter=tree.is_ignored),
> > +                                   revision and to_rev or None)
> >              revno = tree.branch.revision_id_to_revno(
> >                  _mod_revision.ensure_null(tree.last_revision()))
> >              note('Updated to revision %d.' % (revno,))
> >
> 
> I'd rather you just set to_rev appropriately rather than the
> pseudo-ternary here.

It is not easily done as for lightweighted checkouts the
revision may change due to an update of the related branch
... well this is what I think it is ...

> > === modified file 'bzrlib/status.py'
> > --- bzrlib/status.py    2007-07-23 14:27:42 +0000
> > +++ bzrlib/status.py    2007-08-04 00:14:16 +0000
> > @@ -122,7 +122,12 @@
> >          new_is_working_tree = True
> >          if revision is None:
> >              if wt.last_revision() != wt.branch.last_revision():
> > -                warning("working tree is out of date, run 'bzr
> >                  update'")
> > +                b_revno = wt.branch.last_revision_info()[0]
> > +                wt_revno =
> >                  wt.branch.revision_id_to_revno(wt.last_revision())
> > +                missing = b_revno - wt_revno
> > +                warning("Working tree is at revno %d missing %d
> >                  revision%s!" %
> > +                        (wt_revno, missing,
> > +                         (missing > 1) and "s" or ""))
> >              new = wt
> >              old = new.basis_tree()
> >          elif len(revision) > 0:
> 
> Giving more information here is good.
> 
> I don't think you should remove the suggestion to run update.  It
> might be even better to say "working tree is out of date with its
> branch", or something else that helps  the user understand what's
> happening.  Suggestions?
> 
> Getting the branch last revision is potentially expensive (for
> checkouts), so please just do it once and remember it.
> 
> In general we don't put '!' at the end of messages.  Stay calm. :-)
> 
> >
> > === modified file 'bzrlib/workingtree.py'
> > --- bzrlib/workingtree.py       2007-07-25 21:25:00 +0000
> > +++ bzrlib/workingtree.py       2007-08-04 02:05:49 +0000
> > @@ -2001,7 +2001,7 @@
> >          """
> >          raise NotImplementedError(self.unlock)
> >
> > -    def update(self, change_reporter=None):
> > +    def update(self, change_reporter=None, to_revision=None):
> >          """Update a working tree along its branch.
> >
> >          This will update the branch if its bound too, which means
> >          we have
> 
> It's almost obvious but please document the parameter.  Maybe it
> should be to_revision_id to be more clear.

I missed that in the bundle I just sent, I will send another
one ...

> I don't think you have a workingtree_implementation test of this new
> parameter and you probably need one, to make sure it works with
> every kind of tree.

I am not quite sure what to do here, just call it once
without the new and once with the new extra parameter?

> > @@ -2096,6 +2099,8 @@
> >                  parent_trees.append(
> >                      (old_tip,
> >                      self.branch.repository.revision_tree(old_tip)))
> >              self.set_parent_trees(parent_trees)
> > +            if to_revision is not None:
> > +                self.set_parent_ids([to_rev])
> >              last_rev = parent_trees[0][0]
> >          else:
> >              # the working tree had the same last-revision as the
> >              # master
> >
> 
> Is that needed if we've done set_parent_trees?  I think not?

I get 7 failed test if I do not set both.

> Could you please update your patch and resubmit.

Sure, there will be some more rounds IMHO ;-)

Cheers Robert