Launchpad itself

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

  1. bug643715
  2. Merge into devel
Proposed by Gary Poster
Status: Work in progress
Proposed branch: lp:~gary/launchpad/bug643715
Merge into: lp:launchpad
Diff against target: 96 lines (+28/-9)
1 file modified
lib/devscripts/sourcecode.py (+28/-9)
To merge this branch: bzr merge lp:~gary/launchpad/bug643715
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Needs Fixing
Jonathan Lange (community) Approve
Review via email: mp+36060@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote :

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

Thanks for doing this.

review: Approve
Reply
Revision history for this message
Aaron Bentley (abentley) wrote :

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
Reply
Revision history for this message
Gary Poster (gary) 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.

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?

Reply
Revision history for this message
Aaron Bentley (abentley) wrote :

-----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-----

Reply

Unmerged revisions

11582. By Gary Poster

make lint happy

11581. By Gary Poster

import missing exception

11580. By Gary Poster

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/devscripts/sourcecode.py'
2--- lib/devscripts/sourcecode.py 2010-04-19 11:05:07 +0000
3+++ lib/devscripts/sourcecode.py 2010-09-20 19:31:23 +0000
4@@ -16,7 +16,11 @@
5 import sys
6
7 from bzrlib.branch import Branch
8-from bzrlib.errors import BzrError, NotBranchError, IncompatibleRepositories
9+from bzrlib.errors import (
10+ BzrError,
11+ NotBranchError,
12+ IncompatibleRepositories,
13+ NoSuchRevision)
14 from bzrlib.plugin import load_plugins
15 from bzrlib.revisionspec import RevisionSpec
16 from bzrlib.trace import enable_default_logging, report_exception
17@@ -135,14 +139,13 @@
18 return spec.as_revision_id(from_branch)
19 # else return None
20
21-
22 def _format_revision_name(revision, tip=False):
23 """Formatting helper to return human-readable identifier for revision.
24
25 If ``tip`` is True, the revision value will be ignored.
26 """
27 if not tip and revision:
28- return 'revision %s' % (revision,)
29+ return 'revision %s' % (revision, )
30 else:
31 return 'tip'
32
33@@ -178,6 +181,17 @@
34 possible_transports=possible_transports)
35
36
37+def get_revno(wt):
38+ """Get the revno of a working tree."""
39+ # Ripped from bzrlib.builtins.cmd_revno.run.
40+ revid = wt.last_revision()
41+ try:
42+ revno_t = wt.branch.revision_id_to_dotted_revno(revid)
43+ except NoSuchRevision:
44+ revno_t = ('???', )
45+ return ".".join(str(n) for n in revno_t)
46+
47+
48 def update_branches(sourcecode_directory, update_branches,
49 possible_transports=None, tip=False, quiet=False):
50 """Update the existing branches in sourcecode."""
51@@ -193,6 +207,12 @@
52 print 'Updating %s to %s' % (
53 project, _format_revision_name(revision, tip))
54 local_tree = WorkingTree.open(destination)
55+ # See if we can shortcut: do we already have the desired revision?
56+ if not tip and revision == get_revno(local_tree):
57+ if not quiet:
58+ print ' (No change)'
59+ continue
60+ # Apparently we do not. Open the remote branch.
61 try:
62 remote_branch = Branch.open(
63 branch_url, possible_transports=possible_transports)
64@@ -210,10 +230,10 @@
65 remote_branch, stop_revision=revision_id, overwrite=True,
66 possible_transports=possible_transports)
67 except IncompatibleRepositories:
68- # XXX JRV 20100407: Ideally remote_branch.bzrdir._format
69+ # XXX JRV 20100407: Ideally remote_branch.bzrdir._format
70 # should be passed into upgrade() to ensure the format is the same
71- # locally and remotely. Unfortunately smart server branches
72- # have their _format set to RemoteFormat rather than an actual
73+ # locally and remotely. Unfortunately smart server branches
74+ # have their _format set to RemoteFormat rather than an actual
75 # format instance.
76 upgrade(destination)
77 # Upgraded, repoen working tree
78@@ -277,7 +297,6 @@
79 # differ from each other (because of developers fiddling with things), we can
80 # take a survey of all of them, and choose the most popular.
81
82-
83 def main(args):
84 parser = optparse.OptionParser("usage: %prog [options] [root [conffile]]")
85 parser.add_option(
86@@ -305,8 +324,8 @@
87 if len(args) > 3:
88 parser.error("Too many arguments.")
89 if not options.quiet:
90- print 'Sourcecode: %s' % (sourcecode_directory,)
91- print 'Config: %s' % (config_filename,)
92+ print 'Sourcecode: %s' % (sourcecode_directory, )
93+ print 'Config: %s' % (config_filename, )
94 enable_default_logging()
95 # Tell bzr to use the terminal (if any) to show progress bars
96 ui.ui_factory = ui.make_ui_for_terminal(