env: environments can be named or created in directories

- `spack env create <name>` works as before

- `spack env create <path>` now works as well -- environments can be
  created in their own directories outside of Spack.

- `spack install` will look for a `spack.yaml` file in the current
  directory, and will install the entire project from the environment

- The Environment class has been refactored so that it does not depend on
  the internal Spack environment root; it just takes a path and operates
  on an environment in that path (so internal and external envs are
  handled the same)

- The named environment interface has been hoisted to the
  spack.environment module level.

- env.yaml is now spack.yaml in all places.  It was easier to go with one
  name for these files than to try to handle logic for both env.yaml and
  spack.yaml.

lib/spack/llnl/util/filesystem.py

@@ -538,6 +538,30 @@ def hash_directory(directory):
     return md5_hash.hexdigest()
 
 
+@contextmanager
+def write_tmp_and_move(filename):
+    """Write to a temporary file, then move into place."""
+    dirname = os.path.dirname(filename)
+    basename = os.path.basename(filename)
+    tmp = os.path.join(dirname, '.%s.tmp' % basename)
+    with open(tmp, 'w') as f:
+        yield f
+    shutil.move(tmp, filename)
+
+
+@contextmanager
+def open_if_filename(str_or_file, mode='r'):
+    """Takes either a path or a file object, and opens it if it is a path.
+
+    If it's a file object, just yields the file object.
+    """
+    if isinstance(str_or_file, six.string_types):
+        with open(str_or_file, mode) as f:
+            yield f
+    else:
+        yield str_or_file
+
+
 def touch(path):
     """Creates an empty file at the specified path."""
     perms = (os.O_WRONLY | os.O_CREAT | os.O_NONBLOCK | os.O_NOCTTY)

lib/spack/spack/cmd/env.py

@@ -48,7 +48,7 @@
 ]
 
 
-def get_env(args, cmd_name):
+def get_env(args, cmd_name, fail_on_error=True):
     """Get target environment from args, or from environment variables.
 
     This is used by a number of commands for handling the environment
@@ -67,11 +67,16 @@ def get_env(args, cmd_name):
     if not env:
         env = os.environ.get('SPACK_ENV')
         if not env:
+            if not fail_on_error:
+                return None
             tty.die(
                 'spack env %s requires an active environment or an argument'
                 % cmd_name)
 
-    return ev.read(env)
+    environment = ev.disambiguate(env)
+    if not environment:
+        tty.die('no such environment: %s' % env)
+    return environment
 
 
 #
@@ -86,6 +91,13 @@ def env_activate_setup_parser(subparser):
     shells.add_argument(
         '--csh', action='store_const', dest='shell', const='csh',
         help="print csh commands to activate the environment")
+    shells.add_argument(
+        '-d', '--dir', action='store_true', default=False,
+        help="force spack to treat env as a directory, not a name")
+
+    subparser.add_argument(
+        '-p', '--prompt', action='store_true', default=False,
+        help="decorate the command line prompt when activating")
     subparser.add_argument(
         metavar='env', dest='activate_env',
         help='name of environment to activate')
@@ -110,29 +122,38 @@ def env_activate(args):
         tty.msg(*msg)
         return 1
 
-    if not ev.exists(env):
+    if ev.exists(env) and not args.dir:
-        tty.die("No such environment: '%s'" % env)
+        spack_env = ev.root(env)
+        short_name = env
+        env_prompt = '[%s]' % env
 
-    env_name_prompt = '[%s] ' % env
+    elif ev.is_env_dir(env):
+        spack_env = os.path.abspath(env)
+        short_name = os.path.basename(os.path.abspath(env))
+        env_prompt = '[%s]' % short_name
+
+    else:
+        tty.die("No such environment: '%s'" % env)
 
     if args.shell == 'csh':
         # TODO: figure out how to make color work for csh
-        sys.stdout.write('''\
+        sys.stdout.write('setenv SPACK_ENV %s;\n' % spack_env)
-setenv SPACK_ENV %s;
+        sys.stdout.write('alias despacktivate "spack env deactivate";\n')
-setenv SPACK_OLD_PROMPT "${prompt}";
+        if args.prompt:
-set prompt="%s ${prompt}";
+            sys.stdout.write('if (! $?SPACK_OLD_PROMPT ) '
-alias despacktivate "spack env deactivate";
+                             'setenv SPACK_OLD_PROMPT "${prompt}";\n')
-''' % (env, env_name_prompt))
+            sys.stdout.write('set prompt="%s ${prompt}";\n' % env_prompt)
+
     else:
         if 'color' in os.environ['TERM']:
-            env_name_prompt = colorize('@G{%s} ' % env_name_prompt, color=True)
+            env_prompt = colorize('@G{%s} ' % env_prompt, color=True)
 
-        sys.stdout.write('''\
+        sys.stdout.write('export SPACK_ENV=%s;\n' % spack_env)
-export SPACK_ENV=%s;
+        sys.stdout.write("alias despacktivate='spack env deactivate';\n")
-if [ -z "${SPACK_OLD_PS1}" ]; then export SPACK_OLD_PS1="${PS1}"; fi;
+        if args.prompt:
-export PS1="%s ${PS1}";
+            sys.stdout.write('if [ -z "${SPACK_OLD_PS1}" ]; then\n')
-alias despacktivate='spack env deactivate'
+            sys.stdout.write('export SPACK_OLD_PS1="${PS1}"; fi;\n')
-''' % (env, env_name_prompt))
+            sys.stdout.write('export PS1="%s ${PS1}";\n' % env_prompt)
 
 
 #
@@ -168,19 +189,19 @@ def env_deactivate(args):
         tty.die('No environment is currently active.')
 
     if args.shell == 'csh':
-        sys.stdout.write('''\
+        sys.stdout.write('unsetenv SPACK_ENV;\n')
-unsetenv SPACK_ENV;
+        sys.stdout.write('if ( $?SPACK_OLD_PROMPT ) '
-set prompt="${SPACK_OLD_PROMPT}";
+                         'set prompt="$SPACK_OLD_PROMPT" && '
-unsetenv SPACK_OLD_PROMPT;
+                         'unsetenv SPACK_OLD_PROMPT;\n')
-unalias despacktivate;
+        sys.stdout.write('unalias despacktivate;\n')
-''')
+
     else:
-        sys.stdout.write('''\
+        sys.stdout.write('unset SPACK_ENV; export SPACK_ENV;\n')
-unset SPACK_ENV; export SPACK_ENV;
+        sys.stdout.write('unalias despacktivate;\n')
-export PS1="$SPACK_OLD_PS1";
+        sys.stdout.write('if [ -n "$SPACK_OLD_PS1" ]; then\n')
-unset SPACK_OLD_PS1; export SPACK_OLD_PS1;
+        sys.stdout.write('export PS1="$SPACK_OLD_PS1";\n')
-unalias despacktivate;
+        sys.stdout.write('unset SPACK_OLD_PS1; export SPACK_OLD_PS1;\n')
-''')
+        sys.stdout.write('fi;\n')
 
 
 #
@@ -189,32 +210,40 @@ def env_deactivate(args):
 def env_create_setup_parser(subparser):
     """create a new environment"""
     subparser.add_argument('env', help='name of environment to create')
-    subparser.add_argument('envfile', nargs='?', default=None,
+    subparser.add_argument(
-                           help='YAML initialization file (optional)')
+        '-d', '--dir', action='store_true',
+        help='create an environment in a specific directory')
+    subparser.add_argument(
+        'envfile', nargs='?', default=None,
+        help='optional init file; can be spack.yaml or spack.lock')
 
 
 def env_create(args):
     if args.envfile:
         with open(args.envfile) as f:
-            _env_create(args.env, f)
+            _env_create(args.env, f, args.dir)
     else:
-        _env_create(args.env)
+        _env_create(args.env, None, args.dir)
 
 
-def _env_create(name, env_yaml=None):
+def _env_create(name_or_path, init_file=None, dir=False):
     """Create a new environment, with an optional yaml description.
 
     Arguments:
-        name (str): name of the environment to create
+        name_or_path (str): name of the environment to create, or path to it
-        env_yaml (str or file): yaml text or file object containing
+        init_file (str or file): optional initialization file -- can be
-            configuration information.
+            spack.yaml or spack.lock
+        dir (bool): if True, create an environment in a directory instead
+            of a named environment
     """
-    if os.path.exists(ev.root(name)):
+    if dir:
-        tty.die("'%s': environment already exists" % name)
+        env = ev.Environment(name_or_path, init_file)
-
+        env.write()
-    env = ev.Environment(name, env_yaml)
+        tty.msg("Created environment in %s" % env.path)
-    env.write()
+    else:
-    tty.msg("Created environment '%s' in %s" % (name, env.path))
+        env = ev.create(name_or_path, init_file)
+        env.write()
+        tty.msg("Created environment '%s' in %s" % (name_or_path, env.path))
     return env
 
 
@@ -229,13 +258,6 @@ def env_destroy_setup_parser(subparser):
 
 
 def env_destroy(args):
-    for env in args.env:
-        if not ev.exists(env):
-            tty.die("No such environment: '%s'" % env)
-        elif not os.access(ev.root(env), os.W_OK):
-            tty.die("insufficient permissions to modify environment: '%s'"
-                    % args.env)
-
     if not args.yes_to_all:
         answer = tty.get_yes_or_no(
             'Really destroy %s %s?' % (
@@ -246,7 +268,7 @@ def env_destroy(args):
             tty.die("Will not destroy any environments")
 
     for env in args.env:
-        ev.Environment(env).destroy()
+        ev.destroy(env)
         tty.msg("Successfully destroyed environment '%s'" % env)
 
 
@@ -315,7 +337,7 @@ def env_remove_setup_parser(subparser):
 
 
 def env_remove(args):
-    env = get_env(args, 'remove')
+    env = get_env(args, 'remove <spec>')
 
     if args.all:
         env.clear()
@@ -340,13 +362,13 @@ def env_concretize_setup_parser(subparser):
 
 def env_concretize(args):
     env = get_env(args, 'status')
-    _env_concretize(env, use_repo=bool(args.exact_env), force=args.force)
+    _env_concretize(env, use_repo=args.use_env_repo, force=args.force)
 
 
 def _env_concretize(env, use_repo=False, force=False):
     """Function body separated out to aid in testing."""
-    new_specs = env.concretize(force=force)
+    env.concretize(force=force)
-    env.write(dump_packages=new_specs)
+    env.write()
 
 
 # REMOVE
@@ -416,7 +438,12 @@ def env_status_setup_parser(subparser):
 
 
 def env_status(args):
-    env = get_env(args, 'status')
+    env = get_env(args, 'status', fail_on_error=False)
+    if not env:
+        tty.msg('No active environment')
+        return
+
+    tty.msg('In environment %s' % env.path)
 
     # TODO: option to show packages w/ multiple instances?
     env.status(

lib/spack/spack/cmd/install.py

@@ -11,11 +11,12 @@
 import llnl.util.filesystem as fs
 import llnl.util.tty as tty
 
-import spack.paths
 import spack.build_environment
 import spack.cmd
 import spack.cmd.common.arguments as arguments
+import spack.environment as ev
 import spack.fetch_strategy
+import spack.paths
 import spack.report
 from spack.error import SpackError
 
@@ -156,8 +157,8 @@ def install_spec(cli_args, kwargs, spec):
     def install(spec, kwargs):
         env = spack.environment.active
         if env:
-            new_specs = env.install(spec, kwargs)
+            env.install(spec, kwargs)
-            env.write(dump_packages=new_specs)
+            env.write()
         else:
             spec.package.do_install(**kwargs)
 
@@ -186,7 +187,15 @@ def install(spec, kwargs):
 
 def install(parser, args, **kwargs):
     if not args.package and not args.specfiles:
-        tty.die("install requires at least one package argument or yaml file")
+        # if there is a spack.yaml file, then install the packages in it.
+        if os.path.exists(ev.manifest_name):
+            env = ev.Environment(os.getcwd())
+            env.concretize()
+            env.write()
+            env.install_all()
+            return
+        else:
+            tty.die("install requires a package argument or a spack.yaml file")
 
     if args.jobs is not None:
         if args.jobs <= 0:

lib/spack/spack/environment.py

@@ -7,7 +7,6 @@
 import re
 import sys
 import shutil
-from contextlib import contextmanager
 from six.moves import zip_longest
 
 import jsonschema
@@ -38,21 +37,25 @@
 env_path = os.path.join(spack.paths.var_path, 'environments')
 
 
-#: Name of the input yaml file in an environment
+#: Name of the input yaml file for an environment
-env_yaml_name = 'env.yaml'
+manifest_name = 'spack.yaml'
 
 
-#: Name of the lock file with concrete specs
+#: Name of the input yaml file for an environment
-env_lock_name = 'env.lock'
+lockfile_name = 'spack.lock'
 
 
-#: default env.yaml file to put in new environments
+#: Name of the directory where environments store repos, logs, views
-default_env_yaml = """\
+env_subdir_name = '.spack-env'
+
+
+#: default spack.yaml file to put in new environments
+default_manifest_yaml = """\
 # This is a Spack Environment file.
 #
 # It describes a set of packages to be installed, along with
 # configuration settings.
-env:
+spack:
   # add package specs to the `specs` list
   specs:
   -
@@ -63,8 +66,8 @@
 #: version of the lockfile format. Must increase monotonically.
 lockfile_format_version = 1
 
-#: legal first keys in an environment.yaml file
+#: legal first keys in the spack.yaml manifest file
-env_schema_keys = ('env', 'spack')
+env_schema_keys = ('spack', 'env')
 
 #: jsonschema validator for environments
 _validator = None
@@ -82,7 +85,7 @@ def validate_env_name(name):
     return name
 
 
-def activate(name, exact=False):
+def activate(env, use_env_repo=False):
     """Activate an environment.
 
     To activate an environment, we add its configuration scope to the
