spack buildcache: allow unsigned tarballs (#7440)

This re-adds the option to create and install unsigned tarballs, now
with the -u option (--unsigned) rather than the -y option.

This also changes the "keys" command, replacing the -y/--yes-to-all
option with the -t/--trust option (which has the same effect but is
more-clearly named).

lib/spack/spack/binary_distribution.py

@@ -254,8 +254,8 @@ def generate_index(outdir, indexfile_path):
     f.close()
 
 
-def build_tarball(spec, outdir, force=False, rel=False, allow_root=False,
+def build_tarball(spec, outdir, force=False, rel=False, unsigned=False,
-                  key=None):
+                  allow_root=False, key=None):
     """
     Build a tarball from given spec and put it into the directory structure
     used at the mirror (following <tarball_directory_name>).
@@ -337,17 +337,20 @@ def build_tarball(spec, outdir, force=False, rel=False, allow_root=False,
     with open(specfile_path, 'w') as outfile:
         outfile.write(yaml.dump(spec_dict))
     # sign the tarball and spec file with gpg
-    sign_tarball(key, force, specfile_path)
+    if not unsigned:
+        sign_tarball(key, force, specfile_path)
     # put tarball, spec and signature files in .spack archive
     with closing(tarfile.open(spackfile_path, 'w')) as tar:
         tar.add(name='%s' % tarfile_path, arcname='%s' % tarfile_name)
         tar.add(name='%s' % specfile_path, arcname='%s' % specfile_name)
-        tar.add(name='%s.asc' % specfile_path,
+        if not unsigned:
-                arcname='%s.asc' % specfile_name)
+            tar.add(name='%s.asc' % specfile_path,
+                    arcname='%s.asc' % specfile_name)
 
     # cleanup file moved to archive
     os.remove(tarfile_path)
-    os.remove('%s.asc' % specfile_path)
+    if not unsigned:
+        os.remove('%s.asc' % specfile_path)
 
     # create an index.html for the build_cache directory so specs can be found
     if os.path.exists(indexfile_path):
@@ -435,7 +438,8 @@ def relocate_package(workdir, allow_root):
                                  allow_root)
 
 
-def extract_tarball(spec, filename, allow_root=False, force=False):
+def extract_tarball(spec, filename, allow_root=False, unsigned=False,
+                    force=False):
     """
     extract binary tarball for given package into install area
     """
@@ -456,19 +460,19 @@ def extract_tarball(spec, filename, allow_root=False, force=False):
 
     with closing(tarfile.open(spackfile_path, 'r')) as tar:
         tar.extractall(tmpdir)
-
+    if not unsigned:
-    if os.path.exists('%s.asc' % specfile_path):
+        if os.path.exists('%s.asc' % specfile_path):
-        try:
+            try:
-            Gpg.verify('%s.asc' % specfile_path, specfile_path)
+                Gpg.verify('%s.asc' % specfile_path, specfile_path)
-        except Exception as e:
+            except Exception as e:
+                shutil.rmtree(tmpdir)
+                tty.die(str(e))
+        else:
             shutil.rmtree(tmpdir)
-            tty.die(str(e))
+            raise NoVerifyException(
-    else:
+                "Package spec file failed signature verification.\n"
-        shutil.rmtree(tmpdir)
+                "Use spack buildcache keys to download "
-        raise NoVerifyException(
+                "and install a key for verification from the mirror.")
-            "Package spec file failed signature verification.\n"
-            "Use spack buildcache keys to download "
-            "and install a key for verification from the mirror.")
     # get the sha256 checksum of the tarball
     checksum = checksum_tarball(tarfile_path)
 
@@ -580,7 +584,7 @@ def get_specs(force=False):
     return specs
 
 
-def get_keys(install=False, yes_to_all=False, force=False):
+def get_keys(install=False, trust=False, force=False):
     """
     Get pgp public keys available on mirror
     """
@@ -617,9 +621,9 @@ def get_keys(install=False, yes_to_all=False, force=False):
                         continue
             tty.msg('Found key %s' % link)
             if install:
-                if yes_to_all:
+                if trust:
                     Gpg.trust(stage.save_filename)
                     tty.msg('Added this key to trusted keys.')
                 else:
                     tty.msg('Will not add this key to trusted keys.'
-                            'Use -y to override')
+                            'Use -t to install all downloaded keys')

lib/spack/spack/cmd/buildcache.py

@@ -45,6 +45,9 @@ def setup_parser(subparser):
                              " before creating tarballs.")
     create.add_argument('-f', '--force', action='store_true',
                         help="overwrite tarball if it exists.")
+    create.add_argument('-u', '--unsigned', action='store_true',
+                        help="create unsigned buildcache" +
+                             " tarballs for testing")
     create.add_argument('-a', '--allow_root', action='store_true',
                         help="allow install root string in binary files " +
                              "after RPATH substitution")
@@ -67,6 +70,9 @@ def setup_parser(subparser):
     install.add_argument('-a', '--allow_root', action='store_true',
                          help="allow install root string in binary files " +
                               "after RPATH substitution")
+    install.add_argument('-u', '--unsigned', action='store_true',
+                         help="install unsigned buildcache" +
+                              " tarballs for testing")
     install.add_argument(
         'packages', nargs=argparse.REMAINDER,
         help="specs of packages to install biuldache for")
@@ -85,8 +91,8 @@ def setup_parser(subparser):
         '-i', '--install', action='store_true',
         help="install Keys pulled from mirror")
     dlkeys.add_argument(
-        '-y', '--yes_to_all', action='store_true',
+        '-t', '--trust', action='store_true',
-        help="answer yes to all trust questions")
+        help="trust all downloaded keys")
     dlkeys.add_argument('-f', '--force', action='store_true',
                         help="force new download of keys")
     dlkeys.set_defaults(func=getkeys)
@@ -189,15 +195,6 @@ def createtarball(args):
     signkey = None
     if args.key:
         signkey = args.key
-    allow_root = False
-    force = False
-    relative = False
-    if args.allow_root:
-        allow_root = True
-    if args.force:
-        force = True
-    if args.rel:
-        relative = True
 
     matches = find_matching_specs(pkgs, False, False)
     for match in matches:
@@ -222,8 +219,8 @@ def createtarball(args):
 
     for spec in specs:
         tty.msg('creating binary cache file for package %s ' % spec.format())
-        bindist.build_tarball(spec, outdir, force,
+        bindist.build_tarball(spec, outdir, args.force, args.rel,
-                              relative, allow_root, signkey)
+                              args.unsigned, args.allow_root, signkey)
 
 
 def installtarball(args):
@@ -232,13 +229,7 @@ def installtarball(args):
         tty.die("build cache file installation requires" +
                 " at least one package spec argument")
     pkgs = set(args.packages)
-    multiple = False
+    matches = match_downloaded_specs(pkgs, args.multiple, args.force)
-    if args.multiple:
-        multiple = True
-    force = False
-    if args.force:
-        force = True
-    matches = match_downloaded_specs(pkgs, multiple, force)
 
     for match in matches:
         install_tarball(match, args)
@@ -249,23 +240,18 @@ def install_tarball(spec, args):
     if s.external or s.virtual:
         tty.warn("Skipping external or virtual package %s" % spec.format())
         return
-    allow_root = False
-    if args.allow_root:
-        allow_root = True
-    force = False
-    if args.force:
-        force = True
     for d in s.dependencies(deptype=('link', 'run')):
         tty.msg("Installing buildcache for dependency spec %s" % d)
         install_tarball(d, args)
     package = spack.repo.get(spec)
-    if s.concrete and package.installed and not force:
+    if s.concrete and package.installed and not args.force:
         tty.warn("Package for spec %s already installed." % spec.format())
     else:
         tarball = bindist.download_tarball(spec)
         if tarball:
             tty.msg('Installing buildcache for spec %s' % spec.format())
-            bindist.extract_tarball(spec, tarball, allow_root, force)
+            bindist.extract_tarball(spec, tarball, args.allow_root,
+                                    args.unsigned, args.force)
             spack.store.db.reindex(spack.store.layout)
         else:
             tty.die('Download of binary cache file for spec %s failed.' %
@@ -297,16 +283,7 @@ def listspecs(args):
 
 def getkeys(args):
     """get public keys available on mirrors"""
-    install = False
+    bindist.get_keys(args.install, args.trust, args.force)
-    if args.install:
-        install = True
-    yes_to_all = False
-    if args.yes_to_all:
-        yes_to_all = True
-    force = False
-    if args.force:
-        force = True
-    bindist.get_keys(install, yes_to_all, force)
 
 
 def buildcache(parser, args):

lib/spack/spack/package.py

@@ -1288,7 +1288,8 @@ def try_install_from_binary_cache(self, explicit):
         tty.msg('Installing %s from binary cache' % self.name)
         tarball = binary_distribution.download_tarball(binary_spec)
         binary_distribution.extract_tarball(
-            binary_spec, tarball, allow_root=False, force=False)
+            binary_spec, tarball, allow_root=False,
+            unsigned=False, force=False)
         spack.store.db.add(self.spec, spack.store.layout, explicit=explicit)
         return True
 

lib/spack/spack/test/packaging.py

@@ -157,34 +157,34 @@ def test_buildcache(mock_archive, tmpdir):
     else:
         # create build cache without signing
         args = parser.parse_args(
-            ['create', '-d', mirror_path, '-y', str(spec)])
+            ['create', '-d', mirror_path, '-u', str(spec)])
         buildcache.buildcache(parser, args)
 
         # Uninstall the package
         pkg.do_uninstall(force=True)
 
         # install build cache without verification
-        args = parser.parse_args(['install', '-y', str(spec)])
+        args = parser.parse_args(['install', '-u', str(spec)])
         buildcache.install_tarball(spec, args)
 
         # test overwrite install without verification
-        args = parser.parse_args(['install', '-f', '-y', str(pkghash)])
+        args = parser.parse_args(['install', '-f', '-u', str(pkghash)])
         buildcache.buildcache(parser, args)
 
         # create build cache with relative path
         args = parser.parse_args(
-            ['create', '-d', mirror_path, '-f', '-r', '-y', str(pkghash)])
+            ['create', '-d', mirror_path, '-f', '-r', '-u', str(pkghash)])
         buildcache.buildcache(parser, args)
 
         # Uninstall the package
         pkg.do_uninstall(force=True)
 
         # install build cache
-        args = parser.parse_args(['install', '-y', str(spec)])
+        args = parser.parse_args(['install', '-u', str(spec)])
         buildcache.install_tarball(spec, args)
 
         # test overwrite install
-        args = parser.parse_args(['install', '-f', '-y', str(pkghash)])
+        args = parser.parse_args(['install', '-f', '-u', str(pkghash)])
         buildcache.buildcache(parser, args)
 
     # Validate the relocation information