ensure that vr picks up installed config.

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) - Merge new CLI
+options with earlier ones when validating against source.
+
 ## __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,6 +27,7 @@
     load_rose_config,
     process_config,
     record_cylc_install_options,
+    retrieve_installed_cli_opts,
     rose_config_exists,
 )
 
@@ -40,7 +41,15 @@ def pre_configure(srcdir: Path, opts: 'Values') -> dict:
         # nothing to do here
         return {}
 
-    # load the Rose config
+    # 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

@@ -883,6 +883,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 +910,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 +961,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 +973,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 +1005,61 @@ 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
+    )
+
+    # 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
+
+    if any(template_variables):
+        opts.rose_template_vars = [
+            f'{k}={v.value}'
+            for k, v in template_variables.value.items()
+            if v.state == ''
+        ]
+
+    # Get all other keys (-D):
+    new_defines = []
+    for l1_key, l1_item in cli_config.value.items():
+        if isinstance(l1_item.value, Dict):
+            for l2_key, l2_item in l1_item.value.items():
+                new_defines.append(f'[{l1_key}]{l2_key}={l2_item.value}')
+        else:
+            new_defines.append(f'{l1_key}={l1_item.value}')
+    opts.defines = new_defines
+
+    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

tests/conftest.py

@@ -16,16 +16,18 @@
 
 import asyncio
 from functools import partial
+import importlib
 from io import StringIO
 from pathlib import Path
 from shlex import split
-from shutil import rmtree
+from shutil import rmtree, copytree
 from subprocess import run
 from time import sleep
 from types import SimpleNamespace
 from uuid import uuid4
 
 import pytest
+from pytest import UsageError
 
 from cylc.flow import __version__ as CYLC_VERSION
 from cylc.flow.option_parsers import Options
@@ -169,15 +171,20 @@ def _pytest_passed(request: pytest.FixtureRequest) -> bool:
     ))
 
 
-def _cylc_validate_cli(capsys, caplog):
-    """Access the validate CLI"""
-    async def _inner(srcpath, args=None):
-        parser = validate_gop()
+def _cylc_inspection_cli(capsys, caplog, script, gop):
+    """Access the CLI for cylc scripts inspecting configurations
+    """
+    async def _inner(srcpath, args=None, n_args=3):
+        parser = gop()
         options = Options(parser, args)()
         output = SimpleNamespace()
 
         try:
-            await cylc_validate(parser, options, str(srcpath))
+            if n_args == 3:
+                await script(parser, options, str(srcpath))
+            if n_args == 2:
+                # Don't include the parser:
+                await script(options, str(srcpath))
             output.ret = 0
             output.exc = ''
         except Exception as exc:
@@ -270,12 +277,76 @@ def mod_cylc_reinstall_cli(mod_test_dir):
 
 @pytest.fixture
 def cylc_validate_cli(capsys, caplog):
-    return _cylc_validate_cli(capsys, caplog)
+    return _cylc_inspection_cli(capsys, caplog, cylc_validate, validate_gop)
 
 
 @pytest.fixture(scope='module')
 def mod_cylc_validate_cli(mod_capsys, mod_caplog):
