Multiple package repos in config.yaml

[msimberg]

Extends config.yaml to accept not only

...
    packages:
        repo: https://github.com/foo/spack-packages.git
        commit: abcdef

but also multiple repos:

...
    packages:
        myrepo:
            repo: https://github.com/bar/other-packages.git
            commit: fedcba
            path: path/to/repo
        builtin:
            repo: https://github.com/foo/spack-packages.git
            commit: abcdef

Order of repos is significant in the same way it's significant in spack config files (https://spack.readthedocs.io/en/latest/repositories.html#search-order-and-overriding-packages). Packages from repos higher up on the list take precedence.

I made a small change so that there are also two implicit repos (currently on main there's one: alps). A repo recipe exists if and only if there are packages in the recipe. alps still exists, but doesn't have the recipe packages. The example above would produce the following list of repos:

• recipe
• alps
• myrepo
• builtin

Currently there's no enforcement that the repo entry has the same name as the repo namespace (similar to spack itself, if I understand it correctly). Is this ok?

There's an optional path property that can be used to point to a non-default subdirectory within the cloned git repo. For spack-packages this is always in repos/spack_repo/builtin, so the default is repos/spack_repo/{name}. In spack-packages there is an index file https://github.com/spack/spack-packages/blob/develop/spack-repo-index.yaml which could be used to locate all the package repos within a git repo. Spack uses this when it clones package repos through git. For now I decided it's overkill to support that. Thoughts?

[albestro]

Currently there's no enforcement that the repo entry has the same name as the repo namespace (similar to spack itself, if I understand it correctly). Is this ok?

I quickly checked and I agree with you. The important part is the matching between namespace (defined in repos.yaml) and the folder name. Not sure what the name is there for, probably just for distinguish the different configurations.

There's an optional path property that can be used to point to a non-default subdirectory within the cloned git repo. For spack-packages this is always in repos/spack_repo/builtin, so the default is repos/spack_repo/{name}. In spack-packages there is an index file https://github.com/spack/spack-packages/blob/develop/spack-repo-index.yaml which could be used to locate all the package repos within a git repo. Spack uses this when it clones package repos through git. For now I decided it's overkill to support that. Thoughts?

I fully agree, unless there will be a reason to do so, I'd add as requirement that additional repos should follow the "standard"/implicit repo structure.

[albestro]

Nice extensions, really nice done! just a few minor comments from my side.

[albestro]

Looking at the test, this is managed at python level with dict.get which returns None if not set, instead of "enforcing" it with the schema.

If it was on purpose, it's ok for me. Just noting this because it might have happened "by accident" and it is not what has been done elsewhere, where default values are used

Suggested change

	"commit": {
	"oneOf": [
	{"type" : "string"},
	{"type" : "null"}
	]
	}
	"commit": {
	"oneOf": [
	{"type" : "string"},
	{"type" : "null"}
	],
	default: null
	}

In case, it applies also to the next more "advanced" schema.

[msimberg]

Semi-intentional, but you might have ideas on how to improve this since I know you've worked more with the schema+validator. The current validator seems to set defaults unconditionally for subobjects in oneOf. It'd e.g. take

...
  packages:
    myrepo:
      repo: https://...

and make it into

...
  packages:
    commit: null
    myrepo:
      repo: https://...

which doesn't pass validation anymore, because it should be one of those two.

It seems I can set a default for commit in the branch where there's a list of repos so I added that default now. But for packages:commit I can't add back the old default null without changing the validator.

---

But that raises a good question: should we even allow commit to be empty? I see now there's a fallback path for it when cloning repos and when commit isn't given it'll just use whatever is checked out by default after cloning a repo. I think this is probably not something we ever want. I couldn't see any recipes in alps-uenv that would set repo but not set the commit.

[albestro]

just a renaming suggestion for the variable

Suggested change

	"name": pkg["name"],
	"path": (recipe.mount / "repos" / "spack_repo" / pkg["name"]).as_posix(),
	}
	for pkg in spack_meta["packages"]
	"name": repo["name"],
	"path": (recipe.mount / "repos" / "spack_repo" / repo["name"]).as_posix(),
	}
	for repo in spack_meta["packages"]

also in other similar loops (both before and after this one), I'd suggest to use repo or pkg_repo or repo_meta...pkg sounds like it's targeting a single package

[msimberg]

I did a few renamings hopefully for the better. I tried to avoid renaming things outside of the diff of this PR.