From c751e15d14655cbf5d78ca6da123ebfb64ef18b1 Mon Sep 17 00:00:00 2001
From: Sam Clarke-Green <sam.clarke@metoffice.gov.uk>
Date: Wed, 13 Aug 2025 15:54:40 +0100
Subject: [PATCH 1/7] Add ability to cache command line arguments

This adds a feature which allows complex command line arguments to be
written to a cache file and then re-used.  Where an argument is
specified in both the cache and on the current command line, the
command line version takes priority.

Cache arguments themselves are not sticky, so if --save-cache and
--use-cache are set on the command line, they will get written to the
cache but ignored when the cache is next loaded.  This prevents the
cache from being inadvertently overwritten with new arguments.
---
 source/fab/cui/arguments.py            | 125 +++++++++++++++++-
 tests/unit_tests/test_cui_arguments.py | 168 ++++++++++++++++++++++++-
 2 files changed, 290 insertions(+), 3 deletions(-)

diff --git a/source/fab/cui/arguments.py b/source/fab/cui/arguments.py
index 39c931e76..404327d22 100644
--- a/source/fab/cui/arguments.py
+++ b/source/fab/cui/arguments.py
@@ -19,6 +19,7 @@
 """
 
 import argparse
+import json
 import os
 from pathlib import Path
 from typing import Callable
@@ -61,6 +62,7 @@ def inner(self, *args, **kwargs):
             self._add_location_group()
             self._add_output_group()
             self._add_info_group()
+            self._add_cache_group()
 
         result = func(self, *args, **kwargs)
 
@@ -79,18 +81,119 @@ def inner(self, *args, **kwargs):
         self._check_fabfile(namespace)
         self._configure_logging(namespace)
 
+        if getattr(namespace, "use_cache", False):
+            # Add cached argument settings if enabled
+            try:
+                self._merge_cached_namespace(
+                    self._option_string_actions.values(), namespace
+                )
+            except json.JSONDecodeError as err:
+                self.error(
+                    "--use-cache: "
+                    f"decode error at line {err.lineno} column {err.colno}"
+                )
+
+        if getattr(namespace, "save_cache", False):
+            # Save arguments to the ache
+            self._save_namespace(namespace)
+
         return result
 
     return inner
 
 
-class FabArgumentParser(argparse.ArgumentParser):
+class _FabArgumentCache:
+
+    # Path to options cache file
+    _cache_file = Path("fab.cache.json")
+
+    # Default cache setting
+    cache = False
+
+    def _save_namespace(self, namespace: argparse.Namespace) -> None:
+        """Save the argument namespace to a cache file.
+
+        Save the contents of the namespace to a cache file for use in
+        a subsequent fab run.  This is intended to simplify the
+        process of setting up and re-executing complex commands.
+
+        :param namespace: values from the argument parser
+        """
+
+        if not self.cache:
+            # Do not save the file if caching is off
+            return
+
+        with self._cache_file.open("wt", encoding="utf-8") as fd:
+            print(json.dumps(namespace.__dict__, default=lambda x: str(x)), file=fd)
+
+    def _merge_cached_namespace(
+        self, action_values, namespace: argparse.Namespace
+    ) -> None:
+        """Merge cached arguments into a namespace.
+
+        Where a cache file exists and argument caching is enabled,
+        load the values from the file and merge them into the
+        namespace instance according to the following rules:
+
+          * command line arguments always take priority
+          * cached values supersede argparse defaults
+          * defaults are used if no other value is available
+          * arguments with no defaults will not be set
+
+        :param namespace: values from the argument parser
+        """
+
+        if not self.cache or not self._cache_file.is_file():
+            # Do nothing if cache is off or file does not exist
+            return
+
+        with self._cache_file.open("rt", encoding="utf-8") as fd:
+            cached_args = json.load(fd)
+
+        for action in action_values:
+            # For each defined option
+            name = action.dest
+            default = getattr(action, "default", None)
+            cache = cached_args.get(name, None)
+            value = getattr(namespace, name, None)
+
+            if (isinstance(value, str) and value.startswith("==")) or (
+                isinstance(default, str)
+                and default.startswith("==")
+                or (name in ("save_cache", "use_cache"))
+            ):
+                # Ignore internal options, e.g. help, and any
+                # save/load cache arguments to avoid caching becoming
+                # sticky
+                continue
+
+            if (
+                value is not None
+                and default is not None
+                and cache is not None
+                and value == default
+            ) or (value is None and cache is not None):
+                # Value is set to the default.  It probably hasn't
+                # been set on the command line, so use the cached
+                # value instead
+                value = cache
+
+                if action.type is not None:
+                    # Convert the value if necessary
+                    value = action.type(value)
+
+            setattr(namespace, name, value)
+
+
+class FabArgumentParser(argparse.ArgumentParser, _FabArgumentCache):
     """Fab command argument parser."""
 
     def __init__(self, *args, **kwargs):
 
         self.version = kwargs.pop("version", str(fab_version))
         self.fabfile = full_path_type(kwargs.pop("fabfile", "FabFile") or "FabFile")
+        self.cache = kwargs.pop("cache", False)
 
         if "usage" not in kwargs:
             # Default to a simplified usage line
@@ -178,6 +281,26 @@ def _add_info_group(self):
                 "--version", action="version", version=f"%(prog)s {self.version}"
             )
 
+    def _add_cache_group(self) -> None:
+
+        if not self.cache:
+            # Do not add the group if caching is disabled
+            return
+
+        group = self.add_argument_group("fab cache arguments")
+
+        group.add_argument(
+            "--save-cache",
+            action="store_true",
+            help="create a command line argument cache",
+        )
+
+        group.add_argument(
+            "--use-cache",
+            action="store_true",
+            help="use a command line argument cache if it exists",
+        )
+
     def _configure_logging(self, namespace: argparse.Namespace) -> None:
         """Configure output logging.
 
