[gnome-ostree] Checksum patches for smarter rebuild

[Colin Walters <walters src gnome org>]

commit 0e756d9307af7a4242e9ecfa0faf8e6c771138ff
Author: Colin Walters <walters verbum org>
Date:   Wed Aug 15 15:48:43 2012 -0400

    Checksum patches for smarter rebuild
    
    Right now a rebuild gets triggered due to the revision of gnome-ostree
    changing, even if the actual content of the patches haven't.
    
    Optimize this by caching the sha256sum of each patch in the build
    metadata, and avoid rebuilding if the patches haven't changed.

 src/ostbuild/pyostbuild/buildutil.py               |   12 +++
 src/ostbuild/pyostbuild/builtin_build.py           |   81 +++++++++++++++-----
 src/ostbuild/pyostbuild/builtin_checkout.py        |   39 ++++------
 .../pyostbuild/builtin_chroot_compile_one.py       |    4 +-
 src/ostbuild/pyostbuild/vcs.py                     |   19 +++++
 5 files changed, 110 insertions(+), 45 deletions(-)
---
diff --git a/src/ostbuild/pyostbuild/buildutil.py b/src/ostbuild/pyostbuild/buildutil.py
index 94ad63b..7855a99 100755
--- a/src/ostbuild/pyostbuild/buildutil.py
+++ b/src/ostbuild/pyostbuild/buildutil.py
@@ -185,3 +185,15 @@ def resolve_component_meta(snapshot, component_meta):
         result['branch'] = 'master'
 
     return result
+
+def get_patch_paths_for_component(patchdir, component):
+    patches = component.get('patches')
+    patch_subdir = patches.get('subdir', None)
+    if patch_subdir is not None:
+        patchdir = os.path.join(patchdir, patch_subdir)
+    else:
+        patchdir = self.patchdir
+    result = []
+    for patch in patches['files']:
+        result.append(os.path.join(patchdir, patch))
+    return result
diff --git a/src/ostbuild/pyostbuild/builtin_build.py b/src/ostbuild/pyostbuild/builtin_build.py
index e703da1..1350b7d 100755
--- a/src/ostbuild/pyostbuild/builtin_build.py
+++ b/src/ostbuild/pyostbuild/builtin_build.py
@@ -75,6 +75,42 @@ class OstbuildBuild(builtins.Builtin):
         else:
             log("No previous build; skipping source diff")
 
+    def _needs_rebuild(self, previous_metadata, new_metadata):
+        build_keys = ['config-opts', 'src', 'revision']
+        for k in build_keys:
+            if (k not in new_metadata) or (previous_metadata[k] != new_metadata[k]):
+                return 'key %r differs' % (k, )
+            
+        if 'patches' in previous_metadata:
+            if 'patches' not in new_metadata:
+                return 'patches differ'
+            old_patches = previous_metadata['patches']
+            new_patches = new_metadata['patches']
+            old_files = old_patches['files']
+            new_files = new_patches['files']
+            if len(old_files) != len(new_files):
+                return 'patches differ'
+            old_sha256sums = old_patches.get('files_sha256sums')
+            new_sha256sums = new_patches.get('files_sha256sums')
+            if ((old_sha256sums is None or new_sha256sums is None) or
+                len(old_sha256sums) != len(new_sha256sums) or
+                old_sha256sums != new_sha256sums):
+                return 'patch sha256sums differ'
+        return None
+
+    def _compute_sha256sums_for_patches(self, patchdir, component):
+        patches = buildutil.get_patch_paths_for_component(patchdir, component)
+        result = []
+
+        for patch in patches:
+            csum = hashlib.sha256()
+            f = open(patch)
+            patchdata = f.read()
+            csum.update(patchdata)
+            f.close()
+            result.append(csum.hexdigest())
+        return result
+
     def _build_one_component(self, component, architecture):
         basename = component['name']
 
@@ -84,14 +120,16 @@ class OstbuildBuild(builtins.Builtin):
         current_vcs_version = component.get('revision')
 
         expanded_component = self.expand_component(component)
