From c9e214f6d37af9320afc3a3bca304143c91254e7 Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com> Date: Thu, 22 Aug 2019 11:08:23 -0700 Subject: Spack BundlePackage: a group of other packages (#11981) This adds a special package type to Spack which is used to aggregate a set of packages that a user might commonly install together; it does not include any source code itself and does not require a download URL like other Spack packages. It may include an 'install' method to generate scripts, and Spack will run post-install hooks (including module generation). * Add new BundlePackage type * Update the Xsdk package to be a BundlePackage and remove the 'install' method (previously it had a noop install method) * "spack create --template" now takes "bundle" as an option * Rename cmd_create_repo fixture to "mock_test_repo" and relocate it to shared pytest fixtures * Add unit tests for BundlePackage behavior --- lib/spack/spack/cmd/create.py | 90 ++++++++++++++++++-------- lib/spack/spack/cmd/info.py | 24 ++++--- lib/spack/spack/cmd/url.py | 6 ++ lib/spack/spack/directives.py | 50 ++++++++++----- lib/spack/spack/fetch_strategy.py | 52 +++++++++++++++ lib/spack/spack/package.py | 114 +++++++++++++++++++++------------ lib/spack/spack/pkgkit.py | 4 +- lib/spack/spack/test/cmd/create.py | 107 +++++++++++++++++++++++-------- lib/spack/spack/test/cmd/info.py | 3 +- lib/spack/spack/test/concretize.py | 8 ++- lib/spack/spack/test/conftest.py | 50 +++++++++++++++ lib/spack/spack/test/install.py | 35 +++++++++- lib/spack/spack/test/package_sanity.py | 2 +- lib/spack/spack/test/packages.py | 19 ++++++ lib/spack/spack/test/url_fetch.py | 77 ++++++++-------------- lib/spack/spack/version.py | 9 +++ 16 files changed, 475 insertions(+), 175 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/cmd/create.py b/lib/spack/spack/cmd/create.py index 59e3274bdb..fd2a00d6cb 100644 --- a/lib/spack/spack/cmd/create.py +++ b/lib/spack/spack/cmd/create.py @@ -11,7 +11,6 @@ import re import llnl.util.tty as tty from llnl.util.filesystem import mkdirp -import spack.cmd import spack.util.web import spack.repo from spack.spec import Spec @@ -58,35 +57,33 @@ class {class_name}({base_class_name}): # FIXME: Add a proper url for your package's homepage here. homepage = "http://www.example.com" - url = "{url}" +{url_def} {versions} {dependencies} -{body} +{body_def} ''' -class PackageTemplate(object): - """Provides the default values to be used for the package file template""" +class BundlePackageTemplate(object): + """ + Provides the default values to be used for a bundle package file template. + """ - base_class_name = 'Package' + base_class_name = 'BundlePackage' dependencies = """\ # FIXME: Add dependencies if required. # depends_on('foo')""" - body = """\ - def install(self, spec, prefix): - # FIXME: Unknown build system - make() - make('install')""" + url_def = " # There is no URL since there is no code to download." + body_def = " # There is no need for install() since there is no code." - def __init__(self, name, url, versions): + def __init__(self, name, versions): self.name = name self.class_name = mod_to_class(name) - self.url = url self.versions = versions def write(self, pkg_path): @@ -98,10 +95,29 @@ class PackageTemplate(object): name=self.name, class_name=self.class_name, base_class_name=self.base_class_name, - url=self.url, + url_def=self.url_def, versions=self.versions, dependencies=self.dependencies, - body=self.body)) + body_def=self.body_def)) + + +class PackageTemplate(BundlePackageTemplate): + """Provides the default values to be used for the package file template""" + + base_class_name = 'Package' + + body_def = """\ + def install(self, spec, prefix): + # FIXME: Unknown build system + make() + make('install')""" + + url_line = """ url = \"{url}\"""" + + def __init__(self, name, url, versions): + super(PackageTemplate, self).__init__(name, versions) + + self.url_def = self.url_line.format(url=url) class AutotoolsPackageTemplate(PackageTemplate): @@ -376,6 +392,7 @@ templates = { 'autotools': AutotoolsPackageTemplate, 'autoreconf': AutoreconfPackageTemplate, 'cmake': CMakePackageTemplate, + 'bundle': BundlePackageTemplate, 'qmake': QMakePackageTemplate, 'scons': SconsPackageTemplate, 'waf': WafPackageTemplate, @@ -438,7 +455,7 @@ class BuildSystemGuesser: # Most octave extensions are hosted on Octave-Forge: # http://octave.sourceforge.net/index.html # They all have the same base URL. - if 'downloads.sourceforge.net/octave/' in url: + if url is not None and 'downloads.sourceforge.net/octave/' in url: self.build_system = 'octave' return @@ -507,15 +524,22 @@ def get_name(args): # Default package name name = 'example' - if args.name: + if args.name is not None: # Use a user-supplied name if one is present name = args.name - tty.msg("Using specified package name: '{0}'".format(name)) - elif args.url: + if len(args.name.strip()) > 0: + tty.msg("Using specified package name: '{0}'".format(name)) + else: + tty.die("A package name must be provided when using the option.") + elif args.url is not None: # Try to guess the package name based on the URL try: name = parse_name(args.url) - tty.msg("This looks like a URL for {0}".format(name)) + if name != args.url: + desc = 'URL' + else: + desc = 'package name' + tty.msg("This looks like a {0} for {1}".format(desc, name)) except UndetectableNameError: tty.die("Couldn't guess a name for this package.", " Please report this bug. In the meantime, try running:", @@ -567,21 +591,27 @@ def get_versions(args, name): BuildSystemGuesser object """ - # Default version, hash, and guesser - versions = """\ + # Default version with hash + hashed_versions = """\ # FIXME: Add proper versions and checksums here. # version('1.2.3', '0123456789abcdef0123456789abcdef')""" + # Default version without hash + unhashed_versions = """\ + # FIXME: Add proper versions here. + # version('1.2.4')""" + + # Default guesser guesser = BuildSystemGuesser() - if args.url: + if args.url is not None and args.template != 'bundle': # Find available versions try: url_dict = spack.util.web.find_versions_of_archive(args.url) except UndetectableVersionError: # Use fake versions tty.warn("Couldn't detect version in: {0}".format(args.url)) - return versions, guesser + return hashed_versions, guesser if not url_dict: # If no versions were found, revert to what the user provided @@ -591,6 +621,8 @@ def get_versions(args, name): versions = spack.util.web.get_checksums_for_versions( url_dict, name, first_stage_function=guesser, keep_stage=args.keep_stage) + else: + versions = unhashed_versions return versions, guesser @@ -610,15 +642,14 @@ def get_build_system(args, guesser): Returns: str: The name of the build system template to use """ - # Default template template = 'generic' - if args.template: + if args.template is not None: # Use a user-supplied template if one is present template = args.template tty.msg("Using specified package template: '{0}'".format(template)) - elif args.url: + elif args.url is not None: # Use whatever build system the guesser detected template = guesser.build_system if template == 'generic': @@ -681,8 +712,11 @@ def create(parser, args): build_system = get_build_system(args, guesser) # Create the package template object + constr_args = {'name': name, 'versions': versions} package_class = templates[build_system] - package = package_class(name, url, versions) + if package_class != BundlePackageTemplate: + constr_args['url'] = url + package = package_class(**constr_args) tty.msg("Created template for {0} package".format(package.name)) # Create a directory for the new package diff --git a/lib/spack/spack/cmd/info.py b/lib/spack/spack/cmd/info.py index c67de5b102..37a1aeb0a4 100644 --- a/lib/spack/spack/cmd/info.py +++ b/lib/spack/spack/cmd/info.py @@ -174,16 +174,19 @@ def print_text_info(pkg): not v.isdevelop(), v) preferred = sorted(pkg.versions, key=key_fn).pop() + url = '' + if pkg.has_code: + url = fs.for_package_version(pkg, preferred) - f = fs.for_package_version(pkg, preferred) - line = version(' {0}'.format(pad(preferred))) + color.cescape(f) + line = version(' {0}'.format(pad(preferred))) + color.cescape(url) color.cprint(line) color.cprint('') color.cprint(section_title('Safe versions: ')) for v in reversed(sorted(pkg.versions)): - f = fs.for_package_version(pkg, v) - line = version(' {0}'.format(pad(v))) + color.cescape(f) + if pkg.has_code: + url = fs.for_package_version(pkg, v) + line = version(' {0}'.format(pad(v))) + color.cescape(url) color.cprint(line) color.cprint('') @@ -193,12 +196,13 @@ def print_text_info(pkg): for line in formatter.lines: color.cprint(line) - color.cprint('') - color.cprint(section_title('Installation Phases:')) - phase_str = '' - for phase in pkg.phases: - phase_str += " {0}".format(phase) - color.cprint(phase_str) + if hasattr(pkg, 'phases') and pkg.phases: + color.cprint('') + color.cprint(section_title('Installation Phases:')) + phase_str = '' + for phase in pkg.phases: + phase_str += " {0}".format(phase) + color.cprint(phase_str) for deptype in ('build', 'link', 'run'): color.cprint('') diff --git a/lib/spack/spack/cmd/url.py b/lib/spack/spack/cmd/url.py index 604640851e..3fe4e0ac6a 100644 --- a/lib/spack/spack/cmd/url.py +++ b/lib/spack/spack/cmd/url.py @@ -258,6 +258,12 @@ def url_stats(args): for pkg in spack.repo.path.all_packages(): npkgs += 1 + if not pkg.has_code: + for _ in pkg.versions: + inc('No code', 'total') + nvers += 1 + continue + # look at each version for v, args in pkg.versions.items(): # figure out what type of fetcher it is diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index 009be89e06..d877d4a193 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -17,13 +17,14 @@ definition to modify the package, for example: The available directives are: - * ``version`` + * ``conflicts`` * ``depends_on`` - * ``provides`` * ``extends`` * ``patch`` - * ``variant`` + * ``provides`` * ``resource`` + * ``variant`` + * ``version`` """ @@ -44,7 +45,7 @@ import spack.variant from spack.dependency import Dependency, default_deptype, canonical_deptype from spack.fetch_strategy import from_kwargs from spack.resource import Resource -from spack.version import Version +from spack.version import Version, VersionChecksumError __all__ = [] @@ -138,8 +139,8 @@ class DirectiveMeta(type): cls, name, bases, attr_dict) def __init__(cls, name, bases, attr_dict): - # The class is being created: if it is a package we must ensure - # that the directives are called on the class to set it up + # The instance is being initialized: if it is a package we must ensure + # that the directives are called to set it up. if 'spack.pkg' in cls.__module__: # Ensure the presence of the dictionaries associated @@ -260,16 +261,22 @@ directive = DirectiveMeta.directive @directive('versions') def version(ver, checksum=None, **kwargs): - """Adds a version and metadata describing how to fetch its source code. + """Adds a version and, if appropriate, metadata for fetching its code. - Metadata is stored as a dict of ``kwargs`` in the package class's - ``versions`` dictionary. + The ``version`` directives are aggregated into a ``versions`` dictionary + attribute with ``Version`` keys and metadata values, where the metadata + is stored as a dictionary of ``kwargs``. - The ``dict`` of arguments is turned into a valid fetch strategy - later. See ``spack.fetch_strategy.for_package_version()``. + The ``dict`` of arguments is turned into a valid fetch strategy for + code packages later. See ``spack.fetch_strategy.for_package_version()``. """ def _execute_version(pkg): - if checksum: + if checksum is not None: + if hasattr(pkg, 'has_code') and not pkg.has_code: + raise VersionChecksumError( + "{0}: Checksums not allowed in no-code packages" + "(see '{1}' version).".format(pkg.name, ver)) + kwargs['checksum'] = checksum # Store kwargs for the package to later with a fetch_strategy. @@ -451,18 +458,23 @@ def patch(url_or_filename, level=1, when=None, working_dir=".", **kwargs): """ def _execute_patch(pkg_or_dep): + pkg = pkg_or_dep + if isinstance(pkg, Dependency): + pkg = pkg.pkg + + if hasattr(pkg, 'has_code') and not pkg.has_code: + raise UnsupportedPackageDirective( + 'Patches are not allowed in {0}: package has no code.'. + format(pkg.name)) + when_spec = make_when_spec(when) if not when_spec: return - # if this spec is identical to some other, then append this + # If this spec is identical to some other, then append this # patch to the existing list. cur_patches = pkg_or_dep.patches.setdefault(when_spec, []) - pkg = pkg_or_dep - if isinstance(pkg, Dependency): - pkg = pkg.pkg - global _patch_order_index ordering_key = (pkg.name, _patch_order_index) _patch_order_index += 1 @@ -639,3 +651,7 @@ class CircularReferenceError(DirectiveError): class DependencyPatchError(DirectiveError): """Raised for errors with patching dependencies.""" + + +class UnsupportedPackageDirective(DirectiveError): + """Raised when an invalid or unsupported package directive is specified.""" diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index 7623972d90..32239d81ce 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/lib/spack/spack/fetch_strategy.py @@ -165,6 +165,51 @@ class FetchStrategy(with_metaclass(FSMeta, object)): return cls.url_attr in args +class BundleFetchStrategy(FetchStrategy): + """ + Fetch strategy associated with bundle, or no-code, packages. + + Having a basic fetch strategy is a requirement for executing post-install + hooks. Consequently, this class provides the API but does little more + than log messages. + + TODO: Remove this class by refactoring resource handling and the link + between composite stages and composite fetch strategies (see #11981). + """ + #: This is a concrete fetch strategy for no-code packages. + enabled = True + + #: There is no associated URL keyword in ``version()`` for no-code + #: packages but this property is required for some strategy-related + #: functions (e.g., check_pkg_attributes). + url_attr = '' + + def fetch(self): + tty.msg("No code to fetch.") + return True + + def check(self): + tty.msg("No code to check.") + + def expand(self): + tty.msg("No archive to expand.") + + def reset(self): + tty.msg("No code to reset.") + + def archive(self, destination): + tty.msg("No code to archive.") + + @property + def cachable(self): + tty.msg("No code to cache.") + return False + + def source_id(self): + tty.msg("No code to be uniquely identified.") + return '' + + @pattern.composite(interface=FetchStrategy) class FetchStrategyComposite(object): """Composite for a FetchStrategy object. @@ -1117,6 +1162,12 @@ def _from_merged_attrs(fetcher, pkg, version): def for_package_version(pkg, version): """Determine a fetch strategy based on the arguments supplied to version() in the package description.""" + + # No-code packages have a custom fetch strategy to work around issues + # with resource staging. + if not pkg.has_code: + return BundleFetchStrategy() + check_pkg_attributes(pkg) if not isinstance(version, Version): @@ -1159,6 +1210,7 @@ def from_list_url(pkg): """If a package provides a URL which lists URLs for resources by version, this can can create a fetcher for a URL discovered for the specified package's version.""" + if pkg.list_url: try: versions = pkg.fetch_remote_versions() diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 5f3cfef6f0..50e483ffd6 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -137,10 +137,8 @@ class PackageMeta( spack.directives.DirectiveMeta, spack.mixins.PackageMixinsMeta ): - """Conveniently transforms attributes to permit extensible phases - - Iterates over the attribute 'phases' and creates / updates private - InstallPhase attributes in the class that is being initialized + """ + Package metaclass for supporting directives (e.g., depends_on) and phases """ phase_fmt = '_InstallPhase_{0}' @@ -148,7 +146,14 @@ class PackageMeta( _InstallPhase_run_after = {} def __new__(cls, name, bases, attr_dict): + """ + Instance creation is preceded by phase attribute transformations. + Conveniently transforms attributes to permit extensible phases by + iterating over the attribute 'phases' and creating / updating private + InstallPhase attributes in the class that will be initialized in + __init__. + """ if 'phases' in attr_dict: # Turn the strings in 'phases' into InstallPhase instances # and add them as private attributes @@ -338,11 +343,16 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): ***The Package class*** - A package defines how to fetch, verify (via, e.g., sha256), build, - and install a piece of software. A Package also defines what other - packages it depends on, so that dependencies can be installed along - with the package itself. Packages are written in pure python by - users of Spack. + At its core, a package consists of a set of software to be installed. + A package may focus on a piece of software and its associated software + dependencies or it may simply be a set, or bundle, of software. The + former requires defining how to fetch, verify (via, e.g., sha256), build, + and install that software and the packages it depends on, so that + dependencies can be installed along with the package itself. The latter, + sometimes referred to as a ``no-source`` package, requires only defining + the packages to be built. + + Packages are written in pure Python. There are two main parts of a Spack package: @@ -353,12 +363,12 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): used as input to the concretizer. 2. **Package instances**. Once instantiated, a package is - essentially an installer for a particular piece of - software. Spack calls methods like ``do_install()`` on the - ``Package`` object, and it uses those to drive user-implemented - methods like ``patch()``, ``install()``, and other build steps. - To install software, An instantiated package needs a *concrete* - spec, which guides the behavior of the various install methods. + essentially a software installer. Spack calls methods like + ``do_install()`` on the ``Package`` object, and it uses those to + drive user-implemented methods like ``patch()``, ``install()``, and + other build steps. To install software, an instantiated package + needs a *concrete* spec, which guides the behavior of the various + install methods. Packages are imported from repos (see ``repo.py``). @@ -377,12 +387,15 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): p = Package() # Done for you by spack - p.do_fetch() # downloads tarball from a URL + p.do_fetch() # downloads tarball from a URL (or VCS) p.do_stage() # expands tarball in a temp directory p.do_patch() # applies patches to expanded source p.do_install() # calls package's install() function p.do_uninstall() # removes install directory + although packages that do not have code have nothing to fetch so omit + ``p.do_fetch()``. + There are also some other commands that clean the build area: .. code-block:: python @@ -392,7 +405,7 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): # re-expands the archive. The convention used here is that a ``do_*`` function is intended to be - called internally by Spack commands (in spack.cmd). These aren't for + called internally by Spack commands (in ``spack.cmd``). These aren't for package writers to override, and doing so may break the functionality of the Package class. @@ -400,7 +413,8 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): override anything in this class. That is not usually required. For most use cases. Package creators typically just add attributes - like ``url`` and ``homepage``, or functions like ``install()``. + like ``homepage`` and, for a code-based package, ``url``, or functions + such as ``install()``. There are many custom ``Package`` subclasses in the ``spack.build_systems`` package that make things even easier for specific build systems. @@ -410,6 +424,10 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): # These are default values for instance variables. # + #: Most Spack packages are used to install source or binary code while + #: those that do not can be used to install a set of other Spack packages. + has_code = True + #: By default we build in parallel. Subclasses can override this. parallel = True @@ -482,7 +500,7 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): #: Do not include @ here in order not to unnecessarily ping the users. maintainers = [] - #: List of attributes which affect do not affect a package's content. + #: List of attributes to be excluded from a package's hash. metadata_attrs = ['homepage', 'url', 'list_url', 'extendable', 'parallel', 'make_jobs'] @@ -497,21 +515,6 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): # a binary cache. self.installed_from_binary_cache = False - # Check versions in the versions dict. - for v in self.versions: - assert (isinstance(v, Version)) - - # Check version descriptors - for v in sorted(self.versions): - assert (isinstance(self.versions[v], dict)) - - # Version-ize the keys in versions dict - try: - self.versions = dict((Version(v), h) - for v, h in self.versions.items()) - except ValueError as e: - raise ValueError("In package %s: %s" % (self.name, e.message)) - # Set a default list URL (place to find available versions) if not hasattr(self, 'list_url'): self.list_url = None @@ -1032,6 +1035,9 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): if not self.spec.concrete: raise ValueError("Can only fetch concrete packages.") + if not self.has_code: + raise InvalidPackageOpError("Can only fetch a package with a URL.") + start_time = time.time() checksum = spack.config.get('config:checksum') if checksum and self.version not in self.versions: @@ -1069,11 +1075,20 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): if not self.spec.concrete: raise ValueError("Can only stage concrete packages.") - self.do_fetch(mirror_only) # this will create the stage - self.stage.expand_archive() + # Always create the stage directory at this point. Why? A no-code + # package may want to use the installation process to install metadata. + self.stage.create() + + # Fetch/expand any associated code. + if self.has_code: + self.do_fetch(mirror_only) + self.stage.expand_archive() - if not os.listdir(self.stage.path): - raise FetchError("Archive was empty for %s" % self.name) + if not os.listdir(self.stage.path): + raise FetchError("Archive was empty for %s" % self.name) + else: + # Support for post-install hooks requires a stage.source_path + mkdirp(self.stage.source_path) def do_patch(self): """Applies patches if they haven't been applied already.""" @@ -1542,7 +1557,7 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): Package._install_bootstrap_compiler(dep.package, **kwargs) dep.package.do_install(**dep_kwargs) - # Then, install the compiler if it is not already installed. + # Then install the compiler if it is not already installed. if install_deps: Package._install_bootstrap_compiler(self, **kwargs) @@ -1674,8 +1689,8 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): # Ensure the metadata path exists as well mkdirp(spack.store.layout.metadata_path(self.spec), mode=perms) - # Fork a child to do the actual installation - # we preserve verbosity settings across installs. + # Fork a child to do the actual installation. + # Preserve verbosity settings across installs. PackageBase._verbose = spack.build_environment.fork( self, build_process, dirty=dirty, fake=fake) @@ -2357,6 +2372,19 @@ env_flags = PackageBase.env_flags build_system_flags = PackageBase.build_system_flags +class BundlePackage(PackageBase): + """General purpose bundle, or no-code, package class.""" + #: There are no phases by default but the property is required to support + #: post-install hooks (e.g., for module generation). + phases = [] + #: This attribute is used in UI queries that require to know which + #: build-system class we are using + build_system_class = 'BundlePackage' + + #: Bundle packages do not have associated source or binary code. + has_code = False + + class Package(PackageBase): """General purpose class with a single ``install`` phase that needs to be coded by packagers. @@ -2528,6 +2556,10 @@ class NoURLError(PackageError): "Package %s has no version with a URL." % cls.__name__) +class InvalidPackageOpError(PackageError): + """Raised when someone tries perform an invalid operation on a package.""" + + class ExtensionError(PackageError): """Superclass for all errors having to do with extension packages.""" diff --git a/lib/spack/spack/pkgkit.py b/lib/spack/spack/pkgkit.py index ff3288f48a..7ad7279e73 100644 --- a/lib/spack/spack/pkgkit.py +++ b/lib/spack/spack/pkgkit.py @@ -11,7 +11,9 @@ Everything in this module is automatically imported into Spack package files. import llnl.util.filesystem from llnl.util.filesystem import * -from spack.package import Package, run_before, run_after, on_package_attributes +from spack.package import \ + Package, BundlePackage, \ + run_before, run_after, on_package_attributes from spack.package import inject_flags, env_flags, build_system_flags from spack.build_systems.makefile import MakefilePackage from spack.build_systems.aspell_dict import AspellDictPackage diff --git a/lib/spack/spack/test/cmd/create.py b/lib/spack/spack/test/cmd/create.py index d3da1cce2f..030140c74e 100644 --- a/lib/spack/spack/test/cmd/create.py +++ b/lib/spack/spack/test/cmd/create.py @@ -9,29 +9,13 @@ import pytest import spack.cmd.create import spack.util.editor - +from spack.url import UndetectableNameError from spack.main import SpackCommand create = SpackCommand('create') -@pytest.fixture("module") -def cmd_create_repo(tmpdir_factory): - repo_namespace = 'cmd_create_repo' - repodir = tmpdir_factory.mktemp(repo_namespace) - repodir.ensure(spack.repo.packages_dir_name, dir=True) - yaml = repodir.join('repo.yaml') - yaml.write(""" -repo: - namespace: cmd_create_repo -""") - - repo = spack.repo.RepoPath(str(repodir)) - with spack.repo.swap(repo): - yield repo, repodir - - @pytest.fixture(scope='module') def parser(): """Returns the parser for the module""" @@ -40,18 +24,91 @@ def parser(): return prs -def test_create_template(parser, cmd_create_repo): +@pytest.mark.parametrize('args,name_index,expected', [ + (['test-package'], 0, [r'TestPackage(Package)', r'def install(self']), + (['-n', 'test-named-package', 'file://example.tar.gz'], 1, + [r'TestNamedPackage(Package)', r'def install(self']), + (['file://example.tar.gz'], -1, + [r'Example(Package)', r'def install(self']), + (['-t', 'bundle', 'test-bundle'], 2, [r'TestBundle(BundlePackage)']) +]) +def test_create_template(parser, mock_test_repo, args, name_index, expected): """Test template creation.""" - repo, repodir = cmd_create_repo + repo, repodir = mock_test_repo - name = 'test-package' - args = parser.parse_args(['--skip-editor', name]) - spack.cmd.create.create(parser, args) + constr_args = parser.parse_args(['--skip-editor'] + args) + spack.cmd.create.create(parser, constr_args) - filename = repo.filename_for_package_name(name) + pkg_name = args[name_index] if name_index > -1 else 'example' + filename = repo.filename_for_package_name(pkg_name) assert os.path.exists(filename) with open(filename, 'r') as package_file: content = ' '.join(package_file.readlines()) - for entry in [r'TestPackage(Package)', r'def install(self']: - assert content.find(entry) > -1 + for entry in expected: + assert entry in content + + +@pytest.mark.parametrize('name,expected', [ + (' ', 'name must be provided'), + ('bad#name', 'name can only contain'), +]) +def test_create_template_bad_name(parser, mock_test_repo, name, expected): + """Test template creation with bad name options.""" + constr_args = parser.parse_args(['--skip-editor', '-n', name]) + with pytest.raises(SystemExit, matches=expected): + spack.cmd.create.create(parser, constr_args) + + +def test_build_system_guesser_no_stage(parser): + """Test build system guesser when stage not provided.""" + guesser = spack.cmd.create.BuildSystemGuesser() + + # Ensure get the expected build system + with pytest.raises(AttributeError, + matches="'NoneType' object has no attribute"): + guesser(None, '/the/url/does/not/matter') + + +def test_build_system_guesser_octave(parser): + """ + Test build system guesser for the special case, where the same base URL + identifies the build system rather than guessing the build system from + files contained in the archive. + """ + url, expected = 'downloads.sourceforge.net/octave/', 'octave' + guesser = spack.cmd.create.BuildSystemGuesser() + + # Ensure get the expected build system + guesser(None, url) + assert guesser.build_system == expected + + # Also ensure get the correct template + args = parser.parse_args([url]) + bs = spack.cmd.create.get_build_system(args, guesser) + assert bs == expected + + +@pytest.mark.parametrize('url,expected', [ + ('testname', 'testname'), + ('file://example.com/archive.tar.gz', 'archive'), +]) +def test_get_name_urls(parser, url, expected): + """Test get_name with different URLs.""" + args = parser.parse_args([url]) + name = spack.cmd.create.get_name(args) + assert name == expected + + +def test_get_name_error(parser, monkeypatch): + """Test get_name UndetectableNameError exception path.""" + def _parse_name_offset(path, v): + raise UndetectableNameError(path) + + monkeypatch.setattr(spack.url, 'parse_name_offset', _parse_name_offset) + + url = 'downloads.sourceforge.net/noapp/' + args = parser.parse_args([url]) + + with pytest.raises(SystemExit, matches="Couldn't guess a name"): + spack.cmd.create.get_name(args) diff --git a/lib/spack/spack/test/cmd/info.py b/lib/spack/spack/test/cmd/info.py index 24b3fc2bd0..1928794560 100644 --- a/lib/spack/spack/test/cmd/info.py +++ b/lib/spack/spack/test/cmd/info.py @@ -41,7 +41,8 @@ def mock_print(monkeypatch, info_lines): 'trilinos', 'boost', 'python', - 'dealii' + 'dealii', + 'xsdk' # a BundlePackage ]) def test_it_just_runs(pkg): info(pkg) diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index eee0199726..b82993dd8e 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -10,7 +10,7 @@ import spack.architecture import spack.concretize import spack.repo -from spack.concretize import find_spec +from spack.concretize import find_spec, NoValidVersionError from spack.spec import Spec, CompilerSpec from spack.spec import ConflictsInSpecError, SpecError from spack.version import ver @@ -565,3 +565,9 @@ class TestConcretize(object): # Make sure the concrete spec are top-level specs with no dependents for spec in concrete_specs: assert not spec.dependents() + + @pytest.mark.parametrize('spec', ['noversion', 'noversion-bundle']) + def test_noversion_pkg(self, spec): + """Test concretization failures for no-version packages.""" + with pytest.raises(NoValidVersionError, match="no valid versions"): + Spec(spec).concretized() diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index d54f2fc732..b247d484c5 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -944,3 +944,53 @@ def invalid_spec(request): """Specs that do not parse cleanly due to invalid formatting. """ return request.param + + +@pytest.fixture("module") +def mock_test_repo(tmpdir_factory): + """Create an empty repository.""" + repo_namespace = 'mock_test_repo' + repodir = tmpdir_factory.mktemp(repo_namespace) + repodir.ensure(spack.repo.packages_dir_name, dir=True) + yaml = repodir.join('repo.yaml') + yaml.write(""" +repo: + namespace: mock_test_repo +""") + + repo = spack.repo.RepoPath(str(repodir)) + with spack.repo.swap(repo): + yield repo, repodir + + shutil.rmtree(str(repodir)) + + +########## +# Class and fixture to work around problems raising exceptions in directives, +# which cause tests like test_from_list_url to hang for Python 2.x metaclass +# processing. +# +# At this point only version and patch directive handling has been addressed. +########## + +class MockBundle(object): + has_code = False + name = 'mock-bundle' + versions = {} + + +@pytest.fixture +def mock_directive_bundle(): + """Return a mock bundle package for directive tests.""" + return MockBundle() + + +@pytest.fixture +def clear_directive_functions(): + """Clear all overidden directive functions for subsequent tests.""" + yield + + # Make sure any directive functions overidden by tests are cleared before + # proceeding with subsequent tests that may depend on the original + # functions. + spack.directives.DirectiveMeta._directives_to_be_executed = [] diff --git a/lib/spack/spack/test/install.py b/lib/spack/spack/test/install.py index 85e1c5050c..e0a83f20b2 100644 --- a/lib/spack/spack/test/install.py +++ b/lib/spack/spack/test/install.py @@ -9,7 +9,8 @@ import shutil from llnl.util.filesystem import mkdirp, touch, working_dir -from spack.package import InstallError, PackageBase, PackageStillNeededError +from spack.package import \ + InstallError, InvalidPackageOpError, PackageBase, PackageStillNeededError import spack.patch import spack.repo import spack.store @@ -326,6 +327,38 @@ def test_uninstall_by_spec_errors(mutable_database): PackageBase.uninstall_by_spec(rec.spec) +def test_nosource_pkg_install(install_mockery, mock_fetch, mock_packages): + """Test install phases with the nosource package.""" + spec = Spec('nosource').concretized() + pkg = spec.package + + # Make sure install works even though there is no associated code. + pkg.do_install() + + # Also make sure an error is raised if `do_fetch` is called. + with pytest.raises(InvalidPackageOpError, + match="fetch a package with a URL"): + pkg.do_fetch() + + +def test_nosource_pkg_install_post_install( + install_mockery, mock_fetch, mock_packages): + """Test install phases with the nosource package with post-install.""" + spec = Spec('nosource-install').concretized() + pkg = spec.package + + # Make sure both the install and post-install package methods work. + pkg.do_install() + + # Ensure the file created in the package's `install` method exists. + install_txt = os.path.join(spec.prefix, 'install.txt') + assert os.path.isfile(install_txt) + + # Ensure the file created in the package's `post-install` method exists. + post_install_txt = os.path.join(spec.prefix, 'post-install.txt') + assert os.path.isfile(post_install_txt) + + def test_pkg_build_paths(install_mockery): # Get a basic concrete spec for the trivial install package. spec = Spec('trivial-install-test-package').concretized() diff --git a/lib/spack/spack/test/package_sanity.py b/lib/spack/spack/test/package_sanity.py index ed103c8d74..e1a16e80af 100644 --- a/lib/spack/spack/test/package_sanity.py +++ b/lib/spack/spack/test/package_sanity.py @@ -56,7 +56,7 @@ def test_all_virtual_packages_have_default_providers(): def test_package_version_consistency(): - """Make sure all versions on builtin packages can produce a fetcher.""" + """Make sure all versions on builtin packages produce a fetcher.""" for name in spack.repo.all_package_names(): pkg = spack.repo.get(name) spack.fetch_strategy.check_pkg_attributes(pkg) diff --git a/lib/spack/spack/test/packages.py b/lib/spack/spack/test/packages.py index 145c8fb2aa..186b0d0007 100644 --- a/lib/spack/spack/test/packages.py +++ b/lib/spack/spack/test/packages.py @@ -12,6 +12,8 @@ from spack.paths import mock_packages_path from spack.util.naming import mod_to_class from spack.spec import Spec from spack.util.package_hash import package_content +from spack.version import VersionChecksumError +import spack.directives @pytest.mark.usefixtures('config', 'mock_packages') @@ -369,3 +371,20 @@ def test_rpath_args(mutable_database): rpath_args = rec.spec.package.rpath_args assert '-rpath' in rpath_args assert 'mpich' in rpath_args + + +def test_bundle_version_checksum(mock_directive_bundle, + clear_directive_functions): + """Test raising exception on a version checksum with a bundle package.""" + with pytest.raises(VersionChecksumError, match="Checksums not allowed"): + version = spack.directives.version('1.0', checksum='1badpkg') + version(mock_directive_bundle) + + +def test_bundle_patch_directive(mock_directive_bundle, + clear_directive_functions): + """Test raising exception on a patch directive with a bundle package.""" + with pytest.raises(spack.directives.UnsupportedPackageDirective, + match="Patches are not allowed"): + patch = spack.directives.patch('mock/patch.txt') + patch(mock_directive_bundle) diff --git a/lib/spack/spack/test/url_fetch.py b/lib/spack/spack/test/url_fetch.py index cc9560350a..c43005ff62 100644 --- a/lib/spack/spack/test/url_fetch.py +++ b/lib/spack/spack/test/url_fetch.py @@ -85,67 +85,46 @@ def test_fetch( assert 'echo Building...' in contents -def test_from_list_url(mock_packages, config): - pkg = spack.repo.get('url-list-test') - - # These URLs are all in the url-list-test package and should have - # checksums taken from the package. - spec = Spec('url-list-test @0.0.0').concretized() - pkg = spack.repo.get(spec) - fetch_strategy = from_list_url(pkg) - assert isinstance(fetch_strategy, URLFetchStrategy) - assert os.path.basename(fetch_strategy.url) == 'foo-0.0.0.tar.gz' - assert fetch_strategy.digest == 'abc000' - - spec = Spec('url-list-test @1.0.0').concretized() - pkg = spack.repo.get(spec) +@pytest.mark.parametrize('spec,url,digest', [ + ('url-list-test @0.0.0', 'foo-0.0.0.tar.gz', 'abc000'), + ('url-list-test @1.0.0', 'foo-1.0.0.tar.gz', 'abc100'), + ('url-list-test @3.0', 'foo-3.0.tar.gz', 'abc30'), + ('url-list-test @4.5', 'foo-4.5.tar.gz', 'abc45'), + ('url-list-test @2.0.0b2', 'foo-2.0.0b2.tar.gz', 'abc200b2'), + ('url-list-test @3.0a1', 'foo-3.0a1.tar.gz', 'abc30a1'), + ('url-list-test @4.5-rc5', 'foo-4.5-rc5.tar.gz', 'abc45rc5'), +]) +def test_from_list_url(mock_packages, config, spec, url, digest): + """ + Test URLs in the url-list-test package, which means they should + have checksums in the package. + """ + specification = Spec(spec).concretized() + pkg = spack.repo.get(specification) fetch_strategy = from_list_url(pkg) assert isinstance(fetch_strategy, URLFetchStrategy) - assert os.path.basename(fetch_strategy.url) == 'foo-1.0.0.tar.gz' - assert fetch_strategy.digest == 'abc100' + assert os.path.basename(fetch_strategy.url) == url + assert fetch_strategy.digest == digest - spec = Spec('url-list-test @3.0').concretized() - pkg = spack.repo.get(spec) - fetch_strategy = from_list_url(pkg) - assert isinstance(fetch_strategy, URLFetchStrategy) - assert os.path.basename(fetch_strategy.url) == 'foo-3.0.tar.gz' - assert fetch_strategy.digest == 'abc30' - spec = Spec('url-list-test @4.5').concretized() - pkg = spack.repo.get(spec) - fetch_strategy = from_list_url(pkg) - assert isinstance(fetch_strategy, URLFetchStrategy) - assert os.path.basename(fetch_strategy.url) == 'foo-4.5.tar.gz' - assert fetch_strategy.digest == 'abc45' +def test_from_list_url_unspecified(mock_packages, config): + """Test non-specific URLs from the url-list-test package.""" + pkg = spack.repo.get('url-list-test') - spec = Spec('url-list-test @2.0.0b2').concretized() + spec = Spec('url-list-test @2.0.0').concretized() pkg = spack.repo.get(spec) fetch_strategy = from_list_url(pkg) assert isinstance(fetch_strategy, URLFetchStrategy) - assert os.path.basename(fetch_strategy.url) == 'foo-2.0.0b2.tar.gz' - assert fetch_strategy.digest == 'abc200b2' + assert os.path.basename(fetch_strategy.url) == 'foo-2.0.0.tar.gz' + assert fetch_strategy.digest is None - spec = Spec('url-list-test @3.0a1').concretized() - pkg = spack.repo.get(spec) - fetch_strategy = from_list_url(pkg) - assert isinstance(fetch_strategy, URLFetchStrategy) - assert os.path.basename(fetch_strategy.url) == 'foo-3.0a1.tar.gz' - assert fetch_strategy.digest == 'abc30a1' - spec = Spec('url-list-test @4.5-rc5').concretized() - pkg = spack.repo.get(spec) - fetch_strategy = from_list_url(pkg) - assert isinstance(fetch_strategy, URLFetchStrategy) - assert os.path.basename(fetch_strategy.url) == 'foo-4.5-rc5.tar.gz' - assert fetch_strategy.digest == 'abc45rc5' +def test_nosource_from_list_url(mock_packages, config): + """This test confirms BundlePackages do not have list url.""" + pkg = spack.repo.get('nosource') - # this one is not in the url-list-test package. - spec = Spec('url-list-test @2.0.0').concretized() - pkg = spack.repo.get(spec) fetch_strategy = from_list_url(pkg) - assert isinstance(fetch_strategy, URLFetchStrategy) - assert os.path.basename(fetch_strategy.url) == 'foo-2.0.0.tar.gz' - assert fetch_strategy.digest is None + assert fetch_strategy is None def test_hash_detection(checksum_type): diff --git a/lib/spack/spack/version.py b/lib/spack/spack/version.py index 286943119f..0bb5a767e7 100644 --- a/lib/spack/spack/version.py +++ b/lib/spack/spack/version.py @@ -30,6 +30,7 @@ from bisect import bisect_left from functools import wraps from six import string_types +import spack.error from spack.util.spack_yaml import syaml_dict @@ -848,3 +849,11 @@ def ver(obj): return obj else: raise TypeError("ver() can't convert %s to version!" % type(obj)) + + +class VersionError(spack.error.SpackError): + """This is raised when something is wrong with a version.""" + + +class VersionChecksumError(VersionError): + """Raised for version checksum errors.""" -- cgit v1.2.3-70-g09d2