diff --git a/CHANGES.md b/CHANGES.md index 85a0f774925..d6332923fa9 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -35,6 +35,10 @@ Maintenance release. ### Fixes +[#5033](https://github.com/cylc/cylc-flow/pull/5033) - Running `cylc clean` +on a top level dir containing run dir(s) will now remove that top level dir +in addition to the run(s) (if there is nothing else inside it). + [#5007](https://github.com/cylc/cylc-flow/pull/5007) - Fix for `cylc broadcast` cycle point validation in the UI. diff --git a/cylc/flow/scripts/clean.py b/cylc/flow/scripts/clean.py index 50354992565..b53e9120692 100644 --- a/cylc/flow/scripts/clean.py +++ b/cylc/flow/scripts/clean.py @@ -47,7 +47,7 @@ $ cylc clean foo/bar --rm log --rm work # Remove all job log files from the 2020 cycle points - cylc clean foo/bar --rm 'log/job/2020*' + $ cylc clean foo/bar --rm 'log/job/2020*' # Remove all .csv files $ cylc clean foo/bar --rm '**/*.csv' diff --git a/cylc/flow/workflow_files.py b/cylc/flow/workflow_files.py index f688d3725af..344da8b5130 100644 --- a/cylc/flow/workflow_files.py +++ b/cylc/flow/workflow_files.py @@ -835,13 +835,8 @@ def clean(reg: str, run_dir: Path, rm_dirs: Optional[Set[str]] = None) -> None: if '' not in symlink_dirs: # if run dir isn't a symlink dir and hasn't been deleted yet remove_dir_and_target(run_dir) - # Tidy up if necessary - # Remove any empty parents of run dir up to ~/cylc-run/ - remove_empty_parents(run_dir, reg) - for symlink, target in symlink_dirs.items(): - # Remove empty parents of symlink target up to /cylc-run/ - remove_empty_parents(target, Path(reg, symlink)) + # Tidy up if necessary # Remove `runN` symlink if it's now broken runN = run_dir.parent / WorkflowFiles.RUN_N if ( @@ -850,6 +845,20 @@ def clean(reg: str, run_dir: Path, rm_dirs: Optional[Set[str]] = None) -> None: os.readlink(str(runN)) == run_dir.name ): runN.unlink() + # Remove _cylc-install if it's the only thing left + cylc_install_dir = run_dir.parent / WorkflowFiles.Install.DIRNAME + for entry in run_dir.parent.iterdir(): + if entry == cylc_install_dir: + continue + break + else: # no break + if cylc_install_dir.is_dir(): + remove_dir_or_file(cylc_install_dir) + # Remove any empty parents of run dir up to ~/cylc-run/ + remove_empty_parents(run_dir, reg) + for symlink, target in symlink_dirs.items(): + # Remove empty parents of symlink target up to /cylc-run/ + remove_empty_parents(target, Path(reg, symlink)) def get_symlink_dirs(reg: str, run_dir: Union[Path, str]) -> Dict[str, Path]: diff --git a/tests/functional/cylc-clean/03-multi.t b/tests/functional/cylc-clean/03-multi.t index 46af765c372..1410b967551 100644 --- a/tests/functional/cylc-clean/03-multi.t +++ b/tests/functional/cylc-clean/03-multi.t @@ -18,45 +18,115 @@ # Test cleaning multiple run dirs . "$(dirname "$0")/test_header" -set_test_number 14 +if ! command -v 'tree' > /dev/null; then + skip_all '"tree" command not available' +fi +set_test_number 18 + +FUNCTIONAL_DIR="${TEST_SOURCE_DIR_BASE%/*}" + +# ----------------------------------------------------------------------------- for _ in 1 2; do install_workflow "$TEST_NAME_BASE" basic-workflow true done -exists_ok "${WORKFLOW_RUN_DIR}/run1" -exists_ok "${WORKFLOW_RUN_DIR}/run2" +TEST_NAME="tree-pre-clean-1" +run_ok "${TEST_NAME}" tree --noreport --charset=ascii -L 4 "${HOME}/cylc-run/${CYLC_TEST_REG_BASE}" +# Note: backticks need to be escaped in the heredoc +cmp_ok "${TEST_NAME}.stdout" << __TREE__ +${HOME}/cylc-run/${CYLC_TEST_REG_BASE} +\`-- ${FUNCTIONAL_DIR} + \`-- cylc-clean + \`-- ${TEST_NAME_BASE} + |-- _cylc-install + |-- run1 + |-- run2 + \`-- runN -> run2 +__TREE__ # Test trying to clean multiple run dirs without --yes fails: run_fail "${TEST_NAME_BASE}-no" cylc clean "$WORKFLOW_NAME" exists_ok "${WORKFLOW_RUN_DIR}/run1" exists_ok "${WORKFLOW_RUN_DIR}/run2" - -# Should work with --yes: +# Should work with --yes (removes top level dir too): run_ok "${TEST_NAME_BASE}-yes" cylc clean -y "$WORKFLOW_NAME" -exists_fail "${WORKFLOW_RUN_DIR}/run1" -exists_fail "${WORKFLOW_RUN_DIR}/run2" +exists_fail "${HOME}/cylc-run/${CYLC_TEST_REG_BASE}" -# Should continue cleaning a list of worflows even if one fails. +# ----------------------------------------------------------------------------- +# Should continue cleaning a list of workflows even if one fails. for _ in 1 2; do install_workflow "$TEST_NAME_BASE" basic-workflow true done -exists_ok "${WORKFLOW_RUN_DIR}/run1" -exists_ok "${WORKFLOW_RUN_DIR}/run2" +TEST_NAME="tree-pre-clean-2" +run_ok "${TEST_NAME}" tree --noreport --charset=ascii -L 4 "${HOME}/cylc-run/${CYLC_TEST_REG_BASE}" +cmp_ok "${TEST_NAME}.stdout" << __TREE__ +${HOME}/cylc-run/${CYLC_TEST_REG_BASE} +\`-- ${FUNCTIONAL_DIR} + \`-- cylc-clean + \`-- ${TEST_NAME_BASE} + |-- _cylc-install + |-- run1 + |-- run2 + \`-- runN -> run2 +__TREE__ mkdir "${WORKFLOW_RUN_DIR}/run1/.service" touch "${WORKFLOW_RUN_DIR}/run1/.service/db" # corrupted db! -TEST_NAME="${TEST_NAME_BASE}-yes-no" +TEST_NAME="${TEST_NAME_BASE}-yes-no" run_fail "${TEST_NAME}" \ cylc clean -y "$WORKFLOW_NAME/run1" "$WORKFLOW_NAME/run2" grep_ok "Cannot clean .*/run1" "${TEST_NAME}.stderr" -e -exists_ok "${WORKFLOW_RUN_DIR}/run1" -exists_fail "${WORKFLOW_RUN_DIR}/run2" +TEST_NAME="tree-post-clean-2" +run_ok "${TEST_NAME}" tree --noreport --charset=ascii -L 4 "${HOME}/cylc-run/${CYLC_TEST_REG_BASE}" +cmp_ok "${TEST_NAME}.stdout" << __TREE__ +${HOME}/cylc-run/${CYLC_TEST_REG_BASE} +\`-- ${FUNCTIONAL_DIR} + \`-- cylc-clean + \`-- ${TEST_NAME_BASE} + |-- _cylc-install + \`-- run1 +__TREE__ + +purge + +# ----------------------------------------------------------------------------- +# Should not clean top level dir if not empty. + +install_workflow "$TEST_NAME_BASE" basic-workflow true + +touch "${WORKFLOW_RUN_DIR}/jellyfish.txt" + +TEST_NAME="tree-pre-clean-3" +run_ok "${TEST_NAME}" tree --noreport --charset=ascii -L 4 "${HOME}/cylc-run/${CYLC_TEST_REG_BASE}" +cmp_ok "${TEST_NAME}.stdout" << __TREE__ +${HOME}/cylc-run/${CYLC_TEST_REG_BASE} +\`-- ${FUNCTIONAL_DIR} + \`-- cylc-clean + \`-- ${TEST_NAME_BASE} + |-- _cylc-install + |-- jellyfish.txt + |-- run1 + \`-- runN -> run1 +__TREE__ + +run_ok "${TEST_NAME}" cylc clean -y "$WORKFLOW_NAME" + +TEST_NAME="tree-post-clean-3" +run_ok "${TEST_NAME}" tree --noreport --charset=ascii -L 4 "${HOME}/cylc-run/${CYLC_TEST_REG_BASE}" +cmp_ok "${TEST_NAME}.stdout" << __TREE__ +${HOME}/cylc-run/${CYLC_TEST_REG_BASE} +\`-- ${FUNCTIONAL_DIR} + \`-- cylc-clean + \`-- ${TEST_NAME_BASE} + |-- _cylc-install + \`-- jellyfish.txt +__TREE__ purge diff --git a/tests/unit/test_workflow_files.py b/tests/unit/test_workflow_files.py index 5e14f5c2867..01f028a3e47 100644 --- a/tests/unit/test_workflow_files.py +++ b/tests/unit/test_workflow_files.py @@ -1412,6 +1412,28 @@ def test_remote_clean_cmd( assert constructed_cmd == ['clean', '--local-only', reg, *expected_args] +def test_clean_top_level(tmp_run_dir: Callable): + """Test that cleaning last remaining run dir inside a workflow dir removes + the top level dir if it's empty (excluding _cylc-install).""" + # Setup + reg = 'blue/planet/run1' + run_dir: Path = tmp_run_dir(reg, installed=True, named=True) + cylc_install_dir = run_dir.parent / WorkflowFiles.Install.DIRNAME + assert cylc_install_dir.is_dir() + runN_symlink = run_dir.parent / WorkflowFiles.RUN_N + assert runN_symlink.exists() + # Test + clean(reg, run_dir) + assert not run_dir.parent.parent.exists() + # Now check that if the top level dir is not empty, it doesn't get removed + run_dir: Path = tmp_run_dir(reg, installed=True, named=True) + jellyfish_file = (run_dir.parent / 'jellyfish.txt') + jellyfish_file.touch() + clean(reg, run_dir) + assert cylc_install_dir.is_dir() + assert jellyfish_file.exists() + + def test_get_workflow_source_dir_numbered_run(tmp_path): """Test get_workflow_source_dir returns correct source for numbered run""" cylc_install_dir = (