Repository: JonathonReinhart/scuba

Issue:
Enhanced aliases
Allow complex, multi-line aliases, using the [common script schema](https://github.com/JonathonReinhart/scuba/blob/master/doc/yaml-reference.md#common-script-schema), like this:

```yaml
image: foo
aliases:
  build:
    script:
      - echo "build script starting"
      - make -j4
      - other stuff
```

When an alias is used, a script will be generated from it and injected into the container for execution.

*If an alias is a multiline script, then no additional arguments will be allowed in the scuba invocation.*

A somewhat important change made by this is that *all aliases will now be executed by a shell*, instead of directly, as before. This alone may warrant a v2.0 bump, and we should then probably execute everything (even non-aliases) via the shell.

Discussion:

Patch:
diff --git a/example/alias_multiline/.scuba.yml b/example/alias_multiline/.scuba.yml
new file mode 100644
index 0000000..90d7d1b
--- /dev/null
+++ b/example/alias_multiline/.scuba.yml
@@ -0,0 +1,9 @@
+image: !from_yaml ../common.yml image
+aliases:
+  simple: echo 'a simple alias'
+
+  complex:
+    script:
+      - echo 'this is a complex script'
+      - date
+      - uname -a
diff --git a/example/alias_multiline/run_example.sh b/example/alias_multiline/run_example.sh
new file mode 100755
index 0000000..019c32d
--- /dev/null
+++ b/example/alias_multiline/run_example.sh
@@ -0,0 +1,3 @@
+#!/bin/bash
+cd $(dirname $0)
+scuba complex
diff --git a/example/common.yml b/example/common.yml
new file mode 100644
index 0000000..cd78a52
--- /dev/null
+++ b/example/common.yml
@@ -0,0 +1,1 @@
+image: debian:8.2
diff --git a/example/run_all.sh b/example/run_all.sh
index c734038..9cabda1 100755
--- a/example/run_all.sh
+++ b/example/run_all.sh
@@ -7,3 +7,4 @@ cd $(dirname $0)
 ./external_yaml_simple/run_example.sh
 ./external_yaml_nested/run_example.sh
 ./scubainit_hooks/run_example.sh
+./alias_multiline/run_example.sh
diff --git a/scuba/__main__.py b/scuba/__main__.py
index a1e2c7a..a0f4ef3 100644
--- a/scuba/__main__.py
+++ b/scuba/__main__.py
@@ -230,21 +230,24 @@ class ScubaDive(object):
             self.add_option('--tty')
 
         # Process any aliases
-        cmd = self.config.process_command(self.user_command)
+        try:
+            script = self.config.process_command(self.user_command)
+        except ConfigError as cfgerr:
+            raise ScubaError(str(cfgerr))
 
         '''
         Normally, if the user provides no command to "docker run", the image's
         default CMD is run. Because we set the entrypiont, scuba must emulate the
         default behavior itself.
         '''
-        if len(cmd) == 0:
+        if len(script) == 0:
             # No user-provided command; we want to run the image's default command
             verbose_msg('No user command; getting command from image')
             try:
-                cmd = get_image_command(self.config.image)
+                script = [get_image_command(self.config.image)]
             except DockerError as e:
                 raise ScubaError(str(e))
-            verbose_msg('{0} Cmd: "{1}"'.format(self.config.image, cmd))
+            verbose_msg('{0} Cmd: "{1}"'.format(self.config.image, script[0]))
 
         # The user command is executed via a generated shell script
         with self.open_scubadir_file('command.sh', 'wt') as f:
@@ -252,7 +255,9 @@ class ScubaDive(object):
             writeln(f, '#!/bin/sh')
             writeln(f, '# Auto-generated from scuba')
             writeln(f, 'set -e')
-            writeln(f, shell_quote_cmd(cmd))
+            for cmd in script:
+                writeln(f, shell_quote_cmd(cmd))
+
 
 
     def open_scubadir_file(self, name, mode):
diff --git a/scuba/config.py b/scuba/config.py
index 250b92e..9a7fd41 100644
--- a/scuba/config.py
+++ b/scuba/config.py
@@ -122,10 +122,7 @@ class ScubaConfig(object):
 
         self._image = data['image']
 
-        self._aliases = {}
-        for alias, cmdstr in data.get('aliases', {}).items():
-            self._aliases[alias] = shlex_split(cmdstr)
-
+        self._load_aliases(data)
         self._load_hooks(data)
 
 
@@ -169,6 +166,13 @@ class ScubaConfig(object):
         raise ConfigError("{0}: must be string or dict".format(name))
 
 
+    def _load_aliases(self, data):
+        self._aliases = {}
+
+        for name, node in data.get('aliases', {}).items():
+            self._aliases[name] = [shlex_split(cmd) for cmd in self._process_script(node, name)]
+
+
     def _load_hooks(self, data):
         self._hooks = {}
 
@@ -193,13 +197,30 @@ class ScubaConfig(object):
 
 
     def process_command(self, command):
-        if command:
-            rep = self.aliases.get(command[0])
-            if rep:
-                command.pop(0)
-                command = rep + command
+        '''Processes a user command using aliases
+
+        Arguments:
+            command     A user command list (e.g. argv)
+
+        Returns: A "script" - a list of command lists
+        '''
+        if not command:
+            return command
+
+        script = self.aliases.get(command[0])
+        if not script:
+            return [command]
+
+        if len(command) > 1:
+            # If an alias is a multiline script, then no additional
+            # arguments will be allowed in the scuba invocation.
+            if len(script) > 1:
+                raise ConfigError('Additional arguments not allowed with multi-line aliases')
+
+            command.pop(0)
+            return [script[0] + command]
 
-        return command
+        return script
 
 
 def load_config(path):


Test patch:
diff --git a/tests/test_config.py b/tests/test_config.py
index fb14e0d..3843dad 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -115,8 +115,8 @@ class TestConfig(TestCase):
         config = scuba.config.load_config('.scuba.yml')
         assert_equals(config.image, 'busybox')
         assert_equals(len(config.aliases), 2)
-        assert_seq_equal(config.aliases['foo'], ['bar'])
-        assert_seq_equal(config.aliases['snap'], ['crackle', 'pop'])
+        assert_seq_equal(config.aliases['foo'], [['bar']])
+        assert_seq_equal(config.aliases['snap'], [['crackle', 'pop']])
 
 
 
@@ -182,13 +182,22 @@ class TestConfig(TestCase):
     ######################################################################
     # process_command
 
+    def test_process_command_empty(self):
+        '''process_command handles no aliases and an empty command'''
+        cfg = scuba.config.ScubaConfig(
+                image = 'na',
+                )
+        result = cfg.process_command([])
+        assert_equal(result, [])
+
+
     def test_process_command_no_aliases(self):
         '''process_command handles no aliases'''
         cfg = scuba.config.ScubaConfig(
                 image = 'na',
                 )
         result = cfg.process_command(['cmd', 'arg1', 'arg2'])
-        assert_equal(result, ['cmd', 'arg1', 'arg2'])
+        assert_equal(result, [['cmd', 'arg1', 'arg2']])
 
     def test_process_command_aliases_unused(self):
         '''process_command handles unused aliases'''
@@ -200,7 +209,7 @@ class TestConfig(TestCase):
                     ),
                 )
         result = cfg.process_command(['cmd', 'arg1', 'arg2'])
