17

First off, apologies for what I'm sure will be obvious is my rudimentary understanding of bash and shells and subprocesses.

I am trying to use Python to automate calls to a program called Freesurfer (actually, the subprogram I'm calling is called recon-all.)

If I were doing this directly at the command line, I'd "source" a script called mySetUpFreeSurfer.sh that does nothing but set three environment variables, and then "source" another script, FreeSurferEnv.sh. FreesurferEnv.sh doesn't seem to me to do anything but set a lot of environment variables and echo some stuff to the terminal, but it's more complicated than the other bash script, so I'm not sure of that.

Here is what I have right now:

from subprocess import Popen, PIPE, call, check_output
import os

root = "/media/foo/"

#I got this function from another Stack Overflow question.

def source(script, update=1):
    pipe = Popen(". %s; env" % script, stdout=PIPE, shell=True)
    data = pipe.communicate()[0]
    env = dict((line.split("=", 1) for line in data.splitlines()))
    if update:
        os.environ.update(env)
    return env

source('~/scripts/mySetUpFreeSurfer.sh')
source('/usr/local/freesurfer/FreeSurferEnv.sh')

for sub_dir in os.listdir(root):
    sub = "s" + sub_dir[0:4]
    anat_dir = os.path.join(root, sub_dir, "anatomical")
    for directory in os.listdir(anat_dir):
        time_dir = os.path.join(anat_dir, directory)
        for d in os.listdir(time_dir):
            dicoms_dir = os.path.join(time_dir, d, 'dicoms')
            dicom_list = os.listdir(dicoms_dir)
            dicom = dicom_list[0]
            path = os.path.join(dicoms_dir, dicom)
            cmd1 = "recon-all -i " + path + " -subjid " + sub
            check_output(cmd1, shell=True)
            call(cmd1, shell=True)
            cmd2 = "recon-all -all -subjid " + sub,
            call(cmd2, shell=True)

This is failing:

Traceback (most recent call last):
     File "/home/katie/scripts/autoReconSO.py", line 28, in <module>
        check_output(cmd1, shell=True)
      File "/usr/lib/python2.7/subprocess.py", line 544, in check_output
        raise CalledProcessError(retcode, cmd, output=output)
    CalledProcessError: Command 'recon-all -i /media/foo/bar -subjid s1001' returned non-zero exit status 127

I maybe understand why this is. My "calls" later in the script are raising new subprocesses that do not inherit environment variables from the processes that are raised by invocation of the source() function. I have done a number of things to try to confirm my understanding. One example -- I put these lines:

mkdir ~/testFreeSurferEnv
export TEST_ENV_VAR=~/testFreeSurferEnv

in the FreeSurferEnv.sh script. The directory gets made just fine, but in the Python script this:

cmd = 'mkdir $TEST_ENV_VAR/test'
check_output(cmd, shell=True)

fails like this:

File "/usr/lib/python2.7/subprocess.py", line 544, in check_output
    raise CalledProcessError(retcode, cmd, output=output)
CalledProcessError: Command 'mkdir $TEST_ENV_VAR/test' returned non-zero exit status 1

QUESTION:

How can I make the subprocess that runs "recon-all" inherit the environment variables it needs? Or how can I do everything I need to do -- run the scripts to set the environment variables, and call recon-all, in the same process? Or should I approach the problem another way? Or do I likely misunderstand the problem?

Katie
  • 808
  • 1
  • 11
  • 28
  • related: [Calling the “source” command from subprocess.Popen](http://stackoverflow.com/q/7040592/4279) – jfs May 01 '16 at 11:58

2 Answers2

24

If you look at the docs for Popen, it takes an env parameter:

If env is not None, it must be a mapping that defines the environment variables for the new process; these are used instead of inheriting the current process’ environment, which is the default behavior.

You've written a function that extracts the environment you want from your sourced scripts and puts it into a dict. Just pass the result as the env to the scripts you want to use it. For example:

env = {}
env.update(os.environ)
env.update(source('~/scripts/mySetUpFreeSurfer.sh'))
env.update(source('/usr/local/freesurfer/FreeSurferEnv.sh'))

# …

check_output(cmd, shell=True, env=env)
abarnert
  • 354,177
  • 51
  • 601
  • 671
  • What you say makes so much sense. I obviously didn't understand that function and had somehow gotten the idea it was updating the environment variables as a side effect. However, my problem is not yet solved. I get env['TEST_ENV_VAR'] equal to ~/testFreeSurferEnv. So far so good. But check_output(cmd, shell=True, env=env) still fails with the same error. call(cmd, shell=True, env=env) and Popen(cmd, shell=True, env=env) don't throw any kind of exception, but they don't make the test subdirectory either. – Katie Dec 18 '13 at 23:12
  • @Katie: There are other reasons `mkdir` could be failing. If you try running `check_output('echo $TEST_ENV_VAR/test', shell=True, env=env)`, do you get back `~/testFreeSurferEnv/test`, or does that fail too? When I try the test (explicitly setting `env['TEST_ENV_VAR']` instead of going through all your other code, because you say all that other code works) on both OS X and Linux with both Python 2.7 and 3.3 or 3.4, it works in every case. – abarnert Dec 19 '13 at 00:59
  • That's not failing. I think I was inclined to think my problem was unsolved because last night, when I tried using that source function the correct way, the recon-all command was also still failing with exit status 127. Sadly, I can't really investigate this right now, because I used the "use Python to write a single bash script" method suggested by unutbu to start my job, and changing environment variables at this point could cause havoc. I'm curious to understand the underlying situation better, but atm I have to leave it alone. I really appreciate the help. – Katie Dec 19 '13 at 21:15
  • Is there no direct way to extract all the environment variables from a subprocess? The proposal from @unutbu is pretty decent, but I sure wish there was a built-in. – JDong Apr 20 '18 at 18:00
  • @JDong That's a limitation of the design of Unix (and of Windows NT, for that matter). – abarnert Apr 21 '18 at 00:25
5

Regarding

If I were doing this directly at the command line, I'd "source" a script called mySetUpFreeSurfer.sh that does nothing but set three environment variables, and then "source" another script, FreeSurferEnv.sh.

I think you would be better off using Python to automate the process of writing a shell script newscript.sh, and then calling this script with one call subprocess.check_output (instead of many calls to Popen, check_output, call, etc.):

newscript.sh:

#!/bin/bash
source ~/scripts/mySetUpFreeSurfer.sh
source /usr/local/freesurfer/FreeSurferEnv.sh
recon-all -i /media/foo/bar -subjid s1001
...

and then calling

subprocess.check_output(['newscript.sh'])

import subprocess
import tempfile
import os
import stat


with tempfile.NamedTemporaryFile(mode='w', delete=False) as f:
    f.write('''\
#!/bin/bash
source ~/scripts/mySetUpFreeSurfer.sh
source /usr/local/freesurfer/FreeSurferEnv.sh
''')
    root = "/media/foo/"
    for sub_dir in os.listdir(root):
        sub = "s" + sub_dir[0:4]
        anat_dir = os.path.join(root, sub_dir, "anatomical")
        for directory in os.listdir(anat_dir):
            time_dir = os.path.join(anat_dir, directory)
            for d in os.listdir(time_dir):
                dicoms_dir = os.path.join(time_dir, d, 'dicoms')
                dicom_list = os.listdir(dicoms_dir)
                dicom = dicom_list[0]
                path = os.path.join(dicoms_dir, dicom)
                cmd1 = "recon-all -i {}  -subjid {}\n".format(path, sub)
                f.write(cmd1)
                cmd2 = "recon-all -all -subjid {}\n".format(sub)
                f.write(cmd2)

filename = f.name
os.chmod(filename, stat.S_IRUSR | stat.S_IXUSR)
subprocess.call([filename])
os.unlink(filename)

By the way,

def source(script, update=1):
    pipe = Popen(". %s; env" % script, stdout=PIPE, shell=True)
    data = pipe.communicate()[0]
    env = dict((line.split("=", 1) for line in data.splitlines()))
    if update:
        os.environ.update(env)
    return env

is broken. For example, if script contains something like

VAR=`ls -1`
export VAR

then

. script; env

may return output like

VAR=file1
file2
file3

which will result in source(script) raising a ValueError:

env = dict((line.split("=", 1) for line in data.splitlines()))
ValueError: dictionary update sequence element #21 has length 1; 2 is required

There is a way to fix source: have env separate environment variables with a zero byte instead of the ambiguous newline:

def source(script, update=True):
    """
    http://pythonwise.blogspot.fr/2010/04/sourcing-shell-script.html (Miki Tebeka)
    http://stackoverflow.com/questions/3503719/#comment28061110_3505826 (ahal)
    """
    import subprocess
    import os
    proc = subprocess.Popen(
        ['bash', '-c', 'set -a && source {} && env -0'.format(script)], 
        stdout=subprocess.PIPE, shell=False)
    output, err = proc.communicate()
    output = output.decode('utf8')
    env = dict((line.split("=", 1) for line in output.split('\x00') if line))
    if update:
        os.environ.update(env)
    return env

Fixable or not, however, you are still probably better off constructing a conglomerate shell script (as shown above) than you would be parsing env and passing env dicts to subprocess calls.

unutbu
  • 842,883
  • 184
  • 1,785
  • 1,677
  • I am going to try this! Just trying solutions one at a time. – Katie Dec 18 '13 at 23:20
  • This worked great, thanks so much! I am going to remember the trick of just using Python to write a single bash script forever, and I also learned some new Python from your answer. Maybe you should note how that source function is broken here: http://stackoverflow.com/a/12708396/2066083 ? That's where I got it. – Katie Dec 18 '13 at 23:46