Allow per-version URLs instead of one single URL per package.

11 files changed, 189 insertions, 81 deletions

diff --git a/lib/spack/llnl/util/lang.py b/lib/spack/llnl/util/lang.py
index 7590fb1298..ce7d0197f0 100644
--- a/lib/spack/llnl/util/lang.py
+++ b/lib/spack/llnl/util/lang.py
@@ -119,9 +119,8 @@ def caller_locals():
 
 
 def get_calling_package_name():
-    """Make sure that the caller is a class definition, and return
-       the module's name.  This is useful for getting the name of
-       spack packages from inside a relation function.
+    """Make sure that the caller is a class definition, and return the
+       module's name.
     """
     stack = inspect.stack()
     try:
@@ -144,8 +143,9 @@ def get_calling_package_name():
 def attr_required(obj, attr_name):
     """Ensure that a class has a required attribute."""
     if not hasattr(obj, attr_name):
-        tty.die("No required attribute '%s' in class '%s'"
-                % (attr_name, obj.__class__.__name__))
+        raise RequiredAttributeError(
+            "No required attribute '%s' in class '%s'"
+            % (attr_name, obj.__class__.__name__))
 
 
 def attr_setdefault(obj, name, value):
@@ -259,3 +259,8 @@ def in_function(function_name):
         return False
     finally:
         del stack
+
+
+class RequiredAttributeError(ValueError):
+    def __init__(self, message):
+        super(RequiredAttributeError, self).__init__(message)
diff --git a/lib/spack/spack/__init__.py b/lib/spack/spack/__init__.py
index d0cf8804ba..50fe453cfb 100644
--- a/lib/spack/spack/__init__.py
+++ b/lib/spack/spack/__init__.py
@@ -32,7 +32,7 @@
 # TODO: maybe this should be separated out and should go in build_environment.py?
 # TODO: it's not clear where all the stuff that needs to be included in packages
 #       should live.  This file is overloaded for spack core vs. for packages.
-__all__ = ['Package', 'when', 'provides', 'depends_on',
+__all__ = ['Package', 'when', 'provides', 'depends_on', 'version',
            'patch', 'Version', 'working_dir', 'which', 'Executable',
            'filter_file', 'change_sed_delimiter']
 
@@ -146,6 +146,6 @@ sys_type = None
 #
 from llnl.util.filesystem import working_dir
 from spack.package import Package
-from spack.relations import depends_on, provides, patch
+from spack.relations import *
 from spack.multimethod import when
 from spack.version import Version
diff --git a/lib/spack/spack/cmd/create.py b/lib/spack/spack/cmd/create.py
index cc9a1342e7..1a1a19a4b6 100644
--- a/lib/spack/spack/cmd/create.py
+++ b/lib/spack/spack/cmd/create.py
@@ -70,7 +70,7 @@ class ${class_name}(Package):
     homepage = "http://www.example.com"
     url      = "${url}"
 
-    versions = ${versions}
+${versions}
 
     def install(self, spec, prefix):
         # FIXME: Modify the configure line to suit your build system here.
@@ -114,13 +114,11 @@ class ConfigureGuesser(object):
             self.configure = '%s\n        # %s' % (autotools, cmake)
 
 
-def make_version_dict(ver_hash_tuples):
-    max_len = max(len(str(v)) for v,hfg in ver_hash_tuples)
-    width = max_len + 2
-    format = "%-" + str(width) + "s : '%s',"
-    sep = '\n                 '
-    return '{ ' + sep.join(format % ("'%s'" % v, h)
-                           for v, h in ver_hash_tuples) + ' }'
+def make_version_calls(ver_hash_tuples):
+    """Adds a version() call to the package for each version found."""
+    max_len = max(len(str(v)) for v, h in ver_hash_tuples)
+    format = "    version(%%-%ds, '%%s')" % (max_len + 2)
+    return '\n'.join(format % ("'%s'" % v, h) for v, h in ver_hash_tuples)
 
 
 def get_name():
@@ -195,7 +193,7 @@ def create(parser, args):
                 configure=guesser.configure,
                 class_name=mod_to_class(name),
                 url=url,
-                versions=make_version_dict(ver_hash_tuples)))
+                versions=make_version_calls(ver_hash_tuples)))
 
     # If everything checks out, go ahead and edit.
     spack.editor(pkg_path)
diff --git a/lib/spack/spack/cmd/edit.py b/lib/spack/spack/cmd/edit.py
index c96cf75c9b..3647186a3c 100644
--- a/lib/spack/spack/cmd/edit.py
+++ b/lib/spack/spack/cmd/edit.py
@@ -44,7 +44,7 @@ class ${class_name}(Package):
     homepage = "http://www.example.com"
     url      = "http://www.example.com/${name}-1.0.tar.gz"
 
-    versions = { '1.0' : '0123456789abcdef0123456789abcdef' }
+    version('1.0', '0123456789abcdef0123456789abcdef')
 
     def install(self, spec, prefix):
         configure("--prefix=%s" % prefix)
diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py
index f5775ef1bf..eb497711b7 100644
--- a/lib/spack/spack/concretize.py
+++ b/lib/spack/spack/concretize.py
@@ -72,7 +72,7 @@ class DefaultConcretizer(object):
         if valid_versions:
             spec.versions = ver([valid_versions[-1]])
         else:
-            spec.versions = ver([pkg.default_version])
+            raise NoValidVerionError(spec)
 
 
     def concretize_architecture(self, spec):
@@ -158,3 +158,11 @@ class UnavailableCompilerVersionError(spack.error.SpackError):
         super(UnavailableCompilerVersionError, self).__init__(
             "No available compiler version matches '%s'" % compiler_spec,
             "Run 'spack compilers' to see available compiler Options.")
+
+
+class NoValidVerionError(spack.error.SpackError):
+    """Raised when there is no available version for a package that
+       satisfies a spec."""
+    def __init__(self, spec):
+        super(NoValidVerionError, self).__init__(
+            "No available version of %s matches '%s'" % (spec.name, spec.versions))
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 79a6c2362e..90e77b5e82 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -296,9 +296,12 @@ class Package(object):
     """
 
     #
