Fix create, diy, edit, and repo commands to use multiple repos.

6 files changed, 247 insertions, 92 deletions

diff --git a/lib/spack/spack/cmd/create.py b/lib/spack/spack/cmd/create.py
index 1a60875de8..7cea39cb55 100644
--- a/lib/spack/spack/cmd/create.py
+++ b/lib/spack/spack/cmd/create.py
@@ -36,7 +36,9 @@ import spack.cmd
 import spack.cmd.checksum
 import spack.url
 import spack.util.web
+from spack.spec import Spec
 from spack.util.naming import *
+from spack.repository import Repo, RepoError
 import spack.util.crypto as crypto
 
 from spack.util.executable import which
@@ -85,21 +87,34 @@ ${versions}
 """)
 
 
+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 setup_parser(subparser):
     subparser.add_argument('url', nargs='?', help="url of package archive")
     subparser.add_argument(
-        '--keep-stage', action='store_true', dest='keep_stage',
+        '--keep-stage', action='store_true',
         help="Don't clean up staging area when command completes.")
     subparser.add_argument(
-        '-n', '--name', dest='alternate_name', default=None,
+        '-n', '--name', dest='alternate_name', default=None, metavar='NAME',
         help="Override the autodetected name for the created package.")
     subparser.add_argument(
-        '-p', '--package-repo', dest='package_repo', default=None,
-        help="Create the package in the specified packagerepo.")
+        '-r', '--repo', default=None,
+        help="Path to a repository where the package should be created.")
+    subparser.add_argument(
+        '-N', '--namespace',
+        help="Specify a namespace for the package. Must be the namespace of "
+        "a repository registered with Spack.")
     subparser.add_argument(
         '-f', '--force', action='store_true', dest='force',
         help="Overwrite any existing package file with the same name.")
 
+    setup_parser.subparser = subparser
+
 
 class ConfigureGuesser(object):
     def __call__(self, stage):
@@ -137,16 +152,7 @@ class ConfigureGuesser(object):
         self.build_system = build_system
 
 
-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 create(parser, args):
-    url = args.url
-
+def guess_name_and_version(url, args):
     # Try to deduce name and version of the new package from the URL
     version = spack.url.parse_version(url)
     if not version:
@@ -163,21 +169,52 @@ def create(parser, args):
             tty.die("Couldn't guess a name for this package. Try running:", "",
                     "spack create --name <name> <url>")
 
-    package_repo = args.package_repo
-
-    if not valid_module_name(name):
+    if not valid_fully_qualified_module_name(name):
         tty.die("Package name can only contain A-Z, a-z, 0-9, '_' and '-'")
 
-    tty.msg("This looks like a URL for %s version %s." % (name, version))
-    tty.msg("Creating template for package %s" % name)
+    return name, version
 
-    # Create a directory for the new package.
-    pkg_path = spack.repo.filename_for_package_name(name, package_repo)
-    if os.path.exists(pkg_path) and not args.force:
-        tty.die("%s already exists." % pkg_path)
+
+def find_repository(spec, args):
+    # figure out namespace for spec
+    if spec.namespace and args.namespace and spec.namespace != args.namespace:
+        tty.die("Namespaces '%s' and '%s' do not match." % (spec.namespace, args.namespace))
+
+    if not spec.namespace and args.namespace:
+        spec.namespace = args.namespace
+
+    # Figure out where the new package should live.
+    repo_path = args.repo
+    if repo_path is not None:
+        try:
+            repo = Repo(repo_path)
+            if spec.namespace and spec.namespace != repo.namespace:
+                tty.die("Can't create package with namespace %s in repo with namespace %s."
+                        % (spec.namespace, repo.namespace))
+        except RepoError as e:
+            tty.die(str(e))
     else:
-        mkdirp(os.path.dirname(pkg_path))
+        if spec.namespace:
+            repo = spack.repo.get_repo(spec.namespace, None)
+            if not repo:
+                tty.die("Unknown namespace: %s" % spec.namespace)
+        else:
+            repo = spack.repo.first_repo()
+
+    # Set the namespace on the spec if it's not there already
+    if not spec.namespace:
+        spec.namespace = repo.namespace
+
+    return repo
+
+
+def fetch_tarballs(url, name, args):
+    """Try to find versions of the supplied archive by scraping the web.
+
+    Prompts the user to select how many to download if many are found.
 
