Repository: osbuild/osbuild
Pull request #1618 (2024-02-28 12:05:47)

osbuild: first step towards stage separation - allow meta.json for stages
[Reopening based on the request from Simon https://github.com/osbuild/osbuild/pull/1460#issuecomment-1968609226]

Instead of always parsing the python stage to load meta information
allow the user of a new `{stage}-meta.json` file. This is a first
step towards allowing modules to be written in a different language
than python. It also has some practical advantages:
- slightly faster as it avoids calling python to output the schemas
- easier to write schemas as this can be done in a real json editor
  now
- more extensible in a future where stages maybe binaries with
  shlib dependencies that are only satisfied in the buildroot
  but not on the host

An example/toy stage that uses this can be seen in https://github.com/osbuild/osbuild/compare/main...mvo5:stage-separation-json-2?expand=1 (the last two commits)



supakeen: Thank you for this, one comment only.

The only thing necessary in my opinion is to have a chat about the name of the `-meta.json` files otherwise this is a good plan and opens up fun new avenues for modules.
mvo5: > Thank you for this, one comment only.

Thanks for the review, really appreciated. You raise a very good point, this should be addressed now.
 
> The only thing necessary in my opinion is to have a chat about the name of the `-meta.json` files otherwise this is a good plan and opens up fun new avenues for modules.

I have no super strong opinion here, maybe @achilleas-k or @ondrejbudai have ideas? The only reason why I changed it from `-meta.json` to `.meta-json` is that I kinda like the idea that `os.path.splitext()` is what gives us the information what extension the file has. But it could be `.mjson, .jsonm, .metaj, jmeta, json-meta` etc. I not even strongly against `-meta.json` it just feels a little uncommon to use half of the file extension and half a `-` :)
supakeen: I'd still like inputs from @achilleas-k, @ondrejbudai, and/or @thozza about this filename distraction :)
mvo5: > I'd still like inputs from @achilleas-k, @ondrejbudai, and/or @thozza about this filename distraction :)

Yeah, it needs a second review anyway so let's make sure we get input on this along :)
achilleas-k: > Nice work!
> 
> FWIW, I'm in favor of `.meta.json` instead of `.meta-json` due to the reason to make the file be recognized as JSON.

Same!
achilleas-k: This is an idea we've been playing with for a while and I like the approach here but I wonder if we should go all the way and pull out all the schemas with the introduction of the change and really commit to it.  I'm afraid if we don't, it'll just be a curiosity we tried once and forgot about.

A more general question (or suggestion): What if the schema was still part of the stage file but stages printed it when called with a `--schema` flag?  I think we discussed this before and decided on the current approach but I'd like to revisit the idea.
**Upside**: Fewer changes (in patch length) to convert existing code.  Every stage would get an identical patch where they implement `--schema` which prints the existing global constant.
**Neutral**: Most of the benefits from the current approach are retained.  `--schema` becomes part of the stage API but lets us write stages in any language as long as they implement it.  Stages _can_ ship their schema in separate files and simply `cat` their schema file when called.
**Downside**: Writing json as a text blob in Python is annoying vs writing in a dedicated file with syntax highlighting and linting while editing (though we can still have them separate if we want).
supakeen: @achilleas-k The reason for not continuing down the 'command-line API to stages' was that it requires all libraries used by stage to be available on the host.

Patch:
diff --git a/.ruff.toml b/.ruff.toml
index 20122a7a..7b8950db 100644
--- a/.ruff.toml
+++ b/.ruff.toml
@@ -1,5 +1,10 @@
 line-length = 120
 
+extend-exclude = [
+    "*.json",
+]
+force-exclude = true
+
 [lint]
 # enabled the `isort` and `pylint` additional linters, see `ruff linter` output
 # or: https://docs.astral.sh/ruff/rules/