-
-        # TODO - deduplicate this with chroot_compile_one
-        current_meta_io = StringIO()
-        json.dump(expanded_component, current_meta_io, indent=4, sort_keys=True)
-        current_metadata_text = current_meta_io.getvalue()
-        sha = hashlib.sha256()
-        sha.update(current_metadata_text)
-        current_meta_digest = sha.hexdigest()
+        
+        if 'patches' in expanded_component:
+            patchdir = vcs.checkout_patches(self.mirrordir,
+                                            self.patchdir,
+                                            expanded_component,
+                                            patches_path=self.args.patches_path)
+            patches_sha256sums = self._compute_sha256sums_for_patches(patchdir, expanded_component)
+            expanded_component['patches']['files_sha256sums'] = patches_sha256sums
+        else:
+            patchdir = None
 
         force_rebuild = (self.buildopts.force_rebuild or
                          basename in self.force_build_components)
@@ -109,44 +147,45 @@ class OstbuildBuild(builtins.Builtin):
                                                           '/_ostbuild-meta.json'])
             sha = hashlib.sha256()
             sha.update(previous_metadata_text)
-            previous_meta_digest = sha.hexdigest()
 
             previous_metadata = json.loads(previous_metadata_text)
             previous_vcs_version = previous_metadata.get('revision')
 
             log("Previous build of %s is ostree:%s " % (buildname, previous_build_version))
 
-            if current_meta_digest == previous_meta_digest:
+            rebuild_reason = self._needs_rebuild(previous_metadata, expanded_component)
+            if rebuild_reason is None:
                 if not force_rebuild:
                     log("Reusing cached build at %s" % (previous_vcs_version)) 
                     return previous_build_version
                 else:
                     log("Build forced regardless") 
             else:
-                previous_vcs_version = previous_metadata.get('revision')
-                if current_vcs_version == previous_vcs_version:
-                    if self.buildopts.skip_vcs_matches and not force_rebuild:
-                        log("Metadata differs; git revision %s unchanged" % (previous_vcs_version, ))
-                        return previous_build_version
-                    for k,v in expanded_component.iteritems():
-                        previous_v = previous_metadata.get(k)
-                        if v != previous_v:
-                            log("Key %r differs: old: %r new: %r" % (k, previous_v, v))
-                else:
-                    log("Metadata differs; git revision changed from '%s' to '%s'" % (previous_vcs_version, current_vcs_version))
+                    log("Need rebuild of %s: %s" % (buildname, rebuild_reason, ) )
         else:
             log("No previous build for '%s' found" % (buildname, ))
 
+        (fd, temp_metadata_path) = tempfile.mkstemp(suffix='.json', prefix='ostbuild-metadata-')
+        os.close(fd)
+        f = open(temp_metadata_path, 'w')
+        json.dump(expanded_component, f, indent=4, sort_keys=True)
+        f.close()
+
         checkoutdir = os.path.join(self.workdir, 'checkouts')
         component_src = os.path.join(checkoutdir, buildname)
         fileutil.ensure_parent_dir(component_src)
         child_args = ['ostbuild', 'checkout', '--snapshot=' + self.snapshot_path,
                       '--checkoutdir=' + component_src,
+                      '--metadata-path=' + temp_metadata_path,
                       '--clean', '--overwrite', basename]
         if self.args.patches_path:
             child_args.append('--patches-path=' + self.args.patches_path)
+        elif patchdir is not None:
+            child_args.append('--patches-path=' + patchdir)
         run_sync(child_args)
 
+        os.unlink(temp_metadata_path)
+
         artifact_meta = dict(component)
 
         logdir = os.path.join(self.workdir, 'logs', buildname)
diff --git a/src/ostbuild/pyostbuild/builtin_checkout.py b/src/ostbuild/pyostbuild/builtin_checkout.py
index 153ea8a..ee215ac 100755
--- a/src/ostbuild/pyostbuild/builtin_checkout.py
+++ b/src/ostbuild/pyostbuild/builtin_checkout.py
@@ -42,6 +42,7 @@ class OstbuildCheckout(builtins.Builtin):
         parser.add_argument('--overwrite', action='store_true')
         parser.add_argument('--prefix')
         parser.add_argument('--patches-path')
+        parser.add_argument('--metadata-path')
         parser.add_argument('--snapshot')
         parser.add_argument('--checkoutdir')
         parser.add_argument('-a', '--active-tree', action='store_true')
