Create, install and relocate tarballs of installed packages. #4854

gartung · 2017-07-21T16:29:32Z

This is a resubmit of previous binary_packages branch #1013 and includes work started in #445

A build cache is a tarball of the install directory of a spack installed package.

spack buildcache create <>

saves the install path, a list of binary files and a list of text files in install_directory/.spack/binary_distribution
creates a build_cache directory
creates a tarball of the package install directory
signs the tarball using gpg
creates ".spack" file with signature and install directory tarball
puts a renamed copy of ".spec" file in build_cache direcory
puts ".spack" in a subdirectory of build_cache named by platform then compiler
recurses all link and run dependencies of package creating build caches for them
optinally make all rpaths, ids, and deps relative if they are in the spack install tree before creating tarball of install directory

options:

-d : directory to create build_cache direcory in, default "."
-f : overwrite ".spack" in build_cache if it exists
-r : make paths in binaries relative before creating tarball
-key : the kep to sign package with, defaults to first key in spack gpg directory
-ns : don't sign the package with gpg
<> : list of package specs or package hashes with leading /

spack buildcache list <>

checks the list of configured mirrors for a build_cache directory
downloads all ".spec" files found
prints a list of spack buildcache install commands using the package hash and the equivalent concretized short spec

options:

<> string to be matched to matched to begining of listed concretized short specs,
eg. "spack buildcache list gcc" with print only commands to install gcc package(s)

spack buildcache install <>

checks the list of configured mirrors for ".spack" file matching the explicit hash or the hash generated from the input spec
downloads ".spack" file for packages and all link and run dependencies based on ".spec" file which contains all hashes
unpacks ".spack" file and verifies tarball with gpg signature file
unpacks tarball into new install path
changes all rpaths ( ids and deps on macOS ) from original install path to new install path in all binaries
changes all text file replacing original install path with new install path
calls spack reindex routine to add package to installed package db
repeats unpack and verify procedure for dependency .spack files

options:

-f : remove install directory if it exists before unpacking tarball
-nv : don't verify package with gpg
<> : list of package specs or package hashes with leading /

spack buildcache keys

checks the list of configured mirrors for ".key" files in build_cache directory and a prints a list of keys
optionally adds key to trusted keys

options:

-i : trust the keys downloaded with prompt for each
-y : answer yes to all

A build cache is a tarball of the install directory of a spack installed package. spack buildcache create <> * saves the install path, a list of binary files and a list of text files in install_directory/.spack/binary_distribution * creates a build_cache directory * creates a tarball of the package install directory * signs the tarball using gpg * creates ".spack" file with signature and install directory tarball * puts a renamed copy of ".spec" file in build_cache direcory * puts ".spack" in a subdirectory of build_cache named by platform then compiler * recurses all link and run dependencies of package creating build caches for them * optinally make all rpaths, ids, and deps relative if they are in the spack install tree before creating tarball of install directory options: -d : directory to create build_cache direcory in, default "." -f : overwrite ".spack" in build_cache if it exists -r : make paths in binaries relative before creating tarball -key : the kep to sign package with, defaults to first key in spack gpg directory -ns : don't sign the package with gpg <> : list of package specs or package hashes with leading / spack buildcache list <> * checks the list of configured mirrors for a build_cache directory * downloads all ".spec" files found * prints a list of spack buildcache install commands using the package hash and the equivalent concretized short spec options: <> string to be matched to matched to begining of listed concretized short specs, eg. "spack buildcache list gcc" with print only commands to install gcc package(s) spack buildcache install <> * checks the list of configured mirrors for ".spack" file matching the explicit hash or the hash generated from the input spec * downloads ".spack" file for packages and all link and run dependencies based on ".spec" file which contains all hashes * unpacks ".spack" file and verifies tarball with gpg signature file * unpacks tarball into new install path * changes all rpaths ( ids and deps on macOS ) from original install path to new install path in all binaries * changes all text file replacing original install path with new install path * calls spack reindex routine to add package to installed package db * repeats unpack and verify procedure for dependency .spack files options: -f : remove install directory if it exists before unpacking tarball -nv : don't verify package with gpg <> : list of package specs or package hashes with leading / spack buildcache keys * checks the list of configured mirrors for ".key" files in build_cache directory and a prints a list of keys * optionally adds key to trusted keys options: -i : trust the keys downloaded with prompt for each -y : answer yes to all

gartung · 2017-07-21T16:31:25Z

@mathstuf @adamjstewart
This is a resubmit of #1013

mathstuf