diff --git a/Schutzfile b/Schutzfile
index 349965ba..896106cf 100644
--- a/Schutzfile
+++ b/Schutzfile
@@ -120,14 +120,14 @@
           {
             "title": "updates",
             "name": "updates",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/f38/f38-x86_64-updates-released-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/f38/f38-x86_64-updates-released-20240313"
           }
         ],
         "aarch64": [
           {
             "title": "updates",
             "name": "updates",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/f38/f38-aarch64-updates-released-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/f38/f38-aarch64-updates-released-20240313"
           }
         ]
       },
@@ -137,14 +137,14 @@
           {
             "title": "updates-modular",
             "name": "updates-modular",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/f38/f38-x86_64-updates-released-modular-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/f38/f38-x86_64-updates-released-modular-20240313"
           }
         ],
         "aarch64": [
           {
             "title": "updates-modular",
             "name": "updates-modular",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/f38/f38-aarch64-updates-released-modular-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/f38/f38-aarch64-updates-released-modular-20240313"
           }
         ]
       }
@@ -179,14 +179,14 @@
           {
             "title": "fedora",
             "name": "fedora",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/f40/f40-x86_64-branched-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/f40/f40-x86_64-branched-20240313"
           }
         ],
         "aarch64": [
           {
             "title": "fedora",
             "name": "fedora",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/f40/f40-aarch64-branched-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/f40/f40-aarch64-branched-20240313"
           }
         ]
       }
@@ -200,34 +200,34 @@
           {
             "title": "baseos",
             "name": "baseos",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el9/cs9-x86_64-baseos-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el9/cs9-x86_64-baseos-20240313"
           },
           {
             "title": "appstream",
             "name": "appstream",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el9/cs9-x86_64-appstream-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el9/cs9-x86_64-appstream-20240313"
           },
           {
             "title": "crb",
             "name": "crb",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el9/cs9-x86_64-crb-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el9/cs9-x86_64-crb-20240313"
           }
         ],
         "aarch64": [
           {
             "title": "baseos",
             "name": "baseos",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el9/cs9-aarch64-baseos-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el9/cs9-aarch64-baseos-20240313"
           },
           {
             "title": "appstream",
             "name": "appstream",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el9/cs9-aarch64-appstream-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el9/cs9-aarch64-appstream-20240313"
           },
           {
             "title": "crb",
             "name": "crb",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el9/cs9-aarch64-crb-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el9/cs9-aarch64-crb-20240313"
           }
         ]
       }
@@ -241,34 +241,34 @@
           {
             "title": "RHEL-8-RPMREPO-NIGHTLY-BaseOS",
             "name": "baseos",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el8/el8-x86_64-baseos-n8.10-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el8/el8-x86_64-baseos-n8.10-20240313"
           },
           {
             "title": "RHEL-8-RPMREPO-NIGHTLY-AppStream",
             "name": "appstream",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el8/el8-x86_64-appstream-n8.10-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el8/el8-x86_64-appstream-n8.10-20240313"
           },
           {
             "title": "RHEL-8-RPMREPO-NIGHTLY-CRB",
             "name": "crb",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el8/el8-x86_64-crb-n8.10-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el8/el8-x86_64-crb-n8.10-20240313"
           }
         ],
         "aarch64": [
           {
             "title": "RHEL-8-RPMREPO-NIGHTLY-BaseOS",
             "name": "baseos",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el8/el8-aarch64-baseos-n8.10-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el8/el8-aarch64-baseos-n8.10-20240313"
           },
           {
             "title": "RHEL-8-RPMREPO-NIGHTLY-AppStream",
             "name": "appstream",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el8/el8-aarch64-appstream-n8.10-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el8/el8-aarch64-appstream-n8.10-20240313"
           },
           {
             "title": "RHEL-8-RPMREPO-NIGHTLY-CRB",
             "name": "crb",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el8/el8-aarch64-crb-n8.10-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el8/el8-aarch64-crb-n8.10-20240313"
           }
         ]
       }
