launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/fix-update-cache into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #794088 in Launchpad itself: "update-sourcecode updates cache when already up-to-date"
  https://bugs.launchpad.net/launchpad/+bug/794088

For more details, see:
https://code.launchpad.net/~abentley/launchpad/fix-update-cache/+merge/63717

= Summary =
Fix bug #794088: update-sourcecode updates cache when already up-to-date

== Proposed fix ==
Change the check for whether to write the cache so that it is written only if it has changed.

== Pre-implementation notes ==
None

== Implementation details ==
We have to listify the last revision so that it is not a tuple (old cache values have lists, not tuples).

== Tests ==
None

== Demo and Q/A ==
Create a stale branch in sourcecode, e.g. by using "bzr pull --overwrite -r -2"
Run utilities/update-sourcecode

The branch will be updated, but the cache file will not be updated, and it will not print a message advising you to commit the cache file.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/devscripts/sourcecode.py
-- 
https://code.launchpad.net/~abentley/launchpad/fix-update-cache/+merge/63717
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/fix-update-cache into lp:launchpad.

=== modified file 'lib/devscripts/sourcecode.py'
--- lib/devscripts/sourcecode.py	2011-05-24 17:13:58 +0000
+++ lib/devscripts/sourcecode.py	2011-06-07 15:11:43 +0000
@@ -63,6 +63,7 @@
         optional = False
     return branch_name, branch_url, revision, optional
 
+
 def load_cache(cache_filename):
     try:
         cache_file = open(cache_filename, 'rb')
@@ -74,6 +75,7 @@
     with cache_file:
         return json.load(cache_file)
 
+
 def interpret_config(config_entries, public_only):
     """Interpret a configuration stream, as parsed by 'parse_config_file'.
 
@@ -214,16 +216,17 @@
 
 def update_cache(cache, cache_filename, changed, sourcecode_directory, quiet):
     """Update the cache with the changed branches."""
-    if len(changed) == 0:
-        return
-    if not quiet:
-        print 'Cache updated.  Please commit "%s".' % cache_filename
+    old_cache = dict(cache)
     for project, (branch_url, revision, optional) in changed.iteritems():
         destination = os.path.join(sourcecode_directory, project)
         branch = Branch.open(destination)
-        cache[project] = branch.last_revision_info()
+        cache[project] = list(branch.last_revision_info())
+    if cache == old_cache:
+        return
     with open(cache_filename, 'wb') as cache_file:
         json.dump(cache, cache_file, indent=4)
+    if not quiet:
+        print 'Cache updated.  Please commit "%s".' % cache_filename
 
 
 def update_branches(sourcecode_directory, update_branches,
@@ -258,10 +261,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