pw_env_setup: add command() to Environment

Add command() function to Environment class. Also add hash() function so
hashing can happen when the caller wants it. Change 'pw doctor' to use
command() function.

Minor tweaks to environment.py, mostly making classes public for callers
of a future conditional() method to use, but that's not needed quite
yet. Also adding docstrings.

Changed bootstrap.sh to say "error during activate" when in "activate"
mode.

Change-Id: Ib37563caf3f9c437bdb2f7fcf491ba51ea8fba5d
Bug: 147
diff --git a/bootstrap.sh b/bootstrap.sh
index 7c23487..dbca00b 100644
--- a/bootstrap.sh
+++ b/bootstrap.sh
@@ -85,6 +85,8 @@
 _PW_IS_BOOTSTRAP=$?
 
 if [ $_PW_IS_BOOTSTRAP -eq 0 ]; then
+  _PW_NAME="bootstrap"
+
   if [ -z "$PW_ENVSETUP_QUIET" ]; then
     _pw_green "  BOOTSTRAP! Bootstrap may take a few minutes; please be patient.\n"
   fi
@@ -104,6 +106,8 @@
 
   $PYTHON $PW_ROOT/pw_env_setup/py/pw_env_setup/env_setup.py --shell-file $SETUP_SH
 else
+  _PW_NAME="activate"
+
   if [ -z "$PW_ENVSETUP_QUIET" ]; then
     _pw_green "  ACTIVATOR! This sets your shell environment variables.\n"
   fi
@@ -112,18 +116,22 @@
 if [ -f $SETUP_SH ]; then
   . $SETUP_SH
 
-  if [ $_PW_IS_BOOTSTRAP -eq 0 ] && [ -z "$PW_ENVSETUP_QUIET" ]; then
-    echo
-    echo "To activate this environment in the future, run this in your "
-    echo "terminal:"
-    echo
-    _pw_green "  . ./activate.sh\n"
+  if [ $? == 0 ]; then
+    if [ $_PW_IS_BOOTSTRAP -eq 0 ] && [ -z "$PW_ENVSETUP_QUIET" ]; then
+      echo "To activate this environment in the future, run this in your "
+      echo "terminal:"
+      echo
+      _pw_green "  . ./activate.sh\n"
+    fi
+  else
+    _pw_red "Error during $_PW_NAME--see messages above."
   fi
 else
-  _pw_red "Error during bootstrap--see messages above."
+  _pw_red "Error during $_PW_NAME--see messages above."
 fi
 
 unset _PW_IS_BOOTSTRAP
+unset _PW_NAME
 unset _PIGWEED_BANNER
 unset _pw_abspath
 unset _pw_red
diff --git a/pw_env_setup/py/pw_env_setup/env_setup.py b/pw_env_setup/py/pw_env_setup/env_setup.py
index cdb73b9..ba90542 100755
--- a/pw_env_setup/py/pw_env_setup/env_setup.py
+++ b/pw_env_setup/py/pw_env_setup/env_setup.py
@@ -197,9 +197,23 @@
         self._log('')
         self._env.echo('')
 
+        self._env.hash()
+
+        self._env.echo(Color.bold('Sanity checking the environment:'))
+        self._env.echo()
+
+        log_level = 'warn' if 'PW_ENVSETUP_QUIET' in os.environ else 'info'
+        doctor = ['pw', '--no-banner', '--loglevel', log_level, 'doctor']
+
+        self._env.command(doctor)
+        self._env.echo()
+
+        self._env.echo(
+            Color.bold('Environment looks good, you are ready to go!'))
+        self._env.echo()
+
         with open(self._shell_file, 'w') as outs:
             self._env.write(outs)
-            self.write_sanity_check(outs)
 
         return 0
 
@@ -281,42 +295,6 @@
 
         return _Result(_Result.Status.DONE)
 
-    def write_sanity_check(self, fd):
-        """Call pw doctor after setting environment variables."""
-
-        echo_empty = 'echo.' if self._is_windows else 'echo'
-
-        if not self._quiet:
-            # Not quoting args to echo because Windows will treat them as if
-            # they're already quoted and Linux will just space-separate them.
-            fd.write('echo {}\n'.format(
-                Color.bold('Sanity checking the environment:')))
-            fd.write('{}\n'.format(echo_empty))
-
-        log_level = 'warn' if 'PW_ENVSETUP_QUIET' in os.environ else 'info'
-        doctor = ' '.join(
-            ['pw', '--no-banner', '--loglevel', log_level, 'doctor'])
-
-        if self._is_windows:
-            fd.write('{}\n'.format(doctor))
-            fd.write('if %ERRORLEVEL% EQU 0 (\n')
-        else:
-            fd.write('if {}; then\n'.format(doctor))
-
-        if not self._quiet:
-            fd.write('  {}\n'.format(echo_empty))
-            # Again, not quoting args to echo.
-            fd.write('  echo {}\n'.format(
-                Color.bold('Environment looks good, you are ready to go!')))
-
-        if self._is_windows:
-            fd.write(')\n')
-        else:
-            # If PW_ENVSETUP_QUIET is set, there might not be anything inside
-            # the if which is an error. Always echo nothing at the end.
-            fd.write('  echo -n\n')
-            fd.write('fi\n')
-
 
 def parse(argv=None):
     """Parse command-line arguments."""
