Function remove_module_lines can produce an invalid bash script

[SeanBryan51]

For compiling CABLE with a custom build script, benchcab will try to remove any lines that invoke the module command in the build script so that benchcab can load the modules specified in config.yaml when compiling the executables. This ensures that when we run CABLE we have the correct module dependencies loaded (see related issue: #94). This is done via the remove_module_lines function:

benchcab/benchcab/model.py

Lines 154 to 162 in dd9bc12

	def remove_module_lines(file_path: Path) -> None:
	"""Remove lines from `file_path` that call the environment modules package."""
	with file_path.open("r", encoding="utf-8") as file:
	contents = file.read()
	with file_path.open("w", encoding="utf-8") as file:
	for line in contents.splitlines(True):
	cmds = shlex.split(line, comments=True)
	if "module" not in cmds:
	file.write(line)

However, this approach of removing lines from the bash script is brittle and can easily produce a broken bash script. For example, a bash script snippet such as the following would break when applying the above function:

if condition; then
    module load foo
fi

I think it is best to avoid modifying the build script as much as possible.

We could instead disable module invocations by creating a wrapper around the module command which prevents any module load or module add statements from being executed. To invoke the wrapper instead of the actual command when calling subprocess.run, we can prepend the path to the wrapper to the PATH environment variable (see here for using python subprocess with a modified environment). This approach could be relevant for #244 if the user specifies a list of shell commands to compile CABLE instead of a build script path.

[SeanBryan51]

I'm going to attempt to implement this, we'll see how I go.

[SeanBryan51]

After discussing with @ccarouge, we have decided to add support for spack and to deprecate support for legacy CABLE build systems in benchcab. This adds the requirement that users will only be able to test benchcab on branches that can be built by spack. ACCESS-NRI will help with transitioning legacy build systems to use the latest build system compatible with spack.

Moving to spack will solve existing issues around supporting bespoke build scripts (e.g. #244, #249). This is the best way forward considering support for legacy build systems is a temporary that will be deprecated sooner or later.

Closing this issue.