diff --git a/tests/unit_tests/test_cui_arguments.py b/tests/unit_tests/test_cui_arguments.py
index 8e86cf211..becf415ff 100644
--- a/tests/unit_tests/test_cui_arguments.py
+++ b/tests/unit_tests/test_cui_arguments.py
@@ -10,14 +10,16 @@
 """
 
 import sys
+import json
 import os
 import argparse
 from pathlib import Path
-from fab.cui.arguments import full_path_type, FabArgumentParser
+from fab.cui.arguments import full_path_type, FabArgumentParser, _FabArgumentCache
 
 import pytest
 from typing import Optional
 from pyfakefs.fake_filesystem import FakeFilesystem
+from unittest.mock import Mock
 
 
 class TestFullPathType:
@@ -214,7 +216,9 @@ def test_outupt_with_quiet(self, argv, capsys):
             pytest.param(
                 [], None, Path("fab-workspace").expanduser().resolve(), id="default"
             ),
-            pytest.param([], Path("/tmp/fab"), Path("/tmp/fab").resolve(), id="environment"),
+            pytest.param(
+                [], Path("/tmp/fab"), Path("/tmp/fab").resolve(), id="environment"
+            ),
             pytest.param(
                 ["--workspace", "/run/fab"],
                 Path("/tmp/fab"),
@@ -296,3 +300,163 @@ def test_version(self, capsys, monkeypatch):
         captured = capsys.readouterr()
 
         assert captured.out.startswith("fab ")
+
+
+class TestArgumentCache:
+    """Test the ability to cache and re-load argument values."""
+
+    def test_save_disabled(self, fs: FakeFilesystem):
+        """Test save namespace."""
+
+        cache = _FabArgumentCache()
+        nspace = argparse.Namespace(a=1, b=2, c="xyz")
+
+        cache.cache = False
+        cache._save_namespace(nspace)
+        assert not cache._cache_file.exists()
+
+    def test_save_enabled(self, fs: FakeFilesystem):
+        """Test save namespace."""
+
+        cache = _FabArgumentCache()
+        nspace = argparse.Namespace(a=1, b=2, c="xyz")
+
+        cache.cache = True
+        cache._save_namespace(nspace)
+        assert cache._cache_file.exists()
+
+        with cache._cache_file.open("rt", encoding="utf-8") as fd:
+            values = json.load(fd)
+
+        assert values.get("a", "") == 1
+        assert values.get("b", "") == 2
+        assert values.get("c", "") == "xyz"
+
+        return
+
+    def test_cache_merge_disabled(self):
+
+        cache = _FabArgumentCache()
+        nspace = argparse.Namespace(a=1, b=2, c="xyz")
+
+        cache.cache = True
+        cache._save_namespace(nspace)
+        assert cache._cache_file.exists()
+
+        cache.cache = False
+        result = argparse.Namespace()
+        cache._merge_cached_namespace([], result)
+
+        assert getattr(result, "a", None) is None
+
+    @pytest.mark.parametrize(
+        "kwaction,value,expected",
+        [
+            pytest.param(
+                {"dest": "a", "default": 10, "type": None},
+                5,
+                5,
+                id="user value",
+            ),
+            pytest.param(
+                {"dest": "a", "default": 10, "type": None},
+                None,
+                1,
+                id="cache value",
+            ),
+            pytest.param(
+                {"dest": "a", "default": 10, "type": None},
+                10,
+                1,
+                id="default override",
+            ),
+            pytest.param(
+                {"dest": "b", "type": Path},
+                None,
+                Path("/dir"),
+                id="cache type conversion",
+            ),
+        ],
+    )
+    def test_cache_merge(self, kwaction, value, expected):
+
+        cache = _FabArgumentCache()
+        nspace = argparse.Namespace(a=1, b=Path("/dir"))
+
+        cache.cache = True
+        cache._save_namespace(nspace)
+
+        result = argparse.Namespace()
+        setattr(result, kwaction["dest"], value)
+        cache._merge_cached_namespace([Mock(**kwaction)], result)
+
+        assert getattr(result, kwaction["dest"], "") == expected
+
+
+class TestArgumentCaching:
+    """Test caching with the argument parser."""
+
+    def test_disabled(self, fs: FakeFilesystem):
+
+        parser = FabArgumentParser(cache=False)
+        parser.add_argument("--sarg", type=str)
+        parser.add_argument("--path", type=Path)
+        parser.parse_args(["--sarg=abc", "--path=/foobar"])
+
+        assert not parser._cache_file.is_file()
+
+    def test_enabled(self, fs: FakeFilesystem):
+
+        parser = FabArgumentParser(cache=True)
+        parser.add_argument("--sarg", type=str)
+        parser.add_argument("--path", type=Path)
+        parser.parse_args(["--save-cache", "--sarg=abc", "--path=/foobar"])
+
+        assert parser._cache_file.is_file()
+
+    def test_load(self, fs: FakeFilesystem):
+
+        parser = FabArgumentParser(cache=True)
+        parser.add_argument("--sarg", type=str)
+        parser.add_argument("--path", type=Path)
+        parser.parse_args(["--save-cache", "--sarg=abc", "--path=/foobar"])
+
+        assert parser._cache_file.is_file()
+
+        args = parser.parse_args(["--use-cache"])
+
+        assert hasattr(args, "sarg") and args.sarg == "abc"
+        assert (
+            hasattr(args, "path")
+            and isinstance(args.path, Path)
+            and args.path.name == "foobar"
+        )
+
+    def test_ignore_cache_args(self):
+        """Check caching arguments themselves are not cached."""
+
+        parser = FabArgumentParser(cache=True)
+        parser.add_argument("--sarg", type=str)
+        parser.parse_args(["--save-cache", "--sarg=abc"])
+
+        assert parser._cache_file.is_file()
+
+        args = parser.parse_args(["--use-cache"])
+
+        assert hasattr(args, "sarg") and args.sarg == "abc"
+        assert hasattr(args, "save_cache") and not args.save_cache
+
+    def test_error(self, capsys):
+        """Check a corrupt cache file raises an error."""
+
+        parser = FabArgumentParser(cache=True)
+
+        with parser._cache_file.open("wt", encoding="utf-8") as fd:
+            print("# invalid data", file=fd)
+
+        with pytest.raises(SystemExit) as err:
+            parser.parse_args(["--use-cache"])
+        assert err.value.code == 2
+
+        captured = capsys.readouterr()
+        assert "--use-cache: decode error at line 1 column 1" in captured.err

From d32a790f2e39e0fcb96cf3ac73613e295d3bfe61 Mon Sep 17 00:00:00 2001
From: Sam Clarke-Green <sam.clarke@metoffice.gov.uk>
Date: Fri, 15 Aug 2025 13:45:10 +0100
Subject: [PATCH 2/7] Try to fix cache test and improve docstrings

A problem with TestArgumentCaching::test_disabled seems to be tripping
up the CI testing on github even though it works interactively.  Try
to fix this by removing the cache file before running the test.

Also take the opportunity to add some missing docstrings to the test
cases.
---
 tests/unit_tests/test_cui_arguments.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/unit_tests/test_cui_arguments.py b/tests/unit_tests/test_cui_arguments.py
index becf415ff..8e1d1c90e 100644
--- a/tests/unit_tests/test_cui_arguments.py
+++ b/tests/unit_tests/test_cui_arguments.py
@@ -335,6 +335,7 @@ def test_save_enabled(self, fs: FakeFilesystem):
         return
 
     def test_cache_merge_disabled(self):
+        """Check cache does not get merged when disabled."""
 
         cache = _FabArgumentCache()
         nspace = argparse.Namespace(a=1, b=2, c="xyz")
@@ -379,6 +380,7 @@ def test_cache_merge_disabled(self):
         ],
     )
     def test_cache_merge(self, kwaction, value, expected):
+        """Check cache merges command line args correctly."""
 
         cache = _FabArgumentCache()
         nspace = argparse.Namespace(a=1, b=Path("/dir"))
@@ -397,8 +399,11 @@ class TestArgumentCaching:
     """Test caching with the argument parser."""
 
     def test_disabled(self, fs: FakeFilesystem):
+        """Check parser with caching disabled."""
 
         parser = FabArgumentParser(cache=False)
+        parser._cache_file.unlink(missing_ok=True)
+
         parser.add_argument("--sarg", type=str)
         parser.add_argument("--path", type=Path)
         parser.parse_args(["--sarg=abc", "--path=/foobar"])
@@ -406,6 +411,7 @@ def test_disabled(self, fs: FakeFilesystem):
         assert not parser._cache_file.is_file()
 
     def test_enabled(self, fs: FakeFilesystem):
+        """Check parser writes cache file."""
 
         parser = FabArgumentParser(cache=True)
         parser.add_argument("--sarg", type=str)
@@ -415,6 +421,7 @@ def test_enabled(self, fs: FakeFilesystem):
         assert parser._cache_file.is_file()
 
     def test_load(self, fs: FakeFilesystem):
+        """Check parser loads cached file."""
 
         parser = FabArgumentParser(cache=True)
         parser.add_argument("--sarg", type=str)

From f35b9f7e368a1bf5123791ec217238acd940d97e Mon Sep 17 00:00:00 2001
From: Sam Clarke-Green <sam.clarke@metoffice.gov.uk>
Date: Fri, 15 Aug 2025 13:52:14 +0100
Subject: [PATCH 3/7] Another attempt to fix cache file bugs

---
 tests/unit_tests/test_cui_arguments.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/unit_tests/test_cui_arguments.py b/tests/unit_tests/test_cui_arguments.py
index 8e1d1c90e..b5075f8f2 100644
--- a/tests/unit_tests/test_cui_arguments.py
+++ b/tests/unit_tests/test_cui_arguments.py
@@ -414,6 +414,8 @@ def test_enabled(self, fs: FakeFilesystem):
         """Check parser writes cache file."""
 
         parser = FabArgumentParser(cache=True)
+        parser._cache_file.unlink(missing_ok=True)
+
         parser.add_argument("--sarg", type=str)
         parser.add_argument("--path", type=Path)
         parser.parse_args(["--save-cache", "--sarg=abc", "--path=/foobar"])
@@ -424,6 +426,8 @@ def test_load(self, fs: FakeFilesystem):
         """Check parser loads cached file."""
 
         parser = FabArgumentParser(cache=True)
+        parser._cache_file.unlink(missing_ok=True)
+
         parser.add_argument("--sarg", type=str)
         parser.add_argument("--path", type=Path)
         parser.parse_args(["--save-cache", "--sarg=abc", "--path=/foobar"])
@@ -443,6 +447,8 @@ def test_ignore_cache_args(self):
         """Check caching arguments themselves are not cached."""
 
         parser = FabArgumentParser(cache=True)
+        parser._cache_file.unlink(missing_ok=True)
+
         parser.add_argument("--sarg", type=str)
         parser.parse_args(["--save-cache", "--sarg=abc"])
 

From 0f8a86e0aa318975c9acd08bc2f1eee5918d2e90 Mon Sep 17 00:00:00 2001
From: Sam Clarke-Green <sam.clarke@metoffice.gov.uk>
Date: Fri, 15 Aug 2025 14:19:08 +0100
Subject: [PATCH 4/7] Use tmp_path not pyfakefs in cache tests

This replaces the pyfakefs calls with tmp_path in the cache tests to
prevent failures with older versions of python from tripping up the CI
tests on github.
---
 tests/unit_tests/test_cui_arguments.py | 35 +++++++++++++++-----------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/tests/unit_tests/test_cui_arguments.py b/tests/unit_tests/test_cui_arguments.py
index b5075f8f2..dda07fd3e 100644
--- a/tests/unit_tests/test_cui_arguments.py
+++ b/tests/unit_tests/test_cui_arguments.py
@@ -305,20 +305,22 @@ def test_version(self, capsys, monkeypatch):
 class TestArgumentCache:
     """Test the ability to cache and re-load argument values."""
 
-    def test_save_disabled(self, fs: FakeFilesystem):
+    def test_save_disabled(self, tmp_path):
         """Test save namespace."""
 
         cache = _FabArgumentCache()
+        cache._cache_file = tmp_path / _FabArgumentCache._cache_file
         nspace = argparse.Namespace(a=1, b=2, c="xyz")
 
         cache.cache = False
         cache._save_namespace(nspace)
         assert not cache._cache_file.exists()
 
-    def test_save_enabled(self, fs: FakeFilesystem):
+    def test_save_enabled(self, tmp_path):
         """Test save namespace."""
 
         cache = _FabArgumentCache()
+        cache._cache_file = tmp_path / _FabArgumentCache._cache_file
         nspace = argparse.Namespace(a=1, b=2, c="xyz")
 
         cache.cache = True
@@ -334,10 +336,11 @@ def test_save_enabled(self, fs: FakeFilesystem):
 
         return
 
-    def test_cache_merge_disabled(self):
+    def test_cache_merge_disabled(self, tmp_path):
         """Check cache does not get merged when disabled."""
 
         cache = _FabArgumentCache()
+        cache._cache_file = tmp_path / _FabArgumentCache._cache_file
         nspace = argparse.Namespace(a=1, b=2, c="xyz")
 
         cache.cache = True
@@ -369,20 +372,21 @@ def test_cache_merge_disabled(self):
                 {"dest": "a", "default": 10, "type": None},
                 10,
                 1,
-                id="default override",
+                id="defaults",
             ),
             pytest.param(
                 {"dest": "b", "type": Path},
                 None,
                 Path("/dir"),
-                id="cache type conversion",
+                id="conversion",
             ),
         ],
     )
-    def test_cache_merge(self, kwaction, value, expected):
+    def test_cache_merge(self, kwaction, value, expected, tmp_path):
         """Check cache merges command line args correctly."""
 
         cache = _FabArgumentCache()
+        cache._cache_file = tmp_path / _FabArgumentCache._cache_file
         nspace = argparse.Namespace(a=1, b=Path("/dir"))
 
         cache.cache = True
@@ -398,11 +402,11 @@ def test_cache_merge(self, kwaction, value, expected):
 class TestArgumentCaching:
     """Test caching with the argument parser."""
 
-    def test_disabled(self, fs: FakeFilesystem):
+    def test_disabled(self, tmp_path):
         """Check parser with caching disabled."""
 
         parser = FabArgumentParser(cache=False)
-        parser._cache_file.unlink(missing_ok=True)
+        parser._cache_file = tmp_path / FabArgumentParser._cache_file
 
         parser.add_argument("--sarg", type=str)
         parser.add_argument("--path", type=Path)
@@ -410,11 +414,11 @@ def test_disabled(self, fs: FakeFilesystem):
 
         assert not parser._cache_file.is_file()
 
-    def test_enabled(self, fs: FakeFilesystem):
+    def test_enabled(self, tmp_path):
         """Check parser writes cache file."""
 
         parser = FabArgumentParser(cache=True)
-        parser._cache_file.unlink(missing_ok=True)
+        parser._cache_file = tmp_path / FabArgumentParser._cache_file
 
         parser.add_argument("--sarg", type=str)
         parser.add_argument("--path", type=Path)
@@ -422,11 +426,11 @@ def test_enabled(self, fs: FakeFilesystem):
 
         assert parser._cache_file.is_file()
 
-    def test_load(self, fs: FakeFilesystem):
+    def test_load(self, tmp_path):
         """Check parser loads cached file."""
 
         parser = FabArgumentParser(cache=True)
-        parser._cache_file.unlink(missing_ok=True)
+        parser._cache_file = tmp_path / FabArgumentParser._cache_file
 
         parser.add_argument("--sarg", type=str)
         parser.add_argument("--path", type=Path)
@@ -443,11 +447,11 @@ def test_load(self, fs: FakeFilesystem):
             and args.path.name == "foobar"
         )
 
-    def test_ignore_cache_args(self):
+    def test_ignore_cache_args(self, tmp_path):
         """Check caching arguments themselves are not cached."""
 
         parser = FabArgumentParser(cache=True)
-        parser._cache_file.unlink(missing_ok=True)
+        parser._cache_file = tmp_path / FabArgumentParser._cache_file
 
         parser.add_argument("--sarg", type=str)
         parser.parse_args(["--save-cache", "--sarg=abc"])
@@ -459,10 +463,11 @@ def test_ignore_cache_args(self):
         assert hasattr(args, "sarg") and args.sarg == "abc"
         assert hasattr(args, "save_cache") and not args.save_cache
 
-    def test_error(self, capsys):
+    def test_error(self, tmp_path, capsys):
         """Check a corrupt cache file raises an error."""
 
         parser = FabArgumentParser(cache=True)
+        parser._cache_file = tmp_path / FabArgumentParser._cache_file
 
         with parser._cache_file.open("wt", encoding="utf-8") as fd:
             print("# invalid data", file=fd)

From 267903579f53f6e9c72383c38e51161ff1b25147 Mon Sep 17 00:00:00 2001
From: Sam Clarke-Green <sam.clarke@metoffice.gov.uk>
Date: Wed, 3 Sep 2025 14:32:16 +0100
Subject: [PATCH 5/7] Move more caching features into the mixin

Some of the caching features were defined in the wrong places; the
argument group was added in FabArgumentParser and the post-parse_args
setup was entirely contained in the _parser_wrapper decorator.  This
prevents the class from being self-contained and makes it harder to
reuse.  This moves the methods into the _FabArgumentCache class.

This also creates a CachingArgumentParser which is a simple
combination of _FabArgumentCache and ArgumentParser.  This provides a
self-contained parser for use by infrastructure that does not use the
fab CUI.
---
 source/fab/cui/arguments.py | 110 ++++++++++++++++++++++++------------
 1 file changed, 74 insertions(+), 36 deletions(-)

diff --git a/source/fab/cui/arguments.py b/source/fab/cui/arguments.py
index 404327d22..5950d0d2d 100644
--- a/source/fab/cui/arguments.py
+++ b/source/fab/cui/arguments.py
@@ -62,7 +62,11 @@ def inner(self, *args, **kwargs):
             self._add_location_group()
             self._add_output_group()
             self._add_info_group()
-            self._add_cache_group()
+
+            if hasattr(self, "_add_cache_group"):
+                # Only call if the wrapped class includes features
+                # from the _FabArgumentCache mixin
+                self._add_cache_group()
 
         result = func(self, *args, **kwargs)
 
@@ -81,21 +85,10 @@ def inner(self, *args, **kwargs):
         self._check_fabfile(namespace)
         self._configure_logging(namespace)
 
-        if getattr(namespace, "use_cache", False):
-            # Add cached argument settings if enabled
-            try:
-                self._merge_cached_namespace(
-                    self._option_string_actions.values(), namespace
-                )
-            except json.JSONDecodeError as err:
-                self.error(
-                    "--use-cache: "
-                    f"decode error at line {err.lineno} column {err.colno}"
-                )
-
-        if getattr(namespace, "save_cache", False):
-            # Save arguments to the ache
-            self._save_namespace(namespace)
+        if hasattr(self, "_process_cache_args"):
+            # Only handle the caching arguments when running with the
+            # _FabArgumentCache mixin
+            self._process_cache_args(namespace)
 
         return result
 
@@ -103,6 +96,7 @@ def inner(self, *args, **kwargs):
 
 
 class _FabArgumentCache:
+    """Mixin which caches arguments passed on the command line."""
 
     # Path to options cache file
     _cache_file = Path("fab.cache.json")
@@ -110,6 +104,27 @@ class _FabArgumentCache:
     # Default cache setting
     cache = False
 
+    def _add_cache_group(self) -> None:
+        """Add a group containing cache arguments to the parser."""
+
+        if not self.cache:
+            # Do not add the group if caching is disabled
+            return
+
+        group = self.add_argument_group("fab cache arguments")
+
+        group.add_argument(
+            "--save-cache",
+            action="store_true",
+            help="create a command line argument cache",
+        )
+
+        group.add_argument(
+            "--use-cache",
+            action="store_true",
+            help="use a command line argument cache if it exists",
+        )
+
     def _save_namespace(self, namespace: argparse.Namespace) -> None:
         """Save the argument namespace to a cache file.
 
