force `subprocess.run` to check return code is 0

In general, this should help to capture unintentional errors.

Where the change has created failures, they have been fixed. (Mostly because the git repository that was looked for was missing.)

mpsd-software-environment.py

@@ -4,6 +4,7 @@ import datetime
 import os
 import subprocess
 import sys
+import time
 from pathlib import Path
 from typing import List, Tuple
 
@@ -44,6 +45,90 @@ class os_chdir:
         os.chdir(self.saved_dir)
 
 
+def run(*args, counter=[0], **kwargs):
+    """
+    Call subprocess.run(*args, **kwargs) and print logging data.
+
+    Conveniene function to call `subprocess.run` and provide some metadata
+    about the call.
+
+    Parameters
+    ----------
+    args : tuple
+        passed on to subprocess.run(*args). For example
+        ("ls -l") or (["ls", "-l"])
+    counter : TYPE, optional
+        list with one integer, starting from [0].
+        This is (a Python hack) to count the number of
+        calls of this function, so the different calls of subprocess.run
+        are easier to follow in the log files.
+    kwargs : dict
+        keyword-value arguments to be passed to subprocess.run. For example,
+        `shell=True`.
+
+    Returns
+    -------
+    process : subprocess.CompletedProcess
+        CompletedProcess object as returned by `subprocess.run` .
+
+    Examples
+    --------
+    >>> run(['date', '+%Y-%m-%d'])
+    ##-03 Starting subprocess.run(['date', '+%Y-%m-%d']) with options
+    ##-03   getcwd=/Users/fangohr/git/mpsd-software-environments
+    ##-03   COMMAND=date +%Y-%m-%d
+    2023-05-30
+    ##-03   Completed in 0.0054s.
+    ##-03
+    CompletedProcess(args=['date', '+%Y-%m-%d'], returncode=0)
+
+    >>> run(['date +%Y-%m-%d'], shell=True)
+    ##-04 Starting subprocess.run(['date +%Y-%m-%d']) with options shell=True
+    ##-04   getcwd=/Users/fangohr/git/mpsd-software-environments
+    ##-04   COMMAND=date +%Y-%m-%d
+    2023-05-30
+    ##-04   Completed in 0.0069s.
+    ##-04
+    CompletedProcess(args=['date +%Y-%m-%d'], returncode=0)
+    """
+    # token is printed in front of every meta-data line - useful for
+    # searching the logs. Starts with "##-00", then "##-01", ...
+    token = f"##-{counter[0]:02d}"
+
+    counter[0] += 1  # increase counter
+
+    # make command nicely readable: ["ls", "-l"] -> "ls -l"
+    assert isinstance(args, tuple)
+    assert len(args) == 1
+    arg = args[0]
+    # either args is a tuple containing a string | Example: ('ls -1',)
+    if isinstance(arg, str):
+        command = arg
+        # or we have a tuple containing a list of strings.
+        # Example: (['ls', '-1'],)
+    elif isinstance(arg, list):
+        command = " ".join(arg)
+    else:
+        # we do not expect this to happen
+        raise NotImplementedError(f"{arg=}, {args=}")
+
+    # make options (such as `shell=True`) nicely readable
+    options = ", ".join([f"{key}={value}" for key, value in kwargs.items()])
+
+    # provide information about upcoming subprocess.run call
+    print(f"{token} Starting subprocess.run({arg}) with options {options}")
+    print(f"{token}   getcwd={os.getcwd()}")
+    print(f"{token}   COMMAND={command}")
+
+    time_start = time.time()
+    process = subprocess.run(*args, **kwargs)
+    execution_time = time.time() - time_start
+
+    print(f"{token}   Completed in {execution_time:.4f}s.")
+    print(f"{token}")  # near-empty line to make reading logs easier
+    return process
+
+
 def setup_log_cmd(
     mpsd_release: str, script_dir: str, msg: str = None, *args, **kwargs
 ) -> None:
@@ -86,7 +171,7 @@ def setup_log_cmd(
                 # script branch and commit hash
                 with os_chdir(script_dir):
                     script_branch = (
-                        subprocess.run(
+                        run(
                             ["git", "rev-parse", "--abbrev-ref", "HEAD"],
                             stdout=subprocess.PIPE,
                             check=True,
@@ -95,7 +180,7 @@ def setup_log_cmd(
                         .strip()
                     )
                     script_commit_hash = (
-                        subprocess.run(
+                        run(
                             ["git", "rev-parse", "--short", "HEAD"],
                             stdout=subprocess.PIPE,
                             check=True,
@@ -139,7 +224,7 @@ def create_dir_structure(mpsd_release: str, script_dir: Path) -> None:
     with os_chdir(release_base_dir):
         # Clone the spack-environments repo if it doesn't exist
         if not os.path.exists("spack-environments"):
-            subprocess.run(
+            run(
                 [
                     "git",
                     "clone",
@@ -150,14 +235,15 @@ def create_dir_structure(mpsd_release: str, script_dir: Path) -> None:
         with os_chdir("spack-environments"):
             # Git fetch and checkout the release branch and git pull
             # to be sure that the resulting repo is up to date
-            subprocess.run(["git", "fetch", "--all"], check=True)
-            checkout_result = subprocess.run(["git", "checkout", mpsd_release])
+            run(["git", "fetch", "--all"], check=True)
+            checkout_result = run(["git", "checkout", mpsd_release], check=True)
+
             if checkout_result.returncode != 0:
                 raise Exception(
                     "Release branch does not exist in spack-environment repo \n."
                     "Check for typos."
                 )
-            subprocess.run(["git", "pull"], check=True)
+            run(["git", "pull"], check=True)
 
 
 def get_release_info(mpsd_release: str, script_dir: Path) -> Tuple[str, str, List[str]]:
@@ -176,27 +262,25 @@ def get_release_info(mpsd_release: str, script_dir: Path) -> Tuple[str, str, Lis
       toolchains for the release.
 
     Raises:
-    - Exception: If the release directory does not exist. Run `create_dir_structure()`
-      first.
+    - FileNotFoundError: If the release directory does not exist. Run
+      `create_dir_structure()` first.
     """
     # Get the info for release
     release_base_dir = script_dir / mpsd_release
     if not os.path.exists(release_base_dir):
-        raise Exception(
+        raise FileNotFoundError(
             "Release directory does not exist. Run create_dir_structure() first."
         )
     with os_chdir(release_base_dir):
         with os_chdir("spack-environments"):
             # Get the branch and commit hash of the spack-environments repo
             spe_commit_hash = (
-                subprocess.run(
-                    ["git", "rev-parse", "HEAD"], stdout=subprocess.PIPE, check=True
-                )
+                run(["git", "rev-parse", "HEAD"], stdout=subprocess.PIPE, check=True)
                 .stdout.decode()
                 .strip()
             )
             spe_branch = (
-                subprocess.run(
+                run(
                     ["git", "rev-parse", "--abbrev-ref", "HEAD"],
                     stdout=subprocess.PIPE,
                     check=True,
@@ -261,7 +345,7 @@ def install_environment(
                     cache when installing toolchains. Defaults to False.
 
     Raises:
-        Exception: If a requested toolchain is not available in the specified release.
+        ValueError: If a requested toolchain is not available in the specified release.
 
     Returns:
         None
@@ -299,7 +383,7 @@ def install_environment(
 
     for toolchain in toolchains:
         if toolchain not in available_toolchains:
-            raise Exception(
+            raise ValueError(
                 f"Toolchain '{toolchain}' is not available in release {mpsd_release}."
             )
 
@@ -334,7 +418,7 @@ def install_environment(
                     "{toolchain}"
                 ),
             )
-            subprocess.run(
+            run(
                 f"bash {spack_setup_script} {' '.join(install_flags)} {toolchain} 2>&1 "
                 f"| tee -a {install_log_file} ",
                 shell=True,