diff --git a/.cspell/ok-unknown-words.txt b/.cspell/ok-unknown-words.txt index ceea956fe..126cf2489 100644 --- a/.cspell/ok-unknown-words.txt +++ b/.cspell/ok-unknown-words.txt @@ -296,6 +296,7 @@ minmax mkmf Mkmf mkmfclone +mktemplate mocsy modelyaml modindex diff --git a/fre/__init__.py b/fre/__init__.py index 9aeab8ac6..cfe401402 100644 --- a/fre/__init__.py +++ b/fre/__init__.py @@ -10,7 +10,7 @@ fre_logger = logging.getLogger(__name__) -FORMAT = "[%(levelname)5s:%(filename)24s:%(funcName)20s] %(message)s" +FORMAT = "[%(levelname)5s:%(filename)24s:%(funcName)24s] %(message)s" logging.basicConfig(level = logging.WARNING, format = FORMAT, filename = None, diff --git a/fre/make/create_checkout_script.py b/fre/make/create_checkout_script.py index 521360aa5..0dedc3ec3 100644 --- a/fre/make/create_checkout_script.py +++ b/fre/make/create_checkout_script.py @@ -7,15 +7,39 @@ import os import subprocess import logging -import fre.yamltools.combine_yamls_script as cy from typing import Optional +import fre.yamltools.combine_yamls_script as cy from .gfdlfremake import varsfre, yamlfre, checkout, targetfre # set up logging fre_logger = logging.getLogger(__name__) +def baremetal_checkout_write(model_yaml, src_dir, jobs, pc, execute): + """ + Write the checkout script for bare-metal build + """ + fre_checkout = checkout.checkout("checkout.sh",src_dir) + fre_checkout.writeCheckout(model_yaml.compile.getCompileYaml(),jobs,pc) + fre_checkout.finish(model_yaml.compile.getCompileYaml(),pc) + + # Make checkout script executable + os.chmod(src_dir+"/checkout.sh", 0o744) + fre_logger.info("Checkout script created in %s/checkout.sh", src_dir ) + + if execute: + fre_checkout.run() + +def container_checkout_write(model_yaml, src_dir, tmp_dir, jobs, pc): + """ + Write the checkout script for container build + """ + fre_checkout = checkout.checkoutForContainer("checkout.sh", src_dir, tmp_dir) + fre_checkout.writeCheckout(model_yaml.compile.getCompileYaml(), jobs, pc) + fre_checkout.finish(model_yaml.compile.getCompileYaml(), pc) + fre_logger.info("Checkout script created in ./%s/checkout.sh", tmp_dir) + def checkout_create(yamlfile: str, platform: str, target: str, no_parallel_checkout: Optional[bool] = None, - njobs: int = 4, execute: Optional[bool] = False, verbose: Optional[bool] = None): + njobs: int = 4, execute: Optional[bool] = False, force_checkout: Optional[bool] = False): """ Creates the checkout script for bare-metal or container build The checkout script will clone component repositories, defined @@ -34,8 +58,6 @@ def checkout_create(yamlfile: str, platform: str, target: str, no_parallel_check :type njobs: int :param execute: Run the created checkout script to check out source code :type execute: bool - :param verbose: Increase verbosity output - :type verbose: bool :raises ValueError: - Error if 'jobs' param is not an integer - Error if platform does not exist in platforms yaml configuration @@ -48,7 +70,6 @@ def checkout_create(yamlfile: str, platform: str, target: str, no_parallel_check # Define variables yml = yamlfile name = yamlfile.split(".")[0] - run = execute jobs = str(njobs) pcheck = no_parallel_checkout @@ -59,13 +80,7 @@ def checkout_create(yamlfile: str, platform: str, target: str, no_parallel_check else: pc = " &" - if verbose: - fre_logger.setLevel(level = logging.DEBUG) - else: - fre_logger.setLevel(level = logging.INFO) - src_dir="src" - baremetal_run = False # This is needed if there are no bare metal runs ## Split and store the platforms and targets in a list plist = platform @@ -90,6 +105,7 @@ def checkout_create(yamlfile: str, platform: str, target: str, no_parallel_check for target_name in tlist: target = targetfre.fretarget(target_name) + fre_logger.setLevel(level = logging.INFO) ## Loop through the platforms specified on the command line ## If the platform is a baremetal platform, write the checkout script and run it once ## This should be done separately and serially because bare metal platforms should all be using @@ -98,50 +114,57 @@ def checkout_create(yamlfile: str, platform: str, target: str, no_parallel_check if model_yaml.platforms.hasPlatform(platform_name): pass else: - raise ValueError (platform_name + " does not exist in platforms.yaml") + raise ValueError (f"{platform_name} does not exist in platforms.yaml") platform = model_yaml.platforms.getPlatformFromName(platform_name) # create the source directory for the platform if not platform["container"]: - src_dir = platform["modelRoot"] + "/" + fremake_yaml["experiment"] + "/src" + src_dir = f'{platform["modelRoot"]}/{fremake_yaml["experiment"]}/src' # if the source directory does not exist, it is created if not os.path.exists(src_dir): - os.system("mkdir -p " + src_dir) + os.system(f"mkdir -p {src_dir}") # if the checkout script does not exist, it is created - if not os.path.exists(src_dir+"/checkout.sh"): - fre_checkout = checkout.checkout("checkout.sh",src_dir) - fre_checkout.writeCheckout(model_yaml.compile.getCompileYaml(),jobs,pc) - fre_checkout.finish(model_yaml.compile.getCompileYaml(),pc) - # Make checkout script executable - os.chmod(src_dir+"/checkout.sh", 0o744) - fre_logger.info("\nCheckout script created in %s/checkout.sh \n", src_dir ) - - # Run the checkout script - if run: - fre_checkout.run() - else: - return - - else: - fre_logger.info("\n....Checkout script PREVIOUSLY created in %s/checkout.sh \n", - src_dir) - if run: + if not os.path.exists(f"{src_dir}/checkout.sh"): + # Create and run (if --execute passed) the checkout script + baremetal_checkout_write(model_yaml, src_dir, jobs, pc, execute) + elif os.path.exists(f"{src_dir}/checkout.sh") and force_checkout: + fre_logger.info("Checkout script PREVIOUSLY created in %s/checkout.sh", src_dir) + fre_logger.info("*** REMOVING CHECKOUT SCRIPT ***") + # Remove the checkout script + os.remove(f"{src_dir}/checkout.sh") + + # Re-create and run (if --execute passed) the checkout script + baremetal_checkout_write(model_yaml, src_dir, jobs, pc, execute) + + elif os.path.exists(f"{src_dir}/checkout.sh") and not force_checkout: + fre_logger.info("Checkout script PREVIOUSLY created in %s/checkout.sh", src_dir) + if execute: try: subprocess.run(args=[src_dir+"/checkout.sh"], check=True) - except: - raise OSError("\nThere was an error with the checkout script "+src_dir+"/checkout.sh.", - "\nTry removing test folder: "+platform["modelRoot"] +"\n") - + except Exception as exc: + raise OSError(f"\nThere was an error with the checkout script {src_dir}/checkout.sh.", + f"\nTry removing test folder: {platform['modelRoot']}\n") from exc else: return else: - src_dir = platform["modelRoot"] + "/" + fremake_yaml["experiment"] + "/src" - bld_dir = platform["modelRoot"] + "/" + fremake_yaml["experiment"] + "/exec" - tmp_dir = "tmp/"+platform_name + src_dir = f'{platform["modelRoot"]}/{fremake_yaml["experiment"]}/src' + tmp_dir = f"tmp/{platform_name}" pc = "" #Set this way because containers do not support the parallel checkout - fre_checkout = checkout.checkoutForContainer("checkout.sh", src_dir, tmp_dir) - fre_checkout.writeCheckout(model_yaml.compile.getCompileYaml(),jobs,pc) - fre_checkout.finish(model_yaml.compile.getCompileYaml(),pc) - fre_logger.info("\nCheckout script created at %s/checkout.sh \n", tmp_dir) + if not os.path.exists(tmp_dir): + os.system(f"mkdir -p {tmp_dir}") + # If checkout script does not exist, it is created + if not os.path.exists(f"{tmp_dir}/checkout.sh"): + container_checkout_write(model_yaml, src_dir, tmp_dir, jobs, pc) + # If checkout script exists and force_checkout is used: + elif os.path.exists(f"{tmp_dir}/checkout.sh") and force_checkout: + fre_logger.info("Checkout script PREVIOUSLY created in %s/checkout.sh", tmp_dir) + fre_logger.info("*** REMOVING CHECKOUT SCRIPT ***") + # remove + os.remove(f"{tmp_dir}/checkout.sh") + # recreate + container_checkout_write(model_yaml, src_dir, tmp_dir, jobs, pc) + # If checkout script exists but force_checkout is not used + elif os.path.exists(f"{tmp_dir}/checkout.sh") and not force_checkout: + fre_logger.info("Checkout script PREVIOUSLY created in %s/checkout.sh", tmp_dir) diff --git a/fre/make/create_docker_script.py b/fre/make/create_docker_script.py index ba1bd2c87..d4fd31402 100644 --- a/fre/make/create_docker_script.py +++ b/fre/make/create_docker_script.py @@ -106,9 +106,9 @@ def dockerfile_create(yamlfile:str, platform:str, target:str, execute: Optional[ former_log_level = fre_logger.level fre_logger.setLevel(logging.INFO) - fre_logger.info("\ntmpDir created in " + currDir + "/tmp") - fre_logger.info("Dockerfile created in " + currDir +"\n") - fre_logger.info("Container build script created at "+dockerBuild.userScriptPath+"\n\n") + fre_logger.info("tmpDir created in " + currDir + "/tmp") + fre_logger.info("Dockerfile created in " + currDir) + fre_logger.info("Container build script created at "+dockerBuild.userScriptPath) fre_logger.setLevel(former_log_level) # run the script if option is given diff --git a/fre/make/create_makefile_script.py b/fre/make/create_makefile_script.py index fe1233582..b8bdcbe11 100644 --- a/fre/make/create_makefile_script.py +++ b/fre/make/create_makefile_script.py @@ -93,7 +93,7 @@ def makefile_create(yamlfile: str, platform: str, target:str): freMakefile.writeMakefile() former_log_level = fre_logger.level fre_logger.setLevel(logging.INFO) - fre_logger.info("\nMakefile created at " + bldDir + "/Makefile" + "\n") + fre_logger.info("Makefile created in " + bldDir + "/Makefile") fre_logger.setLevel(former_log_level) else: bldDir = platform["modelRoot"] + "/" + fremakeYaml["experiment"] + "/exec" @@ -115,5 +115,5 @@ def makefile_create(yamlfile: str, platform: str, target:str): freMakefile.writeMakefile() former_log_level = fre_logger.level fre_logger.setLevel(logging.INFO) - fre_logger.info("\nMakefile created at " + tmpDir + "/Makefile" + "\n") + fre_logger.info("Makefile created in " + tmpDir + "/Makefile") fre_logger.setLevel(former_log_level) diff --git a/fre/make/fremake.py b/fre/make/fremake.py index 30549a843..f2ab1f76b 100644 --- a/fre/make/fremake.py +++ b/fre/make/fremake.py @@ -19,7 +19,7 @@ """ PARALLEL_OPT_HELP = """Number of concurrent model compiles (default 1) """ -JOBS_OPT_HELP = """Number of jobs to run simultaneously. Used for make -jJOBS (parallelism with make) and git clone recursive --njobs=JOBS (# of submodules fetched simultaneously) +JOBS_OPT_HELP = """Number of jobs to run simultaneously; default=4. Used for make -jJOBS (parallelism with make) and git clone recursive --njobs=JOBS (# of submodules fetched simultaneously) """ NO_PARALLEL_CHECKOUT_OPT_HELP = """Use this option if you do not want a parallel checkout. The default is to have parallel checkouts. @@ -75,14 +75,17 @@ def make_cli(): is_flag = True, default = False, help = "Use this to run the created compilation script.") +@click.option("--force-checkout", + is_flag = True, + help = "Force checkout in case the source directory exists.") @click.option("-v", "--verbose", is_flag = True, help = VERBOSE_OPT_HELP) -def all(yamlfile, platform, target, nparallel, njobs, no_parallel_checkout, no_format_transfer, execute, verbose): +def all(yamlfile, platform, target, nparallel, njobs, no_parallel_checkout, no_format_transfer, execute, verbose, force_checkout): """ - Perform all fre make functions; run checkout and compile scripts to create model executable or container""" run_fremake_script.fremake_run( - yamlfile, platform, target, nparallel, njobs, no_parallel_checkout, no_format_transfer, execute, verbose) + yamlfile, platform, target, nparallel, njobs, no_parallel_checkout, no_format_transfer, execute, verbose, force_checkout) @make_cli.command() @click.option("-y", @@ -115,14 +118,13 @@ def all(yamlfile, platform, target, nparallel, njobs, no_parallel_checkout, no_f is_flag = True, default = False, help = "Use this to run the created checkout script.") -@click.option("-v", - "--verbose", +@click.option("--force-checkout", is_flag = True, - help = VERBOSE_OPT_HELP) -def checkout_script(yamlfile, platform, target, no_parallel_checkout, njobs, execute, verbose): + help = "Force checkout in case the source directory exists.") +def checkout_script(yamlfile, platform, target, no_parallel_checkout, njobs, execute, force_checkout): """ - Write the checkout script """ create_checkout_script.checkout_create( - yamlfile, platform, target, no_parallel_checkout, njobs, execute, verbose) + yamlfile, platform, target, no_parallel_checkout, njobs, execute, force_checkout) @make_cli.command @click.option("-y", diff --git a/fre/make/run_fremake_script.py b/fre/make/run_fremake_script.py index ba5c07864..d3d7c054c 100644 --- a/fre/make/run_fremake_script.py +++ b/fre/make/run_fremake_script.py @@ -21,6 +21,7 @@ def fremake_run(yamlfile:str, platform:str, target:str, no_parallel_checkout: Optional[bool] = None, no_format_transfer: Optional[bool] = False, execute: Optional[bool] = False, + force_checkout: Optional[bool] = False, verbose: Optional[bool] = None): """ Runs all of fre make code @@ -44,6 +45,8 @@ def fremake_run(yamlfile:str, platform:str, target:str, :param execute: Run the created compile script or dockerfile to create a model executable or container :type execute: bool + :param force_checkout: Re-create the checkout script if changes were made to configurations + :type force_checkout: bool :param verbose: Increase verbosity output :type verbose: bool """ @@ -73,7 +76,7 @@ def fremake_run(yamlfile:str, platform:str, target:str, #checkout fre_logger.info("Running fre make: calling checkout_create") checkout_create(yamlfile, platform, target, no_parallel_checkout, - njobs, execute, verbose) + njobs, execute, force_checkout) #makefile fre_logger.info("Running fre make: calling makefile_create") diff --git a/fre/make/tests/test_create_checkout.py b/fre/make/tests/test_create_checkout.py index 91006584c..f1a0c3110 100644 --- a/fre/make/tests/test_create_checkout.py +++ b/fre/make/tests/test_create_checkout.py @@ -2,24 +2,23 @@ Test fre make checkout-script """ from pathlib import Path -import os -import subprocess -from fre.make import create_checkout_script import shutil +from fre.make import create_checkout_script # Set example yaml paths, input directory TEST_DIR = str(Path("fre/make/tests")) YAMLFILE = str(Path(f"{TEST_DIR}/null_example/null_model.yaml")) -#set platform and target +# Set platform and target PLATFORM = ["ci.gnu"] +CONTAINER_PLATFORM = ["hpcme.2023"] TARGET = ["debug"] -#set output directory -# Set home for ~/cylc-src location in script -#run checkout command +# Set output directory OUT = f"{TEST_DIR}/checkout_out" -os.environ["TEST_BUILD_DIR"] = OUT + +# Set expected line that should be in checkout script +EXPECTED_LINE = "git clone --recursive --jobs=2 https://github.com/NOAA-GFDL/FMS.git -b 2025.04 FMS" def test_nullyaml_exists(): """ @@ -31,56 +30,182 @@ def test_nullyaml_filled(): """ Make sure null.yaml is not an empty file """ - sum(1 for _ in open(f'{YAMLFILE}')) > 1 + with open(YAMLFILE,'r') as f: + assert sum(1 for _ in f) >1 -def test_checkout_script_exists(): +def test_checkout_script_exists(monkeypatch): """ - Make sure checkout file exists + Test checkout-script was successful and that file exists. + Also checks that the default behavior is a parallel checkout. """ - os.environ["TEST_BUILD_DIR"] = OUT # env vars seem to be carrying over from other tests, need to set it again + # Set output directory as home for fre make output + monkeypatch.setenv("TEST_BUILD_DIR", OUT) + shutil.rmtree(f"{OUT}/fremake_canopy/test", ignore_errors=True) create_checkout_script.checkout_create(YAMLFILE, PLATFORM, TARGET, no_parallel_checkout = False, - njobs = False, execute = False, - verbose = False) + njobs = 2, + execute = False, + force_checkout = False) #assert result.exit_code == 0 assert Path(f"{OUT}/fremake_canopy/test/null_model_full/src/checkout.sh").exists() -def test_checkout_verbose(): + # A parallel checkout is done by default - check for subshells (parallelism in script) + expected_line = f"({EXPECTED_LINE}) &" + with open(f"{OUT}/fremake_canopy/test/null_model_full/src/checkout.sh", 'r') as f1: + content = f1.read() + assert expected_line in content + +def test_checkout_execute(monkeypatch): """ - check if --verbose option works + check if --execute option works """ + # Set output directory as home for fre make output + monkeypatch.setenv("TEST_BUILD_DIR", OUT) + + shutil.rmtree(f"{OUT}/fremake_canopy/test", ignore_errors=True) create_checkout_script.checkout_create(YAMLFILE, PLATFORM, TARGET, no_parallel_checkout = False, - njobs = False, + njobs = 2, + execute = True, + force_checkout = False) + + # Check for checkout script and for some resulting folders from running the script + assert all([Path(f"{OUT}/fremake_canopy/test/null_model_full/src/checkout.sh").exists(), + Path(f"{OUT}/fremake_canopy/test/null_model_full/src/FMS").is_dir(), + any(Path(f"{OUT}/fremake_canopy/test/null_model_full/src/FMS").iterdir()), + Path(f"{OUT}/fremake_canopy/test/null_model_full/src/coupler").is_dir(), + any(Path(f"{OUT}/fremake_canopy/test/null_model_full/src/coupler").iterdir())]) + +def test_checkout_no_parallel_checkout(monkeypatch): + """ + check if --no_parallel_checkout option works + """ + # Set output directory as home for fre make output + monkeypatch.setenv("TEST_BUILD_DIR", OUT) + + shutil.rmtree(f"{OUT}/fremake_canopy/test", ignore_errors=True) + create_checkout_script.checkout_create(YAMLFILE, + PLATFORM, + TARGET, + no_parallel_checkout = True, + njobs = 2, execute = False, - verbose = True) + force_checkout = False) + assert Path(f"{OUT}/fremake_canopy/test/null_model_full/src/checkout.sh").exists() + + expected_line = EXPECTED_LINE + with open(f"{OUT}/fremake_canopy/test/null_model_full/src/checkout.sh", 'r') as f: + content = f.read() -def test_checkout_execute(): + assert all([expected_line in content, + "pids" not in content, + "&" not in content]) + +def test_bm_checkout_force_checkout(caplog, monkeypatch): """ - check if --execute option works + Test re-creation of checkout script if --force-checkout is passed. """ + # Set output directory as home for fre make output + monkeypatch.setenv("TEST_BUILD_DIR", OUT) + + # Remove if previously created shutil.rmtree(f"{OUT}/fremake_canopy/test", ignore_errors=True) + + ## Mock checkout script with some content we can check + # Create come checkout script + mock_checkout = Path(f"{OUT}/fremake_canopy/test/null_model_full/src") + mock_checkout.mkdir(parents = True) + + # Write checkout + with open(f"{mock_checkout}/checkout.sh", 'w') as f: + f.write("mock bare-metal checkout content") + + # Check mock script was created correctly + assert Path(f"{mock_checkout}/checkout.sh").exists() + # Check mock content + with open(f"{mock_checkout}/checkout.sh", 'r') as f: + assert f.read() == "mock bare-metal checkout content" + + # Re-create checkout script create_checkout_script.checkout_create(YAMLFILE, PLATFORM, TARGET, no_parallel_checkout = False, njobs = 2, - execute = True, - verbose = False) + execute = False, + force_checkout = True) + + # Check it exists, check output, check content + assert all([Path(f"{OUT}/fremake_canopy/test/null_model_full/src/checkout.sh").exists(), + "Checkout script PREVIOUSLY created" in caplog.text, + "*** REMOVING CHECKOUT SCRIPT ***" in caplog.text, + "Checkout script created" in caplog.text]) + + # Check one expected line is now populating the re-created checkout script + expected_line = f"({EXPECTED_LINE}) &" + + with open(f"{OUT}/fremake_canopy/test/null_model_full/src/checkout.sh", 'r') as f2: + content = f2.read() + + assert all([expected_line in content, + "pids" in content, + "mock bare-metal checkout content" not in content]) -def test_checkout_no_parallel_checkout(): +def test_container_checkout_force_checkout(caplog): """ - check if --no_parallel_checkout option works + Test for the re-creation of the checkout script if + --force-checkout is passed. """ + # Remove if previously created from a different test + shutil.rmtree(f"./tmp/{CONTAINER_PLATFORM[0]}", ignore_errors=True) + + ## Mock checkout script with some content we can check + # Create come checkout script + mock_checkout = Path(f"./tmp/{CONTAINER_PLATFORM[0]}") + mock_checkout.mkdir(parents = True) + + # Write checkout + with open(f"{mock_checkout}/checkout.sh", 'w') as f: + f.write("mock container checkout content") + + # Check mock script was created correctly and check content + assert Path(f"{mock_checkout}/checkout.sh").exists() + with open(f"{mock_checkout}/checkout.sh", 'r') as f: + assert f.read() == "mock container checkout content" + + # Re-create checkout script create_checkout_script.checkout_create(YAMLFILE, - PLATFORM, + CONTAINER_PLATFORM, TARGET, no_parallel_checkout = True, - njobs = False, + njobs = 2, execute = False, - verbose = False) + force_checkout = True) + + # Check it mock checkout script exists + # Check output for script removal + # Check output for script creation + # Check content of re-created script + assert all([Path(f"{mock_checkout}/checkout.sh").exists(), + "Checkout script PREVIOUSLY created" in caplog.text, + "*** REMOVING CHECKOUT SCRIPT ***" in caplog.text, + "Checkout script created in ./tmp" in caplog.text]) + + # Check for an expected line that should be populating the re-created checkout script + # Check no parenthesis (no parallel checkouts) + # Check content did not just append (no previous content) + + expected_line = EXPECTED_LINE + with open(f"./tmp/{CONTAINER_PLATFORM[0]}/checkout.sh", "r") as f2: + content = f2.read() + + assert all([expected_line in content, + "pids" not in content, + "mock container checkout content" not in content]) + +##test checkout w/o force but if it already exists diff --git a/fre/tests/test_fre_cmor_cli.py b/fre/tests/test_fre_cmor_cli.py index bac762120..7ad4314b6 100644 --- a/fre/tests/test_fre_cmor_cli.py +++ b/fre/tests/test_fre_cmor_cli.py @@ -71,8 +71,8 @@ def test_cli_fre_cmor_help_and_debuglog(): assert result.exit_code == 0 assert Path("TEST_FOO_LOG.log").exists() - log_text_line_1='[ INFO: fre.py: fre] fre_file_handler added to base_fre_logger\n' - log_text_line_2='[DEBUG: fre.py: fre] click entry-point function call done.\n' + log_text_line_1='[ INFO: fre.py: fre] fre_file_handler added to base_fre_logger\n' + log_text_line_2='[DEBUG: fre.py: fre] click entry-point function call done.\n' with open( "TEST_FOO_LOG.log", 'r') as log_text: line_list=log_text.readlines() assert log_text_line_1 in line_list[0] @@ -90,7 +90,7 @@ def test_cli_fre_cmor_help_and_infolog(): assert result.exit_code == 0 assert Path("TEST_FOO_LOG.log").exists() - log_text_line_1='[ INFO: fre.py: fre] fre_file_handler added to base_fre_logger\n' + log_text_line_1='[ INFO: fre.py: fre] fre_file_handler added to base_fre_logger\n' with open( "TEST_FOO_LOG.log", 'r') as log_text: line_list=log_text.readlines() assert log_text_line_1 in line_list[0]