@@ -185,6 +200,25 @@ def _merge_cached_namespace(
 
             setattr(namespace, name, value)
 
+    def _process_cache_args(self, namespace: argparse.Namespace) -> None:
+        """Handle cache arguments in Namespace instance."""
+
+        if getattr(namespace, "use_cache", False):
+            # Add cached argument settings if enabled
+            try:
+                self._merge_cached_namespace(
+                    self._option_string_actions.values(), namespace
+                )
+            except json.JSONDecodeError as err:
+                self.error(
+                    "--use-cache: "
+                    f"decode error at line {err.lineno} column {err.colno}"
+                )
+
+        if getattr(namespace, "save_cache", False):
+            # Save arguments to the ache
+            self._save_namespace(namespace)
+
 
 class FabArgumentParser(argparse.ArgumentParser, _FabArgumentCache):
     """Fab command argument parser."""
@@ -281,26 +315,6 @@ def _add_info_group(self):
                 "--version", action="version", version=f"%(prog)s {self.version}"
             )
 
-    def _add_cache_group(self) -> None:
-
-        if not self.cache:
-            # Do not add the group if caching is disabled
-            return
-
-        group = self.add_argument_group("fab cache arguments")
-
-        group.add_argument(
-            "--save-cache",
-            action="store_true",
-            help="create a command line argument cache",
-        )
-
-        group.add_argument(
-            "--use-cache",
-            action="store_true",
-            help="use a command line argument cache if it exists",
-        )
-
     def _configure_logging(self, namespace: argparse.Namespace) -> None:
         """Configure output logging.
 
@@ -411,3 +425,27 @@ def parse_args(self, *args, **kwargs):
     def parse_known_args(self, *args, **kwargs):
 
         return super().parse_known_args(*args, **kwargs)
+
+
+class CachingArgumentParser(_FabArgumentCache, argparse.ArgumentParser):
+    """Argument parser which can save option to a cache.
+
+    This adds a default group of argument options and
+    """
+
+    # Enable the cache functionality by default
+    cache = True
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self._add_cache_group()
+
+    def parse_args(self, *args, **kwargs):
+        namespace = super().parse_args(*args, **kwargs)
+        self._process_cache_args(namespace)
+        return namespace
+
+    def parse_known_args(self, *args, **kwargs):
+        namespace, rest = super().parse_known_args(*args, **kwargs)
+        self._process_cache_args(namespace)
+        return namespace, rest

From cadabb0710a69f247511b400155aae78d96e3716 Mon Sep 17 00:00:00 2001
From: Sam Clarke-Green <sam.clarke@metoffice.gov.uk>
Date: Wed, 3 Sep 2025 15:37:10 +0100
Subject: [PATCH 6/7] Extend the caching funcationality to FabBase

Add the CachingArgumentParser class to FabBase as a drop-in
replacement for argparse.ArgumentParser.  Also add some tests
confirming the core caching functionality works as expected with
FabBase.
---
 source/fab/fab_base/fab_base.py            | 14 ++++---
 tests/unit_tests/fab_base/test_fab_base.py | 44 ++++++++++++++++++++++
 2 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/source/fab/fab_base/fab_base.py b/source/fab/fab_base/fab_base.py
index d3ef10a1d..c54856276 100755
--- a/source/fab/fab_base/fab_base.py
+++ b/source/fab/fab_base/fab_base.py
@@ -31,6 +31,8 @@
 from fab.steps.preprocess import preprocess_c, preprocess_fortran
 from fab.tools import Category, ToolBox, ToolRepository
 
+from fab.cui.arguments import CachingArgumentParser
+
 
 class FabBase:
     '''
@@ -293,7 +295,7 @@ def define_site_platform_target(self) -> None:
         # Use `argparser.parse_known_args` to just handle --site and
         # --platform. We also suppress help (all of which will be handled
         # later, including proper help messages)
-        parser = argparse.ArgumentParser(add_help=False)
+        parser = CachingArgumentParser(add_help=False)
         parser.add_argument("--site", "-s", type=str, default="$SITE")
         parser.add_argument("--platform", "-p", type=str, default="$PLATFORM")
 
@@ -340,8 +342,8 @@ def site_specific_setup(self) -> None:
 
     def define_command_line_options(
             self,
-            parser: Optional[argparse.ArgumentParser] = None
-            ) -> argparse.ArgumentParser:
+            parser: Optional[CachingArgumentParser] = None
+            ) -> CachingArgumentParser:
         '''
         Defines command line options. Can be overwritten by a derived
         class which can provide its own instance (to easily allow for a
@@ -353,7 +355,7 @@ class which can provide its own instance (to easily allow for a
 
         if not parser:
             # The formatter class makes sure to print default settings
-            parser = argparse.ArgumentParser(
+            parser = CachingArgumentParser(
                 description=("A Fab-based build system. Note that if --suite "
                              "is specified, this will change the default for "
                              "compiler and linker"),
@@ -437,7 +439,7 @@ class which can provide its own instance (to easily allow for a
         return parser
 
     def handle_command_line_options(self,
-                                    parser: argparse.ArgumentParser) -> None:
+                                    parser: CachingArgumentParser) -> None:
         '''
         Analyse the actual command line options using the specified parser.
         The base implementation will handle the `--suite` parameter, and
@@ -445,7 +447,7 @@ def handle_command_line_options(self,
         variables). Needs to be overwritten to handle additional options
         specified by a derived script.
 
-        :param argparse.ArgumentParser parser: the argument parser.
+        :param CachingArgumentParser parser: the argument parser.
         '''
         # pylint: disable=too-many-branches
         self._args = parser.parse_args(sys.argv[1:])
diff --git a/tests/unit_tests/fab_base/test_fab_base.py b/tests/unit_tests/fab_base/test_fab_base.py
index 82098452a..98c06e41e 100644
--- a/tests/unit_tests/fab_base/test_fab_base.py
+++ b/tests/unit_tests/fab_base/test_fab_base.py
@@ -7,6 +7,7 @@
 Tests the FabBase class
 """
 import inspect