Testing shows that if I do not have keys installed, the package contains empty .asc files. If we're making a unsigned package (which didn't error or even warn), the .asc files should not be present.

mathstuf · 2017-07-24T19:37:48Z

lib/spack/docs/binary_caches.rst

+Finding and installing build cache files
+------------------------------------
+
+To find install build caches a spack mirror must be configured with


To find install build caches reads oddly. To find installed build caches perhaps?

mathstuf · 2017-07-24T19:38:19Z

lib/spack/docs/binary_caches.rst

+.. _binary_caches:
+
+Build caches
+============================


The "titlelines" should match the title length (also applies to later titles).

mathstuf · 2017-07-24T19:38:59Z

lib/spack/docs/binary_caches.rst

+Tarballs are created for all of its link and run 
+dependency packages as well.
+Compressed tarballs are signed with gpg and 
+signature and tarball and put in a ".spack" tarfile. 


There's no need to mention that it's a tarfile. Also, the wordwrapping here is odd. Consider reflowing?

mathstuf · 2017-07-24T19:39:58Z

lib/spack/docs/binary_caches.rst

+
+options:
+
+-d : directory to create build_cache direcory in, 


This looks like it could use some markup. Also, build_cache is not an (English) word :) .

mathstuf · 2017-07-24T19:41:33Z

lib/spack/docs/binary_caches.rst

+-key : the kep to sign package with, 
+       defaults to first key in spack gpg directory
+-ns : don't sign the package with gpg
+<> : list of package specs or package hashes with leading /


I don't think this is how this has been specified elsewhere in the docs.

mathstuf · 2017-07-24T19:43:07Z

lib/spack/docs/binary_caches.rst

+-r : make paths in binaries relative before creating tarball
+-key : the kep to sign package with, 
+       defaults to first key in spack gpg directory
+-ns : don't sign the package with gpg


Short options should be a single letter with the long option using the full words. It may also be better to discuss the various flags in prose where how the command is usually used is discussed (see the docs for spack gpg which go discuss a series of actions which make sense using the command).

mathstuf · 2017-07-24T19:43:31Z

lib/spack/spack/binary_distribution.py

@@ -0,0 +1,372 @@
+# "Benedikt Hegner (CERN)"
+# "Patrick Gartung (FNAL)"


This should have the standard Spack copyright header, no?

mathstuf · 2017-07-24T19:50:17Z

lib/spack/spack/cmd/buildcache.py

+    create.add_argument('-ns', '--nosign', action='store_true')
+    create.add_argument('-k', '--key', metavar='key',
+                        type=str, default=None,
+                        help="gpg2 key for signing.")


This should just be Key for signing. No need to mention that it's a GPG key.

mathstuf · 2017-07-24T20:12:01Z

And if I use -k, the package is also unsigned (empty .asc files).

gartung · 2017-07-24T20:29:35Z

The -k option should work now.

mathstuf · 2017-07-24T20:59:40Z

Hmm, I'm still getting an empty .asc file. I created a new key and am trying to create the package using:

bin/spack buildcache create -k 0A3CAFD27242FF43 -d opt/spack/buildcaches/test-gpg cmake

gartung · 2017-07-24T21:06:10Z

lib/spack/spack/binary_distribution.py

+    # Sign the packages.
+    if sign:
+        if key is None:
+            keys = Gpg.signing_keys()


@mathstuf Do I need to init the Pgp interface for the signing to work?

I found the bug.

gartung · 2017-07-24T21:28:24Z

It worked on my mac because I had just made the bug fix.

mathstuf · 2017-07-24T21:43:05Z

Yep, that works now :) . Thanks. I'll test it out more later this week.

…ome/gartung/spack/bin:/usr/lib64/qt-3.3/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/home/gartung/.local/bin:/home/gartung/bin, call more fuctions from relocate.pywq

…ested in pytest env

Will review the newest code.

…should have made read-only files writable.

scheibelp · 2017-08-10T21:28:44Z

lib/spack/spack/binary_distribution.py

+        if force:
+            os.remove(specfile_path)
+        else:
+            return NoOverwriteException(specfile_path)


-> raise NoOverwriteException (you must raise exceptions vs. returning them)

scheibelp · 2017-08-10T21:30:42Z

lib/spack/spack/binary_distribution.py

+        try:
+            sign_tarball(yes_to_all, key, force, specfile_path)
+            signed = True
+        except (NoGpgException, PickKeyException, NoKeyException) as e:


In this case you don't have to surround with try/except, the exception will terminate and be caught in the caller (generally speaking when you have a try/except clause and the except just has a raise then you don't need to wrap with a try/except)