+
+    """
     versions = spack.util.web.find_versions_of_archive(url)
     rkeys = sorted(versions.keys(), reverse=True)
     versions = OrderedDict(zip(rkeys, (versions[v] for v in rkeys)))
@@ -196,13 +233,35 @@ def create(parser, args):
             default=5, abort='q')
 
         if not archives_to_fetch:
-            tty.msg("Aborted.")
-            return
+            tty.die("Aborted.")
+
+    sorted_versions = sorted(versions.keys(), reverse=True)
+    sorted_urls = [versions[v] for v in sorted_versions]
+    return sorted_versions[:archives_to_fetch], sorted_urls[:archives_to_fetch]
+
+
+def create(parser, args):
+    url = args.url
+    if not url:
+        setup_parser.subparser.print_help()
+        return
+
+    # Figure out a name and repo for the package.
+    name, version = guess_name_and_version(url, args)
+    spec = Spec(name)
+    name = spec.name  # factors out namespace, if any
+    repo = find_repository(spec, args)
+
+    tty.msg("This looks like a URL for %s version %s." % (name, version))
+    tty.msg("Creating template for package %s" % name)
+
+    # Fetch tarballs (prompting user if necessary)
+    versions, urls = fetch_tarballs(url, name, args)
 
+    # Try to guess what configure system is used.
     guesser = ConfigureGuesser()
     ver_hash_tuples = spack.cmd.checksum.get_checksums(
-        versions.keys()[:archives_to_fetch],
-        [versions[v] for v in versions.keys()[:archives_to_fetch]],
+        versions, urls,
         first_stage_function=guesser,
         keep_stage=args.keep_stage)
 
@@ -214,7 +273,7 @@ def create(parser, args):
         name = 'py-%s' % name
 
     # Create a directory for the new package.
-    pkg_path = spack.repo.filename_for_package_name(name)
+    pkg_path = repo.filename_for_package_name(name)
     if os.path.exists(pkg_path) and not args.force:
         tty.die("%s already exists." % pkg_path)
     else:
diff --git a/lib/spack/spack/cmd/diy.py b/lib/spack/spack/cmd/diy.py
index 1acbebbc15..9df53312f8 100644
--- a/lib/spack/spack/cmd/diy.py
+++ b/lib/spack/spack/cmd/diy.py
@@ -69,7 +69,7 @@ def diy(self, args):
                 sys.exit(1)
             else:
                 tty.msg("Running 'spack edit -f %s'" % spec.name)
-                edit_package(spec.name, True)
+                edit_package(spec.name, spack.repo.first_repo(), None, True)
                 return
 
         if not spec.version.concrete:
diff --git a/lib/spack/spack/cmd/edit.py b/lib/spack/spack/cmd/edit.py
index e0688dc96b..a20e40df9b 100644
--- a/lib/spack/spack/cmd/edit.py
+++ b/lib/spack/spack/cmd/edit.py
@@ -30,6 +30,8 @@ from llnl.util.filesystem import mkdirp, join_path
 
 import spack
 import spack.cmd
+from spack.spec import Spec
+from spack.repository import Repo
 from spack.util.naming import mod_to_class
 
 description = "Open package files in $EDITOR"
@@ -53,9 +55,16 @@ class ${class_name}(Package):
 """)
 
 
-def edit_package(name, force=False):
-    path = spack.repo.filename_for_package_name(name)
+def edit_package(name, repo_path, namespace, force=False):
+    if repo_path:
+        repo = Repo(repo_path)
+    elif namespace:
+        repo = spack.repo.get_repo(namespace)
+    else:
+        repo = spack.repo
+    path = repo.filename_for_package_name(name)
 
+    spec = Spec(name)
     if os.path.exists(path):
         if not os.path.isfile(path):
             tty.die("Something's wrong.  '%s' is not a file!" % path)
