Skip to content

Commit

Permalink
Merge pull request #6364 from MetRonnie/cylc-clean
Browse files Browse the repository at this point in the history 
`cylc clean`: fix bug where `--rm share` would not clean `share/cycle` symlink first
  • Loading branch information
@wxtim
wxtim authored Sep 26, 2024
2 parents 87bb2d4 + 128e584 commit a722b26
Show file tree
Hide file tree
Showing 6 changed files with 212 additions and 117 deletions.
1 change: 1 addition & 0 deletions changes.d/6364.fix.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed bug where `cylc clean <workflow> --rm share` would not take care of removing the target of the `share/cycle` symlink directory.
30 changes: 18 additions & 12 deletions cylc/flow/clean.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
)
from cylc.flow.pathutil import (
get_workflow_run_dir,
is_relative_to,
parse_rm_dirs,
remove_dir_and_target,
remove_dir_or_file,
Expand Down Expand Up @@ -116,12 +117,8 @@ def _clean_check(opts: 'Values', id_: str, run_dir: Path) -> None:
# Thing to clean must be a dir or broken symlink:
if not run_dir.is_dir() and not run_dir.is_symlink():
raise FileNotFoundError(f"No directory to clean at {run_dir}")
db_path = (
run_dir / WorkflowFiles.Service.DIRNAME / WorkflowFiles.Service.DB
)
if opts.local_only and not db_path.is_file():
# Will reach here if this is cylc clean re-invoked on remote host
# (workflow DB only exists on scheduler host); don't need to worry
if opts.no_scan:
# This is cylc clean re-invoked on remote host; don't need to worry
# about contact file.
return
try:
Expand Down Expand Up @@ -259,7 +256,7 @@ def glob_in_run_dir(
"""Execute a (recursive) glob search in the given run directory.
Returns list of any absolute paths that match the pattern. However:
* Does not follow symlinks (apart from the spcedified symlink dirs).
* Does not follow symlinks (apart from the specified symlink dirs).
* Also does not return matching subpaths of matching directories (because
that would be redundant).
Expand All @@ -281,6 +278,9 @@ def glob_in_run_dir(
results: List[Path] = []
subpath_excludes: Set[Path] = set()
for path in matches:
# Iterate down through ancestors (starting at the run dir) to
# weed out redundant subpaths of matched directories and subpaths of
# non-standard symlinks
for rel_ancestor in reversed(path.relative_to(run_dir).parents):
ancestor = run_dir / rel_ancestor
if ancestor in subpath_excludes:
Expand Down Expand Up @@ -326,13 +326,19 @@ def _clean_using_glob(
LOG.info(f"No files matching '{pattern}' in {run_dir}")
return
# First clean any matching symlink dirs
for path in abs_symlink_dirs:
if path in matches:
remove_dir_and_target(path)
if path == run_dir:
for symlink_dir in abs_symlink_dirs:
# Note: must clean e.g. share/cycle/ before share/ if the former
# is a symlink even if only the latter was specified.
if (
any(is_relative_to(symlink_dir, path) for path in matches)
and symlink_dir.is_symlink()
):
remove_dir_and_target(symlink_dir)
if symlink_dir == run_dir:
# We have deleted the run dir
return
matches.remove(path)
if symlink_dir in matches:
matches.remove(symlink_dir)
# Now clean the rest
for path in matches:
remove_dir_or_file(path)
Expand Down
3 changes: 2 additions & 1 deletion cylc/flow/pathutil.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,8 @@ def parse_rm_dirs(rm_dirs: Iterable[str]) -> Set[str]:


def is_relative_to(path1: Union[Path, str], path2: Union[Path, str]) -> bool:
"""Return whether or not path1 is relative to path2.
"""Return whether or not path1 is relative to path2 (including if they are
the same path).
Normalizes both paths to avoid trickery with paths containing `..`
somewhere in them.
Expand Down
222 changes: 146 additions & 76 deletions tests/unit/filetree.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,21 @@
'file.txt': None
}
},
# Symlinks are represented by pathlib.Path, with the target represented
# by the relative path from the tmp_path directory:
'symlink': Path('dir/another-dir')
# Symlinks are represented by the Symlink class, with the target
# represented by the relative path from the tmp_path directory:
'symlink': Symlink('dir/another-dir')
}
"""

from pathlib import Path
from pathlib import Path, PosixPath
from typing import Any, Dict, List


class Symlink(PosixPath):
"""A class to represent a symlink target."""
...


def create_filetree(
filetree: Dict[str, Any], location: Path, root: Path
) -> None:
Expand All @@ -53,10 +58,9 @@ def create_filetree(
if isinstance(entry, dict):
path.mkdir(exist_ok=True)
create_filetree(entry, path, root)
elif isinstance(entry, Path):
elif isinstance(entry, Symlink):
path.symlink_to(root / entry)
else:

path.touch()


Expand All @@ -79,89 +83,155 @@ def get_filetree_as_list(


FILETREE_1 = {
'cylc-run': {'foo': {'bar': {
'.service': {'db': None},
'flow.cylc': None,
'log': Path('sym/cylc-run/foo/bar/log'),
'mirkwood': Path('you-shall-not-pass/mirkwood'),
'rincewind.txt': Path('you-shall-not-pass/rincewind.txt')
}}},
'sym': {'cylc-run': {'foo': {'bar': {
'log': {
'darmok': Path('you-shall-not-pass/darmok'),
'temba.txt': Path('you-shall-not-pass/temba.txt'),
'bib': {
'fortuna.txt': None
}
}
}}}},
'cylc-run': {
'foo': {
'bar': {
'.service': {
'db': None,
},
'flow.cylc': None,
'log': Symlink('sym/cylc-run/foo/bar/log'),
'mirkwood': Symlink('you-shall-not-pass/mirkwood'),
'rincewind.txt': Symlink('you-shall-not-pass/rincewind.txt'),
},
},
},
'sym': {
'cylc-run': {
'foo': {
'bar': {
'log': {
'darmok': Symlink('you-shall-not-pass/darmok'),
'temba.txt': Symlink('you-shall-not-pass/temba.txt'),
'bib': {
'fortuna.txt': None,
},
},
},
},
},
},
'you-shall-not-pass': { # Nothing in here should get deleted
'darmok': {
'jalad.txt': None
'jalad.txt': None,
},
'mirkwood': {
'spiders.txt': None
'spiders.txt': None,
},
'rincewind.txt': None,
'temba.txt': None
}
'temba.txt': None,
},
}

FILETREE_2 = {
'cylc-run': {'foo': {'bar': Path('sym-run/cylc-run/foo/bar')}},
'sym-run': {'cylc-run': {'foo': {'bar': {
'.service': {'db': None},
'flow.cylc': None,
'share': Path('sym-share/cylc-run/foo/bar/share')
}}}},
'sym-share': {'cylc-run': {'foo': {'bar': {
'share': {
'cycle': Path('sym-cycle/cylc-run/foo/bar/share/cycle')
}
}}}},
'sym-cycle': {'cylc-run': {'foo': {'bar': {
'share': {
'cycle': {
'macklunkey.txt': None
}
}
}}}},
'you-shall-not-pass': {}
'cylc-run': {'foo': {'bar': Symlink('sym-run/cylc-run/foo/bar')}},
'sym-run': {
'cylc-run': {
'foo': {
'bar': {
'.service': {
'db': None,
},
'flow.cylc': None,
'share': Symlink('sym-share/cylc-run/foo/bar/share'),
},
},
},
},
'sym-share': {
'cylc-run': {
'foo': {
'bar': {
'share': {
'cycle': Symlink(
'sym-cycle/cylc-run/foo/bar/share/cycle'
),
},
},
},
},
},
'sym-cycle': {
'cylc-run': {
'foo': {
'bar': {
'share': {
'cycle': {
'macklunkey.txt': None,
},
},
},
},
},
},
'you-shall-not-pass': {},
}

FILETREE_3 = {
'cylc-run': {'foo': {'bar': Path('sym-run/cylc-run/foo/bar')}},
'sym-run': {'cylc-run': {'foo': {'bar': {
'.service': {'db': None},
'flow.cylc': None,
'share': {
'cycle': Path('sym-cycle/cylc-run/foo/bar/share/cycle')
}
}}}},
'sym-cycle': {'cylc-run': {'foo': {'bar': {
'share': {
'cycle': {
'sokath.txt': None
}
}
}}}},
'you-shall-not-pass': {}
'cylc-run': {
'foo': {
'bar': Symlink('sym-run/cylc-run/foo/bar'),
},
},
'sym-run': {
'cylc-run': {
'foo': {
'bar': {
'.service': {
'db': None,
},
'flow.cylc': None,
'share': {
'cycle': Symlink(
'sym-cycle/cylc-run/foo/bar/share/cycle'
),
},
},
},
},
},
'sym-cycle': {
'cylc-run': {
'foo': {
'bar': {
'share': {
'cycle': {
'sokath.txt': None,
},
},
},
},
},
},
'you-shall-not-pass': {},
}

FILETREE_4 = {
'cylc-run': {'foo': {'bar': {
'.service': {'db': None},
'flow.cylc': None,
'share': {
'cycle': Path('sym-cycle/cylc-run/foo/bar/share/cycle')
}
}}},
'sym-cycle': {'cylc-run': {'foo': {'bar': {
'share': {
'cycle': {
'kiazi.txt': None
}
}
}}}},
'you-shall-not-pass': {}
'cylc-run': {
'foo': {
'bar': {
'.service': {
'db': None,
},
'flow.cylc': None,
'share': {
'cycle': Symlink('sym-cycle/cylc-run/foo/bar/share/cycle'),
},
},
},
},
'sym-cycle': {
'cylc-run': {
'foo': {
'bar': {
'share': {
'cycle': {
'kiazi.txt': None,
},
},
},
},
},
},
'you-shall-not-pass': {},
}
Loading

0 comments on commit a722b26

Please sign in to comment.