-    # These variables are defaults for the various relations defined on
-    # packages.  Subclasses will have their own versions of these.
+    # These variables are defaults for the various "relations".
     #
+    """Map of information about Versions of this package.
+       Map goes: Version -> VersionDescriptor"""
+    versions = {}
+
     """Specs of dependency packages, keyed by name."""
     dependencies = {}
 
@@ -317,16 +320,10 @@ class Package(object):
     """By default we build in parallel.  Subclasses can override this."""
     parallel = True
 
-    """Dirty hack for forcing packages with uninterpretable URLs
-       TODO: get rid of this.
-    """
-    force_url = False
-
 
     def __init__(self, spec):
         # These attributes are required for all packages.
         attr_required(self.__class__, 'homepage')
-        attr_required(self.__class__, 'url')
 
         # this determines how the package should be built.
         self.spec = spec
@@ -337,24 +334,32 @@ class Package(object):
         if '.' in self.name:
             self.name = self.name[self.name.rindex('.') + 1:]
 
-        # Make sure URL is an allowed type
-        validate_package_url(self.url)
-
-        # patch up the URL with a new version if the spec version is concrete
-        if self.spec.versions.concrete:
-            self.url = self.url_for_version(self.spec.version)
-
         # This is set by scraping a web page.
         self._available_versions = None
 