+import json
 import os
 from pathlib import Path
 import sys
@@ -491,3 +492,46 @@ def test_build_shared_lib(monkeypatch) -> None:
     mocks["link_shared_object"][1].assert_called_once_with(
         fab_base.config, output_fpath=str(workspace / 'libtest.so'),
         flags=[])
+
+
+def test_caching_options(capsys, monkeypatch) -> None:
+    '''
+    Tests command line argument options are present.
+    '''
+    monkeypatch.setattr(sys, "argv", ["fab", "--help"])
+
+    with pytest.raises(SystemExit):
+        FabBase(name="test-caching")
+
+    captured = capsys.readouterr()
+    assert "--save-cache" in captured.out
+    assert "--use-cache" in captured.out
+
+
+def test_caching_feature(monkeypatch) -> None:
+    '''Test cache save and load features.
+
+    Both these operations are done in a single test to remove the need
+    to duplicate the creation of the cache file for the load test.
+    '''
+
+    # Set an argument and check that it
+    monkeypatch.setattr(sys, "argv", ["fab", "--save-cache",
+                                      "--fflag", "ftn"])
+
+    fab_base = FabBase(name="test-caching")
+    assert fab_base.fortran_compiler_flags_commandline == ["ftn"]
+
+    # Check the cache has been created
+    assert os.path.exists("fab.cache.json")
+
+    # Check the contents of the cached file
+    values = json.load(open("fab.cache.json"))
+    assert "fflags" in values
+    assert values["fflags"] == "ftn"
+
+    # Load the cached values
+    monkeypatch.setattr(sys, "argv", ["fab", "--use-cache"])
+
+    fab_base = FabBase(name="test-caching")
+    assert fab_base.fortran_compiler_flags_commandline == ["ftn"]

