Merge branch 'benbrown/sanitise-ref-sha' into 'master'

Sanitise ref and sha usage

See merge request !389

9 files changed, 36 insertions, 45 deletions

diff --git a/ybd/__main__.py b/ybd/__main__.py
index 88c12a0..7014280 100755
--- a/ybd/__main__.py
+++ b/ybd/__main__.py
@@ -92,8 +92,8 @@ with timer('TOTAL'):
                                 target.get('kind') == 'cluster'):
         log('ARCH', 'No definitions for', config['arch'], exit=True)
 
+    write_cache_key()
     if config.get('mode', 'normal') == 'keys-only':
-        write_cache_key()
         os._exit(0)
 
     cache.cull(config['artifacts'])
diff --git a/ybd/cache.py b/ybd/cache.py
index 17d5517..f3b312d 100644
--- a/ybd/cache.py
+++ b/ybd/cache.py
@@ -183,8 +183,7 @@ def update_manifest(dn, manifest):
             format = '%s %s %s %s %s %s\n'
             m.write(format % (dn['name'], dn['cache'],
                               get_repo_url(dn.get('repo', 'None')),
-                              dn.get('ref', 'None'),
-                              dn.get('unpetrify-ref', 'None'),
+                              dn.get('sha'), dn.get('ref'),
                               md5(get_cache(dn))))
             m.flush()
             return
@@ -192,8 +191,7 @@ def update_manifest(dn, manifest):
         text = {'name': dn['name'],
                 'summary': {'artifact': dn['cache'],
                             'repo': get_repo_url(dn.get('repo', None)),
-                            'sha': dn.get('sha', None),
-                            'ref': dn.get('unpetrify-ref', None),
+                            'sha': dn.get('sha'), 'ref': dn.get('ref'),
                             'md5': md5(get_cache(dn))}}
         m.write(yaml.dump(text, default_flow_style=True))
         m.flush()
diff --git a/ybd/config/defaults.conf b/ybd/config/defaults.conf
index 8d3576b..c464b77 100644
--- a/ybd/config/defaults.conf
+++ b/ybd/config/defaults.conf
@@ -13,7 +13,6 @@ morph-fields: ['arch',
                'max-jobs',
                'morph',
                'name',
-               'orig_ref',
                'path',
                'prefix',
                'products',
diff --git a/ybd/config/ybd.conf b/ybd/config/ybd.conf
index 27e60eb..d9ac612 100644
--- a/ybd/config/ybd.conf
+++ b/ybd/config/ybd.conf
@@ -210,8 +210,8 @@ tar-url: 'http://git.baserock.org/tarballs'
 # else, set TMPDIR
 # TMPDIR: /tmp
 
-# Normally ybd builds at whatever version is set in the 'ref:' field
-# If we set 'track-branches:', then ybd will attempt to build 'unpetrify-ref:'
+# Normally ybd builds at whatever version is set in the 'sha:' field
+# If we set 'track-branches:', then ybd will attempt to build 'ref:'
 # track-branches: [path/to/foo-chunk, path/to/bar-chunk, path/to/bip-chunk]
 # track-branches: True
 
diff --git a/ybd/morphs.py b/ybd/morphs.py
index c99b36d..6ded04d 100644
--- a/ybd/morphs.py
+++ b/ybd/morphs.py
@@ -215,13 +215,17 @@ class Morphs(object):
                         'WARNING: multiple definitions of %s \n' % key,
                         '%s | %s' % (dn.get(key), new_def[key]), exit=exit)
 
-        # If a sha was specified, we want to build it instead of the ref
-        # but preserve the ref in the output <target>.yml file.
-        if new_def.get('sha'):
-            if len(new_def['sha']) != 40:
-                log(new_def, 'ERROR: invalid sha:', new_def['sha'], exit=True)
-            new_def['orig_ref'] = new_def['ref']
-            new_def['ref'] = new_def['sha']
+        sha = new_def.get('sha')
+        if sha:
+            if len(sha) != 40:
+                log(new_def, 'ERROR: invalid sha:', sha, exit=True)
+        if new_def.get('ref'):
+            if not sha and len(new_def['ref']) != 40:
+                log(new_def, 'WARNING: ref is not a sha:', new_def['ref'],
+                    exit=False)
+        if new_def.get('unpetrify-ref'):
+            log(new_def, 'WARNING: \'unpetrify-ref\' is a deprecated field',
+                exit=False)
 
         if dn is None:
             self._data[new_def['path']] = new_def
diff --git a/ybd/pots.py b/ybd/pots.py
index 96686ae..07bb644 100644
--- a/ybd/pots.py
+++ b/ybd/pots.py
@@ -54,17 +54,8 @@ class Pots(object):
         return self._data.get(dn.get('path', dn.keys()[0]))
 
     def save(self, filename):
-
-        # Make a copy, restore any refs which may have been overridden
-        # with the sha field while building.
-        data = copy.deepcopy(self._data)
-        for key, value in data.items():
-            if value.get('orig_ref') is not None:
-                value['ref'] = value['orig_ref']
-                del value['orig_ref']
-
         with open(filename, 'w') as f:
-            f.write(morph_dump(data, self.defaults))
+            f.write(morph_dump(self._data, self.defaults))
         log('CHECK', 'Saved yaml definitions at', filename)
 
     def _load_pots(self, filename):