diff --git a/pw_env_setup/py/pw_env_setup/environment.py b/pw_env_setup/py/pw_env_setup/environment.py
index 6671f22..d363e3e 100644
--- a/pw_env_setup/py/pw_env_setup/environment.py
+++ b/pw_env_setup/py/pw_env_setup/environment.py
@@ -94,7 +94,8 @@
             env.pop(self.name, None)
 
 
-class _Set(_VariableAction):
+class Set(_VariableAction):
+    """Set a variable."""
     def write(self, outs, windows=(os.name == 'nt')):
         if windows:
             outs.write('set {name}={value}\n'.format(**vars(self)))
@@ -106,11 +107,12 @@
         env[self.name] = self.value
 
 
-class _Clear(_VariableAction):
+class Clear(_VariableAction):
+    """Remove a variable from the environment."""
     def __init__(self, *args, **kwargs):
         kwargs['value'] = ''
         kwargs['allow_empty_values'] = True
-        super(_Clear, self).__init__(*args, **kwargs)
+        super(Clear, self).__init__(*args, **kwargs)
 
     def write(self, outs, windows=(os.name == 'nt')):
         if windows:
@@ -123,9 +125,10 @@
             del env[self.name]
 
 
-class _Remove(_VariableAction):
+class Remove(_VariableAction):
+    """Remove a value from a PATH-like variable."""
     def __init__(self, name, value, pathsep, *args, **kwargs):
-        super(_Remove, self).__init__(name, value, *args, **kwargs)
+        super(Remove, self).__init__(name, value, *args, **kwargs)
         self._pathsep = pathsep
 
     def write(self, outs, windows=(os.name == 'nt')):
@@ -167,9 +170,10 @@
         raise BadVariableValue('"{}" contains "="'.format(action.value))
 
 
-class _Prepend(_VariableAction):
+class Prepend(_VariableAction):
+    """Prepend a value to a PATH-like variable."""
     def __init__(self, name, value, join, *args, **kwargs):
-        super(_Prepend, self).__init__(name, value, *args, **kwargs)
+        super(Prepend, self).__init__(name, value, *args, **kwargs)
         self._join = join
 
     def write(self, outs, windows=(os.name == 'nt')):
@@ -185,13 +189,14 @@
         env[self.name] = self._join(self.value, env.get(self.name, ''))
 
     def _check(self):
-        super(_Prepend, self)._check()
+        super(Prepend, self)._check()
         _append_prepend_check(self)
 
 
-class _Append(_VariableAction):
+class Append(_VariableAction):
+    """Append a value to a PATH-like variable. (Uncommon, see Prepend.)"""
     def __init__(self, name, value, join, *args, **kwargs):
-        super(_Append, self).__init__(name, value, *args, **kwargs)
+        super(Append, self).__init__(name, value, *args, **kwargs)
         self._join = join
 
     def write(self, outs, windows=(os.name == 'nt')):
@@ -207,7 +212,7 @@
         env[self.name] = self._join(env.get(self.name, ''), self.value)
 
     def _check(self):
-        super(_Append, self)._check()
+        super(Append, self)._check()
         _append_prepend_check(self)
 
 
@@ -215,12 +220,13 @@
     pass
 
 
-class _Echo(_Action):
+class Echo(_Action):
+    """Echo a value to the terminal."""
     def __init__(self, value, newline, *args, **kwargs):
         # These values act funny on Windows.
         if value.lower() in ('off', 'on'):
             raise BadEchoValue(value)
-        super(_Echo, self).__init__(*args, **kwargs)
+        super(Echo, self).__init__(*args, **kwargs)
         self.value = value
         self._newline = newline
 
@@ -229,7 +235,10 @@
         # pass the command line as is without parsing, so quoting is wrong.
         if windows:
             if self._newline:
-                outs.write('echo {}\n'.format(self.value))
+                if not self.value:
+                    outs.write('echo.\n')
+                else:
+                    outs.write('echo {}\n'.format(self.value))
             else:
                 outs.write('<nul set /p="{}"\n'.format(self.value))
         else:
@@ -245,9 +254,10 @@
         del env  # Unused.
 
 
-class _Comment(_Action):
+class Comment(_Action):
+    """Add a comment to the init script."""
     def __init__(self, value, *args, **kwargs):
-        super(_Comment, self).__init__(*args, **kwargs)
+        super(Comment, self).__init__(*args, **kwargs)
         self.value = value
 
     def write(self, outs, windows=(os.name == 'nt')):
@@ -259,7 +269,30 @@
         del env  # Unused.
 
 