-    return _cylc_validate_cli(mod_capsys, mod_caplog)
+    return _cylc_inspection_cli(
+        mod_capsys, mod_caplog, cylc_validate, validate_gop
+    )
+
+
+@pytest.fixture
+async def cylc_inspect_scripts(capsys, caplog):
+    """Run all the common Cylc Test scripts likely to call pre-configure.
+
+    * config
+    * graph
+    * list
+    * validate
+    * view
+
+    n.b.
+    * Function adds arg ``--reference`` to supress image being displayed.
+    """
+
+    async def _inner(wid, args):
+        results = {}
+
+        # Handle scripts taking a parser or just the output of the parser:
+        for script_name, n_args in {
+            'config': 3,
+            'list': 3,
+            'graph': 3,
+            'view': 2,
+            'validate': 3,
+        }.items():
+
+            # Import the script modules:
+            script_module = importlib.import_module(
+                f'cylc.flow.scripts.{script_name}'
+            )
+
+            # Deal with inconsistent API from Cylc:
+            if hasattr(script_module, '_main'):
+                script = script_module._main
+            elif hasattr(script_module, 'run'):
+                script = script_module.run
+            else:
+                raise UsageError(
+                    f'Script "{script}\'s" module does not contain a '
+                    '"_main" or "run" function'
+                )
+
+            # Supress cylc-graph giving a graphical output:
+            if script_name == 'graph':
+                args['reference'] = True
+
+            results[script_name] = await _cylc_inspection_cli(
+                capsys,
+                caplog,
+                script,
+                script_module.get_option_parser,
+            )(wid, args, n_args=n_args)
+
+        # Check outputs
+        assert all(output.ret == 0 for output in results.values())
+
+        # Return results for more checking if required:
+        return results
+
+    return _inner
 
 
 @pytest.fixture
@@ -457,3 +528,26 @@ def timeout_func(func, message, timeout=5):
         sleep(1)
     else:
         raise TimeoutError(message)
+
+
+@pytest.fixture
+def setup_workflow_source_dir(tmp_path):
+    """Copy a workflow from the codebase to a temp-file-path
+    and provide that path for use in tests
+    """
+
+    def _inner(code_src):
+        nonlocal tmp_path
+        # Set up paths for test:
+        srcpath = tmp_path / 'src'
+        srcpath.mkdir()
+
+        # the files to install are stored in a directory alongside this
+        # test file:
+        datapath = Path(__file__).parent / code_src
+        copytree(datapath, srcpath, dirs_exist_ok=True)
+
+        # Create source workflow:
+        return srcpath, datapath
+
+    yield _inner

tests/functional/15_reinstall_using_old_clivars/flow.cylc

@@ -0,0 +1,11 @@
+#!jinja2
+{{ assert(SUITE is defined, "1.1 Test --rose-template-variable") }}
+{{ assert(DEFINE is defined, "1.2 Test --define") }}
+{{ assert(VAL_FROM_OPT is defined, "1.3 Test --opt-conf-keys") }}
+
+# Just enough Cylc to validate:
+[scheduler]
+    allow implicit tasks = True
+[scheduling]
+    [[graph]]
+        R1 = foo

tests/functional/15_reinstall_using_old_clivars/opt/rose-suite-choc.conf

@@ -0,0 +1,2 @@
+[template variables]
+VAL_FROM_OPT=3

tests/functional/test_pre_configure.py

@@ -219,3 +219,39 @@ def test_cylc_script(monkeypatch, option, envvars, cmd, expect):
         '05_opts_set_from_rose_suite_conf')
     output = run(split(f'{cmd} {srcpath} {option}'), capture_output=True)
     assert expect in output.stdout
+
+
+async def test_validate_against_source(
+    setup_workflow_source_dir,
+    cylc_validate_cli,
+    cylc_install_cli,
+    cylc_inspect_scripts,
+):
+    """Ensure that validation against source picks up
+    on existing configs saved in rose-suite-cylc-install.conf
+    """
+    src, _ = setup_workflow_source_dir(
+        'functional/15_reinstall_using_old_clivars'
+    )
+
+    opts = {
+        "rose_template_vars": ["SUITE=1"],
+        "defines": ["[template variables]DEFINE=2"],
+        "opt_conf_keys": ['choc'],
+    }
+
+    # Check that we can validate the source dir with options:
+    await cylc_inspect_scripts(src, opts)
+
+    # Install our workflow with options.
+    wid, _ = await cylc_install_cli(src, opts=opts)
+
+    # Check all scripts:
+    await cylc_inspect_scripts(wid, {"against_source": True})
+
+    # Reinstall fails if we clear rose install opts:
+    clear_install_validate = await cylc_validate_cli(
+        wid, {"against_source": True, 'clear_rose_install_opts': True}
+    )
+    assert clear_install_validate.ret != 0
+    assert 'Jinja2 Assertion Error' in str(clear_install_validate.exc.args[0])