@@ -61,7 +62,12 @@ class OstbuildCheckout(builtins.Builtin):
         component_name = args.component
 
         found = False
-        component = self.get_expanded_component(component_name)
+        if args.metadata_path is not None:
+            f = open(args.metadata_path)
+            component = json.load(f)
+            f.close()
+        else:
+            component = self.get_expanded_component(component_name)
         (keytype, uri) = buildutil.parse_src_key(component['src'])
 
         is_local = (keytype == 'local')
@@ -93,29 +99,16 @@ class OstbuildCheckout(builtins.Builtin):
             else:
                 vcs.clean(keytype, checkoutdir)
 
-        patches = component.get('patches')
-        if patches is not None:
-            if self.args.patches_path:
-                (patches_keytype, patches_uri) = ('local', self.args.patches_path)
-            else:
-                (patches_keytype, patches_uri) = buildutil.parse_src_key(patches['src'])
-            if patches_keytype == 'git':
-                patches_mirror = buildutil.get_mirrordir(self.mirrordir, patches_keytype, patches_uri)
-                vcs.get_vcs_checkout(self.mirrordir, patches_keytype, patches_uri,
-                                     self.patchdir, patches['branch'],
-                                     overwrite=True)
-                patchdir = self.patchdir
-            else:
-                patchdir = patches_uri
-
-            patch_subdir = patches.get('subdir', None)
-            if patch_subdir is not None:
-                patchdir = os.path.join(patchdir, patch_subdir)
+        if 'patches' in component:
+            if args.patches_path is None:
+                patchdir = vcs.checkout_patches(self.mirrordir,
+                                                self.patchdir,
+                                                component)
             else:
-                patchdir = self.patchdir
-            for patch in patches['files']:
-                patch_path = os.path.join(patchdir, patch)
-                run_sync(['git', 'am', '--ignore-date', '-3', patch_path], cwd=checkoutdir)
+                patchdir = args.patches_path
+            patches = buildutil.get_patch_paths_for_component(patchdir, component)
+            for patch in patches:
+                run_sync(['git', 'am', '--ignore-date', '-3', patch], cwd=checkoutdir)
 
         metadata_path = os.path.join(checkoutdir, '_ostbuild-meta.json')
         f = open(metadata_path, 'w')
diff --git a/src/ostbuild/pyostbuild/builtin_chroot_compile_one.py b/src/ostbuild/pyostbuild/builtin_chroot_compile_one.py
index 083a0a0..8e48655 100755
--- a/src/ostbuild/pyostbuild/builtin_chroot_compile_one.py
+++ b/src/ostbuild/pyostbuild/builtin_chroot_compile_one.py
@@ -170,7 +170,9 @@ class OstbuildChrootCompileOne(builtins.Builtin):
         else:
             component_name = self.get_component_from_cwd()
 
-        component = self.get_expanded_component(component_name)
+        f = open('_ostbuild-meta.json')
+        component = json.load(f)
+        f.close()
 
         log("Building component: %r" % (component, ))
 
diff --git a/src/ostbuild/pyostbuild/vcs.py b/src/ostbuild/pyostbuild/vcs.py
index d9090ed..2b74e6c 100755
--- a/src/ostbuild/pyostbuild/vcs.py
+++ b/src/ostbuild/pyostbuild/vcs.py
@@ -161,3 +161,22 @@ def fetch(mirrordir, keytype, uri, branch, keep_going=False):
     ensure_vcs_mirror(mirrordir, keytype, uri, branch, fetch=True,
                       fetch_keep_going=keep_going)
     
+def checkout_patches(mirrordir, patchdir, component, patches_path=None):
+    patches = component.get('patches')
+    if patches is None:
+        return []
+    
+    if patches_path is not None:
+        (patches_keytype, patches_uri) = ('local', patches_path)
+        patchdir = patches_uri
+    else:
+        (patches_keytype, patches_uri) = parse_src_key(patches['src'])
+        assert patches_keytype == 'git'
+        patches_mirror = get_mirrordir(mirrordir, patches_keytype, patches_uri)
+        get_vcs_checkout(mirrordir, patches_keytype, patches_uri,
+                         patchdir, patches['revision'],
+                         overwrite=True)
+
+    return patchdir
+
+