`cylc vr`: installed options are not applied before validate

[oliver-sanders]

An interaction betwen cylc validate --against-source and cylc-rose is not working right.

Example:

Workflow:

opt/rose-suite-myopt.conf

[template variables]
SITE="foo"

rose-suite.conf

flow.cylc

#!Jinja2

{{ assert(SITE is defined, 'please define a site') }}

Steps:

$ cylc install . -O myopt
INSTALLED tmp.GBuczSOxyL/run4 from /var/tmp/tmp.GBuczSOxyL
$ cylc vr tmp.GBuczSOxyL
...
Jinja2Error: Jinja2 Assertion Error: please define a site
File /var/tmp/tmp.GBuczSOxyL/flow.cylc
  #!Jinja2

  {{ assert(SITE is defined, 'please define a site') }} <-- Exception

[oliver-sanders]

Have taken a look.

The problem is that we are not loading the rose-suite-cylc-install.conf before attempting to validate the workflow. As a result the installed options are not used.

I think the solution is to detect the against_source option in cylc-rose, this tells us that we are trying to validate a source dir against installed options. On detection of this variable, we load in the rose-suite-cylc-install.conf and insert it into the config. The tricky bit is getting this done before the ConfigTree is evaluated as this is the bit that actually loads and applies optional configs.

I've managed to make the above example pass with the following diff:

cylc-flow

diff --git a/cylc/flow/scripts/validate_reinstall.py b/cylc/flow/scripts/validate_reinstall.py
index fa53461b2..2ad37c122 100644
--- a/cylc/flow/scripts/validate_reinstall.py
+++ b/cylc/flow/scripts/validate_reinstall.py
@@ -39,6 +39,7 @@ Note:
   in the installed workflow to ensure the change can be safely applied.
 """

+from pathlib import Path
 import sys
 from typing import TYPE_CHECKING

@@ -74,7 +75,10 @@ from cylc.flow.scripts.reload import (
     run as cylc_reload
 )
 from cylc.flow.terminal import cli_function
-from cylc.flow.workflow_files import detect_old_contact_file
+from cylc.flow.workflow_files import (
+    detect_old_contact_file,
+    get_workflow_run_dir,
+)

 import asyncio

@@ -167,7 +171,7 @@ async def vr_cli(parser: COP, options: 'Values', workflow_id: str):
         return 1

     # Force on the against_source option:
-    options.against_source = True
+    options.against_source = Path(get_workflow_run_dir(workflow_id))

     # Run cylc validate
     log_subcommand('validate --against-source', workflow_id)

cylc-rose

diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py
index 503382d..8f0ccdc 100644
--- a/cylc/rose/entry_points.py
+++ b/cylc/rose/entry_points.py
@@ -41,6 +41,20 @@ def pre_configure(srcdir: Path, opts: 'Values') -> dict:
         return {}

     # load the Rose config
+    opt_conf_keys_env = ''
+    if getattr(opts, 'against_source', False) and isinstance(
+        opts.against_source, Path
+    ):
+        # if opts.against_source is a path then we are validating a source
+        # directory against installed options
+        rundir = opts.against_source
+        # NOTE: we only need to load the rose-suite-cylc-install for this
+        cli_config, _config = record_cylc_install_options(
+            srcdir, rundir, opts, modify=False
+        )
+        if cli_config.value.get('opts'):
+            opts.opt_conf_keys = cli_config.value.get('opts').value.split(' ')
+
     config_tree = load_rose_config(Path(srcdir), opts=opts)

     # extract plugin return information from the Rose config
diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py
index 8ccb3a1..cbf9ff2 100644
--- a/cylc/rose/utilities.py
+++ b/cylc/rose/utilities.py
@@ -883,6 +883,7 @@ def record_cylc_install_options(
     srcdir: Path,
     rundir: Path,
     opts: 'Values',
+    modify=True,
 ) -> Tuple[ConfigNode, ConfigNode]:
     """Create/modify files recording Cylc install config options.

@@ -909,6 +910,10 @@ 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 but not write
+            them.

     Returns:
         Tuple - (cli_config, rose_suite_conf)
@@ -957,8 +962,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 +974,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

But this is still a way off:

• The -S and -D options need preserving too.
• I think the ROSE_SUITE_OPT_CONF_KEYS env var needs to be ignored during the load_rose_config stage of the pre_configure plugin to prevent it from being appended twice.

[oliver-sanders]

Renaming as this issue is not constrained to optional configs.