@@ -90,8 +93,8 @@ def activate(name, exact=False):
     environment.
 
     Arguments:
-        name (str): name of the environment to activate
+        env (Environment): the environment to activate
-        exact (bool): use the packages exactly as they appear in the
+        use_env_repo (bool): use the packages exactly as they appear in the
             environment's repository
 
     TODO: Add support for views here.  Activation should set up the shell
@@ -99,9 +102,9 @@ def activate(name, exact=False):
     """
     global active
 
-    active = read(name)
+    active = env
     prepare_config_scope(active)
-    if exact:
+    if use_env_repo:
         spack.repo.path.put_first(active.repo)
 
     tty.debug("Using environmennt '%s'" % active.name)
@@ -121,58 +124,86 @@ def deactivate():
         return
 
     deactivate_config_scope(active)
-    spack.repo.path.remove(active.repo)
+
+    # use _repo so we only remove if a repo was actually constructed
+    if active._repo:
+        spack.repo.path.remove(active._repo)
 
     tty.debug("Deactivated environmennt '%s'" % active.name)
     active = None
 
 
-@contextmanager
+def disambiguate(env, env_dir=None):
-def env_context(env):
+    """Used to determine whether an environment is named or a directory."""
-    """Context manager that activates and deactivates an environment."""
+    if env:
-    old_active = active
+        if exists(env):
-    activate(env)
+            # treat env as a name
+            return read(env)
+        env_dir = env
 
-    yield
+    if not env_dir:
+        env_dir = os.environ.get(spack_env_var)
+        if not env_dir:
+            return None
 
-    deactivate()
+    if os.path.isdir(env_dir):
-    if old_active:
+        if is_env_dir(env_dir):
-        activate(old_active)
+            return Environment(env_dir)
+        else:
+            raise EnvError('no environment in %s' % env_dir)
+        return
+
+    return None
 
 
 def root(name):
     """Get the root directory for an environment by name."""
+    validate_env_name(name)
     return os.path.join(env_path, name)
 
 
 def exists(name):
-    """Whether an environment exists or not."""
+    """Whether an environment with this name exists or not."""
-    return os.path.exists(root(name))
+    if not valid_env_name(name):
+        return False
+    return os.path.isdir(root(name))
 
 
-def manifest_path(name):
+def is_env_dir(path):
-    return os.path.join(root(name), env_yaml_name)
+    """Whether a directory contains a spack environment."""
+    return os.path.isdir(path) and os.path.exists(
+        os.path.join(path, manifest_name))
 
 
-def lockfile_path(name):
+def read(name):
-    return os.path.join(root(name), env_lock_name)
+    """Get an environment with the supplied name."""
+    validate_env_name(name)
+    if not exists(name):
+        raise EnvError("no such environment '%s'" % name)
+    return Environment(root(name))
 
 
-def dotenv_path(env_root):
+def create(name, init_file=None):
-    """@return Directory in an environment that is owned by Spack"""
+    """Create a named environment in Spack."""
-    return os.path.join(env_root, '.env')
+    validate_env_name(name)
+    if exists(name):
+        raise EnvError("'%s': environment already exists" % name)
+    return Environment(root(name), init_file)
 
 
-def repos_path(dotenv_path):
+def destroy(name):
-    return os.path.join(dotenv_path, 'repos')
+    """Destroy a named environment."""
-
+    validate_env_name(name)
-
+    if not exists(name):
-def log_path(dotenv_path):
+        raise EnvError("no such environment '%s'" % name)
-    return os.path.join(dotenv_path, 'logs')
+    if not os.access(root(name), os.W_OK):
+        raise EnvError(
+            "insufficient permissions to modify environment: '%s'" % name)
+    shutil.rmtree(root(name))
 
 
 def config_dict(yaml_data):
-    """Get the configuration scope section out of an env.yaml"""
+    """Get the configuration scope section out of an spack.yaml"""
     key = spack.config.first_existing(yaml_data, env_schema_keys)
     return yaml_data[key]
 
@@ -187,7 +218,7 @@ def list_environments():
     candidates = sorted(os.listdir(env_path))
     names = []
     for candidate in candidates:
-        yaml_path = os.path.join(root(candidate), env_yaml_name)
+        yaml_path = os.path.join(root(candidate), manifest_name)
         if valid_env_name(candidate) and os.path.exists(yaml_path):
             names.append(candidate)
     return names
@@ -246,53 +277,96 @@ def _write_yaml(data, str_or_file):
 
 
 class Environment(object):
-    def __init__(self, name, env_yaml=None):
+    def __init__(self, path, init_file=None):
-        """Create a new environment, optionally with an initialization file.
+        """Create a new environment.
+
+        The environment can be optionally initialized with either a
+        spack.yaml or spack.lock file.
 
         Arguments:
-            name (str): name for this environment
+            path (str): path to the root directory of this environment
-            env_yaml (str or file): raw YAML or a file to initialize the
+            init_file (str or file object): filename or file object to
-                environment
+                initialize the environment
         """
