Repository navigation
refactor(tools): share catalog reader between installer and documentation - #196
Merged
lurtz merged 1 commit intoNov 3, 2026
Merged
Conversation
AlexanderLanin
changed the title
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues block the shared catalog-reader extraction.
No open issues 🎉
What changed in this PR
This PR separates tool metadata reading from DevContainer installation, supporting the catalog consolidation in #190. The installer and README generator now share a standard-library-only reader while preserving tool definitions, pinned versions, and delivery behavior.
Changes:
- Extract native and Python catalog readers into
catalog.py, preserving literal-only parsing. - Update consumers and supporting configuration for shared-reader use, including copied tools.
- Add regression coverage and CI execution for catalog validation and script independence.
| File | Description |
|---|---|
tools/tests/catalog_test.py |
Tests metadata consistency, invalid catalogs, duplicates, and copied scripts. |
tools/internal/sync_readme.py |
Reads metadata directly from the shared catalog module. |
tools/internal/README.md |
Documents the reader/installer boundary and regression command. |
tools/internal/devcontainer/install.py |
Replaces local catalog readers with shared imports. |
tools/internal/catalog.py |
Provides shared metadata types, parsing, validation, and lookup functions. |
tools/BUILD.bazel |
Includes the shared reader in launcher test runfiles. |
.pre-commit-config.yaml |
Watches shared-reader changes for README regeneration. |
.github/workflows/ci.yaml |
Runs the shared-reader regression tests. |
Review effort: Balanced
1 of 10 tasks
lurtz
approved these changes
Nov 3, 2026
This was referenced Nov 3, 2026
etas-contrib-automation Bot
pushed a commit
to etas-contrib/score_devcontainer
that referenced
this pull request
Nov 5, 2026
…lipse-score#198) ## Why Adding or removing a tool currently means following several installation scripts and separate version files. Maintainers need one place to see which tools each environment provides, and to change that selection without rewriting installation steps. For developers, basedpyright and pytest become available to all DevContainer users alongside pre-commit, rather than being installed into one user's home directory. They also gain Bazel targets using the same package versions. Invalid selections, missing declared dependencies and unsupported platform artifacts are reported before installation starts. ## What changes - **Tool selection:** `tools/environments.json` lists the directly managed tools for the two existing container features and this repository's own setup. - **Package definitions:** `tools/providers.json` holds Python/npm packages, APT policies and special setup metadata. Native release versions and checksums stay in the existing lockfiles; Bazel reads Python definitions from the same JSON source. - **Installation:** each setup requests its selection with one installer call. The installer manages temporary build packages, tool setup, cleanup and required privileges, while npm retains the development user's environment. Tool-specific setup lives beside the installer. - **Cleanup and fixes:** remove the replaced YAML definitions and yq bootstrap, and repair config/state parent ownership without changing existing private permissions. Existing tool versions and APT policies are retained. Distribution-selected packages still follow Ubuntu; the change does not make those packages exactly pinned. Part of eclipse-score#190; builds on eclipse-score#196. --- ## Possible split if this is too large If this PR is too large to review as one change, we could split it into the following independently usable steps. Extractions eclipse-score#200 and eclipse-score#201 are ready for review, each based directly on `main` and independent of the other. This PR retains the complete implementation until the extractions land. | PR | Scope | Result | | --- | --- | --- | | 1. Fix directory ownership (eclipse-score#200) | Repair config/state parent ownership while preserving private permissions, with regression coverage. | An independent bugfix. | | 2. Consolidate package definitions (eclipse-score#201) | Introduce `providers.json` and the Bazel adapter; replace the Python/YAML definitions and yq. Adapt existing installers to read the new metadata and expose the additional Python Bazel targets. | Shared definitions, with existing installation flows largely retained. | | 3. Migrate repository setup | Add environment selection, validation and native/Python/npm installation with privilege handling; migrate `post_create_command.sh`. | The new model is used by this repository's own setup. | | 4. Migrate container features | Add APT, temporary build dependencies and declared setup scripts; migrate both features, including system-wide Python tools, and remove their inline installation logic. | The complete behavior proposed by this PR. | Steps 2 → 3 → 4 build on each other. Step 1 is independent and should land before step 4. If step 4 is still too large, it can be split further by `bazel-feature` and `s-core-local`. Tests and documentation would accompany their respective changes; intermediate steps would keep one source for each version rather than introducing duplicate definitions.
etas-contrib-automation Bot
pushed a commit
to etas-contrib/score_devcontainer
that referenced
this pull request
Nov 6, 2026
…pse-score#203) Setup scripts and feature tests need the catalog’s version and checksum metadata. Keeping its preparation in the shared reader makes that data available independently of the container installer and keeps metadata rules together after eclipse-score#198. Move the metadata preparation to `catalog.load_setup_metadata()` and have the installer use it for setup environments and `shell-metadata`. Move the existing metadata assertion to the catalog tests and document the boundary. Existing shell exports and installation behavior are preserved. Part of eclipse-score#190; follows eclipse-score#196 and eclipse-score#198.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tool documentation now reads catalog metadata without importing the
DevContainer installer. A shared, standard-library-only reader keeps metadata
queries independent of installation code while preserving existing versions,
definitions, and delivery behavior.
tools/internal/catalog.py, retaining the literal-only parsing boundary.direct script execution and copied container tools.
documentation hook, and document the reader/installer boundary.
All tool definitions stay at their existing paths. The Python catalog remains
limited to pre-commit, and the shell/Bazel launcher API is unchanged.
Related #190.