@@ -282,34 +282,34 @@
           {
             "title": "RHEL-9-RPMREPO-NIGHTLY-BaseOS",
             "name": "baseos",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el9/el9-x86_64-baseos-n9.4-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el9/el9-x86_64-baseos-n9.4-20240313"
           },
           {
             "title": "RHEL-9-RPMREPO-NIGHTLY-AppStream",
             "name": "appstream",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el9/el9-x86_64-appstream-n9.4-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el9/el9-x86_64-appstream-n9.4-20240313"
           },
           {
             "title": "RHEL-9-RPMREPO-NIGHTLY-CRB",
             "name": "crb",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el9/el9-x86_64-crb-n9.4-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el9/el9-x86_64-crb-n9.4-20240313"
           }
         ],
         "aarch64": [
           {
             "title": "RHEL-9-RPMREPO-NIGHTLY-BaseOS",
             "name": "baseos",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el9/el9-aarch64-baseos-n9.4-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el9/el9-aarch64-baseos-n9.4-20240313"
           },
           {
             "title": "RHEL-9-RPMREPO-NIGHTLY-AppStream",
             "name": "appstream",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el9/el9-aarch64-appstream-n9.4-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el9/el9-aarch64-appstream-n9.4-20240313"
           },
           {
             "title": "RHEL-9-RPMREPO-NIGHTLY-CRB",
             "name": "crb",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el9/el9-aarch64-crb-n9.4-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el9/el9-aarch64-crb-n9.4-20240313"
           }
         ]
       }
@@ -323,14 +323,14 @@
           {
             "title": "baseos",
             "name": "baseos",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el8/cs8-x86_64-baseos-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el8/cs8-x86_64-baseos-20240313"
           }
         ],
         "aarch64": [
           {
             "title": "baseos",
             "name": "baseos",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el8/cs8-aarch64-baseos-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el8/cs8-aarch64-baseos-20240313"
           }
         ]
       },
@@ -340,14 +340,14 @@
           {
             "title": "appstream",
             "name": "appstream",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el8/cs8-x86_64-appstream-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el8/cs8-x86_64-appstream-20240313"
           }
         ],
         "aarch64": [
           {
             "title": "appstream",
             "name": "appstream",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el8/cs8-aarch64-appstream-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el8/cs8-aarch64-appstream-20240313"
           }
         ]
       },
@@ -357,14 +357,14 @@
           {
             "title": "powertools",
             "name": "powertools",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el8/cs8-x86_64-powertools-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el8/cs8-x86_64-powertools-20240313"
           }
         ],
         "aarch64": [
           {
             "title": "powertools",
             "name": "powertoosl",
-            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el8/cs8-aarch64-powertools-20240301"
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el8/cs8-aarch64-powertools-20240313"
           }
         ]
       }
diff --git a/osbuild/meta.py b/osbuild/meta.py
index 50c7c6cf..44cbe3cd 100644
--- a/osbuild/meta.py
+++ b/osbuild/meta.py
@@ -275,6 +275,54 @@ class Schema:
         return self.check().valid
 
 