-        assert_equal(result, ['cmd', 'arg1', 'arg2'])
+        assert_equal(result, [['cmd', 'arg1', 'arg2']])
 
     def test_process_command_aliases_used_noargs(self):
         '''process_command handles aliases with no args'''
@@ -212,7 +221,7 @@ class TestConfig(TestCase):
                     ),
                 )
         result = cfg.process_command(['apple', 'arg1', 'arg2'])
-        assert_equal(result, ['banana', 'arg1', 'arg2'])
+        assert_equal(result, [['banana', 'arg1', 'arg2']])
 
     def test_process_command_aliases_used_withargs(self):
         '''process_command handles aliases with args'''
@@ -224,7 +233,40 @@ class TestConfig(TestCase):
                     ),
                 )
         result = cfg.process_command(['apple', 'arg1', 'arg2'])
-        assert_equal(result, ['banana', 'cherry', 'pie is good', 'arg1', 'arg2'])
+        assert_equal(result, [['banana', 'cherry', 'pie is good', 'arg1', 'arg2']])
+
+    def test_process_command_multiline_aliases_used(self):
+        '''process_command handles multiline aliases'''
+        cfg = scuba.config.ScubaConfig(
+                image = 'na',
+                aliases = dict(
+                    apple = dict(script=[
+                        'banana cherry "pie is good"',
+                        'so is peach',
+                    ]),
+                    cat = 'dog',
+                    ),
+                )
+        result = cfg.process_command(['apple'])
+        assert_equal(result, [
+            ['banana', 'cherry', 'pie is good'],
+            ['so', 'is', 'peach'],
+        ])
+
+    def test_process_command_multiline_aliases_forbid_user_args(self):
+        '''process_command raises ConfigError when args are specified with multiline aliases'''
+        cfg = scuba.config.ScubaConfig(
+                image = 'na',
+                aliases = dict(
+                    apple = dict(script=[
+                        'banana cherry "pie is good"',
+                        'so is peach',
+                    ]),
+                    cat = 'dog',
+                    ),
+                )
+        assert_raises(scuba.config.ConfigError, cfg.process_command, ['apple', 'ARGS', 'NOT ALLOWED'])
+
 
     ############################################################################
     # Hooks
diff --git a/tests/test_main.py b/tests/test_main.py
index 7207a98..474af21 100644
--- a/tests/test_main.py
+++ b/tests/test_main.py
@@ -160,6 +160,23 @@ class TestMain(TestCase):
         # ConfigError -> exit(128)
         self.run_scuba([], 128)
 
+    def test_multiline_alias_no_args_error(self):
+        '''Verify config errors from passing arguments to multi-line alias are caught'''
+        with open('.scuba.yml', 'w') as f:
+            f.write('''
+                image: {image}
+                aliases:
+                  multi:
+                    script:
+                      - echo multi
+                      - echo line
+                      - echo alias
+                '''.format(image=DOCKER_IMAGE))
+
+        # ConfigError -> exit(128)
+        self.run_scuba(['multi', 'with', 'args'], 128)
+
+
 
     def test_version(self):
         '''Verify scuba prints its version for -v'''