-class _BlankLine(_Action):
+class Command(_Action):
+    """Run a command."""
+    def __init__(self, command, *args, **kwargs):
+        exit_on_error = kwargs.pop('exit_on_error', True)
+        super(Command, self).__init__(*args, **kwargs)
+        assert isinstance(command, (list, tuple))
+        self.command = command
+        self.exit_on_error = exit_on_error
+
+    def write(self, outs, windows=(os.name == 'nt')):
+        # TODO(mohrr) use shlex.quote here?
+        outs.write('{}\n'.format(' '.join(self.command)))
+        if not self.exit_on_error:
+            return
+
+        if windows:
+            pass  # TODO(pwbug/147) Fill in.
+        else:
+            # Assume failing command produced relevant output.
+            outs.write('if [ $? != 0 ]; then\n  return 1\nfi\n')
+
+
+class BlankLine(_Action):
+    """Write a blank line to the init script."""
     def write(  # pylint: disable=no-self-use
         self, outs, windows=(os.name == 'nt')):
         del windows  # Unused.
@@ -269,6 +302,24 @@
         del env  # Unused.
 
 
+class Hash(_Action):
+    def write(self, outs, windows=(os.name == 'nt')):  # pylint: disable=no-self-use
+        if windows:
+            return
+
+        outs.write('''
+# This should detect bash and zsh, which have a hash command that must be
+# called to get it to forget past commands. Without forgetting past
+# commands the $PATH changes we made may not be respected.
+if [ -n "${BASH:-}" -o -n "${ZSH_VERSION:-}" ] ; then
+  hash -r\n
+fi
+''')
+
+    def apply(self, env):  # pylint: disable=no-self-use
+        del env  # Unused.
+
+
 # TODO(mohrr) remove disable=useless-object-inheritance once in Python 3.
 # pylint: disable=useless-object-inheritance
 class Environment(object):
@@ -306,13 +357,15 @@
     # operations should not invoke each other (this is why _remove() exists).
 
     def set(self, name, value):
+        """Set a variable."""
         name = self.normalize_key(name)
-        self._actions.append(_Set(name, value))
+        self._actions.append(Set(name, value))
         self._blankline()
 
     def clear(self, name):
+        """Remove a variable."""
         name = self.normalize_key(name)
-        self._actions.append(_Clear(name))
+        self._actions.append(Clear(name))
         self._blankline()
 
     def _remove(self, name, value):
@@ -320,44 +373,60 @@
 
         name = self.normalize_key(name)
         if self.get(name, None):
-            self._actions.append(_Remove(name, value, self._pathsep))
+            self._actions.append(Remove(name, value, self._pathsep))
 
     def remove(self, name, value):
+        """Remove a value from a PATH-like variable."""
         self._remove(name, value)
         self._blankline()
 
     def append(self, name, value):
-        """Add a value to the end of a variable. Rarely used, see prepend()."""
+        """Add a value to a PATH-like variable. Rarely used, see prepend()."""
 
         name = self.normalize_key(name)
         if self.get(name, None):
             self._remove(name, value)
-            self._actions.append(_Append(name, value, self._join))
+            self._actions.append(Append(name, value, self._join))
         else:
-            self._actions.append(_Set(name, value))
+            self._actions.append(Set(name, value))
         self._blankline()
 
     def prepend(self, name, value):
-        """Add a value to the beginning of a variable."""
+        """Add a value to the beginning of a PATH-like variable."""
 
         name = self.normalize_key(name)
         if self.get(name, None):
             self._remove(name, value)
-            self._actions.append(_Prepend(name, value, self._join))
+            self._actions.append(Prepend(name, value, self._join))
         else:
-            self._actions.append(_Set(name, value))
+            self._actions.append(Set(name, value))
         self._blankline()
 
-    def echo(self, value, newline=True):
-        self._actions.append(_Echo(value, newline))
-        self._blankline()
+    def echo(self, value='', newline=True):
+        """Echo a value to the terminal."""
+
+        self._actions.append(Echo(value, newline))
+        if value:
+            self._blankline()
 
     def comment(self, comment):
-        self._actions.append(_Comment(comment))
+        """Add a comment to the init script."""
+        self._actions.append(Comment(comment))
+        self._blankline()
+
+    def command(self, command, exit_on_error=True):
+        """Run a command."""
+
+        self._actions.append(Command(command, exit_on_error=exit_on_error))
         self._blankline()
 
     def _blankline(self):
-        self._actions.append(_BlankLine())
+        self._actions.append(BlankLine())
+
+    def hash(self):
+        """If required by the shell rehash the PATH variable."""
+        self._actions.append(Hash())
+        self._blankline()
 
     def write(self, outs):
         """Writes a shell init script to outs."""
@@ -367,16 +436,6 @@
         for action in self._actions:
             action.write(outs, windows=self._windows)
 
-        if not self._windows:
-            outs.write(
-                '# This should detect bash and zsh, which have a hash \n'
-                '# command that must be called to get it to forget past \n'
-                '# commands. Without forgetting past commands the $PATH \n'
-                '# changes we made may not be respected.\n'
-                'if [ -n "${BASH:-}" -o -n "${ZSH_VERSION:-}" ] ; then\n'
-                '    hash -r\n'
-                'fi\n')
-
     @contextlib.contextmanager
     def __call__(self, export=True):
         """Set environment as if this was written to a file and sourced.