-        # versions should be a dict from version to checksum, for safe versions
-        # of this package.  If it's not present, make it an empty dict.
-        if not hasattr(self, 'versions'):
-            self.versions = {}
-
-        if not isinstance(self.versions, dict):
-            raise ValueError("versions attribute of package %s must be a dict!"
-                             % self.name)
+        # Sanity check some required variables that could be
+        # overridden by package authors.
+        def sanity_check_dict(attr_name):
+            if not hasattr(self, attr_name):
+                raise PackageError("Package %s must define %s" % attr_name)
+
+            attr = getattr(self, attr_name)
+            if not isinstance(attr, dict):
+                raise PackageError("Package %s has non-dict %s attribute!"
+                                   % (self.name, attr_name))
+        sanity_check_dict('versions')
+        sanity_check_dict('dependencies')
+        sanity_check_dict('conflicted')
+        sanity_check_dict('patches')
+
+        # Check versions in the versions dict.
+        for v in self.versions:
+            assert(isinstance(v, Version))
+
+        # Check version descriptors
+        for v in sorted(self.versions):
+            vdesc = self.versions[v]
+            assert(isinstance(vdesc, spack.relations.VersionDescriptor))
 
         # Version-ize the keys in versions dict
         try:
@@ -366,6 +371,10 @@ class Package(object):
         # stage used to build this package.
         self._stage = None
 
+        # patch up self.url based on the actual version
+        if self.spec.concrete:
+            self.url = self.url_for_version(self.version)
+
         # Set a default list URL (place to find available versions)
         if not hasattr(self, 'list_url'):
             self.list_url = None
@@ -375,18 +384,6 @@ class Package(object):
 
 
     @property
-    def default_version(self):
-        """Get the version in the default URL for this package,
-           or fails."""
-        try:
-            return url.parse_version(self.__class__.url)
-        except UndetectableVersionError:
-            raise PackageError(
-                "Couldn't extract a default version from %s." % self.url,
-                " You must specify it explicitly in the package file.")
-
-
-    @property
     def version(self):
         if not self.spec.concrete:
             raise ValueError("Can only get version of concrete package.")
@@ -514,16 +511,50 @@ class Package(object):
            override this, e.g. for boost versions where you need to ensure that there
            are _'s in the download URL.
         """
-        if self.force_url:
-            return self.default_version
         return str(version)
 
 
     def url_for_version(self, version):
-        """Gives a URL that you can download a new version of this package from."""
-        if self.force_url:
-            return self.url
-        return url.substitute_version(self.__class__.url, self.url_version(version))
+        """Returns a URL that you can download a new version of this package from."""
+        if not isinstance(version, Version):
+            version = Version(version)
+
+        def nearest_url(version):
+            """Finds the URL for the next lowest version with a URL.
+               If there is no lower version with a URL, uses the
+               package url property. If that isn't there, uses a
+               *higher* URL, and if that isn't there raises an error.
+            """
+            url = getattr(self, 'url', None)
+            for v in sorted(self.versions):
+                if v > version and url:
+                    break
+                if self.versions[v].url:
+                    url = self.versions[v].url
+            if not url:
+                raise PackageVersionError(v)
+            return url
+
+        if version in self.versions:
+            vdesc = self.versions[version]
+            if not vdesc.url:
+                base_url = nearest_url(version)
+                vdesc.url = url.substitute_version(
+                    base_url, self.url_version(version))
+            return vdesc.url
+        else:
+            return nearest_url(version)
+
+
+    @property
+    def default_url(self):
+        if self.concrete:
+            return self.url_for_version(self.version)
+        else:
+            url = getattr(self, 'url', None)
+            if url:
+                return url
+
 
 
     def remove_prefix(self):
@@ -548,7 +579,7 @@ class Package(object):
         self.stage.fetch()
 
         if spack.do_checksum and self.version in self.versions:
-            digest = self.versions[self.version]
+            digest = self.versions[self.version].checksum
             self.stage.check(digest)
             tty.msg("Checksum passed for %s@%s" % (self.name, self.version))
 
@@ -779,6 +810,9 @@ class Package(object):
 
 
     def fetch_available_versions(self):
+        if not hasattr(self, 'url'):
+            raise VersionFetchError(self.__class__)
+
         # If not, then try to fetch using list_url
         if not self._available_versions:
             try:
@@ -865,7 +899,6 @@ def print_pkg(message):
     print message
 
 
-
 class FetchError(spack.error.SpackError):
     """Raised when something goes wrong during fetch."""
     def __init__(self, message, long_msg=None):
@@ -889,3 +922,19 @@ class InvalidPackageDependencyError(PackageError):
        its dependencies."""
     def __init__(self, message):
         super(InvalidPackageDependencyError, self).__init__(message)
