Remove dependence on build script

Currently benchcab uses the `offline/build3.sh` script to build the
CABLE executable. The `build3.sh` is a wrapper around `Makefile`,
`parallel_cable` and `serial_cable` scripts. Using `build3.sh` is
convenient for CABLE developers, however it has its draw backs when
building executables for benchcab, these are outlined in #138.

This change removes the dependence on `build3.sh` from the build and
instead invokes the `Makefile`, `parallel_cable` and `serial_cable`
scripts directly.

Fixes #138

benchcab/internal.py

@@ -100,6 +100,23 @@
 # CABLE standard output filename
 CABLE_STDOUT_FILENAME = "out.txt"
 
+OFFLINE_SOURCE_FILES = [
+    "science/albedo/*90",
+    "science/radiation/*90",
+    "science/canopy/*90",
+    "science/casa-cnp/*90",
+    "science/gw_hydro/*90",
+    "science/misc/*90",
+    "science/roughness/*90",
+    "science/soilsnow/*90",
+    "science/landuse/*90",
+    "offline/*90",
+    "util/*90",
+    "params/*90",
+    "science/sli/*90",
+    "science/pop/*90",
+]
+
 # Contains the default science configurations used to run the CABLE test suite
 # (when a science config file is not provided by the user)
 DEFAULT_SCIENCE_CONFIGURATIONS = [

benchcab/repository.py

@@ -1,5 +1,6 @@
 """A module containing functions and data structures for manipulating CABLE repositories."""
 
+import os
 import shlex
 import shutil
 import stat
@@ -74,25 +75,97 @@ def svn_info_show_item(self, item: str) -> str:
         )
         return proc.stdout.strip()
 
-    def build(self, modules: list[str], verbose=False) -> None:
+    def build(
+        self,
+        modules: list[str],
+        offline_source_files: Optional[list[str]] = None,
+        verbose=False,
+    ) -> None:
         """Build CABLE using the default build script or a custom build script."""
 
         if self.build_script:
+            self._custom_build(modules=modules, verbose=verbose)
+            return
+
+        mpi = internal.MPI
+        print(
+            f"Compiling CABLE {'with MPI' if mpi else 'serially'} for "
+            f"realisation {self.name}..."
+        )
+
+        path_to_repo = self.root_dir / internal.SRC_DIR / self.name
+        tmp_dir = path_to_repo / "offline" / ".tmp"
+        if not tmp_dir.exists():
+            if verbose:
+                print(f"mkdir {tmp_dir}")
+            tmp_dir.mkdir()
+
+        for pattern in (
+            offline_source_files
+            if offline_source_files
+            else internal.OFFLINE_SOURCE_FILES
+        ):
+            for path in path_to_repo.glob(pattern):
+                if not path.is_file():
+                    continue
+                if verbose:
+                    print(
+                        f"cp -p {path.relative_to(self.root_dir)} "
+                        f"{tmp_dir.relative_to(self.root_dir)}"
+                    )
+                shutil.copy2(path, tmp_dir)
+
+        src, dest = path_to_repo / "offline" / "Makefile", tmp_dir
+        if verbose:
             print(
-                "Compiling CABLE using custom build script for "
-                f"realisation {self.name}..."
+                f"cp -p {src.relative_to(self.root_dir)} {dest.relative_to(self.root_dir)}"
             )
-        else:
+        shutil.copy2(src, dest)
+
+        src, dest = path_to_repo / "offline" / "parallel_cable", tmp_dir
+        if verbose:
             print(
-                f"Compiling CABLE {'with MPI' if internal.MPI else 'serially'} for "
-                f"realisation {self.name}..."
+                f"cp -p {src.relative_to(self.root_dir)} {dest.relative_to(self.root_dir)}"
             )