+META_JSON_SCHEMA = {
+    "type": "object",
+    "additionalProperties": False,
+    "propertyNames": {
+        "not": {
+            "const": "description",
+        },
+    },
+    "required": ["summary", "description"],
+    "oneOf": [
+        {
+            "required": [
+                "schema"
+            ]
+        },
+        {
+            "required": [
+                "schema_2"
+            ]
+        },
+    ],
+    "properties": {
+        "summary": {
+            "type": "string",
+        },
+        "description": {
+            "type": "array",
+            "minItems": 1,
+            "items": {
+                "type": "string",
+            },
+        },
+        "capabilities": {
+            "type": "array",
+            "items": {
+                "type": "string",
+            },
+        },
+        "schema": {
+            "type": "object",
+        },
+        "schema_2": {
+            "type": "object",
+        }
+    }
+}
+
+
 class ModuleInfo:
     """Meta information about a stage
 
@@ -409,6 +457,54 @@ class ModuleInfo:
 
     @classmethod
     def load(cls, root, klass, name) -> Optional["ModuleInfo"]:
+        base = cls.MODULES.get(klass)
+        if not base:
+            raise ValueError(f"Unsupported type: {klass}")
+        path = os.path.join(root, base, name)
+
+        try:
+            return cls._load_from_json(path, klass, name)
+        except FileNotFoundError:
+            pass
+        return cls._load_from_py(path, klass, name)
+
+    @classmethod
+    def _load_from_json(cls, path, klass, name) -> Optional["ModuleInfo"]:
+        meta_json_suffix = ".meta.json"
+        with open(path + meta_json_suffix, encoding="utf-8") as fp:
+            try:
+                meta = json.load(fp)
+            except json.decoder.JSONDecodeError as e:
+                raise SyntaxError("Invalid schema: " + str(e)) from e
+
+        schema = Schema(META_JSON_SCHEMA, "meta.json validator")
+        res = schema.validate(meta)
+        if not res.valid:
+            # the python code is very leaniant with invalid schemas
+            # so just print a warning here for now to stay close to
+            # what the old code was doing
+            errs = res.as_dict()["errors"]
+            # it would be nice to have a proper logger here
+            print(f"WARNING: schema for {path} is invalid: {errs}", file=sys.stderr)
+            return None
+
+        long_description = meta.get("description", "no description provided")
+        if isinstance(long_description, list):
+            long_description = "\n".join(long_description)
+
+        info = {
+            "schema": {
+                "1": meta.get("schema", {}),
+                "2": meta.get("schema_2", {}),
+            },
+            "desc": meta.get("summary", "no summary provided"),
+            "info": long_description,
+            "caps": meta.get("capabilities", set()),
+        }
+        return cls(klass, name, path, info)
+
+    @classmethod
+    def _load_from_py(cls, path, klass, name) -> Optional["ModuleInfo"]:
         names = ["SCHEMA", "SCHEMA_2", "CAPABILITIES"]
 
         def filter_type(lst, target):
@@ -417,11 +513,6 @@ class ModuleInfo:
         def targets(a):
             return [t.id for t in filter_type(a.targets, ast.Name)]
 
-        base = cls.MODULES.get(klass)
-        if not base:
-            raise ValueError(f"Unsupported type: {klass}")
-
-        path = os.path.join(root, base, name)
         try:
             with open(path, encoding="utf8") as f:
                 data = f.read()
@@ -434,6 +525,8 @@ class ModuleInfo:
 
         docstring = ast.get_docstring(tree)
         doclist = docstring.split("\n") if docstring else []
+        summary = doclist[0] if len(doclist) > 0 else ""
+        long_description = "\n".join(doclist[1:]) if len(doclist) > 0 else ""
 
         assigns = filter_type(tree.body, ast.Assign)
         values = {
@@ -454,8 +547,8 @@ class ModuleInfo:
                 "1": parse_schema(values.get("SCHEMA")),
                 "2": parse_schema(values.get("SCHEMA_2")),
             },
-            'desc': doclist[0],
-            'info': "\n".join(doclist[1:]),
+            'desc': summary,
+            'info': long_description,
             'caps': parse_caps(values.get("CAPABILITIES"))
         }
         return cls(klass, name, path, info)
@@ -585,7 +678,7 @@ class Index:
             raise ValueError(f"Unsupported nodule class: {klass}")
 
         path = os.path.join(self.path, module_path)
-        modules = filter(lambda f: os.path.isfile(f"{path}/{f}"),
+        modules = filter(lambda f: os.path.isfile(f"{path}/{f}") and not path.endswith(".meta.json"),
                          os.listdir(path))
         return list(modules)
 
diff --git a/stages/org.osbuild.noop b/stages/org.osbuild.noop
index c44e7606..85f04570 100755
--- a/stages/org.osbuild.noop
+++ b/stages/org.osbuild.noop
@@ -2,8 +2,7 @@
 """
 Do Nothing
 
