Conversation
7406385 to
fb3085c
Compare
67ccd6a to
c7c6178
Compare
e966548 to
abe0330
Compare
There was a problem hiding this comment.
Pull Request Overview
Adds support for defining and managing remote git-based Spack package repositories alongside local ones.
- Defines new
RepoDescriptorclasses to parse, initialize, and construct local and remote repos. - Extends the YAML schema to allow
gitobjects withrepository,destination, andrepo_path. - Updates
spack repo add/listcommands, command-line flags, and shell completions for the new options.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| share/spack/spack-completion.fish | Added --name and --repo-path options for spack repo add, updated help text. |
| share/spack/spack-completion.bash | Included --name and --repo-path in bash completion for spack repo add. |
| lib/spack/spack/test/repo.py | New tests for parsing local and git-based repo descriptors. |
| lib/spack/spack/schema/repos.py | Extended repos schema to support remote git repository definitions. |
| lib/spack/spack/repo.py | Introduced RepoDescriptor hierarchy and parsing logic for remote repos. |
| lib/spack/spack/main.py | Integrated auto-fetching of remote repos in the main setup. |
| lib/spack/spack/cmd/repo.py | Refactored repo add into _add_repo, added support for --name, --repo-path, and destination. |
Comments suppressed due to low confidence (3)
lib/spack/spack/test/repo.py:587
- No test covers the case where
repo_pathis provided as a list of strings. Add a test that passesrepo_path: ['one','two']to verifydescriptor.relative_repo_pathsmatches both entries.
descriptor = spack.repo.parse_config_descriptor(
lib/spack/spack/test/repo.py:568
- There is no test for the default destination path when
destinationis omitted. Add a test that omitsdestinationand asserts thatdescriptor.destinationis set to a hash-based cache directory.
def test_parse_config_descriptor_git_1(tmp_path: pathlib.Path):
share/spack/spack-completion.fish:2774
- The completion for positional arguments (path_or_repo and destination) was removed. Add back directory completions for those positions, e.g.:
complete -c spack -n '__fish_spack_using_command_pos 0 repo add' -f -a '(__fish_complete_directories)'
and a similar line for the destination argument.
set -g __fish_spack_optspecs_spack_repo_add h/help name= repo-path= scope=
|
High level thoughts on the schema:
repos:
my_repo:
# you can run even these attrs through `from_kwargs` in `fetch_strategy.py`
git: https://example.com/example/example.git
# optional (and not necessarily all implemented in this PR)
branch: ...
tag: ...
commit: ...
# local_path might be more descriptive but I guess destination is ok
destination:
destination: ~/example # optionalThe way On repo_index:
paths:
- subdir/spack_repo/example/x
- subdir/spack_repo/example/ynow |
7b2e675 to
a329d74
Compare
a329d74 to
ad14202
Compare
There was a problem hiding this comment.
Pull Request Overview
Adds support for cloning and registering Spack package repositories from remote git URLs, including selective sub-repo paths and index file discovery. Key changes:
- Extend
repos.yamlschema andparse_config_descriptorto accept git URLs with optionaldestinationandrepo_paths. - Implement
RemoteRepoDescriptorfor fetching, indexing, and constructing remote repos, and integrate intospack.mainandspack repocommands. - Update CLI (
spack repo add/remove/list), completions (fish/bash), and tests to cover the new functionality.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| share/spack/spack-completion.fish | Updated repo add completion description for plural repositories |
| share/spack/spack-completion.bash | Added --name and --repo-path options to bash completion |
| lib/spack/spack/test/repo.py | Added tests for parsing git vs. local descriptors and index file |
| lib/spack/spack/schema/repos.py | Expanded JSON schema to support git descriptor objects |
| lib/spack/spack/repo.py | Introduced RemoteRepoDescriptor, lock helper, and index reader |
| lib/spack/spack/main.py | Automatically initialize remote repos in the main entry point |
| lib/spack/spack/cmd/repo.py | Extended CLI parser and logic for git-based repo add/remove |
Comments suppressed due to low confidence (1)
|
Should |
f7470d0 to
8e8707d
Compare
8e8707d to
aa41bd2
Compare
Adds support for Spack package repositories from external git repos.
```yaml
repos:
my_repo:
git: https://example.com/example/example.git
destination: ~/example # optional
paths: # optional
- subdir/spack_repo/example/x
- subdir/spack_repo/example/y
```
If `destination` is not configured, Spack clones the git repo to `~/.spack/git_repos/{hash(repository)}`.
Package repositories can put a file `spack-repo-index.yaml` in their root with relative paths to roots of package repositories (i.e. directories that contain `repo.yaml`):
```yaml
repo_index:
paths:
- subdir/spack_repo/example/x
- subdir/spack_repo/example/y
```
so users don't have to put that under `paths` in config. The `spack-repo-index.yaml` is simply a list of paths under `repo_index`. The idea is to avoid duplicating data such as "namespace" already specified in `<git repo>/<repo path>/repo.yaml`, to avoid that there are two sources of truth that go out of sync.
Further, `paths` in user config takes precedence, which allows users to enable specific package repositories in case the git monorepo provides multiple.
Remote package repositories are cloned/initialized in:
* `spack.main`
* `spack repo add`
This is process safe due to the lock in `$SPACK_USER_CACHE_PATH/package-repository.lock`; only one process can clone at a time.
The `spack repo add` command has a few new flags and a new positional arg:
```
spack repo add [-h] [--name NAME] [--path PATH] [--scope ...] path_or_repo [destination]
```
The signature is similar to the familiar `git clone <repository> <directory>`.
The `path_or_repo` argument is detected as a remote git repo if it contains a `:` not preceded by a `/`, which is what git does as well. If in the future we would support package repositories other than local file paths and remote git repos, we can resolve ambiguities with a new flag `[--git | --path | --<other-type>]`, but this is currently unnecessary.
The positional `destination` argument allows users to pick their own clone path, and only applies in case of git repos.
The flag `--path` corresponds to `repos:<name>:paths` and can be repeated to select specific package repositories in a git monorepo, and is also required if the git repo does not provide a `spack-repo-index.yaml` file in its root. Spack will never scan for `repo.yaml` files recursively, it only relies on `spack-repo-index.yaml` and `--repo-path` for package repository roots inside a git repo.
The flag `--name` applies to the *config* name/key in `repos.yaml` under `repos:<name>`. This flag is optional in the common case of adding just one package repository: it is set to the package repository's namespace. In case of monorepos with multiple package repositories, the `--name` flag is required.
Adds support for Spack package repositories from external git repos.
If
destinationis not configured, Spack clones the git repo to~/.spack/git_repos/{hash(repository)}.Package repositories can put a file
spack-repo-index.yamlin their root with relative paths to roots of package repositories (i.e. directories that containrepo.yaml):so users don't have to put that under
pathsin config. Thespack-repo-index.yamlis simply a list of paths underrepo_index. The idea is to avoid duplicating data such as "namespace" already specified in<git repo>/<repo path>/repo.yaml, to avoid that there are two sources of truth that go out of sync.Further,
pathsin user config takes precedence, which allows users to enable specific package repositories in case the git monorepo provides multiple.Remote package repositories are cloned/initialized in:
spack.mainspack repo addThis is process safe due to the lock in
$SPACK_USER_CACHE_PATH/package-repository.lock; only one process can clone at a time.The
spack repo addcommand has a few new flags and a new positional arg:The signature is similar to the familiar
git clone <repository> <destination>.The
path_or_repoargument is detected as a remote git repo if it contains a:not preceded by a/, which is what git does as well. If in the future we would support package repositories other than local file paths and remote git repos, we can resolve ambiguities with a new flag[--git | --path | --<other-type>], but this is currently unnecessary.The positional
destinationargument allows users to pick their own clone path, and only applies in case of git repos.The flag
--repo-pathcorresponds torepos:<name>:pathsand can be repeated to select specific package repositories in a git monorepo, and is also required if the git repo does not provide aspack-repo-index.yamlfile in its root. Spack will never scan forrepo.yamlfiles recursively, it only relies onspack-repo-index.yamland--repo-pathfor package repository roots inside a git repo.The flag
--nameapplies to the config name/key inrepos.yamlunderrepos:<name>. This flag is optional in the common case of adding just one package repository: it is set to the package repository's namespace. In case of monorepos with multiple package repositories, the--nameflag is required.