@@ -80,6 +71,8 @@ class Pots(object):
             treefile = os.path.join(config['artifacts'], '.trees')
             if not os.path.isfile(treefile):
                 treefile = os.path.join(config['defdir'], '.trees')
+            if not os.path.isfile(treefile):
+                return
             with open(treefile) as f:
                 text = f.read()
                 if text:
@@ -88,10 +81,11 @@ class Pots(object):
             for path in self._data:
                 dn = self._data[path]
                 sha1 = dn.get('sha', dn.get('ref'))
-                if sha1 and self._trees.get(path):
-                    if sha1 == self._trees.get(path)[0]:
-                        dn['sha'] = self._trees.get(path)[0]
-                        dn['tree'] = self._trees.get(path)[1]
+                tree_entry = self._trees.get(path)
+                if sha1 and tree_entry:
+                    if sha1 == tree_entry[0]:
+                        dn['sha'] = tree_entry[0]
+                        dn['tree'] = tree_entry[1]
                         count += 1
             log('DEFINITIONS', 'Re-used %s entries from .trees file' % count)
         except:
diff --git a/ybd/release_note.py b/ybd/release_note.py
index 7d44b59..c3c1c93 100644
--- a/ybd/release_note.py
+++ b/ybd/release_note.py
@@ -100,7 +100,7 @@ def log_changes(dn, tmpdir, old_defs, ref):
             try:
                 gitdir = os.path.join(config['gits'],
                                       get_repo_name(dn['repo']))
-                cur_ref = dn.get('sha', dn['ref'])
+                cur_ref = dn['sha']
                 old_ref = old_def.get('sha', old_def['ref'])
                 if old_ref != cur_ref:
                     log(dn, 'Logging git change history', tmpdir)
diff --git a/ybd/repos.py b/ybd/repos.py
index f3b1872..9674ee9 100644
--- a/ybd/repos.py
+++ b/ybd/repos.py
@@ -100,11 +100,9 @@ def get_last_tag(gitdir):
 
 def get_tree(dn):
     info = get_transport_info(dn['repo'])
-    ref = str(dn.get('sha', str(dn.get('ref'))))
-    track = app.config.get('track-branches')
-    if dn.get('unpetrify-ref') and track:
-        if track is True or dn['path'] in track:
-            ref = str(dn['unpetrify-ref'])
+    track = app.config.get('track-branches', [])
+    track = (isinstance(track, list) and dn['path'] in track) or track
+    ref = dn.get('ref') if track else dn.get('sha', dn.get('ref'))
 
     gitdir = info['dir']
     if dn['repo'].startswith('file://') or dn['repo'].startswith('/'):
@@ -124,18 +122,16 @@ def get_tree(dn):
         mirror(dn['name'], dn['repo'])
 
     with app.chdir(gitdir), open(os.devnull, "w") as fnull:
-        if call(['git', 'rev-parse', ref + '^{object}'], stdout=fnull,
-                stderr=fnull):
+        if track or call(['git', 'rev-parse', ref + '^{object}'],
+                         stdout=fnull, stderr=fnull):
             # can't resolve ref. is it upstream?
             app.log(dn, 'Fetching from upstream to resolve %s' % ref)
             update_mirror(dn['name'], dn['repo'], gitdir)
 
         try:
-            tree = check_output(['git', 'rev-parse', ref + '^{tree}'],
-                                universal_newlines=True)[0:-1]
-            sha = check_output(['git', 'rev-parse', ref],
-                               universal_newlines=True)[0:-1]
-            return [tree, sha]
+            # Returns a (tree, sha) tuple.
+            return check_output(['git', 'rev-parse', ref + '^{tree}', ref],
+                                universal_newlines=True).split()
 
         except:
             # either we don't have a git dir, or ref is not unique
@@ -197,7 +193,7 @@ def update_mirror(name, repo, gitdir):
 
 
 def checkout(dn):
-    _checkout(dn['name'], dn['repo'], dn.get('sha', dn['ref']), dn['checkout'])
+    _checkout(dn['name'], dn['repo'], dn['sha'], dn['checkout'])
 
     with app.chdir(dn['checkout']):
         if os.path.exists('.gitmodules') or dn.get('submodules'):
@@ -316,7 +312,7 @@ def checkout_submodules(dn):
         try:
             # list objects in the parent repo tree to find the commit
             # object that corresponds to the submodule
-            commit = check_output(['git', 'ls-tree', dn['ref'], path])
+            commit = check_output(['git', 'ls-tree', dn['sha'], path])
 
             # read the commit hash from the output
             fields = commit.split()
diff --git a/ybd/splitting.py b/ybd/splitting.py
index 3797a1f..770cb64 100644
--- a/ybd/splitting.py
+++ b/ybd/splitting.py
@@ -255,7 +255,7 @@ def write_metafile(rules, splits, dn):
 
     if dn.get('kind', 'chunk') == 'chunk':
         metadata['repo'] = dn.get('repo')
-        metadata['ref'] = dn.get('ref')
+        metadata['ref'] = dn.get('sha')
     else:
         if config.get('artifact-version', 0) not in range(0, 2):
             metadata['repo'] = config['defdir']