From ccb5239a18c1ac4defc0d09cd1464fcaaddadc5f Mon Sep 17 00:00:00 2001
From: Sam Clarke-Green <sam.clarke@metoffice.gov.uk>
Date: Wed, 3 Sep 2025 15:58:08 +0100
Subject: [PATCH 7/7] Ignore type errors for missing methods in mixin

The _FabArgumentCache class is a mixin and only works when added to a
class which also inherits from argparse.ArgumentParser.  Ignore type
complaints related to missing attributes caused by references to
methods defined elsewhere.
---
 source/fab/cui/arguments.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/source/fab/cui/arguments.py b/source/fab/cui/arguments.py
index 5950d0d2d..eff441d91 100644
--- a/source/fab/cui/arguments.py
+++ b/source/fab/cui/arguments.py
@@ -111,7 +111,7 @@ def _add_cache_group(self) -> None:
             # Do not add the group if caching is disabled
             return
 
-        group = self.add_argument_group("fab cache arguments")
+        group = self.add_argument_group("fab cache arguments")  # type: ignore[attr-defined]
 
         group.add_argument(
             "--save-cache",
@@ -207,10 +207,10 @@ def _process_cache_args(self, namespace: argparse.Namespace) -> None:
             # Add cached argument settings if enabled
             try:
                 self._merge_cached_namespace(
-                    self._option_string_actions.values(), namespace
+                    self._option_string_actions.values(), namespace  # type: ignore[attr-defined]
                 )
             except json.JSONDecodeError as err:
-                self.error(
+                self.error(  # type: ignore[attr-defined]
                     "--use-cache: "
                     f"decode error at line {err.lineno} column {err.colno}"
                 )