-        self.name = validate_env_name(name)
+        self.path = os.path.abspath(path)
         self.clear()
 
-        # use read_yaml to preserve comments
+        if init_file:
-        if env_yaml is None:
+            # initialize the environment from a file if provided
-            env_yaml = default_env_yaml
+            with fs.open_if_filename(init_file) as f:
-        self.yaml = _read_yaml(env_yaml)
+                if hasattr(f, 'name') and f.name.endswith('.lock'):
+                    # Initialize the environment from a lockfile
+                    self._read_lockfile(f)
+                    self._set_user_specs_from_lockfile()
+                    self.yaml = _read_yaml(default_manifest_yaml)
+                else:
+                    # Initialize the environment from a spack.yaml file
+                    self._read_manifest(f)
+        else:
+            # read lockfile, if it exists
+            if os.path.exists(self.lock_path):
+                with open(self.lock_path) as f:
+                    self._read_lockfile(f)
 
-        # initialize user specs from the YAML
+            if os.path.exists(self.manifest_path):
+                # read the spack.yaml file, if exists
+                with open(self.manifest_path) as f:
+                    self._read_manifest(f)
+
+            elif self.concretized_user_specs:
+                # if not, take user specs from the lockfile
+                self._set_user_specs_from_lockfile()
+                self.yaml = _read_yaml(default_manifest_yaml)
+            else:
+                # if there's no manifest or lockfile, use the default
+                self._read_manifest(default_manifest_yaml)
+
+    def _read_manifest(self, f):
+        """Read manifest file and set up user specs."""
+        self.yaml = _read_yaml(f)
         spec_list = config_dict(self.yaml).get('specs')
         if spec_list:
-            self.user_specs = [Spec(s) for s in spec_list if s is not None]
+            self.user_specs = [Spec(s) for s in spec_list if s]
+
+    def _set_user_specs_from_lockfile(self):
+        """Copy user_specs from a read-in lockfile."""
+        self.user_specs = [Spec(s) for s in self.concretized_user_specs]
 
     def clear(self):
         self.user_specs = []              # current user specs
         self.concretized_user_specs = []  # user specs from last concretize
         self.concretized_order = []       # roots of last concretize, in order
         self.specs_by_hash = {}           # concretized specs by hash
+        self.new_specs = []               # write packages for these on write()
         self._repo = None                 # RepoPath for this env (memoized)
+        self._previous_active = None      # previously active environment
 
     @property
-    def path(self):
+    def name(self):
-        return root(self.name)
+        return os.path.basename(self.path)
 
     @property
     def manifest_path(self):
-        return manifest_path(self.name)
+        """Path to spack.yaml file in this environment."""
+        return os.path.join(self.path, manifest_name)
 
     @property
     def lock_path(self):
-        return lockfile_path(self.name)
+        """Path to spack.lock file in this environment."""
+        return os.path.join(self.path, lockfile_name)
 
     @property
-    def dotenv_path(self):
+    def env_subdir_path(self):
-        return dotenv_path(self.path)
+        """Path to directory where the env stores repos, logs, views."""
+        return os.path.join(self.path, env_subdir_name)
 
     @property
     def repos_path(self):
-        return repos_path(self.dotenv_path)
+        return os.path.join(self.path, env_subdir_name, 'repos')
+
+    @property
+    def log_path(self):
+        return os.path.join(self.path, env_subdir_name, 'logs')
 
     @property
     def repo(self):
