From 5184edb15f2cf260d561d8fcb7bf56ad147b6555 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@googlemail.com>
Date: Mon, 14 Aug 2017 20:29:18 +0200
Subject: Added a custom action for --clean and --dirty. (#5081)

The action `CleanOrDirtyAction` has been added. It sets the default
value for `dest` to `spack.dirty`, and changes it according to the flags
passed via command line. Added unit tests to check that the arguments
are parsed correctly. Removed lines in `PackageBase` that were setting
the default value of dirty according to what was in the configuration.
---
 lib/spack/spack/cmd/common/arguments.py | 33 +++++++++++++++++++++++++++++----
 lib/spack/spack/package.py              |  4 ----
 lib/spack/spack/test/cmd/install.py     | 24 +++++++++++++++++++++++-
 3 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/lib/spack/spack/cmd/common/arguments.py b/lib/spack/spack/cmd/common/arguments.py
index 46ff44c84a..d1b0418770 100644
--- a/lib/spack/spack/cmd/common/arguments.py
+++ b/lib/spack/spack/cmd/common/arguments.py
@@ -74,6 +74,23 @@ class ConstraintAction(argparse.Action):
         return sorted(specs)
 
 
+class CleanOrDirtyAction(argparse.Action):
+    """Sets the dirty flag in the current namespace"""
+
+    def __init__(self, *args, **kwargs):
+        kwargs['default'] = spack.dirty
+        super(CleanOrDirtyAction, self).__init__(*args, **kwargs)
+
+    def __call__(self, parser, namespace, values, option_string=None):
+        if option_string == '--clean':
+            setattr(namespace, self.dest, False)
+        elif option_string == '--dirty':
+            setattr(namespace, self.dest, True)
+        else:
+            msg = 'expected "--dirty" or "--clean" [got {0} instead]'
+            raise argparse.ArgumentError(msg.format(option_string))
+
+
 _arguments['constraint'] = Args(
     'constraint', nargs=argparse.REMAINDER, action=ConstraintAction,
     help='constraint to select a subset of installed packages')
@@ -93,12 +110,20 @@ _arguments['recurse_dependencies'] = Args(
     help='recursively traverse spec dependencies')
 
 _arguments['clean'] = Args(
-    '--clean', action='store_false', dest='dirty',
-    help='clean environment before installing package')
+    '--clean',
+    action=CleanOrDirtyAction,
+    dest='dirty',
+    help='clean environment before installing package',
+    nargs=0
+)
 
 _arguments['dirty'] = Args(
-    '--dirty', action='store_true', dest='dirty',
-    help='do NOT clean environment before installing')
+    '--dirty',
+    action=CleanOrDirtyAction,
+    dest='dirty',
+    help='do NOT clean environment before installing',
+    nargs=0
+)
 
 _arguments['long'] = Args(
     '-l', '--long', action='store_true',
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index e8e741fefa..86df0ec947 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1218,10 +1218,6 @@ class PackageBase(with_metaclass(PackageMeta, object)):
                 rec = spack.store.db.get_record(self.spec)
                 return self._update_explicit_entry_in_db(rec, explicit)
 
-        # Dirty argument takes precedence over dirty config setting.
-        if dirty is None:
-            dirty = spack.dirty
-
         self._do_install_pop_kwargs(kwargs)
 
         # First, install dependencies recursively.
diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py
index 951d9d85d7..5886d602a2 100644
--- a/lib/spack/spack/test/cmd/install.py
+++ b/lib/spack/spack/test/cmd/install.py
@@ -22,12 +22,24 @@
 # License along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
-from spack.main import SpackCommand
+import argparse
+
+import pytest
 
+import spack.cmd.install
+from spack.main import SpackCommand
 
 install = SpackCommand('install')
 
 
+@pytest.fixture(scope='module')
+def parser():
+    """Returns the parser for the module command"""
+    parser = argparse.ArgumentParser()
+    spack.cmd.install.setup_parser(parser)
+    return parser
+
+
 def _install_package_and_dependency(
         tmpdir, builtin_mock, mock_archive, mock_fetch, config,
         install_mockery):
@@ -64,3 +76,13 @@ def test_install_package_already_installed(
 
     skipped = [line for line in content.split('\n') if 'skipped' in line]
     assert len(skipped) == 2
+
+
+@pytest.mark.parametrize('arguments,expected', [
+    ([], spack.dirty),  # The default read from configuration file
+    (['--clean'], False),
+    (['--dirty'], True),
+])
+def test_install_dirty_flag(parser, arguments, expected):
+    args = parser.parse_args(arguments)
+    assert args.dirty == expected
-- 
cgit v1.2.3-60-g2f50