+
+
+class PackageVersionError(PackageError):
+    """Raised when a version URL cannot automatically be determined."""
+    def __init__(self, version):
+        super(PackageVersionError, self).__init__(
+            "Cannot determine a URL automatically for version %s." % version,
+            "Please provide a url for this version in the package.py file.")
+
+
+class VersionFetchError(PackageError):
+    """Raised when a version URL cannot automatically be determined."""
+    def __init__(self, cls):
+        super(VersionFetchError, self).__init__(
+            "Cannot fetch version for package %s " % cls.__name__ +
+            "because it does not define a default url.")
diff --git a/lib/spack/spack/relations.py b/lib/spack/spack/relations.py
index f46b7dfc84..a7b46cfb33 100644
--- a/lib/spack/spack/relations.py
+++ b/lib/spack/spack/relations.py
@@ -68,6 +68,8 @@ provides
         spack install mpileaks ^mvapich
         spack install mpileaks ^mpich
 """
+__all__ = [ 'depends_on', 'provides', 'patch', 'version' ]
+
 import re
 import inspect
 import importlib
@@ -77,14 +79,38 @@ from llnl.util.lang import *
 import spack
 import spack.spec
 import spack.error
+import spack.url
 
+from spack.version import Version
 from spack.patch import Patch
 from spack.spec import Spec, parse_anonymous_spec
 
 
-"""Adds a dependencies local variable in the locals of
-   the calling class, based on args. """
+class VersionDescriptor(object):
+    """A VersionDescriptor contains information to describe a
+       particular version of a package.  That currently includes a URL
+       for the version along with a checksum."""
+    def __init__(self, checksum, url):
+        self.checksum = checksum
+        self.url = url
+
+
+def version(ver, checksum, **kwargs):
+    """Adds a version and associated metadata to the package."""
+    pkg = caller_locals()
+
+    versions = pkg.setdefault('versions', {})
+    patches  = pkg.setdefault('patches', {})
+
+    ver = Version(ver)
+    url = kwargs.get('url', None)
+
+    versions[ver] = VersionDescriptor(checksum, url)
+
+
 def depends_on(*specs):
+    """Adds a dependencies local variable in the locals of
+       the calling class, based on args. """
     pkg = get_calling_package_name()
 
     dependencies = caller_locals().setdefault('dependencies', {})
diff --git a/lib/spack/spack/test/package_sanity.py b/lib/spack/spack/test/package_sanity.py
index 1a7bc5dc5e..e3de695070 100644
--- a/lib/spack/spack/test/package_sanity.py
+++ b/lib/spack/spack/test/package_sanity.py
@@ -29,19 +29,35 @@ import unittest
 
 import spack
 import spack.url as url
+from spack.packages import PackageDB
+
 
 class PackageSanityTest(unittest.TestCase):
 
-    def test_get_all_packages(self):
-        """Get all packages once and make sure that works."""
+    def check_db(self):
+        """Get all packages in a DB to make sure they work."""
         for name in spack.db.all_package_names():
             spack.db.get(name)
 
 
+    def test_get_all_packages(self):
+        """Get all packages once and make sure that works."""
+        self.check_db()
+
+
+    def test_get_all_mock_packages(self):
+        """Get the mock packages once each too."""
+        tmp = spack.db
+        spack.db = PackageDB(spack.mock_packages_path)
+        self.check_db()
+        spack.db = tmp
+
+
     def test_url_versions(self):
-        """Ensure that url_for_version does the right thing for at least the
-           default version of each package.
-        """
+        """Check URLs for regular packages, if they are explicitly defined."""
         for pkg in spack.db.all_packages():
-            v = url.parse_version(pkg.url)
-            self.assertEqual(pkg.url, pkg.url_for_version(v))
+            for v, vdesc in pkg.versions.items():
+                if vdesc.url:
+                    # If there is a url for the version check it.
+                    v_url = pkg.url_for_version(v)
+                    self.assertEqual(vdesc.url, v_url)
diff --git a/lib/spack/spack/url.py b/lib/spack/spack/url.py
index 1b8120168f..902ce9817d 100644
--- a/lib/spack/spack/url.py
+++ b/lib/spack/spack/url.py
@@ -82,12 +82,16 @@ def parse_version_string_with_indices(path):
     """Try to extract a version string from a filename or URL.  This is taken
        largely from Homebrew's Version class."""
 
