Merge lp:~gary/launchpad/bug643715 into lp:launchpad

[Gary Poster]

This was just a quick itch to scratch as I was updating my tree after being away for a week. It makes calling update-sourcecode much faster.

To test, run

./utilities/update-sourcecode

If you want to play around, you can change utilities/sourcedeps.conf to revert a revision, then run update-sourcecode, and then change the revision back. I could optimize that too, but I don't think that's an important win for the common case, so I'm not worrying about it.

I made a variety of changes for lint, even though some of them didn't make a lot of sense to me. I was surprised in particular by the preference for ('foo', ) over ('foo',). It's alright.

Gary

[Jonathan Lange (jml)]

Its preference for (foo, ) over (foo,) is wrong. Every linting tool that isn't pyflakes is terrible.

Thanks for doing this.

review: Approve

[Aaron Bentley (abentley)]

This branch treats revnos as if they were revision ids, which means that it may not notice cases where uncommit or push --overwrite have been used.

It also appears not to account for changes in the branch location.

It also assumes that "revision" is a revno, when other parts of the code treat it as a RevisionSpec instead.

review: Needs Fixing

[Gary Poster (gary)]

My change was an optimization for the common case. What we had already was "correct" for the edge cases you identified.

On one hand:

- I could make an additional change to handle the case of change in the branch location.

- We use revnos only, practically, so I think that's fine, myself.

However:

- I don't see how to handle an uncommit or an --overwrite.

I think those are pretty unlikely cases, but if we care about handling them, I think this is dead in the water. In that case, Aaron, isn't this actually a Rejected?

[Aaron Bentley (abentley)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/20/2010 04:03 PM, Gary Poster wrote:
> My change was an optimization for the common case. What we had already was "correct" for the edge cases you identified.
>
> On one hand:
>
> - I could make an additional change to handle the case of change in the branch location.
>
> - We use revnos only, practically, so I think that's fine, myself.

That's as may be, but it's actually faster and requires less code to
convert from revision_spec to revision_id and then compare to
wt.last_revision().

> However:
>
> - I don't see how to handle an uncommit or an --overwrite.

You could handle both branch location changes and revno redefinition by
auto-generating a list of revision-ids from update-sourcecode. The
output of this would need to be versioned.

> I think those are pretty unlikely cases

Also, there's merge trunk, commit, push to trunk.

But given that these are all PQM-managed branches and everyone who has
write access to them is pretty sane, I guess I'd agree that those cases
are unlikely.

, but if we care about handling them, I think this is dead in the water.
 In that case, Aaron, isn't this actually a Rejected?

I think we can make it faster without making it less effective.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkyXzFAACgkQ0F+nu1YWqI1pcQCgiU2lEcyIcXnrvBZaAw1Izulj
jVkAn09Z9jlR13ZuWc5Wgv6plAm4nl0p
=YDgF
-----END PGP SIGNATURE-----

Unmerged revisions

11582. By Gary Poster on 2010-09-20

make lint happy

11581. By Gary Poster on 2010-09-20

import missing exception

11580. By Gary Poster on 2010-09-20

optimize case in sourcecode update of already being on the desired revision.

=== modified file 'lib/devscripts/sourcecode.py'
--- lib/devscripts/sourcecode.py	2010-04-19 11:05:07 +0000
+++ lib/devscripts/sourcecode.py	2010-09-20 19:31:23 +0000
@@ -16,7 +16,11 @@
 import sys
 
 from bzrlib.branch import Branch
-from bzrlib.errors import BzrError, NotBranchError, IncompatibleRepositories
+from bzrlib.errors import (
+    BzrError,
+    NotBranchError,
+    IncompatibleRepositories,
+    NoSuchRevision)
 from bzrlib.plugin import load_plugins
 from bzrlib.revisionspec import RevisionSpec
 from bzrlib.trace import enable_default_logging, report_exception
@@ -135,14 +139,13 @@
         return spec.as_revision_id(from_branch)
     # else return None
 
-
 def _format_revision_name(revision, tip=False):
     """Formatting helper to return human-readable identifier for revision.
 
     If ``tip`` is True, the revision value will be ignored.
     """
     if not tip and revision:
-        return 'revision %s' % (revision,)
+        return 'revision %s' % (revision, )
     else:
         return 'tip'
 
@@ -178,6 +181,17 @@
             possible_transports=possible_transports)
 
 
+def get_revno(wt):
+    """Get the revno of a working tree."""
+    # Ripped from bzrlib.builtins.cmd_revno.run.
+    revid = wt.last_revision()
+    try:
+        revno_t = wt.branch.revision_id_to_dotted_revno(revid)
+    except NoSuchRevision:
+        revno_t = ('???', )
+    return ".".join(str(n) for n in revno_t)
+
+
 def update_branches(sourcecode_directory, update_branches,
                     possible_transports=None, tip=False, quiet=False):
     """Update the existing branches in sourcecode."""
@@ -193,6 +207,12 @@
             print 'Updating %s to %s' % (
                     project, _format_revision_name(revision, tip))
         local_tree = WorkingTree.open(destination)
+        # See if we can shortcut: do we already have the desired revision?
+        if not tip and revision == get_revno(local_tree):
+            if not quiet:
+                print '  (No change)'
+            continue
+        # Apparently we do not.  Open the remote branch.
         try:
             remote_branch = Branch.open(
                 branch_url, possible_transports=possible_transports)
@@ -210,10 +230,10 @@
                 remote_branch, stop_revision=revision_id, overwrite=True,
                 possible_transports=possible_transports)
         except IncompatibleRepositories:
-            # XXX JRV 20100407: Ideally remote_branch.bzrdir._format 
+            # XXX JRV 20100407: Ideally remote_branch.bzrdir._format
             # should be passed into upgrade() to ensure the format is the same
-            # locally and remotely. Unfortunately smart server branches 
-            # have their _format set to RemoteFormat rather than an actual 
+            # locally and remotely. Unfortunately smart server branches
+            # have their _format set to RemoteFormat rather than an actual
             # format instance.
             upgrade(destination)
             # Upgraded, repoen working tree
@@ -277,7 +297,6 @@
 # differ from each other (because of developers fiddling with things), we can
 # take a survey of all of them, and choose the most popular.
 
-
 def main(args):
     parser = optparse.OptionParser("usage: %prog [options] [root [conffile]]")
     parser.add_option(
@@ -305,8 +324,8 @@
     if len(args) > 3:
         parser.error("Too many arguments.")
     if not options.quiet:
-        print 'Sourcecode: %s' % (sourcecode_directory,)
-        print 'Config: %s' % (config_filename,)
+        print 'Sourcecode: %s' % (sourcecode_directory, )
+        print 'Config: %s' % (config_filename, )
     enable_default_logging()
     # Tell bzr to use the terminal (if any) to show progress bars
     ui.ui_factory = ui.make_ui_for_terminal(