Put the environment setup code into (mostly) one place.

Author: rupertnash

README.rst

@@ -296,6 +296,30 @@ CWL documents can be expressed as RDF triple graphs.
   cwltool --print-rdf --rdf-serializer=turtle mywf.cwl
 
 
+Environment Variables in cwltool
+--------------------------------
+
+This reference implementation supports several ways of setting
+enviroment variables for tools, in addition to the standard
+``EnvVarRequirement``. The sequence of steps applied to create the
+enviroment is:
+
+0. If the ``--preserve-entire-environment`` flag is present, then begin with the current
+   environment, else begin with an empty environment.
+
+1. Add any variables specified by ``--preserve-environment`` option(s).
+
+2. Set ``TMPDIR`` and ``HOME`` per `the CWL v1.0+ CommandLineTool specification <https://www.commonwl.org/v1.0/CommandLineTool.html#Runtime_environment>`_.
+
+3. Apply any ``EnvVarRequirement`` from the ``CommandLineTool`` description.
+
+4. Apply any manipulations required by any ``cwltool:MPIRequirement`` extensions.
+
+5. Substitute any secrets required by ``Secrets`` extension.
+
+6. Modify the environment in response to ``SoftwareRequirement`` (see below).
+
+
 Leveraging SoftwareRequirements (Beta)
 --------------------------------------
 
@@ -318,10 +342,11 @@ plugins to enable to "resolve" ``SoftwareRequirement`` dependencies.
 
 Using these hints will allow cwltool to modify the environment in
 which your tool runs, for example by loading one or more Environment
-Modules. The environment is constructed as normal (i.e. standard CWL
-runtime enviroment, ``EnvVarRequirement``, ``--preserve-environment``
-are applied), then the environment may modified by the selected tool resolver.  This currently means that you
-cannot override any environment variables set by the selected tool resolver.
+Modules. The environment is constructed as above, then the environment
+may modified by the selected tool resolver.  This currently means that
+you cannot override any environment variables set by the selected tool
+resolver. Note that the enviroment given to the configured dependency
+resolver has the variable `_CWLTOOL` set to `1` to allow introspection.
 
 To discuss some of these plugins and how to configure them, first consider the
 following ``hint`` definition for an example CWL tool.

cwltool/command_line_tool.py

@@ -995,13 +995,14 @@ def register_reader(f: CWLObjectType) -> None:
                         "networkAccess must be a boolean, got: %s" % j.networkaccess
                     )
 
-        j.environment = {}
+        # Build a mapping to hold any EnvVarRequirement
+        required_env = {}
         evr, _ = self.get_requirement("EnvVarRequirement")
         if evr is not None:
             for t3 in cast(List[Dict[str, str]], evr["envDef"]):
-                j.environment[t3["envName"]] = cast(
-                    str, builder.do_eval(t3["envValue"])
-                )
+                required_env[t3["envName"]] = cast(str, builder.do_eval(t3["envValue"]))
+        # Construct the env
+        j.prepare_environment(runtimeContext, required_env)
 
         shellcmd, _ = self.get_requirement("ShellCommandRequirement")
         if shellcmd is not None:

cwltool/job.py

@@ -19,6 +19,7 @@
     Callable,
     Iterable,
     List,
+    Mapping,
     Match,
     MutableMapping,
     MutableSequence,
@@ -419,6 +420,42 @@ def _execute(
             )
             shutil.rmtree(self.tmpdir, True)
 