-    if os.path.isdir(path):
-        stem = os.path.basename(path)
-    elif re.search(r'((?:sourceforge.net|sf.net)/.*)/download$', path):
-        stem = comp.stem(os.path.dirname(path))
-    else:
-        stem = comp.stem(path)
+    # Strip off sourceforge download stuffix.
+    if re.search(r'((?:sourceforge.net|sf.net)/.*)/download$', path):
+        path = os.path.dirname(path)
+
+    # Strip archive extension
+    path = comp.strip_extension(path)
+
+    # Take basename to avoid including parent dirs in version name
+    # Remember the offset of the stem in the full path.
+    stem = os.path.basename(path)
 
     version_types = [
         # GitHub tarballs, e.g. v1.2.3
@@ -137,10 +141,10 @@ def parse_version_string_with_indices(path):
         (r'_((\d+\.)+\d+[a-z]?)[.]orig$', stem),
 
         # e.g. http://www.openssl.org/source/openssl-0.9.8s.tar.gz
-        (r'-([^-]+)', stem),
+        (r'-([^-]+(-alpha|-beta)?)', stem),
 
         # e.g. astyle_1.23_macosx.tar.gz
-        (r'_([^_]+)', stem),
+        (r'_([^_]+(_alpha|_beta)?)', stem),
 
         # e.g. http://mirrors.jenkins-ci.org/war/1.486/jenkins.war
         (r'\/(\d\.\d+)\/', path),
@@ -152,7 +156,9 @@ def parse_version_string_with_indices(path):
         regex, match_string = vtype[:2]
         match = re.search(regex, match_string)
         if match and match.group(1) is not None:
-            return match.group(1), match.start(1), match.end(1)
+            version = match.group(1)
+            start = path.index(version)
+            return version, start, start+len(version)
 
     raise UndetectableVersionError(path)
 
diff --git a/lib/spack/spack/util/compression.py b/lib/spack/spack/util/compression.py
index 7ce8e8c65b..a67576bd50 100644
--- a/lib/spack/spack/util/compression.py
+++ b/lib/spack/spack/util/compression.py
@@ -48,7 +48,7 @@ def decompressor_for(path):
     return tar
 
 
-def stem(path):
+def strip_extension(path):
     """Get the part of a path that does not include its compressed
        type extension."""
     for type in ALLOWED_ARCHIVE_TYPES:
diff --git a/lib/spack/spack/version.py b/lib/spack/spack/version.py
index ce94303a9c..4558f88384 100644
--- a/lib/spack/spack/version.py
+++ b/lib/spack/spack/version.py
@@ -181,7 +181,7 @@ class Version(object):
 
         # Add possible alpha or beta indicator at the end of each segemnt
         # We treat these specially b/c they're so common.
-        wc += '[ab]?)?' * (len(segments) - 1)
+        wc += '(?:[a-z]|alpha|beta)?)?' * (len(segments) - 1)
         return wc