Fix schema: config:spack:packages is a required entry

[albestro]

config:spack:packages is a required entry but the schema has not been updated to reflect that.

The "minimal" required entry for config.yaml is

version: 2
name: blabla
spack:
  repo: https://github.com/spack/spack.git
  packages:
    repo: https://github.com/spack/spack-packages.git

For this PR the easiest way was also the one that made most sense to me.
But in the future, we might opt for having official repos as defaults.

[albestro]

Now I see that maybe this has not been enforced because of back-compatibility.

[msimberg]

Thanks @albestro. I think I initially had it required when the entry was its own top-level entry. I think I lost the required property when moving it under spack.

Now I see that maybe this has not been enforced because of back-compatibility.

That was certainly not a conscious choice...

But in the future, we might opt for having official repos as defaults.

I'm in principle in favour of this, but I also like just keeping everything explicit. I don't feel strongly about either option.

[albestro]

I'm an absolute newbie with JSON schemas, but I took the chance to learn a bit about them to trying improving the config.yaml schema as an experiment.

Main changes:

• define #/$defs/git-repo so that it can be reused for both spack and spack:packages
• make spack subschema conditional on version, so that spack:packages must and should exists just with version:2
• add constraint for version values
• just for config.yaml schema, switch to a newer draft of JSON Schema (for unevaluatedProperties feature, an enhanced version of additionalProperties)
• add a trivial test for defaults of config.yaml v2

I quickly tested the new schema against https://github.com/eth-cscs/alps-uenv/tree/main and eth-cscs/alps-uenv#224, and it didn't report any problem.

At the moment no test has been added for trivially wrong configs (e.g. missing spack:packages in v2), because we could just check that a ValidationError has been raised without actually checking the reason (because the reason is not always meaningful, see next message). In case we'd like to merge this work on the schema and not drop it, I could eventually add this trivial tests without asserting on the actual problem of the validation.

This PR would shift the error detection towards the schema validation phase instead of raising a python exception. On one side, I'm not sure how much nicer it is to get a strange json validation error message compared to a failure in python code, on the other side if previous statement leans towards python errors then one might ask why we have json schemas.

Maybe it is preferrable to have "loose" schemas like they are now, but it sounds like a strange choice. In any case, this PR could just be an interesting (for me) experiment, and we can even drop it if it's not the path we'd like to pursue (or try to cherry-pick a couple of easy things, e.g. required repo field, git-repo definition).

Looking forward to your comments.

[albestro]

A look at validation error messages

Current solution in this PR does not really give useful error messages. I had a quick look at error messages of production version, which might give nicer error messages in some cases, but it's way more loose on many constraints.

name: "test"
version: 1
spack:

Gives

Traceback (most recent call last):
  File "/capstor/store/cscs/cscs/csstaff/ialberto/workspace/stackinator/./validate.py", line 24, in <module>
    config_validator.validate(raw)
    ~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^
  File "/users/ialberto/.cache/uv/environments-v2/validate-a7de8eb85e349bb8/lib/python3.13/site-packages/jsonschema/validators.py", line 451, in validate
    raise error
jsonschema.exceptions.ValidationError: Unevaluated properties are not allowed ('spack' was unexpected)

Failed validating 'unevaluatedProperties' in schema:
    {'$schema': 'http://json-schema.org/draft-2019-09/schema#',
     'title': 'Schema for Spack Stack config.yaml recipe file',
     'type': 'object',
     'unevaluatedProperties': False,
...

On instance:
    {'name': 'test', 'version': 1, 'spack': None}

But apparently it has more information in it that could be helpful

cat <<EOF | ./validate.py -
name: "test"
version: 1
spack:
  miao:
EOF
[0] Unevaluated properties are not allowed ('spack' was unexpected) {'name': 'test', 'version': 1, 'spack': {'miao': None}}
[1] 'repo' is a required property {'miao': None}
[2] Unevaluated properties are not allowed ('miao' was unexpected) {'miao': None, 'commit': None}

or

[daint][ialberto@daint-ln004 stackinator]$ cat <<EOF | ./validate.py -
name: "test"
version: 1
spack:
EOF
[0] Unevaluated properties are not allowed ('spack' was unexpected) {'name': 'test', 'version': 1, 'spack': None}
[1] None is not of type 'object' None