Merge branch 'bf/sudo-env' into 'master'

Bf/sudo env

See merge request fsl/conda/installer!28

fslinstaller.py

@@ -48,7 +48,7 @@ log = logging.getLogger(__name__)
 __absfile__ = op.abspath(__file__).rstrip('c')
 
 
-__version__ = '1.4.3'
+__version__ = '1.5.0'
 """Installer script version number. This must be updated
 whenever a new version of the installer script is released.
 """
@@ -749,6 +749,30 @@ def tempdir(override_dir=None):
             shutil.rmtree(tmpdir)
 
 
+@contextlib.contextmanager
+def tempfilename(permissions=None, delete=True):
+    """Returns a context manager which creates a temporary file, yields its
+    name, then deletes the file on exit.
+    """
+
+    fname = None
+
+    try:
+        tmpf  = tempfile.NamedTemporaryFile(delete=False)
+        fname = tmpf.name
+
+        tmpf.close()
+
+        if permissions:
+            os.chmod(fname, permissions)
+
+        yield fname
+
+    finally:
+        if delete and fname and op.exists(fname):
+            os.remove(fname)
+
+
 def sha256(filename, check_against=None, blocksize=1048576):
     """Calculate the SHA256 checksum of the given file. If check_against
     is provided, it is compared against the calculated checksum, and an
@@ -839,16 +863,29 @@ class Process(object):
     """
 
 
-    def __init__(self, cmd, admin=False, ctx=None, log_output=True, **kwargs):
+    def __init__(self,
+                 cmd,
+                 admin=False,
+                 ctx=None,
+                 log_output=True,
+                 append_env=None,
+                 **kwargs):
         """Run the specified command. Starts threads to capture stdout and
         stderr.
 
         :arg cmd:        Command to run - passed directly to subprocess.Popen
+
         :arg admin:      Run the command with administrative privileges
+
         :arg ctx:        The installer Context. Only used for admin password -
                          can be None if admin is False.
+
         :arg log_output: If True, the command and all of its stdout/stderr are
                          logged.
+
+        :arg append_env: Dictionary of additional environment to be set when
+                         the command is run.
+
         :arg kwargs:     Passed to subprocess.Popen
         """
 
@@ -862,7 +899,9 @@ class Process(object):
         if log_output:
             log.debug('Running %s [as admin: %s]', cmd, admin)
 