…ches Conflicts: lib/spack/spack/binary_distribution.py lib/spack/spack/test/packaging.py

scheibelp

This looks pretty good to me. All new comments are related to documentation.

I tried running the tests on my system and it hung on account of the key generation. It looks like this didn't cause problems for travis though, and I think it may be problematic at the moment to add a private key. @mathstuf is there a way to add signing and verification keys for a unit test in such a way that it doesn't affect spack signing/verifying outside the test?

scheibelp · 2017-08-11T00:59:08Z

lib/spack/docs/binary_caches.rst

+
+spack buildcache install <>
+^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Install "build_cache" available on spack mirror.


I'd prefer: "retrieve specs available from each mirror's build cache and installs all specs which match the input spec"

scheibelp · 2017-08-11T01:10:51Z

lib/spack/docs/binary_caches.rst

+Initial build and later installation do not necessarily happen at the same 
+location. Spack provides a relocation capability and corrects for RPATHs and 
+non-relocatable scripts. However, many packages compile paths into binary 
+artificats directly. In such cases, the build instructions of this package would


-> "artifacts"

Jenkins speak for binaries.

scheibelp · 2017-08-11T01:13:05Z

lib/spack/docs/binary_caches.rst

+-----
+spack buildcache create <>
+^^^^^^^^^^^^^^^^^^^^^^^^^^
+Create tarball of installed spack package, checksum tarball and 


Create tarball of installed spack package

... and all dependencies. Places them in a directory. You can place the directory in a mirror and commands like "spack buildcache install" will search it for pre-compiled packages.

scheibelp · 2017-08-11T01:26:20Z

lib/spack/spack/relocate.py

+
+def macho_make_paths_relative(path_name, old_dir, rpaths, deps, idpath):
+    """
+    Replace old_dir with relative path from dirname(path_name) to old_dir


This makes more sense to me if you remove "to old_dir" at the end of this line. Also, the same replacement is not being done for idpath.

scheibelp · 2017-08-11T01:35:24Z

lib/spack/spack/relocate.py

+    The old install dir in LC_RPATH is replaced with the new install dir using
+    install_name_tool  -rpath old new binary
+    """
+    # avoid error message for libgcc_s


What error comes up for this? I'm curious what context this function gets called on a path_name for libgcc_s

Packaging the gcc compiler.

scheibelp · 2017-08-11T02:20:42Z

That's odd: the failing test is not associated with this PR (and only fails for python 3.4) - could you close and reopen the PR in the github interface? That will rerun the tests.

mathstuf · 2017-08-11T12:16:27Z

is there a way to add signing and verification keys for a unit test in such a way that it doesn't affect spack signing/verifying outside the test?

Yes, the GPG tests do this by setting the directories to use to mock directories instead.

scheibelp · 2017-08-11T17:58:34Z

@mathstuf mentioned that the exceptions you defined should inherit from Exception, so for example update:

class NoGpgException:
    pass

with

class NoGpgException(Exception):
    pass

(I'm repeating this here because I'm having trouble finding this comment in the review)

Other than that this looks good to me and I plan to merge it by 8/14 (pending that minor update)

scheibelp · 2017-08-14T21:36:53Z

@gartung Thanks for the PR, and for all the updates!

gartung changed the title ~~Add the buildcache command and subcommands to spack.~~ Create, install and relocate tarballs of installed packages. Jul 21, 2017

mathstuf requested review from mathstuf and tgamblin July 21, 2017 18:21

convert packaging test to pytest framework

3557fcb

mathstuf previously requested changes Jul 24, 2017

View reviewed changes

documentation updates

c01923b

pass in key if given on command line

713a3e3

gartung force-pushed the build_caches branch from b5a386f to 713a3e3 Compare July 24, 2017 20:43

gartung commented Jul 24, 2017

View reviewed changes

fix order of argumens to build_tarball

1fc1e0f

gartung force-pushed the build_caches branch 3 times, most recently from 40ec524 to a4ccfe7 Compare July 26, 2017 01:28

improve the coverage of testing without calling gpg2

44482ca

gartung force-pushed the build_caches branch from a4ccfe7 to 44482ca Compare July 26, 2017 02:20

gartung added 8 commits July 25, 2017 22:05

make sure gpg2 is available

0b2d3ba

add testing of buildcache command

724c27d

autopep8

052b7cb

keep getting weird failures with the gpg stuff

64e24cc

autopep8 and flake8 fixes

d253646

Merge branch 'develop' into build_caches

514af1e