@@ -63,13 +72,13 @@ def edit_package(name, force=False):
             tty.die("Insufficient permissions on '%s'!" % path)
     elif not force:
         tty.die("No package '%s'.  Use spack create, or supply -f/--force "
-                "to edit a new file." % name)
+                "to edit a new file." % spec.name)
     else:
         mkdirp(os.path.dirname(path))
         with open(path, "w") as pkg_file:
             pkg_file.write(
                 package_template.substitute(
-                    name=name, class_name=mod_to_class(name)))
+                    name=spec.name, class_name=mod_to_class(spec.name)))
 
     spack.editor(path)
 
@@ -79,17 +88,25 @@ def setup_parser(subparser):
         '-f', '--force', dest='force', action='store_true',
         help="Open a new file in $EDITOR even if package doesn't exist.")
 
-    filetypes = subparser.add_mutually_exclusive_group()
-    filetypes.add_argument(
+    excl_args = subparser.add_mutually_exclusive_group()
+
+    # Various filetypes you can edit directly from the cmd line.
+    excl_args.add_argument(
         '-c', '--command', dest='path', action='store_const',
         const=spack.cmd.command_path, help="Edit the command with the supplied name.")
-    filetypes.add_argument(
+    excl_args.add_argument(
         '-t', '--test', dest='path', action='store_const',
         const=spack.test_path, help="Edit the test with the supplied name.")
-    filetypes.add_argument(
+    excl_args.add_argument(
         '-m', '--module', dest='path', action='store_const',
         const=spack.module_path, help="Edit the main spack module with the supplied name.")
 
+    # Options for editing packages
+    excl_args.add_argument(
+        '-r', '--repo', default=None, help="Path to repo to edit package in.")
+    excl_args.add_argument(
+        '-N', '--namespace', default=None, help="Namespace of package to edit.")
+
     subparser.add_argument(
         'name', nargs='?', default=None, help="name of package to edit")
 
@@ -107,7 +124,7 @@ def edit(parser, args):
         spack.editor(path)
 
     elif name:
-        edit_package(name, args.force)
+        edit_package(name, args.repo, args.namespace, args.force)
     else:
         # By default open the directory where packages or commands live.
         spack.editor(path)
diff --git a/lib/spack/spack/cmd/repo.py b/lib/spack/spack/cmd/repo.py
index a792f04cfd..34c755fb67 100644
--- a/lib/spack/spack/cmd/repo.py
+++ b/lib/spack/spack/cmd/repo.py
@@ -44,9 +44,10 @@ def setup_parser(subparser):
     # Create
     create_parser = sp.add_parser('create', help=repo_create.__doc__)
     create_parser.add_argument(
-        'namespace', help="Namespace to identify packages in the repository.")
+        'directory', help="Directory to create the repo in.")
     create_parser.add_argument(
-        'directory', help="Directory to create the repo in.  Defaults to same as namespace.", nargs='?')
+        'namespace', help="Namespace to identify packages in the repository. "
+        "Defaults to the directory name.", nargs='?')
 
     # List
     list_parser = sp.add_parser('list', help=repo_list.__doc__)
@@ -72,14 +73,15 @@ def setup_parser(subparser):
 
 
 def repo_create(args):
-    """Create a new package repo for a particular namespace."""
+    """Create a new package repository."""
+    root = canonicalize_path(args.directory)
     namespace = args.namespace
-    if not re.match(r'\w[\.\w-]*', namespace):
-        tty.die("Invalid namespace: '%s'" % namespace)
 
-    root = args.directory
-    if not root:
-        root = namespace
+    if not args.namespace:
+        namespace = os.path.basename(root)
+
+    if not re.match(r'\w[\.\w-]*', namespace):
+        tty.die("'%s' is not a valid namespace." % namespace)
 
     existed = False
     if os.path.exists(root):
@@ -123,27 +125,22 @@ def repo_create(args):
 
 
 def repo_add(args):
-    """Add a package source to the Spack configuration"""
+    """Add a package source to Spack's configuration."""
     path = args.path
 
-    # check if the path is relative to the spack directory.
-    real_path = path
-    if path.startswith('$spack'):
-        real_path = spack.repository.substitute_spack_prefix(path)
-    elif not os.path.isabs(real_path):
-        real_path = os.path.abspath(real_path)
-        path = real_path
+    # real_path is absolute and handles substitution.
+    canon_path = canonicalize_path(path)
 
     # check if the path exists
-    if not os.path.exists(real_path):
+    if not os.path.exists(canon_path):
         tty.die("No such file or directory: '%s'." % path)
 
     # Make sure the path is a directory.
-    if not os.path.isdir(real_path):
+    if not os.path.isdir(canon_path):
         tty.die("Not a Spack repository: '%s'." % path)
 
     # Make sure it's actually a spack repository by constructing it.
-    repo = Repo(real_path)
+    repo = Repo(canon_path)
 
     # If that succeeds, finally add it to the configuration.
     repos = spack.config.get_config('repos', args.scope)
@@ -152,30 +149,32 @@ def repo_add(args):
     if repo.root in repos or path in repos:
         tty.die("Repository is already registered with Spack: '%s'" % path)
 
-    repos.insert(0, path)
+    repos.insert(0, canon_path)
     spack.config.update_config('repos', repos, args.scope)
     tty.msg("Created repo with namespace '%s'." % repo.namespace)
 
 
 def repo_remove(args):
-    """Remove a repository from the Spack configuration."""
+    """Remove a repository from Spack's configuration."""
     repos = spack.config.get_config('repos', args.scope)
     path_or_namespace = args.path_or_namespace
 
     # If the argument is a path, remove that repository from config.
-    path = os.path.abspath(path_or_namespace)
-    if path in repos:
-        repos.remove(path)
-        spack.config.update_config('repos', repos, args.scope)
-        tty.msg("Removed repository '%s'." % path)
-        return
+    canon_path = canonicalize_path(path_or_namespace)
+    for repo_path in repos:
+        repo_canon_path = canonicalize_path(repo_path)
+        if canon_path == repo_canon_path:
+            repos.remove(repo_path)
+            spack.config.update_config('repos', repos, args.scope)
+            tty.msg("Removed repository '%s'." % repo_path)
+            return
 
     # If it is a namespace, remove corresponding repo
     for path in repos:
         try:
             repo = Repo(path)
             if repo.namespace == path_or_namespace:
-                repos.remove(repo.root)
+                repos.remove(path)
                 spack.config.update_config('repos', repos, args.scope)
                 tty.msg("Removed repository '%s' with namespace %s."
                         % (repo.root, repo.namespace))
@@ -188,7 +187,7 @@ def repo_remove(args):
 
 
 def repo_list(args):
-    """List package sources and their mnemoics"""
+    """Show registered repositories and their namespaces."""
     roots = spack.config.get_config('repos', args.scope)
     repos = []
     for r in roots:
diff --git a/lib/spack/spack/repository.py b/lib/spack/spack/repository.py
index 3367572ef5..31596cee7a 100644
--- a/lib/spack/spack/repository.py
+++ b/lib/spack/spack/repository.py
@@ -54,6 +54,9 @@ repo_config_name   = 'repo.yaml'   # Top-level filename for repo config.
 packages_dir_name  = 'packages'    # Top-level repo directory containing pkgs.
 package_file_name  = 'package.py'  # Filename for packages in a repository.
 
+# Guaranteed unused default value for some functions.
+NOT_PROVIDED = object()
+
 
 def _autospec(function):
     """Decorator that automatically converts the argument of a single-arg
@@ -75,7 +78,15 @@ def _make_namespace_module(ns):
 
 def substitute_spack_prefix(path):
     """Replaces instances of $spack with Spack's prefix."""
-    return path.replace('$spack', spack.prefix)
+    return re.sub(r'^\$spack', spack.prefix, path)
+
+
+def canonicalize_path(path):
+    """Substitute $spack, expand user home, take abspath."""
+    path = substitute_spack_prefix(path)
+    path = os.path.expanduser(path)
+    path = os.path.abspath(path)
+    return path
 
 
 class RepoPath(object):
@@ -109,7 +120,10 @@ class RepoPath(object):
                 repo = Repo(root, self.super_namespace)
                 self.put_last(repo)
             except RepoError as e:
-                tty.warn("Failed to initialize repository at '%s'." % root, e.message)
+                tty.warn("Failed to initialize repository at '%s'." % root,
+                         e.message,
+                         "To remove the bad repository, run this command:",
+                         "    spack repo rm %s" % root)
 
 
     def swap(self, other):
@@ -173,6 +187,31 @@ class RepoPath(object):
             self.repos.remove(repo)
 
 
+    def get_repo(self, namespace, default=NOT_PROVIDED):
+        """Get a repository by namespace.
+        Arguments
+          namespace
+            Look up this namespace in the RepoPath, and return
+            it if found.
+
+        Optional Arguments
+          default
+            If default is provided, return it when the namespace
+            isn't found.  If not, raise an UnknownNamespaceError.
+        """
+        fullspace = '%s.%s' % (self.super_namespace, namespace)
+        if fullspace not in self.by_namespace:
+            if default == NOT_PROVIDED:
+                raise UnknownNamespaceError(namespace)
+            return default
+        return self.by_namespace[fullspace]
+
+
+    def first_repo(self):
+        """Get the first repo in precedence order."""
+        return self.repos[0] if self.repos else None
+
+
     def all_package_names(self):
         """Return all unique package names in all repositories."""
         return self._all_package_names
@@ -229,7 +268,6 @@ class RepoPath(object):
         if fullname in sys.modules:
             return sys.modules[fullname]
 
-
         # partition fullname into prefix and module name.
         namespace, dot, module_name = fullname.rpartition('.')
 
@@ -242,11 +280,23 @@ class RepoPath(object):
         return module
 
 
-    def repo_for_pkg(self, pkg_name):
+    @_autospec
+    def repo_for_pkg(self, spec):
+        """Given a spec, get the repository for its package."""
+        # If the spec already has a namespace, then return the
+        # corresponding repo if we know about it.
+        if spec.namespace:
+            fullspace = '%s.%s' % (self.super_namespace, spec.namespace)
+            if fullspace not in self.by_namespace:
+                raise UnknownNamespaceError(spec.namespace)
+            return self.by_namespace[fullspace]
+
+        # If there's no namespace, search in the RepoPath.
         for repo in self.repos:
-            if pkg_name in repo:
+            if spec.name in repo:
                 return repo
-        raise UnknownPackageError(pkg_name)
+        else:
+            raise UnknownPackageError(spec.name)
 
 
     @_autospec
@@ -255,16 +305,7 @@ class RepoPath(object):
 
            Raises UnknownPackageError if not found.
         """
-        # if the spec has a fully qualified namespace, we grab it
-        # directly and ignore overlay precedence.
-        if spec.namespace:
-            fullspace = '%s.%s' % (self.super_namespace, spec.namespace)
-            if not fullspace in self.by_namespace:
-                raise UnknownPackageError(
-                    "No configured repository contains package %s." % spec.fullname)
-            return self.by_namespace[fullspace].get(spec)
-        else:
-            return self.repo_for_pkg(spec.name).get(spec)
+        return self.repo_for_pkg(spec).get(spec)
 
 
     def dirname_for_package_name(self, pkg_name):
@@ -310,7 +351,7 @@ class Repo(object):
         """
         # Root directory, containing _repo.yaml and package dirs
         # Allow roots to by spack-relative by starting with '$spack'
-        self.root = substitute_spack_prefix(root)
+        self.root = canonicalize_path(root)
 
         # super-namespace for all packages in the Repo
         self.super_namespace = namespace
@@ -330,7 +371,7 @@ class Repo(object):
         # Read configuration and validate namespace
         config = self._read_config()
         check('namespace' in config, '%s must define a namespace.'
-              % join_path(self.root, repo_config_name))
+              % join_path(root, repo_config_name))
 
         self.namespace = config['namespace']
         check(re.match(r'[a-zA-Z][a-zA-Z0-9_.]+', self.namespace),
@@ -524,13 +565,22 @@ class Repo(object):
         return [p for p in self.all_packages() if p.extends(extendee_spec)]
 
 
-    def dirname_for_package_name(self, pkg_name):
+    def _check_namespace(self, spec):
+        """Check that the spec's namespace is the same as this repository's."""
+        if spec.namespace and spec.namespace != self.namespace:
+            raise UnknownNamespaceError(spec.namespace)
+
+
+    @_autospec
+    def dirname_for_package_name(self, spec):
         """Get the directory name for a particular package.  This is the
            directory that contains its package.py file."""
-        return join_path(self.packages_path, pkg_name)
+        self._check_namespace(spec)
+        return join_path(self.packages_path, spec.name)
 
 
-    def filename_for_package_name(self, pkg_name):
+    @_autospec
+    def filename_for_package_name(self, spec):
         """Get the filename for the module we should load for a particular
            package.  Packages for a Repo live in
            ``$root/<package_name>/package.py``
@@ -539,8 +589,8 @@ class Repo(object):
            package doesn't exist yet, so callers will need to ensure
            the package exists before importing.
         """
-        validate_module_name(pkg_name)
-        pkg_dir = self.dirname_for_package_name(pkg_name)
+        self._check_namespace(spec)
+        pkg_dir = self.dirname_for_package_name(spec.name)
         return join_path(pkg_dir, package_file_name)
 
 
@@ -679,6 +729,13 @@ class UnknownPackageError(PackageLoadError):
         self.name = name
 
 
+class UnknownNamespaceError(PackageLoadError):
+    """Raised when we encounter an unknown namespace"""
+    def __init__(self, namespace):
+        super(UnknownNamespaceError, self).__init__(
+            "Unknown namespace: %s" % namespace)
+
+
 class FailedConstructorError(PackageLoadError):
     """Raised when a package's class constructor fails."""
     def __init__(self, name, exc_type, exc_obj, exc_tb):
diff --git a/lib/spack/spack/util/naming.py b/lib/spack/spack/util/naming.py
index 26ca86c77f..5025f15027 100644
--- a/lib/spack/spack/util/naming.py
+++ b/lib/spack/spack/util/naming.py
@@ -8,11 +8,15 @@ from StringIO import StringIO
 import spack
 
 __all__ = ['mod_to_class', 'spack_module_to_python_module', 'valid_module_name',
+           'valid_fully_qualified_module_name', 'validate_fully_qualified_module_name',
            'validate_module_name', 'possible_spack_module_names', 'NamespaceTrie']
 
 # Valid module names can contain '-' but can't start with it.
 _valid_module_re = r'^\w[\w-]*$'
 
+# Valid module names can contain '-' but can't start with it.
+_valid_fully_qualified_module_re = r'^(\w[\w-]*)(\.\w[\w-]*)*$'
+
 
 def mod_to_class(mod_name):
     """Convert a name from module style to class name style.  Spack mostly
@@ -75,16 +79,27 @@ def possible_spack_module_names(python_mod_name):
 
 
 def valid_module_name(mod_name):
-    """Return whether the mod_name is valid for use in Spack."""
+    """Return whether mod_name is valid for use in Spack."""
     return bool(re.match(_valid_module_re, mod_name))
 
 
+def valid_fully_qualified_module_name(mod_name):
+    """Return whether mod_name is a valid namespaced module name."""
+    return bool(re.match(_valid_fully_qualified_module_re, mod_name))
+
+
 def validate_module_name(mod_name):
     """Raise an exception if mod_name is not valid."""
     if not valid_module_name(mod_name):
         raise InvalidModuleNameError(mod_name)
 
 
+def validate_fully_qualified_module_name(mod_name):
+    """Raise an exception if mod_name is not a valid namespaced module name."""
+    if not valid_fully_qualified_module_name(mod_name):
+        raise InvalidFullyQualifiedModuleNameError(mod_name)
+
+
 class InvalidModuleNameError(spack.error.SpackError):
     """Raised when we encounter a bad module name."""
     def __init__(self, name):
@@ -93,6 +108,14 @@ class InvalidModuleNameError(spack.error.SpackError):
         self.name = name
 
 
+class InvalidFullyQualifiedModuleNameError(spack.error.SpackError):
+    """Raised when we encounter a bad full package name."""
+    def __init__(self, name):
+        super(InvalidFullyQualifiedModuleNameError, self).__init__(
+            "Invalid fully qualified package name: " + name)
+        self.name = name
+
+
 class NamespaceTrie(object):
     class Element(object):
         def __init__(self, value):