Reinstall picks up installed config. (#345)

ensure that vr picks up installed config.

* Update pyproject.toml
* don't display warnings about ROSE_ORIG_HOST when not relevant
* updated lower pin on cylc version
* de-flake test, unrelated to PR
* updated test for Python 3.7

---------

Co-authored-by: Oliver Sanders <[email protected]>

CHANGES.md

@@ -6,6 +6,11 @@ creating a new release entry be sure to copy & paste the span tag with the
 updated. Only the first match gets replaced, so it's fine to leave the old
 ones in. -->
 
+## __cylc-rose-1.4.2 (<span actions:bind='release-date'>Upcoming</span>)__
+
+[#345](https://github.com/cylc/cylc-rose/pull/345) - Fix an issue
+where `cylc vr` could report erroneous validation failures.
+
 ## __cylc-rose-1.4.1 (<span actions:bind='release-date'>Released 2024-07-23</span>)__
 
 No significant change - Updated to use feature added at Cylc 8.3.3.

cylc/rose/entry_points.py

@@ -27,7 +27,9 @@
     load_rose_config,
     process_config,
     record_cylc_install_options,
+    retrieve_installed_cli_opts,
     rose_config_exists,
+    sanitize_opts,
 )
 
 if TYPE_CHECKING:
@@ -40,7 +42,17 @@ def pre_configure(srcdir: Path, opts: 'Values') -> dict:
         # nothing to do here
         return {}
 
-    # load the Rose config
+    opts = sanitize_opts(opts)
+
+    # If we are validating against source, load saved CLI options
+    # from previous install, as saved in the rose-suite-cylc-install.conf
+    if (
+        getattr(opts, 'against_source', False)
+        and isinstance(opts.against_source, Path)
+    ):
+        opts = retrieve_installed_cli_opts(srcdir, opts)
+
+    # load the source Rose config
     config_tree = load_rose_config(Path(srcdir), opts=opts)
 
     # extract plugin return information from the Rose config

cylc/rose/stem.py

@@ -575,7 +575,7 @@ def get_rose_stem_opts():
             " the groups into a series of tasks to run."
         ),
         action="append",
-        metavar="PATH/TO/FLOW",
+        metavar="STRING",
         default=[],
         dest="stem_groups")
     rose_stem_options.add_option(

cylc/rose/utilities.py

@@ -16,6 +16,7 @@
 
 """Cylc support for reading and interpreting ``rose-suite.conf`` files."""
 
+from contextlib import suppress
 import itertools
 import os
 from pathlib import Path
@@ -55,6 +56,11 @@
 )
 MESSAGE = 'message'
 ALL_MODES = 'all modes'
+STANDARD_VARS = [
+    ('ROSE_ORIG_HOST', get_host()),
+    ('ROSE_VERSION', ROSE_VERSION),
+    ('CYLC_VERSION', SET_BY_CYLC)
+]
 
 
 class NotARoseSuiteException(Exception):