mimic the output of patchelf by echoing /home/gartung/autopep8/bin:/h…

698d6ab

…ome/gartung/spack/bin:/usr/lib64/qt-3.3/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/home/gartung/.local/bin:/home/gartung/bin, call more fuctions from relocate.pywq

factor out path string replacement for mach-O object so they can be t…

fd40e52

…ested in pytest env

gartung added 2 commits August 10, 2017 10:30

Improve function descriptions. Remove read-only checks since copying …

c7f1810

…should have made read-only files writable.

Add ProcessError import

bec0e52

gartung force-pushed the build_caches branch from ee312f5 to bec0e52 Compare August 10, 2017 15:36

gartung added 2 commits August 10, 2017 15:41

remove chmod lines as this files should be writable after copying

73fdb2c

WIP

b79a47a

scheibelp reviewed Aug 10, 2017

View reviewed changes

gartung added 2 commits August 10, 2017 16:59

call raise not return Exception

cb4d87c

call raise not return Exception

9889d0e

gartung force-pushed the build_caches branch from cb4d87c to 9889d0e Compare August 10, 2017 22:02

gartung added 3 commits August 10, 2017 17:04

remove debug print

e764767

increase coverage?

4006f40

Merge branch 'build_caches' of github.com:gartung/spack into build_ca…

a7f7153

…ches Conflicts: lib/spack/spack/binary_distribution.py lib/spack/spack/test/packaging.py

scheibelp reviewed Aug 11, 2017

View reviewed changes

gartung closed this Aug 11, 2017

gartung reopened this Aug 11, 2017

documentation updates

fe99867

gartung added 2 commits August 11, 2017 13:05

Use superclass Exception

f02d2ba

flake8 nitpick

b11372c

scheibelp merged commit ab56c74 into spack:develop Aug 14, 2017

adamjstewart mentioned this pull request Aug 16, 2017

Clean up docs on and add tab completion for buildcache command #5125

Merged

This was referenced Aug 17, 2017

CUDA 9.0 alpaka-group/alpaka#376

Closed

Docs: Travis-CI Workflow #5133

Merged

gartung deleted the build_caches branch August 24, 2017 16:16

tgamblin added this to the v0.11.0 milestone Nov 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create, install and relocate tarballs of installed packages. #4854

Create, install and relocate tarballs of installed packages. #4854

gartung commented Jul 21, 2017 •

edited

gartung commented Jul 21, 2017 •

edited

mathstuf left a comment

mathstuf Jul 24, 2017

mathstuf Jul 24, 2017

mathstuf Jul 24, 2017

mathstuf Jul 24, 2017

mathstuf Jul 24, 2017

mathstuf Jul 24, 2017

mathstuf Jul 24, 2017

mathstuf Jul 24, 2017

mathstuf commented Jul 24, 2017

gartung commented Jul 24, 2017

mathstuf commented Jul 24, 2017

gartung Jul 24, 2017

gartung Jul 24, 2017

gartung commented Jul 24, 2017

mathstuf commented Jul 24, 2017

scheibelp Aug 10, 2017

scheibelp Aug 10, 2017 •

edited

scheibelp left a comment

scheibelp Aug 11, 2017

scheibelp Aug 11, 2017

gartung Aug 11, 2017

scheibelp Aug 11, 2017

scheibelp Aug 11, 2017

scheibelp Aug 11, 2017

gartung Aug 11, 2017

scheibelp commented Aug 11, 2017

mathstuf commented Aug 11, 2017

scheibelp commented Aug 11, 2017

scheibelp commented Aug 14, 2017

		@@ -0,0 +1,372 @@
		# "Benedikt Hegner (CERN)"
		# "Patrick Gartung (FNAL)"

Create, install and relocate tarballs of installed packages. #4854

Create, install and relocate tarballs of installed packages. #4854

Conversation

gartung commented Jul 21, 2017 • edited

gartung commented Jul 21, 2017 • edited

mathstuf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mathstuf commented Jul 24, 2017

gartung commented Jul 24, 2017

mathstuf commented Jul 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gartung commented Jul 24, 2017

mathstuf commented Jul 24, 2017

Choose a reason for hiding this comment

scheibelp Aug 10, 2017 • edited

Choose a reason for hiding this comment

scheibelp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scheibelp commented Aug 11, 2017

mathstuf commented Aug 11, 2017

scheibelp commented Aug 11, 2017

scheibelp commented Aug 14, 2017

gartung commented Jul 21, 2017 •

edited

gartung commented Jul 21, 2017 •

edited

scheibelp Aug 10, 2017 •

edited