+    def prepare_environment(
+        self, runtimeContext: RuntimeContext, envVarReq: Mapping[str, str]
+    ) -> None:
+        """Set up environment variables.
+
+        Here we prepare the environment for the job, based on any
+        preserved variables and `EnvVarRequirement`. Later, changes due
+        to `MPIRequirement`, `Secrets`, or `SoftwareRequirement` are
+        applied (in that order).
+        """
+        # Start empty
+        env = {}
+
+        # Preserve any env vars
+        vars_to_preserve = runtimeContext.preserve_environment
+        if runtimeContext.preserve_entire_environment is not False:
+            vars_to_preserve = os.environ
+        if vars_to_preserve:
+            for key, value in os.environ.items():
+                if key in vars_to_preserve and key not in env:
+                    env[key] = value
+
+        # Set required env vars
+        env["HOME"] = self.outdir
+        env["TMPDIR"] = self.tmpdir
+        if "PATH" not in env:
+            env["PATH"] = os.environ["PATH"]
+        if "SYSTEMROOT" not in env and "SYSTEMROOT" in os.environ:
+            env["SYSTEMROOT"] = os.environ["SYSTEMROOT"]
+
+        # Apply EnvVarRequirement
+        env.update(envVarReq)
+
+        # Set on ourselves
+        self.environment = env
+
     def process_monitor(self, sproc):  # type: (subprocess.Popen[str]) -> None
         monitor = psutil.Process(sproc.pid)
         # Value must be list rather than integer to utilise pass-by-reference in python
@@ -469,19 +506,6 @@ def run(
 
         self._setup(runtimeContext)
 
-        env = self.environment
-        vars_to_preserve = runtimeContext.preserve_environment
-        if runtimeContext.preserve_entire_environment is not False:
-            vars_to_preserve = os.environ
-        if vars_to_preserve:
-            for key, value in os.environ.items():
-                if key in vars_to_preserve and key not in env:
-                    env[key] = value
-        env["HOME"] = self.outdir
-        env["TMPDIR"] = self.tmpdir
-        if "PATH" not in env:
-            env["PATH"] = os.environ["PATH"]
-
         stage_files(
             self.pathmapper,
             ignore_writable=True,
@@ -504,7 +528,7 @@ def run(
 
         monitor_function = functools.partial(self.process_monitor)
 
-        self._execute([], env, runtimeContext, monitor_function)
+        self._execute([], self.environment, runtimeContext, monitor_function)
 
 
 CONTROL_CODE_RE = r"\x1b\[[0-9;]*[a-zA-Z]"
@@ -832,7 +856,7 @@ def _job_popen(
     stdin_path: Optional[str],
     stdout_path: Optional[str],
     stderr_path: Optional[str],
-    env: MutableMapping[str, str],
+    env: Mapping[str, str],
     cwd: str,
     make_job_dir: Callable[[], str],
     job_script_contents: Optional[str] = None,
@@ -915,15 +939,10 @@ def terminate():  # type: () -> None
         if job_script_contents is None:
             job_script_contents = SHELL_COMMAND_TEMPLATE
 
-        env_copy = {}
-        key = None  # type: Optional[str]
-        for key in env:
-            env_copy[key] = env[key]
-
         job_description = {
             "commands": commands,
             "cwd": cwd,
-            "env": env_copy,
+            "env": env,
             "stdout_path": stdout_path,
             "stderr_path": stderr_path,
             "stdin_path": stdin_path,

cwltool/run_job.py

@@ -7,18 +7,21 @@
 
 
 def handle_software_environment(cwl_env: Dict[str, str], script: str) -> Dict[str, str]:
-    """Update the provided environment variables dictiony by running the script."""
-    res = subprocess.run(["bash", script], shell=False, env=cwl_env)  # nosec
+    """Update the provided environment dict by running the script."""
+    exec_env = cwl_env.copy()
+    exec_env["_CWLTOOL"] = "1"
+    res = subprocess.run(["bash", script], shell=False, env=exec_env)  # nosec
     if res.returncode != 0:
         sys.stderr.write(
             "Error while using SoftwareRequirements to modify environment\n"
         )
         return cwl_env
+
     env = cwl_env.copy()
     with open("output_environment.bash") as env_file:
         for line in env_file:
             key, val = line.split("=", 1)
-            if key in ("_", "PWD", "SHLVL", "TMPDIR", "HOME"):
+            if key in ("_", "PWD", "SHLVL", "TMPDIR", "HOME", "_CWLTOOL"):
                 # Skip some variables that are meaningful to the shell
                 # or set specifically by the CWL runtime environment.
                 continue