feat(config): resolve extension versions from the extension's source tree - #192
Open
mobileoverlord wants to merge 2 commits into
Open
feat(config): resolve extension versions from the extension's source tree#192mobileoverlord wants to merge 2 commits into
mobileoverlord wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for extension version values that are sourced from an extension’s own tree (e.g., Cargo.toml / VERSION) so that composed versions remain stable across path/git/package sources and packaging can reliably ship the required inputs.
Changes:
- Introduces
version: { file, key, format }providers and resolves them during config composition (and in theget_merged_sectionpath used byext package). - Factors extension-tree reading into a reusable
ExtSourceReader(dir/volume/container/dev-fallback) so both config and version resolution use the same access strategy. - Updates
ext packageto include provider files in payloads and skip version “baking” when a provider is used; improves version validation errors to include origin path.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ext_version_source.rs | New integration coverage for provider resolution across source kinds and error reporting. |
| src/utils/version.rs | Adds validate_ext_version to include the originating config path in semver errors. |
| src/utils/mod.rs | Exposes new utility modules for extension source reading and version providers. |
| src/utils/ext_version_source.rs | Implements provider parsing/validation and in-config resolution to literal strings. |
| src/utils/ext_source_reader.rs | Centralizes reading extension files/configs from dir/container/volume/dev-fallback. |
| src/utils/config_edit.rs | Adds a test demonstrating why baking must be skipped for provider-based versions. |
| src/utils/config.rs | Integrates discovery/readers, resolves provider versions during composition, tracks sources. |
| src/commands/ext/package.rs | Ensures provider file is packaged; skips baking when provider is present. |
| src/commands/ext/image.rs | Uses validate_ext_version to report better version error context. |
| src/commands/ext/build.rs | Uses validate_ext_version to report better version error context. |
| src/commands/config_show.rs | Includes resolved extension version in config show --detail output. |
| docs/features/extension-versioning-redesign.md | Documents provider syntax, rationale, rollout, and baking behavior. |
| Cargo.toml | Adds toml dependency needed to extract versions by key from TOML files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…tree
An extension's `version` is an identity field: it has to resolve to the
same string no matter who reads the config. `{{ env.VAR }}` cannot do
that — it binds the value to the caller's environment, and the job that
packages an extension and the build that consumes it are by definition
different environments.
That is why an extension declaring `version: '{{ env.AVOCADO_EXT_VERSION }}'`
(the in-source program extensions do) breaks the moment you point a
project at it with `source: { type: git | path }` to test a local
checkout: the variable isn't set, `env::resolve` warns and substitutes
"", and `ext build` / `ext image` / `ext package` then fail semver
validation with `invalid version ''`. `runtime build` doesn't even
validate — it composes the artifact name `<ext>-.raw`.
Add a mapping form for `version` that names a file inside the
extension's OWN source tree, which is present in every consumption mode
(working copy for `path`, clone for `git`, RPM payload for `package`):
version:
file: VERSION # no key => whole file, trimmed
version:
file: Cargo.toml # parse and navigate
key: package.version
`key` is the discriminator, so there is no format guessing for a plain
VERSION file, and `file: Cargo.toml` with no `key` is a literal read
rather than a surprise parse. `file` must stay inside the extension —
no `..`, no absolute paths. Resolution happens during composition,
before the final interpolation pass, so every existing consumer still
sees `version` as a plain string and nothing downstream changes.
Reading the version file needs the same access ladder that already
existed to read an extension's avocado.yaml — host source path,
in-container includes dir, SDK volume mountpoint, throwaway container,
dev fallback. That ladder was open-coded in config.rs with a
read/error/continue block per strategy; extract it to
`utils::ext_source_reader` so both reads go by the same route. Doing so
also fixes an incidental bug: only `type: path` accepted `avocado.yml`,
so a `.yml` extension shipped in an RPM was invisible once installed.
`ext package` keeps the version file in the payload even when the
extension declares an explicit `package_files` list (which otherwise
replaces the defaults wholesale) — the published avocado.yaml keeps the
provider rather than a baked literal, so a payload missing that file
would be unresolvable for every consumer. The legacy env-template bake
is retained for the form that genuinely cannot resolve downstream, and
is skipped for provider-based extensions.
Also: `config show --detail` now reports each extension's resolved
version, and an invalid version names the config file it came from.
No extension migrates in this change — that needs a released CLI that
understands the provider, since `setup-avocado-cli` installs `latest`.
See the Rollout section in the design doc.
… versions - `ExtSourceReader::read` resolves the joined path and requires it to stay under the extension root. `version.file` already rejects absolute paths and `..`, but a symlink inside a `type: git` tree could still point out. - `get_merged_section_with_board` only resolves a provider for a direct `extensions.<name>` path; a deeper one was read as an extension named `<name>.<sub>`. - The invalid-version error now shows a pre-release and a build-metadata example, both of which the provider flow accepts.
mobileoverlord
force-pushed
the
jschneck/hermetic-ext-version-source
branch
from
August 12, 2026 20:03
1fad6f0 to
1682022
Compare
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.
Problem
An extension's
versionis an identity field — it must resolve to the same string no matter who reads the config.{{ env.VAR }}can't do that: it binds the value to the caller's environment, and the job that packages an extension and the build that consumes it are by definition different environments.The in-source program extensions (
avocado-ext-cli,avocado-ext-connect,avocado-ext-tunnels) declareversion: '{{ env.AVOCADO_EXT_VERSION }}', with CI readingCargo.tomland exporting the variable. Point a project at one of them withsource: { type: git | path }to test a local checkout and it breaks: the variable isn't set,env::resolvewarns and substitutes"", and validation fails.Three hard failure sites (
ext/build.rs,ext/image.rs,ext/package.rs) plus one silent one:runtime/build.rsnever validates and composes the artifact nameavocado-ext-cli-.raw.This also violates an invariant the code already states at
utils/config.rs— "switching between package/path/git doesn't change the composed config hash" — and re-introduces the exact failure classdocs/features/extension-versioning-redesign.mdwas written to eliminate ("wildcards that can't resolve for git/path extensions").Approach
Keep interpolation; move
versionoff the ambient environment and onto a file inside the extension's own source tree, which is present in every consumption mode — the working copy forpath, the clone forgit, the RPM payload forpackage.keyis the discriminator, so there's no format guessing for a plainVERSIONfile, andfile: Cargo.tomlwith nokeyis a literal read rather than a surprise parse.format(toml/json/yaml) is inferred from the extension and can be set explicitly; it's rejected without akey.filemust stay inside the extension — no.., no absolute paths.Resolution runs during composition, before the final interpolation pass, so every existing consumer still sees
versionas a plain string and nothing downstream changes.{{ avocado.* }}and{{ config.* }}remain fine elsewhere — those are consumer-context values that are supposed to differ per build. It's specificallyenvin an identity field that can't work.Notable pieces
Extracted the extension-tree read ladder. Reading the version file needs the same access ladder that already existed for an extension's
avocado.yaml— host source path, in-container includes dir, SDK volume mountpoint, throwaway containercat, dev fallback. That was open-coded inconfig.rswith a read/error/continueblock per strategy; it's nowutils::ext_source_reader, so both reads go by the same route.config.rsis a net ~340-line deletion. This also fixes an incidental bug: onlytype: pathacceptedavocado.yml, so a.ymlextension shipped in an RPM was invisible once installed.The payload must carry the version file. The published
avocado.yamlkeeps the provider rather than a baked literal, soext packagealways appends the provider's file — including on the branch where an explicitpackage_fileslist replaces the defaults wholesale. Forgetting it would only surface when someone consumed the package.The legacy bake is retained, and guarded.
bake_extension_versionstill handles the{{ env.AVOCADO_EXT_VERSION }}form, which genuinely can't resolve downstream. It's skipped for provider-based extensions — baking one would strand the provider'sfile:/key:lines under a replacedversion:scalar.test_bake_extension_version_would_corrupt_a_provider_blockdocuments exactly that.Also:
config show --detailreports each extension's resolvedversion(release CI will read this instead of hand-parsing YAML, which can't see through a provider), and an invalid version now names the config file it came from.Rollout — no extension migrates here
setup-avocado-cliinstallslatest, and a program-tracking extension's release workflow runs on the same tag that publishes the new CLI. So the provider has to be in a released CLI before anyavocado.yamlcan depend on it, or the first tag after merge installs a pre-feature CLI and fails to publish.avocado-cli,avocado-conn, andavocado-ratstay on{{ env.AVOCADO_EXT_VERSION }}and keep working through the bake.avocado.yamlfiles, dropAVOCADO_EXT_VERSION/ theext-versioninput from their workflows and fromavocado-linux/actions, and delete the bake. Staged locally, not yet opened.Step 2 is a payload format change: a CLI predating providers reads the mapping, coerces it to
"0", and fails withinvalid version '0'. There's no graceful degradation —cli_requirementis top-level and isn't merged from a remote extension's config — so publish tonextfirst.Verification
cargo fmt --check,clippy --all-targets --all-features -D warnings, and the full suite are clean (1163 lib + 1171 bin + 8 new integration tests).Beyond tests, verified against real artifacts:
Cargo.tomlwithAVOCADO_EXT_VERSIONunset and no warning — cli1.0.0-rc.1, conn/rat0.1.0.type: path— the reported bug — resolves1.0.0-rc.1.VERSIONfrompackage_files. Payload came out asVERSION,avocado.yaml,extra.txt;Version: 0.4.2; the shippedavocado.yamlkept the provider unbaked. A consumer then resolved0.4.2from that extracted payload. DeletingVERSIONfrom it produces a four-level error chain naming the extension, the field, the file, and the root searched.Unrelated issue found while verifying
avocado ext package avocado-ext-clifails witherror: line 5: Illegal char '-' (0x2d) in: Version: 1.0.0-rc.1. This is pre-existing and unrelated — a scratch extension with a plain literalversion: '1.0.0-rc.1'and no provider fails identically, andgit show HEAD~1:src/commands/ext/package.rshas no sanitization either. CI exported that exact string, so the ext release workflow was already broken on RC tags. RPM wants1.0.0~rc.1. Worth fixing separately before the next tag.CI note: GitHub Actions was in a critical outage when this was opened, so checks may be delayed or spuriously failing.