@@ -305,7 +379,7 @@ def included_config_scopes(self):
 
         Scopes are in order from lowest to highest precedence, i.e., the
         order they should be pushed on the stack, but the opposite of the
-        order they appaer in the env.yaml file.
+        order they appaer in the spack.yaml file.
         """
         scopes = []
 
@@ -402,10 +476,6 @@ def concretize(self, force=False):
         Arguments:
             force (bool): re-concretize ALL specs, even those that were
                already concretized
-
-        Return:
-            (list): list of newly concretized specs
-
         """
         if force:
             # Clear previously concretized specs
@@ -414,85 +484,83 @@ def concretize(self, force=False):
             self.specs_by_hash = {}
 
         # keep any concretized specs whose user specs are still in the manifest
-        new_concretized_user_specs = []
+        old_concretized_user_specs = self.concretized_user_specs
-        new_concretized_order = []
+        old_concretized_order = self.concretized_order
-        new_specs_by_hash = {}
+        old_specs_by_hash = self.specs_by_hash
-        for s, h in zip(self.concretized_user_specs, self.concretized_order):
+
+        self.concretized_user_specs = []
+        self.concretized_order = []
+        self.specs_by_hash = {}
+
+        for s, h in zip(old_concretized_user_specs, old_concretized_order):
             if s in self.user_specs:
-                new_concretized_user_specs.append(s)
+                concrete = old_specs_by_hash[h]
-                new_concretized_order.append(h)
+                self._add_concrete_spec(s, concrete, new=False)
-                new_specs_by_hash[h] = self.specs_by_hash[h]
 
         # concretize any new user specs that we haven't concretized yet
-        new_specs = []
         for uspec in self.user_specs:
-            if uspec not in new_concretized_user_specs:
+            if uspec not in old_concretized_user_specs:
                 tty.msg('Concretizing %s' % uspec)
-                cspec = uspec.concretized()
+                concrete = uspec.concretized()
-                dag_hash = cspec.dag_hash()
+                self._add_concrete_spec(uspec, concrete)
-
-                new_concretized_user_specs.append(uspec)
-                new_concretized_order.append(dag_hash)
-                new_specs_by_hash[dag_hash] = cspec
-                new_specs.append(cspec)
 
                 # Display concretized spec to the user