@@ -119,9 +125,6 @@ def process_config(
     if templating not in config_node.value:
         config_node.set([templating])
 
-    # Get Rose Orig host:
-    rose_orig_host = get_host()
-
     # For each section process variables and add standard variables.
     for section in ['env', templating]:
 
@@ -131,17 +134,13 @@ def process_config(
         # ROSE_ORIG_HOST - If it's the config, replace it, unless it has a
         # comment marking it as having been saved by ``cylc install``.
         # In all cases warn users if the value in their config is not used.
-        for var_name, replace_with in [
-            ('ROSE_ORIG_HOST', rose_orig_host),
-            ('ROSE_VERSION', ROSE_VERSION),
-            ('CYLC_VERSION', SET_BY_CYLC)
-        ]:
+        for var_name, replace_with in STANDARD_VARS:
             # Warn if we're we're going to override a variable:
             if override_this_variable(config_node, section, var_name):
                 user_var = config_node[section].value[var_name].value
                 LOG.warning(
-                    f'[{section}]{var_name}={user_var} from rose-suite.conf '
-                    f'will be ignored: {var_name} will be: {replace_with}'
+                    f'[{section}]{var_name}={user_var}'
+                    f' will be ignored. {var_name} will be: {replace_with}'
                 )
 
             # Handle replacement of stored variable if appropriate:
@@ -883,6 +882,7 @@ def record_cylc_install_options(
     srcdir: Path,
     rundir: Path,
     opts: 'Values',
+    modify: bool = True,
 ) -> Tuple[ConfigNode, ConfigNode]:
     """Create/modify files recording Cylc install config options.
 
@@ -909,6 +909,9 @@ def record_cylc_install_options(
                 Equivalent of ``rose suite-run --define KEY=VAL``
             - rose_template_vars (list of str):
                 Equivalent of ``rose suite-run --define-suite KEY=VAL``
+        modify:
+            If ``True``, the modified rose-suite-cylc-install.conf will be
+            written. If ``False``, this will only read files.
 
     Returns:
         Tuple - (cli_config, rose_suite_conf)
@@ -957,8 +960,9 @@ def record_cylc_install_options(
                 ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING
             ]
 
-    cli_config.comments = [' This file records CLI Options.']
-    dumper.dump(cli_config, str(conf_filepath))
+    if modify:
+        cli_config.comments = [' This file records CLI Options.']
+        dumper.dump(cli_config, str(conf_filepath))
 
     # Merge the opts section of the rose-suite.conf with those set by CLI:
     rose_conf_filepath.touch()
@@ -968,7 +972,8 @@ def record_cylc_install_options(
     )
     identify_templating_section(rose_suite_conf)
 
-    dumper(rose_suite_conf, rose_conf_filepath)
+    if modify:
+        dumper(rose_suite_conf, rose_conf_filepath)
 
     return cli_config, rose_suite_conf
 
@@ -999,3 +1004,94 @@ def copy_config_file(
     shutil.copy2(srcdir_rose_conf, rundir_rose_conf)
 
     return True
+
+
+def retrieve_installed_cli_opts(srcdir, opts):
+    """Merge options from previous install, this install and the CLI.
+
+    Allows validation of merged config for pre-configure where the
+    --against-source argument is used in a Cylc script.
+    """
+    # if opts.against_source is a path then we are validating a source
+    # directory against installed options
+    rundir = opts.against_source
+
+    # If the calling script doesn't have this option (Everything except
+    # Cylc VR) then this defaults to false. If it's true we skip all
+    # following logic:
+    if not getattr(opts, 'clear_rose_install_opts', False):
+        opts.clear_rose_install_opts = False
+    else:
+        return opts
+
+    # NOTE: we only need to load the rose-suite-cylc-install for this
+    cli_config, _ = record_cylc_install_options(
+        srcdir, rundir, opts, modify=False
+    )
+
+    # Set ROSE_ORIG_HOST to ignored, and choose not to ignore it
+    # if this is a validation against source:
+    no_ignore = False
+    if rundir:
+        no_ignore = True
+        for keys, node in cli_config.walk():
+            if keys[-1] == 'ROSE_ORIG_HOST':
+                node.state = cli_config.STATE_SYST_IGNORED
+
+    # Get opt_conf_keys stored in rose-suite-cylc-install.conf
+    opt_conf_keys = cli_config.value.pop('opts').value.split()
+    if any(opt_conf_keys):
+        opts.opt_conf_keys = opt_conf_keys
+
+    # Get --suite-defines/-S
+    # Work out whether user has used "template variables", "jinja2:suite.rc"
+    # or "empy:suite.rc" (There is an assumption that they aren't mixing
+    # them that is not guarded against):
+    for section in SECTIONS:
+        if cli_config.value.get(section, False):
+            template_variables = cli_config.value.pop(section)
+            break
+    # Create new -S list.
+    if any(template_variables):
+        opts.rose_template_vars = [
+            f'{keys[1]}={val.value}'
+            for keys, val in template_variables.walk(no_ignore=no_ignore)
+        ]
+
+    # Get all other keys (-D):
+    new_defines = []
+    for keys, value in cli_config.walk(no_ignore=no_ignore):
+        # Filter out section headings:
+        if not isinstance(value.value, dict):
+            section, key = keys
+            # Don't include section for top level items:
+            section = f"[{section}]" if section else ''
+            new_defines.append(f'{section}{key}={value.value}')
+
+    opts.defines = new_defines
+    return opts
+
+
+def sanitize_opts(opts):
+    """Warn if standard variables which will be overridden
+    have been added to CLI opts, and remove them.
+
+    n.b. This doesn't remove the need to check the input rose-suite.conf
+    for these variables.
+    """
+    options = []
+    for section in ['rose_template_vars', 'defines']:
+        for item in getattr(opts, section, []):
+            options.append((section, item))
+
+    for (section, item), (var_name, replace) in itertools.product(
+        options, STANDARD_VARS
+    ):
+        if re.match(rf'.*\b{var_name}=', item):
+            LOG.warning(
+                f'{section}:{item} from command line args'
+                f' will be ignored: {var_name} will be: {replace}'
+            )
+            with suppress(ValueError):
+                getattr(opts, section).remove(item)
+    return opts

pyproject.toml

@@ -0,0 +1,23 @@
+# Not mandated to use these tools, but if you do:
+
+[tool.ruff]
+line-length = 79
+target-version = "py37"
+
+[tool.ruff.format]
+quote-style = "preserve"
+
+
+[tool.black]
+line-length = 79
+target-version = ['py37']
+skip-string-normalization = true
+
+
+[tool.isort]
+profile = "black"
+line_length = 79
+force_grid_wrap = 2
+lines_after_imports = 2
+combine_as_imports = true
+force_sort_within_sections = true

setup.cfg

@@ -56,7 +56,7 @@ python_requires = >=3.7
 include_package_data = True
 install_requires =
     metomi-rose==2.3.*
-    cylc-flow>8.3.2,<8.4
+    cylc-flow>8.3.5,<8.4
     metomi-isodatetime
     ansimarkup
     jinja2