diff --git a/changes.d/6658.fix.md b/changes.d/6658.fix.md
new file mode 100644
index 00000000000..51876a73e37
--- /dev/null
+++ b/changes.d/6658.fix.md
@@ -0,0 +1 @@
+Fixed `cylc reinstall` not picking up file permissions changes.
diff --git a/cylc/flow/install.py b/cylc/flow/install.py
index 87b8b6b0565..284f7ff9e77 100644
--- a/cylc/flow/install.py
+++ b/cylc/flow/install.py
@@ -213,19 +213,16 @@ def reinstall_workflow(
dry_run=dry_run,
)
- # Add '+++' to -out-format to mark lines passed through formatter.
- rsync_cmd.append('--out-format=+++%o %n%L+++')
+ rsync_cmd.append('--out-format=%i %o %n%L')
+ # %i: itemized changes - needed for rsync to report files with
+ # changed permissions
# Run rsync command:
reinstall_log.info(cli_format(rsync_cmd))
LOG.debug(cli_format(rsync_cmd))
proc = Popen(rsync_cmd, stdout=PIPE, stderr=PIPE, text=True) # nosec
# * command is constructed via internal interface
- stdout, stderr = proc.communicate()
-
- # Strip unwanted output.
- stdout = ('\n'.join(re.findall(r'\+\+\+(.*)\+\+\+', stdout))).strip()
- stderr = stderr.strip()
+ stdout, stderr = (i.strip() for i in proc.communicate())
if proc.returncode != 0:
raise WorkflowFilesError(
diff --git a/cylc/flow/remote.py b/cylc/flow/remote.py
index e0ab7544a39..15ab7ab2f96 100644
--- a/cylc/flow/remote.py
+++ b/cylc/flow/remote.py
@@ -183,6 +183,9 @@ def get_includes_to_rsync(rsync_includes=None):
'--out-format=%o %n%L',
'--no-t'
]
+# %o: the operation (send or del.)
+# %n: filename
+# %L: "-> symlink_target" if applicable
DEFAULT_INCLUDES = [
'/ana/***', # Rose ana analysis modules
diff --git a/cylc/flow/scripts/reinstall.py b/cylc/flow/scripts/reinstall.py
index 1c545944be4..3e55e6edd7f 100644
--- a/cylc/flow/scripts/reinstall.py
+++ b/cylc/flow/scripts/reinstall.py
@@ -68,10 +68,16 @@
"cylc install" even if present in the source directory.
"""
+from functools import partial
from pathlib import Path
+import re
import sys
-from typing import Optional, TYPE_CHECKING, List, Callable
-from functools import partial
+from typing import (
+ TYPE_CHECKING,
+ Callable,
+ List,
+ Optional,
+)
from ansimarkup import parse as cparse
@@ -79,22 +85,26 @@
ServiceFileError,
WorkflowFilesError,
)
-from cylc.flow.install import (
- reinstall_workflow,
-)
+import cylc.flow.flags
+from cylc.flow.install import reinstall_workflow
from cylc.flow.network.multi import call_multi
from cylc.flow.option_parsers import (
+ ID_MULTI_ARG_DOC,
CylcOptionParser as COP,
OptionSettings,
- ID_MULTI_ARG_DOC
)
from cylc.flow.pathutil import get_workflow_run_dir
from cylc.flow.plugins import run_plugins_async
+from cylc.flow.terminal import (
+ DIM,
+ cli_function,
+ is_terminal,
+)
from cylc.flow.workflow_files import (
get_workflow_source_dir,
load_contact_file,
)
-from cylc.flow.terminal import cli_function, DIM, is_terminal
+
if TYPE_CHECKING:
from optparse import Values
@@ -269,9 +279,8 @@ async def reinstall(
update anything, else returns True.
"""
- # run pre_configure plugins
if not dry_run:
- # don't run plugins in dry-mode
+ # run pre_configure plugins
async for _entry_point, _plugin_result in run_plugins_async(
'cylc.pre_configure',
srcdir=src_dir,
@@ -287,18 +296,17 @@ async def reinstall(
dry_run=dry_run,
)
- # display changes
if dry_run:
- if not stdout or stdout == 'send ./':
- # no rsync output == no changes => exit
+ # display changes
+ changes = format_reinstall_output(stdout)
+ if not changes:
return False
# display rsync output
- write('\n'.join(format_rsync_out(stdout)), file=sys.stderr)
+ write('\n'.join(changes), file=sys.stdout)
- # run post_install plugins
if not dry_run:
- # don't run plugins in dry-mode
+ # run post_install plugins
async for _entry_point, _plugin_result in run_plugins_async(
'cylc.post_install',
srcdir=src_dir,
@@ -311,15 +319,15 @@ async def reinstall(
return True
-def format_rsync_out(out: str) -> List[str]:
+def format_reinstall_output(out: str) -> List[str]:
r"""Format rsync stdout for presenting to users.
Note: Output formats of different rsync implementations may differ so keep
this code simple and robust.
Example:
- >>> format_rsync_out(
- ... 'send foo\ndel. bar\nbaz'
+ >>> format_reinstall_output(
+ ... '>f+++++++++ send foo\n*deleting del. bar\nbaz'
... '\ncannot delete non-empty directory: opt'
... ) == [
... cparse('send foo'),
@@ -331,20 +339,23 @@ def format_rsync_out(out: str) -> List[str]:
"""
lines = []
for line in out.splitlines():
- if line[0:4] == 'send':
- # file added or updated
- lines.append(cparse(f'{line}'))
- elif line[0:4] == 'del.':
- # file deleted
- lines.append(cparse(f'{line}'))
- elif line == 'cannot delete non-empty directory: opt':
- # These "cannot delete non-empty directory" messages can arise
- # as a result of excluding files within sub-directories.
- # This opt dir message is likely to occur when a rose-suit.conf
+ if not line or line.startswith('cannot delete non-empty directory:'):
+ # This can arise as a result of deleting a subdir when we are
+ # excluding files within the subdir.
+ # This is likely to occur for the opt dir when a rose-suite.conf
# file is present.
+ # Skip this line as nothing will happen to this dir.
continue
+ match = re.match(r'^(.{11}) (send|del\.) (.*)$', line)
+ if match:
+ summary, operation, file = match.groups()
+ color = 'green' if operation == 'send' else 'red'
+ formatted_line = f"<{color}>{operation} {file}"
+ if cylc.flow.flags.verbosity > 0:
+ formatted_line = f"<{DIM}>{summary} {formatted_line}"
+ lines.append(cparse(formatted_line))
else:
- # other uncategorised log line
+ # shouldn't happen; tolerate unknown rsync implementation?
lines.append(line)
return lines
diff --git a/tests/integration/test_reinstall.py b/tests/integration/test_reinstall.py
index d1d89b66b2d..00f2de7cb12 100644
--- a/tests/integration/test_reinstall.py
+++ b/tests/integration/test_reinstall.py
@@ -19,28 +19,23 @@
import asyncio
from contextlib import asynccontextmanager
from pathlib import Path
+from secrets import token_hex
from types import SimpleNamespace
-from uuid import uuid1
import pytest
-from cylc.flow.exceptions import (
- WorkflowFilesError,
-)
-from cylc.flow.install import (
- reinstall_workflow,
-)
+from cylc.flow.exceptions import WorkflowFilesError
+from cylc.flow.install import reinstall_workflow
from cylc.flow.option_parsers import Options
from cylc.flow.scripts.reinstall import (
get_option_parser as reinstall_gop,
reinstall_cli,
)
-from cylc.flow.workflow_files import (
- WorkflowFiles,
-)
+from cylc.flow.workflow_files import WorkflowFiles
from .utils.entry_points import EntryPointWrapper
+
ReInstallOptions = Options(reinstall_gop())
# cli opts
@@ -66,9 +61,22 @@ def non_interactive(monkeypatch):
)
+@pytest.fixture
+def answer_prompt(monkeypatch: pytest.MonkeyPatch):
+ """Answer reinstall prompt."""
+
+ def inner(answer: str):
+ monkeypatch.setattr(
+ 'cylc.flow.scripts.reinstall._input', lambda x: answer
+ )
+
+ return inner
+
+
@pytest.fixture
def one_src(tmp_path):
- src_dir = tmp_path
+ src_dir = tmp_path / 'src'
+ src_dir.mkdir()
(src_dir / 'flow.cylc').touch()
(src_dir / 'rose-suite.conf').touch()
return SimpleNamespace(path=src_dir)
@@ -76,7 +84,7 @@ def one_src(tmp_path):
@pytest.fixture
def one_run(one_src, test_dir, run_dir):
- w_run_dir = test_dir / str(uuid1())
+ w_run_dir = test_dir / token_hex(4)
w_run_dir.mkdir()
(w_run_dir / 'flow.cylc').touch()
(w_run_dir / 'rose-suite.conf').touch()
@@ -151,7 +159,7 @@ async def test_interactive(
capsys,
capcall,
interactive,
- monkeypatch
+ answer_prompt
):
"""It should perform a dry-run and prompt in interactive mode."""
# capture reinstall calls
@@ -162,11 +170,7 @@ async def test_interactive(
# give it something to reinstall
(one_src.path / 'a').touch()
- # reinstall answering "no" to any prompt
- monkeypatch.setattr(
- 'cylc.flow.scripts.reinstall._input',
- lambda x: 'n'
- )
+ answer_prompt('n')
assert (
await reinstall_cli(opts=ReInstallOptions(), workflow_id=one_run.id)
is False
@@ -177,11 +181,7 @@ async def test_interactive(
assert 'Reinstall canceled, no changes made.' in capsys.readouterr().out
reinstall_calls.clear()
- # reinstall answering "yes" to any prompt
- monkeypatch.setattr(
- 'cylc.flow.scripts.reinstall._input',
- lambda x: 'y'
- )
+ answer_prompt('y')
assert await reinstall_cli(opts=ReInstallOptions(), workflow_id=one_run.id)
# two rsync calls should have been made (i.e. the --dry-run and the real)
@@ -241,7 +241,7 @@ async def test_rsync_stuff(one_src, one_run, capsys, non_interactive):
async def test_rose_warning(
- one_src, one_run, capsys, interactive, monkeypatch
+ one_src, one_run, capsys, interactive, answer_prompt
):
"""It should warn that Rose installed files will be deleted.
@@ -252,11 +252,7 @@ async def test_rose_warning(
'Files created by Rose file installation will show as deleted'
)
- # reinstall answering "no" to any prompt
- monkeypatch.setattr(
- 'cylc.flow.scripts.reinstall._input',
- lambda x: 'n'
- )
+ answer_prompt('n')
(one_src.path / 'a').touch() # give it something to install
# reinstall (with rose-suite.conf file)
@@ -311,6 +307,35 @@ async def test_rsync_fail(one_src, one_run, mock_glbl_cfg, non_interactive):
assert 'An error occurred reinstalling' in str(exc_ctx.value)
+async def test_permissions_change(
+ one_src,
+ one_run,
+ interactive,
+ answer_prompt,
+ monkeypatch: pytest.MonkeyPatch,
+ capsys: pytest.CaptureFixture,
+):
+ """It detects permissions changes."""
+ # Add script file:
+ script_path: Path = one_src.path / 'myscript'
+ script_path.touch()
+ await reinstall_cli(
+ opts=ReInstallOptions(skip_interactive=True), workflow_id=one_run.id
+ )
+ assert (one_run.path / 'myscript').is_file()
+ capsys.readouterr() # clears capsys
+
+ # Change permissions (e.g. user forgot to make it executable before)
+ script_path.chmod(0o777)
+ # Answer "no" to reinstall prompt (we just want to test dry run)
+ answer_prompt('n')
+ await reinstall_cli(
+ opts=ReInstallOptions(), workflow_id=one_run.id
+ )
+ out, _ = capsys.readouterr()
+ assert "send myscript" in out
+
+
@pytest.fixture
def my_install_plugin(monkeypatch):
"""This configures a single post_install plugin.
diff --git a/tests/unit/scripts/test_reinstall.py b/tests/unit/scripts/test_reinstall.py
new file mode 100644
index 00000000000..822069fe054
--- /dev/null
+++ b/tests/unit/scripts/test_reinstall.py
@@ -0,0 +1,56 @@
+# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE.
+# Copyright (C) NIWA & British Crown (Met Office) & Contributors.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see .
+
+from textwrap import dedent
+
+from ansimarkup import parse as cparse
+import pytest
+
+from cylc.flow.scripts.reinstall import format_reinstall_output
+from cylc.flow.terminal import DIM
+
+
+@pytest.mark.parametrize('verbose', [True, False])
+def test_format_reinstall_output(verbose, monkeypatch: pytest.MonkeyPatch):
+ """It should:
+ - colorize the output
+ - remove the itemized changes summary if not in verbose mode
+ - remove the "cannot delete non-empty directory" message
+ """
+ output = dedent("""
+ *deleting del. Cloud.jpg
+ >f+++++++++ send cloud.jpg
+ .f...p..... send foo
+ >fcsTp..... send bar
+ cannot delete non-empty directory: scarf
+ >f+++++++++ send meow.txt
+ cL+++++++++ send garage -> foo
+ """).strip()
+ expected = [
+ f"<{DIM}>*deleting del. Cloud.jpg",
+ f"<{DIM}>>f+++++++++ send cloud.jpg",
+ f"<{DIM}>.f...p..... send foo",
+ f"<{DIM}>>fcsTp..... send bar",
+ f"<{DIM}>>f+++++++++ send meow.txt",
+ f"<{DIM}>cL+++++++++ send garage -> foo",
+ ]
+ if verbose:
+ monkeypatch.setattr('cylc.flow.flags.verbosity', 1)
+ else:
+ # itemized changes summary should not be in output
+ shift = len(f'<{DIM}> ') + 11
+ expected = [i[shift:] for i in expected]
+ assert format_reinstall_output(output) == [cparse(i) for i in expected]