-No-op stage. Prints a JSON dump of the options passed into this stage and
-leaves the tree untouched. Useful for testing, debugging, and wasting time.
+See "org.osbuild.noop.meta.json" for more details.
 """
 
 
@@ -12,21 +11,6 @@ import sys
 
 import osbuild.api
 
-SCHEMA_2 = """
-"options": {
-  "additionalProperties": true
-},
-"devices": {
-  "additionalProperties": true
-},
-"inputs": {
-  "additionalProperties": true
-},
-"mounts": {
-  "additionalProperties": true
-}
-"""
-
 
 def main(_tree, inputs, options):
     print("Not doing anything with these options:", json.dumps(options))
diff --git a/stages/org.osbuild.noop.meta.json b/stages/org.osbuild.noop.meta.json
new file mode 100644
index 00000000..aa9ab33a
--- /dev/null
+++ b/stages/org.osbuild.noop.meta.json
@@ -0,0 +1,23 @@
+{
+    "summary": "Do Nothing",
+    "description": [
+        "No-op stage.\n",
+        "Prints a JSON dump of the options passed into this stage and ",
+        "leaves the tree untouched. Useful for testing, debugging, and ",
+        "wasting time."
+    ],
+    "schema_2": {
+        "options": {
+            "additionalProperties": true
+        },
+        "devices": {
+            "additionalProperties": true
+        },
+        "inputs": {
+            "additionalProperties": true
+        },
+        "mounts": {
+            "additionalProperties": true
+        }
+    }
+}
diff --git a/tox.ini b/tox.ini
index eb07aed0..dbaf7a94 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,11 +1,11 @@
 [tox]
 env_list =
-    py{36,37,38,39,310,311}
+    py{36,37,38,39,310,311,312}
     lint
     type
 
 labels =
-    test = py{36,37,38,39,310,311}
+    test = py{36,37,38,39,310,311,312}
     lint = ruff, autopep8, pylint
     type = mypy,mypy-strict
 
@@ -22,10 +22,11 @@ deps =
     pykickstart
     # required by pykickstart but not pulled in automatically :/
     requests
-    tox-backticks
 
 setenv =
-    LINTABLES = osbuild/ assemblers/* devices/* inputs/*.* mounts/* runners/* sources/*.* stages/*.* inputs/test/*.py stages/test/*.py sources/test/*.py test/ `find ./tools ! -name "*.sh" -type f`
+    LINTABLES = osbuild/ assemblers/* devices/* inputs/*.* mounts/* runners/* sources/*.* stages/*.* inputs/test/*.py stages/test/*.py sources/test/*.py test/ tools/
+    LINTABLES_EXCLUDES = "*.json,*.sh"
+    LINTABLES_EXCLUDES_RE = ".*\.json$,.*\.sh"
     TYPEABLES = osbuild
     TYPEABLES_STRICT = ./osbuild/main_cli.py
 
@@ -47,18 +48,18 @@ commands =
 
 [testenv:autopep8]
 deps =
-    autopep8==2.0.2
-    pycodestyle==2.10.0
+    autopep8==2.0.4
+    pycodestyle==2.11.0
 
 commands =
-    bash -c 'python -m autopep8 --diff --max-line-length 120 -a -a -a -j0 -r --exit-code {env:LINTABLES}'
+    bash -c 'python -m autopep8 --diff --max-line-length 120 -a -a -a -j0 -r --exclude {env:LINTABLES_EXCLUDES} --exit-code {env:LINTABLES}'
 
 [testenv:pylint]
 deps =
     pylint==3.0.2
 
 commands =
-    bash -c 'python -m pylint {env:LINTABLES}'
+    bash -c 'python -m pylint --ignore-patterns {env:LINTABLES_EXCLUDES_RE} {env:LINTABLES}'
 
 [testenv:mypy]
 deps =


Test patch:
diff --git a/test/mod/test_meta.py b/test/mod/test_meta.py
index b7ced843..0294c501 100644
--- a/test/mod/test_meta.py
+++ b/test/mod/test_meta.py
@@ -1,4 +1,7 @@
 import os
+import pathlib
+import re
+import textwrap
 
 import pytest
 
@@ -168,3 +171,185 @@ def test_schema():
     assert not res
     res = schema.validate([1, 2, 3])
     assert res
+
+
+def make_fake_meta_json(tmp_path, name, version, invalid=""):
+    meta_json_path = pathlib.Path(f"{tmp_path}/stages/{name}.meta.json")
+    meta_json_path.parent.mkdir(exist_ok=True)
+    if version == 1:
+        schema_part = """
+        "schema": {
+          "properties": {
+            "json_filename": {
+              "type": "string"
+            }
+          }
+        }
+        """
+    elif version == 2:
+        schema_part = """
+        "schema_2": {
+          "json_devices": {
+            "type": "object"
+          }
+        }
+        """
+    else:
+        pytest.fail(f"unexpected schema version {version}")
+
+    if invalid == "json":
+        schema_part = """
+        "xxx": {not-json}"'
+        """
+    elif invalid == "schema":
+        schema_part = """
+        "not": "following schema"
+        """
+
+    meta_json_path.write_text("""
+    {
+      "summary": "some json summary",
+      "description": [
+        "long text",
+        "with newlines"
+      ],
+      "capabilities": ["CAP_MAC_ADMIN", "CAP_BIG_MAC"],
+      %s
+    }
+    """.replace("\n", " ") % schema_part, encoding="utf-8")
+    return meta_json_path
+
+
+def make_fake_py_module(tmp_path, name, invalid=""):
+    py_path = pathlib.Path(f"{tmp_path}/stages/{name}")
+    py_path.parent.mkdir(exist_ok=True)
+    fake_py = '"""some py summary\nlong description\nwith newline"""'
+    if invalid == "json":
+        fake_py += textwrap.dedent("""
+        SCHEMA = '"xxx": {not-json}"'
+        """)
+    elif invalid == "schema":
+        fake_py += textwrap.dedent("""
+        noT = "following schema"
+        """)
+    else:
+        fake_py += textwrap.dedent("""
+        SCHEMA = '"properties": {"py_filename":{"type": "string"}}'
+        SCHEMA_2 = '"py_devices": {"type":"object"}'
+        CAPABILITIES = ['CAP_MAC_ADMIN']
+    """)
+    py_path.write_text(fake_py, encoding="utf-8")
+
+
+# keep the tests consistent between loading from python and json
+def test_load_from_json(tmp_path):
+    make_fake_meta_json(tmp_path, "org.osbuild.noop", version=1)
+    modinfo = osbuild.meta.ModuleInfo.load(tmp_path, "Stage", "org.osbuild.noop")
+    assert modinfo.desc == "some json summary"
+    assert modinfo.info == "long text\nwith newlines"
+    assert modinfo.caps == ["CAP_MAC_ADMIN", "CAP_BIG_MAC"]
+    assert modinfo.opts == {
+        "1": {"properties": {"json_filename": {"type": "string"}}},
+        "2": {},
+    }
+
+
+def test_load_from_json_invalid_json(tmp_path):
+    make_fake_meta_json(tmp_path, "org.osbuild.noop", version=2, invalid="json")
+    with pytest.raises(SyntaxError) as exc:
+        osbuild.meta.ModuleInfo.load(tmp_path, "Stage", "org.osbuild.noop")
+    assert str(exc.value) == "Invalid schema: Expecting property name enclosed in double quotes: line 2 column 17 (char 201)"
+
+
+def test_load_from_json_invalid_schema(tmp_path, capsys):
+    make_fake_meta_json(tmp_path, "org.osbuild.noop", version=2, invalid="schema")
+    modinfo = osbuild.meta.ModuleInfo.load(tmp_path, "Stage", "org.osbuild.noop")
+    assert modinfo is None
+    assert re.match(r"WARNING: schema for .* is invalid: ", capsys.readouterr().err)
+
+
+def test_load_from_py(tmp_path):
+    make_fake_py_module(tmp_path, "org.osbuild.noop")
+    modinfo = osbuild.meta.ModuleInfo.load(tmp_path, "Stage", "org.osbuild.noop")
+    assert modinfo.desc == "some py summary"
+    assert modinfo.info == "long description\nwith newline"
+    assert modinfo.caps == set(["CAP_MAC_ADMIN"])
+    assert modinfo.opts == {
+        "1": {"properties": {"py_filename": {"type": "string"}}},
+        "2": {"py_devices": {"type": "object"}},
+    }
+
+
+def test_load_from_py_invalid_json(tmp_path):
+    make_fake_py_module(tmp_path, "org.osbuild.noop", invalid="json")
+    with pytest.raises(SyntaxError) as exc:
+        osbuild.meta.ModuleInfo.load(tmp_path, "Stage", "org.osbuild.noop")
+    assert str(exc.value) == "Invalid schema: Expecting property name enclosed in double quotes (org.osbuild.noop, line 4)"
+
+
+def test_load_from_py_invalid_schema(tmp_path):
+    make_fake_py_module(tmp_path, "org.osbuild.noop", invalid="schema")
+    modinfo = osbuild.meta.ModuleInfo.load(tmp_path, "Stage", "org.osbuild.noop")
+    assert modinfo.desc == "some py summary"
+    assert modinfo.info == "long description\nwith newline"
+    assert modinfo.opts == {
+        "1": {},
+        "2": {},
+    }
+
+
+def test_load_from_json_prefered(tmp_path):
+    make_fake_meta_json(tmp_path, "org.osbuild.noop", version=2)
+    make_fake_py_module(tmp_path, "org.osbuild.noop")
+    modinfo = osbuild.meta.ModuleInfo.load(tmp_path, "Stage", "org.osbuild.noop")
+    assert modinfo.desc == "some json summary"
+    assert modinfo.info == "long text\nwith newlines"
+    assert modinfo.caps == ["CAP_MAC_ADMIN", "CAP_BIG_MAC"]
+    assert modinfo.opts == {
+        "1": {},
+        "2": {"json_devices": {"type": "object"}},
+    }
+
+
+@pytest.mark.parametrize("test_input,expected_err", [
+    # happy
+    (
+        {"summary": "some", "description": ["many", "strings"], "schema": {}}, ""), (
+        {"summary": "some", "description": ["many", "strings"], "schema_2": {}}, ""), (
+        {"summary": "some", "description": ["many", "strings"], "schema_2": {}, "capabilities": []}, ""), (
+        {"summary": "some", "description": ["many", "strings"], "schema_2": {}, "capabilities": ["cap1", "cap2"]}, ""),
+    # unhappy
+    (
+        # no description
+        {"summary": "some", "schema": {}},
+        "'description' is a required property",
+    ), (
+        # no summary
+        {"description": ["yes"], "schema": {}},
+        "'summary' is a required property",
+    ), (
+        # schema{,_2} missing
+        {"summary": "some", "description": ["many", "strings"]},
+        " is not valid",
+    ),
+    (
+        # both schema{,_2}
+        {"summary": "some", "description": ["many", "strings"],
+         "schema": {}, "schema_2": {}},
+        " is valid under each ",
+    ), (
+        # capabilities of wrong type
+        {"summary": "some", "description": ["many", "strings"], "schema": {},
+         "capabilities": [1, "cap1"]},
+        " is not of type 'string'",
+    ),
+])
+def test_meta_json_validation_schema(test_input, expected_err):
+    schema = osbuild.meta.Schema(osbuild.meta.META_JSON_SCHEMA, "meta.json validator")
+    res = schema.validate(test_input)
+    errs = [e.as_dict() for e in res.errors]
+    if expected_err:
+        assert len(errs) == 1, f"unexpected error {errs}"
+        assert expected_err in errs[0]["message"]
+    else:
+        assert not errs