+        shutil.copy2(src, dest)
+
+        src, dest = path_to_repo / "offline" / "serial_cable", tmp_dir
+        if verbose:
+            print(
+                f"cp -p {src.relative_to(self.root_dir)} {dest.relative_to(self.root_dir)}"
+            )
+        shutil.copy2(src, dest)
+
+        with chdir(tmp_dir), self.modules_handler.load(modules, verbose=verbose):
+            env = os.environ.copy()
+            env["NCDIR"] = f"{env['NETCDF_ROOT']}/lib/Intel"
+            env["NCMOD"] = f"{env['NETCDF_ROOT']}/include/Intel"
+            env["CFLAGS"] = "-O2 -fp-model precise"
+            env["LDFLAGS"] = f"-L{env['NETCDF_ROOT']}/lib/Intel -O0"
+            env["LD"] = "-lnetcdf -lnetcdff"
+            env["FC"] = "mpif90" if mpi else "ifort"
+
+            self.subprocess_handler.run_cmd(
+                "make -f Makefile", env=env, verbose=verbose
+            )
+            self.subprocess_handler.run_cmd(
+                f"./{'parallel_cable' if mpi else 'serial_cable'} \"{env['FC']}\" "
+                f"\"{env['CFLAGS']}\" \"{env['LDFLAGS']}\" \"{env['LD']}\" \"{env['NCMOD']}\"",
+                env=env,
+                verbose=verbose,
+            )
+
+        exe = internal.CABLE_EXE
+        (tmp_dir / exe).rename(path_to_repo / "offline" / exe)
+
+    def _custom_build(self, modules: list[str], verbose=False):
+        print(
+            "Compiling CABLE using custom build script for "
+            f"realisation {self.name}..."
+        )
 
         build_script_path = (
-            self.root_dir
-            / internal.SRC_DIR
-            / self.name
-            / (self.build_script if self.build_script else "offline/build3.sh")
+            self.root_dir / internal.SRC_DIR / self.name / self.build_script
         )
 
         if not build_script_path.is_file():
@@ -119,15 +192,11 @@ def build(self, modules: list[str], verbose=False) -> None:
             )
         remove_module_lines(tmp_script_path)
 
-        args: list[str] = []
-        if internal.MPI and self.build_script is None:
-            args.append("mpi")
-
         with chdir(build_script_path.parent), self.modules_handler.load(
             modules, verbose=verbose
         ):
             self.subprocess_handler.run_cmd(
-                shlex.join([f"./{tmp_script_path.name}", *args]),
+                f"./{tmp_script_path.name}",
                 verbose=verbose,
             )
 

benchcab/utils/subprocess.py

@@ -22,6 +22,7 @@ def run_cmd(
         capture_output: bool = False,
         output_file: Optional[pathlib.Path] = None,
         verbose: bool = False,
+        env: Optional[dict] = None,
     ) -> subprocess.CompletedProcess:
         """A wrapper around the `subprocess.run` function for executing system commands."""
 
@@ -35,6 +36,7 @@ def run_cmd(
         capture_output: bool = False,
         output_file: Optional[pathlib.Path] = None,
         verbose: bool = False,
+        env: Optional[dict] = None,
     ) -> subprocess.CompletedProcess:
         kwargs: Any = {}
         with contextlib.ExitStack() as stack:
@@ -49,6 +51,9 @@ def run_cmd(
                 kwargs["stdout"] = None if verbose else subprocess.DEVNULL
             kwargs["stderr"] = subprocess.STDOUT
 
+            if env:
+                kwargs["env"] = env
+
             if verbose:
                 print(cmd)
             proc = subprocess.run(cmd, shell=True, check=True, **kwargs)

tests/common.py

@@ -77,19 +77,25 @@ def __init__(self) -> None:
         self.commands: list[str] = []
         self.stdout = "mock standard output"
         self.error_on_call = False
+        self.env = {}
+        self.side_effect = lambda: None
 
     def run_cmd(
         self,
         cmd: str,
         capture_output: bool = False,
         output_file: Optional[Path] = None,
         verbose: bool = False,
+        env: Optional[dict] = None,
     ) -> CompletedProcess:
         self.commands.append(cmd)
+        self.side_effect()
         if self.error_on_call:
             raise CalledProcessError(returncode=1, cmd=cmd, output=self.stdout)
         if output_file:
             output_file.touch()
+        if env:
+            self.env = env
         return CompletedProcess(cmd, returncode=0, stdout=self.stdout)
 
 

tests/test_repository.py

@@ -1,5 +1,6 @@
 """`pytest` tests for repository.py"""
 
+import os
 import io
 import contextlib
 import pytest
@@ -111,24 +112,48 @@ def test_svn_info_show_item():
 def test_build():
     """Tests for `CableRepository.build()`."""
     repo_dir = MOCK_CWD / internal.SRC_DIR / "trunk"
-    build_script_path = repo_dir / "offline" / "build3.sh"
+    offline_dir = repo_dir / "offline"
     custom_build_script_path = repo_dir / "my-custom-build.sh"
     mock_modules = ["foo", "bar"]
-
-    # Success case: execute the default build command
-    build_script_path.parent.mkdir(parents=True, exist_ok=True)
-    build_script_path.touch(exist_ok=True)
+    mock_netcdf_root = "/mock/path/to/root"
+    default_env = {
+        "NCDIR": f"{mock_netcdf_root}/lib/Intel",
+        "NCMOD": f"{mock_netcdf_root}/include/Intel",
+        "CFLAGS": "-O2 -fp-model precise",
+        "LDFLAGS": f"-L{mock_netcdf_root}/lib/Intel -O0",
+        "LD": "-lnetcdf -lnetcdff",
+        "FC": "ifort",
+    }
+
+    # Success case: test default build
+    offline_dir.mkdir(parents=True, exist_ok=True)
+    (offline_dir / "Makefile").touch(exist_ok=True)
+    (offline_dir / "parallel_cable").touch(exist_ok=True)
+    (offline_dir / "serial_cable").touch(exist_ok=True)
+    (offline_dir / "foo.f90").touch(exist_ok=True)
+    os.environ["NETCDF_ROOT"] = mock_netcdf_root
     mock_subprocess = MockSubprocessWrapper()
+    mock_subprocess.side_effect = lambda: (
+        offline_dir / ".tmp" / internal.CABLE_EXE
+    ).touch(exist_ok=True)
     mock_environment_modules = MockEnvironmentModules()
     repo = get_mock_repo(mock_subprocess, mock_environment_modules)
-    repo.build(mock_modules)
-    assert "./tmp-build.sh" in mock_subprocess.commands
+    repo.build(mock_modules, offline_source_files=["offline/*90"])
+    assert (offline_dir / ".tmp" / "foo.f90").exists()
+    assert "make -f Makefile" in mock_subprocess.commands
+    assert (
+        f"./serial_cable \"{default_env['FC']}\" \"{default_env['CFLAGS']}\""
+        f" \"{default_env['LDFLAGS']}\" \"{default_env['LD']}\" \"{default_env['NCMOD']}\""
+        in mock_subprocess.commands
+    )
+    assert all(kv in mock_subprocess.env.items() for kv in default_env.items())
     assert (
         "module load " + " ".join(mock_modules)
     ) in mock_environment_modules.commands
     assert (
         "module unload " + " ".join(mock_modules)
     ) in mock_environment_modules.commands
+    assert (offline_dir / internal.CABLE_EXE).exists()
 
     # Success case: execute the build command for a custom build script
     custom_build_script_path.parent.mkdir(parents=True, exist_ok=True)
@@ -147,11 +172,19 @@ def test_build():
     ) in mock_environment_modules.commands
 
     # Success case: test non-verbose standard output
-    build_script_path.parent.mkdir(parents=True, exist_ok=True)
-    build_script_path.touch(exist_ok=True)
-    repo = get_mock_repo()
+    offline_dir.mkdir(parents=True, exist_ok=True)
+    (offline_dir / "Makefile").touch(exist_ok=True)
+    (offline_dir / "parallel_cable").touch(exist_ok=True)
+    (offline_dir / "serial_cable").touch(exist_ok=True)
+    (offline_dir / "foo.f90").touch(exist_ok=True)
+    os.environ["NETCDF_ROOT"] = mock_netcdf_root
+    mock_subprocess = MockSubprocessWrapper()
+    mock_subprocess.side_effect = lambda: (
+        offline_dir / ".tmp" / internal.CABLE_EXE
+    ).touch(exist_ok=True)
+    repo = get_mock_repo(subprocess_handler=mock_subprocess)
     with contextlib.redirect_stdout(io.StringIO()) as buf:
-        repo.build(mock_modules)
+        repo.build(mock_modules, offline_source_files=["offline/*90"])
     assert buf.getvalue() == "Compiling CABLE serially for realisation trunk...\n"
 
     # Success case: test non-verbose standard output for a custom build script
@@ -166,17 +199,25 @@ def test_build():
     )
 
     # Success case: test verbose standard output
-    build_script_path.parent.mkdir(parents=True, exist_ok=True)
-    build_script_path.touch(exist_ok=True)
-    repo = get_mock_repo()
+    offline_dir.mkdir(parents=True, exist_ok=True)
+    (offline_dir / "Makefile").touch(exist_ok=True)
+    (offline_dir / "parallel_cable").touch(exist_ok=True)
+    (offline_dir / "serial_cable").touch(exist_ok=True)
+    (offline_dir / "foo.f90").touch(exist_ok=True)
+    os.environ["NETCDF_ROOT"] = mock_netcdf_root
+    mock_subprocess = MockSubprocessWrapper()
+    mock_subprocess.side_effect = lambda: (
+        offline_dir / ".tmp" / internal.CABLE_EXE
+    ).touch(exist_ok=True)
+    repo = get_mock_repo(subprocess_handler=mock_subprocess)
     with contextlib.redirect_stdout(io.StringIO()) as buf:
-        repo.build(mock_modules, verbose=True)
+        repo.build(mock_modules, offline_source_files=["offline/*90"], verbose=True)
     assert buf.getvalue() == (
         "Compiling CABLE serially for realisation trunk...\n"
-        f"Copying {build_script_path} to {build_script_path.parent}/tmp-build.sh\n"
-        f"chmod +x {build_script_path.parent}/tmp-build.sh\n"
-        "Modifying tmp-build.sh: remove lines that call environment "
-        "modules\n"
+        "cp -p src/trunk/offline/foo.f90 src/trunk/offline/.tmp\n"
+        "cp -p src/trunk/offline/Makefile src/trunk/offline/.tmp\n"
+        "cp -p src/trunk/offline/parallel_cable src/trunk/offline/.tmp\n"
+        "cp -p src/trunk/offline/serial_cable src/trunk/offline/.tmp\n"
         f"Loading modules: {' '.join(mock_modules)}\n"
         f"Unloading modules: {' '.join(mock_modules)}\n"
     )
@@ -198,14 +239,15 @@ def test_build():
         f"Unloading modules: {' '.join(mock_modules)}\n"
     )
 
-    # Failure case: cannot find default build script
-    build_script_path.parent.mkdir(parents=True, exist_ok=True)
-    build_script_path.touch(exist_ok=True)
-    build_script_path.unlink()
+    # Failure case: cannot find custom build script
+    custom_build_script_path.parent.mkdir(parents=True, exist_ok=True)
+    custom_build_script_path.touch(exist_ok=True)
+    custom_build_script_path.unlink()
     repo = get_mock_repo()
+    repo.build_script = str(custom_build_script_path.relative_to(repo_dir))
     with pytest.raises(
         FileNotFoundError,
-        match=f"The build script, {MOCK_CWD}/src/trunk/offline/build3.sh, could not be "
+        match=f"The build script, {custom_build_script_path}, could not be "
         "found. Do you need to specify a different build script with the 'build_script' "
         "option in config.yaml?",
     ):

tests/test_subprocess.py

@@ -1,5 +1,6 @@
 """`pytest` tests for utils/subprocess.py"""
 
+import os
 import subprocess
 import pytest
 
@@ -79,6 +80,12 @@ def test_run_cmd(capfd):
     assert captured.out == "echo foo\n"
     assert not captured.err
 
+    # Success case: test command is run with environment
+    proc = subprocess_handler.run_cmd(
+        "echo $FOO", capture_output=True, env={"FOO": "bar", **os.environ}
+    )
+    assert proc.stdout == "bar\n"
+
     # Failure case: check non-zero return code throws an exception
     with pytest.raises(subprocess.CalledProcessError):
         subprocess_handler.run_cmd("exit 1")