-                sys.stdout.write(cspec.tree(
+                sys.stdout.write(concrete.tree(
                     recurse_dependencies=True, install_status=True,
                     hashlen=7, hashes=True))
 
-        # save the new concretized state
-        self.concretized_user_specs = new_concretized_user_specs
-        self.concretized_order = new_concretized_order
-        self.specs_by_hash = new_specs_by_hash
-
-        # return only the newly concretized specs
-        return new_specs
-
     def install(self, user_spec, install_args=None):
         """Install a single spec into an environment.
 
         This will automatically concretize the single spec, but it won't
         affect other as-yet unconcretized specs.
-
-        Returns:
-            (Spec): concrete spec if the spec was installed, None if it
-                was already present and installed.
-
         """
         spec = Spec(user_spec)
 
-        # TODO: do a more sophisticated match than just by name
+        if self.add(spec):
-        added = self.add(spec)
-        concrete = None
-        if added:
-            # newly added spec
-            spec = self.user_specs[-1]
             concrete = spec.concretized()
-            h = concrete.dag_hash()
+            self._add_concrete_spec(spec, concrete)
-
-            self.concretized_user_specs.append(spec)
-            self.concretized_order.append(h)
-            self.specs_by_hash[h] = concrete
-
         else:
             # spec might be in the user_specs, but not installed.
             spec = next(s for s in self.user_specs if s.name == spec.name)
-            if spec not in self.concretized_user_specs:
+            concrete = self.specs_by_hash.get(spec.dag_hash())
+            if not concrete:
                 concrete = spec.concretized()
-                self.concretized_user_specs.append(spec)
+                self._add_concrete_spec(spec, concrete)
-                self.concretized_order.append(h)
-                self.specs_by_hash[h] = concrete
 
-        if concrete:
+        concrete.package.do_install(**install_args)
-            spec.package.do_install(**install_args)
+
+    def _add_concrete_spec(self, spec, concrete, new=True):
+        """Called when a new concretized spec is added to the environment.
+
+        This ensures that all internal data structures are kept in sync.
+
+        Arguments:
+            spec (Spec): user spec that resulted in the concrete spec
+            concrete (Spec): spec concretized within this environment
+            new (bool): whether to write this spec's package to the env
+                repo on write()
+        """
+        assert concrete.concrete
+
+        # when a spec is newly concretized, we need to make a note so
+        # that we can write its package to the env repo on write()
+        if new:
+            self.new_specs.append(concrete)
+
+        # update internal lists of specs
+        self.concretized_user_specs.append(spec)
+
+        h = concrete.dag_hash()
+        self.concretized_order.append(h)
+        self.specs_by_hash[h] = concrete
 
     def install_all(self, args=None):
         """Install all concretized specs in an environment."""
 
         # Make sure log directory exists
-        logs_dir = log_path(self.dotenv_path)
+        log_path = self.log_path
-        fs.mkdirp(logs_dir)
+        fs.mkdirp(log_path)
 
         for concretized_hash in self.concretized_order:
             spec = self.specs_by_hash[concretized_hash]
@@ -508,7 +576,7 @@ def install_all(self, args=None):
 
                 # Link the resulting log file into logs dir
                 build_log_link = os.path.join(
-                    logs_dir, '%s-%s.log' % (spec.name, spec.dag_hash(7)))
+                    log_path, '%s-%s.log' % (spec.name, spec.dag_hash(7)))
                 if os.path.exists(build_log_link):
                     os.remove(build_log_link)
                 os.symlink(spec.package.build_log_path, build_log_link)
@@ -623,6 +691,11 @@ def _to_lockfile_dict(self):
 
         return data
 
+    def _read_lockfile(self, file_or_json):
+        """Read a lockfile from a file or from a raw string."""
+        lockfile_dict = sjson.load(file_or_json)
+        self._read_lockfile_dict(lockfile_dict)
+
     def _read_lockfile_dict(self, d):
         """Read a lockfile dictionary into this environment."""
         roots = d['roots']
@@ -645,42 +718,34 @@ def _read_lockfile_dict(self, d):
         self.specs_by_hash = dict(
             (x, y) for x, y in specs_by_hash.items() if x in root_hashes)
 
-    def write(self, dump_packages=None):
+    def write(self):
         """Writes an in-memory environment to its location on disk.
 
-        Arguments:
+        This will also write out package files for each newly concretized spec.
-            dump_packages (list of Spec): specs of packages whose
-                package.py files should be written to the env's repo
         """
         # ensure path in var/spack/environments
         fs.mkdirp(self.path)
 
         if self.specs_by_hash:
             # ensure the prefix/.env directory exists
-            tmp_env = '%s.tmp' % self.dotenv_path
+            fs.mkdirp(self.env_subdir_path)
-            fs.mkdirp(tmp_env)
 
-            # dump package.py files for specified specs
+            for spec in self.new_specs:
-            tmp_repos_path = repos_path(tmp_env)
-            dump_packages = dump_packages or []
-            for spec in dump_packages:
                 for dep in spec.traverse():
                     if not dep.concrete:
                         raise ValueError('specs passed to environment.write() '
                                          'must be concrete!')
 
-                    root = os.path.join(tmp_repos_path, dep.namespace)
+                    root = os.path.join(self.repos_path, dep.namespace)
                     repo = spack.repo.create_or_construct(root, dep.namespace)
                     pkg_dir = repo.dirname_for_package_name(dep.name)
 
                     fs.mkdirp(pkg_dir)
                     spack.repo.path.dump_provenance(dep, pkg_dir)
-
+            self.new_specs = []
-            # move the new .env directory into place.
-            move_move_rm(tmp_env, self.dotenv_path)
 
             # write the lock file last
-            with write_tmp_and_move(self.lock_path) as f:
+            with fs.write_tmp_and_move(self.lock_path) as f:
                 sjson.dump(self._to_lockfile_dict(), stream=f)
         else:
             if os.path.exists(self.lock_path):
@@ -694,55 +759,18 @@ def write(self, dump_packages=None):
         yaml_spec_list[:] = [str(s) for s in self.user_specs]
 
         # if all that worked, write out the manifest file at the top level
-        with write_tmp_and_move(self.manifest_path) as f:
+        with fs.write_tmp_and_move(self.manifest_path) as f:
             _write_yaml(self.yaml, f)
 
+    def __enter__(self):
+        self._previous_active = active
+        activate(self)
+        return
 
-def read(env_name):
+    def __exit__(self, exc_type, exc_val, exc_tb):
-    """Read environment state from disk."""
+        deactivate()
-    env_root = root(env_name)
+        if self._previous_active:
-    if not os.path.isdir(env_root):
+            activate(self._previous_active)
-        raise EnvError("no such environment '%s'" % env_name)
-    if not os.access(env_root, os.R_OK):
-        raise EnvError("can't read environment '%s'" % env_name)
-
-    # read yaml file
-    with open(manifest_path(env_name)) as f:
-        env = Environment(env_name, f.read())
-
-    # read lockfile, if it exists
-    lock_path = lockfile_path(env_name)
-    if os.path.exists(lock_path):
-        with open(lock_path) as f:
-            lockfile_dict = sjson.load(f)
-        env._read_lockfile_dict(lockfile_dict)
-
-    return env
-
-
-def move_move_rm(src, dest):
-    """Move dest out of the way, put src in its place."""
-
-    dirname = os.path.dirname(dest)
-    basename = os.path.basename(dest)
-    old = os.path.join(dirname, '.%s.old' % basename)
-
-    if os.path.exists(dest):
-        shutil.move(dest, old)
-    shutil.move(src, dest)
-    if os.path.exists(old):
-        shutil.rmtree(old)
-
-
-@contextmanager
-def write_tmp_and_move(filename):
-    """Write to a temporary file, then move into place."""
-    dirname = os.path.dirname(filename)
-    basename = os.path.basename(filename)
-    tmp = os.path.join(dirname, '.%s.tmp' % basename)
-    with open(tmp, 'w') as f:
-        yield f
-    shutil.move(tmp, filename)
 
 
 def make_repo_path(root):

lib/spack/spack/main.py

@@ -25,7 +25,7 @@
 import spack.architecture
 import spack.config
 import spack.cmd
-import spack.environment
+import spack.environment as ev
 import spack.hooks
 import spack.paths
 import spack.repo
@@ -325,10 +325,13 @@ def make_argument_parser(**kwargs):
     env_group = parser.add_mutually_exclusive_group()
     env_group.add_argument(
         '-e', '--env', dest='env', metavar='ENV', action='store',
-        help="run spack with a specific environment (see spack env)")
+        help="run with a specific environment (see spack env)")
     env_group.add_argument(
-        '-E', '--exact-env', dest='exact_env', metavar='ENV', action='store',
+        '-E', '--env-dir', metavar='DIR', action='store',
-        help="run spack with a specific environment AND use its repo")
+        help="run with an environment directory (ignore named environments)")
+    parser.add_argument(
+        '--use-env-repo', action='store_true',
+        help="when running in an environment, use its package repository")
 
     parser.add_argument(
         '-k', '--insecure', action='store_true',
@@ -568,6 +571,31 @@ def shell_set(var, value):
             shell_set('_sp_module_prefix', 'not_installed')
 
 
+def activate_environment(env, env_dir, use_env_repo):
+    """Activate an environment from command line arguments or an env var."""
+
+    if env:
+        if ev.exists(env):
+            # treat env as a name
+            ev.activate(ev.read(env), use_env_repo)
+            return
+        env_dir = env
+
+    if not env_dir:
+        env_dir = os.environ.get(spack.environment.spack_env_var)
+        if not env_dir:
+            return
+
+    if os.path.isdir(env_dir):
+        if ev.is_env_dir(env_dir):
+            ev.activate(ev.Environment(env_dir), use_env_repo)
+        else:
+            tty.die('no environment in %s' % env_dir)
+        return
+
+    tty.die('no such environment: %s' % env_dir)
+
+
 def main(argv=None):
     """This is the entry point for the Spack command.
 
@@ -584,13 +612,7 @@ def main(argv=None):
     args, unknown = parser.parse_known_args(argv)
 
     # activate an environment if one was specified on the command line
-    env = args.env or args.exact_env
+    activate_environment(args.env, args.env_dir, args.use_env_repo)
-    if env:
-        spack.environment.activate(env, args.exact_env is not None)
-    else:
-        env = os.environ.get(spack.environment.spack_env_var)
-        if env:
-            spack.environment.activate(env, False)
 
     # make spack.config aware of any command line configuration scopes
     if args.config_scopes:

lib/spack/spack/test/cmd/env.py

@@ -27,7 +27,7 @@
 
 
 def test_add():
-    e = ev.Environment('test')
+    e = ev.create('test')
     e.add('mpileaks')
     assert Spec('mpileaks') in e.user_specs
 
@@ -64,7 +64,7 @@ def test_env_destroy():
 
 
 def test_concretize():
-    e = ev.Environment('test')
+    e = ev.create('test')
     e.add('mpileaks')
     e.concretize()
     env_specs = e._get_environment_specs()
@@ -72,7 +72,7 @@ def test_concretize():
 
 
 def test_env_install_all(install_mockery, mock_fetch):
-    e = ev.Environment('test')
+    e = ev.create('test')
     e.add('cmake-client')
     e.concretize()
     e.install_all()
@@ -81,11 +81,12 @@ def test_env_install_all(install_mockery, mock_fetch):
     assert spec.package.installed
 
 
-def test_env_install(install_mockery, mock_fetch):
+def test_env_install_single_spec(install_mockery, mock_fetch):
     env('create', 'test')
     install = SpackCommand('install')
 
-    with ev.env_context('test'):
+    e = ev.read('test')
+    with e:
         install('cmake-client')
 
     e = ev.read('test')
@@ -94,8 +95,20 @@ def test_env_install(install_mockery, mock_fetch):
     assert e.specs_by_hash[e.concretized_order[0]].name == 'cmake-client'
 
 
+def test_env_install_same_spec_twice(install_mockery, mock_fetch, capfd):
+    env('create', 'test')
+    install = SpackCommand('install')
+
+    e = ev.read('test')
+    with capfd.disabled():
+        with e:
+            install('cmake-client')
+            out = install('cmake-client')
+            assert 'is already installed in' in out
+
+
 def test_remove_after_concretize():
-    e = ev.Environment('test')
+    e = ev.create('test')
 
     e.add('mpileaks')
     e.concretize()
@@ -127,7 +140,7 @@ def test_remove_command():
 
 
 def test_reset_compiler():
-    e = ev.Environment('test')
+    e = ev.create('test')
     e.add('mpileaks')
     e.concretize()
 
@@ -142,7 +155,7 @@ def test_reset_compiler():
 
 
 def test_environment_status():
-    e = ev.Environment('test')
+    e = ev.create('test')
     e.add('mpileaks')
     e.concretize()
     e.add('python')
@@ -156,7 +169,7 @@ def test_environment_status():
 
 
 def test_upgrade_dependency():
-    e = ev.Environment('test')
+    e = ev.create('test')
     e.add('mpileaks ^callpath@0.9')
     e.concretize()
 
@@ -169,36 +182,38 @@ def test_upgrade_dependency():
 
 
 def test_to_lockfile_dict():
-    e = ev.Environment('test')
+    e = ev.create('test')
     e.add('mpileaks')
     e.concretize()
     context_dict = e._to_lockfile_dict()
 
-    e_copy = ev.Environment('test_copy')
+    e_copy = ev.create('test_copy')
 
     e_copy._read_lockfile_dict(context_dict)
     assert e.specs_by_hash == e_copy.specs_by_hash
 
 
 def test_env_repo():
-    e = ev.Environment('testx')
+    e = ev.create('test')
     e.add('mpileaks')
     _env_concretize(e)
 
-    package = e.repo.get(spack.spec.Spec('mpileaks'))
+    package = e.repo.get('mpileaks')
+    assert package.name == 'mpileaks'
     assert package.namespace == 'spack.pkg.builtin.mock'
 
 
 def test_user_removed_spec():
-    """Ensure a user can remove from any position in the env.yaml file."""
+    """Ensure a user can remove from any position in the spack.yaml file."""
-    initial_yaml = """\
+    initial_yaml = StringIO("""\
 env:
   specs:
   - mpileaks
   - hypre
   - libelf
-"""
+""")
-    before = ev.Environment('test', initial_yaml)
+
+    before = ev.create('test', initial_yaml)
     before.concretize()
     before.write()
 
@@ -222,8 +237,57 @@ def test_user_removed_spec():
     assert not any(x.name == 'hypre' for x in env_specs)
 
 
+def test_init_from_lockfile(tmpdir):
+    """Test that an environment can be instantiated from a lockfile."""
+    initial_yaml = StringIO("""\
+env:
+  specs:
+  - mpileaks
+  - hypre
+  - libelf
+""")
+    e1 = ev.create('test', initial_yaml)
+    e1.concretize()
+    e1.write()
+
+    e2 = ev.Environment(str(tmpdir), e1.lock_path)
+
+    for s1, s2 in zip(e1.user_specs, e2.user_specs):
+        assert s1 == s2
+
+    for h1, h2 in zip(e1.concretized_order, e2.concretized_order):
+        assert h1 == h2
+        assert e1.specs_by_hash[h1] == e2.specs_by_hash[h2]
+
+    for s1, s2 in zip(e1.concretized_user_specs, e2.concretized_user_specs):
+        assert s1 == s2
+
+
+def test_init_from_yaml(tmpdir):
+    """Test that an environment can be instantiated from a lockfile."""
+    initial_yaml = StringIO("""\
+env:
+  specs:
+  - mpileaks
+  - hypre
+  - libelf
+""")
+    e1 = ev.create('test', initial_yaml)
+    e1.concretize()
+    e1.write()
+
+    e2 = ev.Environment(str(tmpdir), e1.manifest_path)
+
+    for s1, s2 in zip(e1.user_specs, e2.user_specs):
+        assert s1 == s2
+
+    assert not e2.concretized_order
+    assert not e2.concretized_user_specs
+    assert not e2.specs_by_hash
+
+
 def test_init_with_file_and_remove(tmpdir):
-    """Ensure a user can remove from any position in the env.yaml file."""
+    """Ensure a user can remove from any position in the spack.yaml file."""
     path = tmpdir.join('spack.yaml')
 
     with tmpdir.as_cwd():
@@ -259,7 +323,7 @@ def test_env_with_config():
 """
     spack.package_prefs.PackagePrefs.clear_caches()
 
-    _env_create('test', test_config)
+    _env_create('test', StringIO(test_config))
 
     e = ev.read('test')
     ev.prepare_config_scope(e)
@@ -279,11 +343,9 @@ def test_env_with_included_config_file():
 """
     spack.package_prefs.PackagePrefs.clear_caches()
 
-    _env_create('test', test_config)
+    _env_create('test', StringIO(test_config))
-
     e = ev.read('test')
 
-    print(e.path)
     with open(os.path.join(e.path, 'included-config.yaml'), 'w') as f:
         f.write("""\
 packages:
@@ -309,7 +371,7 @@ def test_env_with_included_config_scope():
 """ % config_scope_path
 
     spack.package_prefs.PackagePrefs.clear_caches()
-    _env_create('test', test_config)
+    _env_create('test', StringIO(test_config))
 
     e = ev.read('test')
 
@@ -339,13 +401,12 @@ def test_env_config_precedence():
   specs:
   - mpileaks
 """
+
     spack.package_prefs.PackagePrefs.clear_caches()
 
-    _env_create('test', test_config)
+    _env_create('test', StringIO(test_config))
-
     e = ev.read('test')
 
-    print(e.path)
     with open(os.path.join(e.path, 'included-config.yaml'), 'w') as f:
         f.write("""\
 packages:
@@ -430,8 +491,6 @@ def test_env_commands_die_with_no_env_arg():
     # these have an optional env arg and raise errors via tty.die
     with pytest.raises(spack.main.SpackCommandError):
         env('concretize')
-    with pytest.raises(spack.main.SpackCommandError):
-        env('status')
     with pytest.raises(spack.main.SpackCommandError):
         env('loads')
     with pytest.raises(spack.main.SpackCommandError):
@@ -444,3 +503,7 @@ def test_env_commands_die_with_no_env_arg():
         env('add')
     with pytest.raises(spack.main.SpackCommandError):
         env('remove')
+
+    # This should NOT raise an error with no environment
+    # it just tells the user there isn't an environment
+    env('status')

share/spack/csh/spack.csh

@@ -77,8 +77,8 @@ case env:
                 set _sp_env_arg=""
                 [ $#_sp_args -gt 1 ] && set _sp_env_arg = ($_sp_args[2])
 
-                if ( "$_sp_env_arg" == "" || "$_sp_env_arg" =~ "-*" ) then
+                if ( "$_sp_env_arg" == "" || "$_sp_args" =~ "*--sh*" || "$_sp_args" =~ "*--csh*" || "$_sp_args" =~ "*-h*" ) then
-                    # no args or does not start with -: just execute
+                    # no args or args contain -h/--help, --sh, or --csh: just execute
                     \spack $_sp_flags env $_sp_args
                 else
                     shift _sp_args  # consume 'activate' or 'deactivate'

share/spack/setup-env.sh

@@ -101,8 +101,9 @@ function spack {
             else
                 case $_sp_arg in
                     activate)
-                        if [ -z "$1" -o "${1#-}" != "$1" ]; then
+                        _a="$@"
-                            # no args or does not start with -: just execute
+                        if [ -z "$1" -o "${_a#*--sh}" != "$_a" -o "${_a#*--csh}" != "$_a" -o "${_a#*-h}" != "$_a" ]; then
+                            # no args or args contain -h/--help, --sh, or --csh: just execute
                             command spack "${args[@]}"
                         else
                             # actual call to activate: source the output