-        self.popen = Process.popen(self.cmd, self.admin, self.ctx, **kwargs)
+        self.popen = Process.popen(
+            self.cmd, self.admin, self.ctx,
+            append_env=append_env, **kwargs)
 
         # threads for consuming stdout/stderr
         self.stdout_thread = threading.Thread(
@@ -998,28 +1037,30 @@ class Process(object):
             line = stream.readline().decode('utf-8')
             if line == '':
                 break
-            else:
-                queue.put(line)
-                if log_output:
-                    log.debug(' [%s]: %s', streamname, line.rstrip())
+            queue.put(line)
+            if log_output:
+                log.debug(' [%s]: %s', streamname, line.rstrip())
 
 
     @staticmethod
-    def popen(cmd, admin=False, ctx=None, **kwargs):
+    def popen(cmd, admin=False, ctx=None, append_env=None, **kwargs):
         """Runs the given command via subprocess.Popen, as administrator if
         requested.
 
-        :arg cmd:    The command to run, as a string
+        :arg cmd:        The command to run, as a string
+
+        :arg admin:      Whether to run with administrative privileges
 
-        :arg admin:  Whether to run with administrative privileges
+        :arg ctx:        The installer Context object. Only required if admin is
+                         True.
 
-        :arg ctx:    The installer Context object. Only required if admin is
-                     True.
+        :arg append_env: Dictionary of additional environment to be set when
+                         the command is run.
 
-        :arg kwargs: Passed to subprocess.Popen. stdin, stdout, and stderr
-                     will be silently clobbered
+        :arg kwargs:     Passed to subprocess.Popen. stdin, stdout, and stderr
+                         will be silently clobbered
 
-        :returns:    The subprocess.Popen object.
+        :returns:        The subprocess.Popen object.
         """
 
         admin = admin and os.getuid() != 0
@@ -1032,19 +1073,51 @@ class Process(object):
         kwargs['stdout'] = sp.PIPE
         kwargs['stderr'] = sp.PIPE
 
-        if admin: proc = Process.sudo_popen(cmd, password, **kwargs)
-        else:     proc = sp.Popen(          cmd,           **kwargs)
+        if admin:
+            proc = Process.sudo_popen(cmd, password, append_env, **kwargs)
+        else:
+            # if append_env has been specified,
+            # add it to the normal env option.
+            if append_env is not None:
+                env = kwargs.get('env', os.environ.copy())
+                env.update(append_env)
+                kwargs['env'] = env
+
+            proc = sp.Popen(cmd, **kwargs)
 
         return proc
 
 
     @staticmethod
-    def sudo_popen(cmd, password, **kwargs):
+    def sudo_popen(cmd, password, append_env=None, **kwargs):
         """Runs "sudo cmd" using subprocess.Popen. Used by Process.popen.
         Assumes that kwargs contains stdin=sp.PIPE
         """
 
-        cmd  = ['sudo', '-S', '-k'] + cmd
+        # sudo will not necessarily propagate environment
+        # variables, and there is no guarantee that the
+        # sudo -E option will work. So here we create a
+        # wrapper shell script with "export VAR=VALUE"
+        # statements for all environment variables that
+        # are set.
+        if append_env is None:
+            append_env = {}
+
+        # Make the wrapper script delete itself
+        # after the command has been executed.
+        with tempfilename(0o755, delete=False) as wrapper:
+            with open(wrapper, 'wt') as f:
+                f.write('#!/usr/bin/env sh\n')
+                f.write('set -e\n')
+                f.write('thisfile=$0\n')
+                f.write('thisdir=$(cd $(dirname $0) && pwd)\n')
+                for k, v in append_env.items():
+                    f.write('export {}="{}"\n'.format(k, v))
+                # shlex.join not available in py27
+                f.write(' '.join(cmd) + '\n')
+                f.write('cd ${thisdir} && rm ${thisfile}\n')
+
+        cmd  = ['sudo', '-S', '-k', wrapper]
         proc = sp.Popen(cmd, **kwargs)
         proc.stdin.write('{}\n'.format(password).encode())
         proc.stdin.flush()
@@ -1089,6 +1162,9 @@ def download_fsl_environment(ctx):
         url          = build['environment']
         checksum     = build.get('sha256',        None)
         basepkgnames = build.get('base_packages', [])
+
+    # disable checksum if env file is passed
+    # via --environment cli option
     else:
         build        = {}
         url          = ctx.args.environment
@@ -1326,13 +1402,15 @@ def install_fsl(ctx):
 
     # Clear any environment variables that refer
     # to existing FSL or conda installations.
-    env = clean_environ()
+    # See Process.sudo_popen regarding append_env
+    env        = clean_environ()
+    append_env = {}
 
     # post-link scripts call $FSLDIR/share/fsl/sbin/createFSLWrapper
     # (part of fsl/base), which will only do its thing if the following
     # env vars are set
-    env['FSL_CREATE_WRAPPER_SCRIPTS'] = '1'
-    env['FSLDIR']                     = ctx.destdir
+    append_env['FSL_CREATE_WRAPPER_SCRIPTS'] = '1'
+    append_env['FSLDIR']                     = ctx.destdir
 
     # FSL environments which source packages from the internal
     # FSL conda channel will refer to the channel as:
@@ -1343,7 +1421,8 @@ def install_fsl(ctx):
     if ctx.args.username: env['FSLCONDA_USERNAME'] = ctx.args.username
     if ctx.args.password: env['FSLCONDA_PASSWORD'] = ctx.args.password
 
-    Process.monitor_progress(commands, output, ctx.need_admin, ctx, env=env)
+    Process.monitor_progress(commands, output, ctx.need_admin, ctx,
+                             append_env=append_env, env=env)
 
 
 def finalise_installation(ctx):

test/test_installer.py

@@ -160,58 +160,63 @@ def check_install(homedir, destdir, version):
 
 
 def test_installer_normal_interactive_usage():
-    with installer_server() as srv:
-        with mock.patch('fslinstaller.FSL_INSTALLER_MANIFEST',
-                        '{}/manifest.json'.format(srv.url)):
-            # accept rel/abs paths
-            for i in range(3):
-                with inst.tempdir() as cwd:
-                    dests = ['fsl', op.join('.', 'fsl'), op.abspath('fsl')]
-                    dest  = dests[i]
-                    with mock_input(dest):
-                        inst.main(['--homedir', cwd])
-                    check_install(cwd, dest, '6.2.0')
-                    shutil.rmtree(dest)
+    with inst.tempdir():
+        with installer_server() as srv:
+            with mock.patch('fslinstaller.FSL_INSTALLER_MANIFEST',
+                            '{}/manifest.json'.format(srv.url)):
+                # accept rel/abs paths
+                for i in range(3):
+                    with inst.tempdir() as cwd:
+                        dests = ['fsl',
+                                 op.join('.', 'fsl'),
+                                 op.abspath('fsl')]
+                        dest  = dests[i]
+                        with mock_input(dest):
+                            inst.main(['--homedir', cwd])
+                        check_install(cwd, dest, '6.2.0')
+                        shutil.rmtree(dest)
 
 
 def test_installer_list_versions():
     platform = inst.Context.identify_platform()
-    with installer_server() as srv:
-        with mock.patch('fslinstaller.FSL_INSTALLER_MANIFEST',
-                        '{}/manifest.json'.format(srv.url)):
-            with inst.tempdir() as cwd:
-                with CaptureStdout() as cap:
-                    with pytest.raises(SystemExit) as e:
-                        inst.main(['--listversions'])
-                    assert e.value.code == 0
+    with inst.tempdir():
+        with installer_server() as srv:
+            with mock.patch('fslinstaller.FSL_INSTALLER_MANIFEST',
+                            '{}/manifest.json'.format(srv.url)):
+                with inst.tempdir() as cwd:
+                    with CaptureStdout() as cap:
+                        with pytest.raises(SystemExit) as e:
+                            inst.main(['--listversions'])
+                        assert e.value.code == 0
 
-                out   = strip_ansi_escape_sequences(cap.stdout)
-                lines = out.split('\n')
+                    out   = strip_ansi_escape_sequences(cap.stdout)
+                    lines = out.split('\n')
 
-                assert '6.1.0' in lines
-                assert '6.2.0' in lines
-                assert '  {} {}/env-6.1.0.yml'.format(platform, srv.url) in lines
-                assert '  {} {}/env-6.2.0.yml'.format(platform, srv.url) in lines
+                    assert '6.1.0' in lines
+                    assert '6.2.0' in lines
+                    assert '  {} {}/env-6.1.0.yml'.format(platform, srv.url) in lines
+                    assert '  {} {}/env-6.2.0.yml'.format(platform, srv.url) in lines
 
 
 def test_installer_normal_cli_usage():
-    with installer_server() as srv:
-        with mock.patch('fslinstaller.FSL_INSTALLER_MANIFEST',
-                        '{}/manifest.json'.format(srv.url)):
-
-            # accept rel/abs paths
-            for i in range(3):
+    with inst.tempdir():
+        with installer_server() as srv:
+            with mock.patch('fslinstaller.FSL_INSTALLER_MANIFEST',
+                            '{}/manifest.json'.format(srv.url)):
+
+                # accept rel/abs paths
+                for i in range(3):
+                    with inst.tempdir() as cwd:
+                        dests = ['fsl', op.join('.', 'fsl'), op.abspath('fsl')]
+                        dest  = dests[i]
+                        inst.main(['--homedir', cwd, '--dest', dest])
+                        check_install(cwd, dest, '6.2.0')
+                        shutil.rmtree(dest)
+
+                # install specific version
                 with inst.tempdir() as cwd:
-                    dests = ['fsl', op.join('.', 'fsl'), op.abspath('fsl')]
-                    dest  = dests[i]
-                    inst.main(['--homedir', cwd, '--dest', dest])
-                    check_install(cwd, dest, '6.2.0')
-                    shutil.rmtree(dest)
-
-            # install specific version
-            with inst.tempdir() as cwd:
-                inst.main(['--homedir', cwd,
-                           '--dest', 'fsl',
-                           '--fslversion', '6.1.0'])
-                check_install(cwd, 'fsl', '6.1.0')
-                shutil.rmtree('fsl')
+                    inst.main(['--homedir', cwd,
+                               '--dest', 'fsl',
+                               '--fslversion', '6.1.0'])
+                    check_install(cwd, 'fsl', '6.1.0')
+                    shutil.rmtree('fsl')

test/test_process.py

@@ -18,6 +18,18 @@ from . import onpath, server
 import fslinstaller as inst
 
 
+SUDO = """
+#!/usr/bin/env bash
+s=$1; shift
+k=$2; shift
+
+echo -n "Password: "
+read password
+echo $password > got_password
+
+"$@"
+""".strip()
+
 
 def test_Process_check_call():
     with inst.tempdir() as cwd:
@@ -106,25 +118,13 @@ def test_Process_monitor_progress():
 
 def test_Process_sudo_popen():
     with inst.tempdir() as cwd:
-        sudo = tw.dedent("""
-        #!/usr/bin/env bash
-        s=$1; shift
-        k=$2; shift
-
-        echo -n "Password: "
-        read password
-        echo $password > got_password
-
-        "$@"
-        """).strip()
-
         cmd = tw.dedent("""
         #!/usr/bin/env bash
 
         echo "Running cmd" > command_output
         """)
 
-        with open('sudo', 'wt') as f: f.write(sudo)
+        with open('sudo', 'wt') as f: f.write(SUDO)
         with open('cmd', 'wt')  as f: f.write(cmd)
         os.chmod('sudo', 0o755)
         os.chmod('cmd',  0o755)
@@ -139,3 +139,53 @@ def test_Process_sudo_popen():
             assert f.read().strip() ==  'password'
         with open('command_output', 'rt') as f:
             assert f.read().strip() == 'Running cmd'
+
+
+def test_Process_popen_append_env():
+
+    script = tw.dedent("""
+    #!/usr/bin/env bash
+    echo "$VAR1" >  output
+    echo "$VAR2" >> output
+    """).strip()
+
+    with inst.tempdir():
+        with open('script', 'wt') as f:
+            f.write(script)
+        os.chmod('script', 0o755)
+
+        append = {'VAR1' : 'var1', 'VAR2' : 'var2'}
+
+        p = inst.Process.popen('./script', append_env=append)
+        p.wait()
+
+        with open('output', 'rt') as f:
+            assert f.read().strip() == 'var1\nvar2'
+
+
+def test_Process_sudo_popen_append_env():
+
+    script = tw.dedent("""
+    #!/usr/bin/env bash
+    echo "$VAR1" >  output
+    echo "$VAR2" >> output
+    """).strip()
+
+    with inst.tempdir() as cwd:
+        with open('sudo',   'wt') as f: f.write(SUDO)
+        with open('script', 'wt') as f: f.write(script)
+        os.chmod('script', 0o755)
+        os.chmod('sudo',   0o755)
+
+        append = {'VAR1' : 'var1', 'VAR2' : 'var2'}
+        path   = op.pathsep.join((cwd, os.environ['PATH']))
+
+        with mock.patch.dict(os.environ, PATH=path):
+            p = inst.Process.sudo_popen(['script'],
+                                        'password',
+                                        stdin=sp.PIPE,
+                                        append_env=append)
+            p.wait()
+
+        with open('output', 'rt') as f:
+            assert f.read().strip() == 'var1\nvar2'