feat(imports)!: version-pinned imports in interface.yaml (#23)#25
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for name@version (and @current) in interface.yaml imports so interface consumers can explicitly pin producer versions, improving determinism of dependency resolution and surfacing invalid pins earlier in the build/dependency graph.
Changes:
- Extend
host/aidl_interface.pyto parse version-pinned imports, track per-import pins, and validate that pinned dependency targets exist (fail-fast vs.KeyErrorduring topo sort). - Update
host/aidl_ops.pyto stop coercing--versiontoint, allowing dotted versions like0.1.0.0. - Broaden
host/CMakeLists.incparsing of-v<version>to accept dotted versions in target names (not just integer ordinals).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| host/CMakeLists.inc | Adjusts CMake target-name version parsing to accept dotted versions for interface targets. |
| host/aidl_ops.py | Passes --version through verbatim to support dotted version strings. |
| host/aidl_interface.py | Parses imports pins, keeps back-compat import list, generates pinned dependency targets, and improves erroring for unresolved pinned deps. |
Comment on lines
+56
to
+79
| def parse_import_spec(spec): | ||
| """Parse a single `imports` entry from interface.yaml. | ||
|
|
||
| Accepts either bare-name form ("audiodecoder") or the versioned | ||
| `name@version` form ("audiodecoder@0.1.0.0", "common@current"). The | ||
| version part is opaque to the parser — any non-empty string is allowed | ||
| and resolved against the on-disk version directory layout downstream. | ||
|
|
||
| Returns: | ||
| (name, version_or_None) — version is `None` for bare-name imports, | ||
| the literal version string otherwise. `@current` is preserved as | ||
| the string "current". | ||
| """ | ||
| if not isinstance(spec, str): | ||
| raise ValueError( | ||
| "imports entry must be a string, got %r" % (spec,)) | ||
| if "@" not in spec: | ||
| return (spec, None) | ||
| name, _, version = spec.partition("@") | ||
| if not name or not version: | ||
| raise ValueError( | ||
| "imports entry %r must be 'name@version' with both parts " | ||
| "non-empty" % (spec,)) | ||
| return (name, version) |
Comment on lines
+169
to
+173
| for imprt, pin in imports.items(): | ||
| if pin == IMPORT_VERSION_UNPINNED: | ||
| lib_deps.append("%s-vcurrent-cpp" % imprt) | ||
| else: | ||
| lib_deps.append("%s-v%s-cpp" % (imprt, pin)) |
Comment on lines
202
to
+208
| elif o == "-v" or o == "--version": | ||
| # Convert the given value to int and then to string. | ||
| # This is required in case the version is given along with the space. | ||
| _gen_version = str(int(a)) | ||
| # Version is passed through verbatim. Integer ordinals ("1", "2") | ||
| # and dotted release strings ("0.1.0.0") are both valid — the | ||
| # latter is used by repos that pin imports via "name@X.Y.Z.W" | ||
| # in interface.yaml (linux_binder_idl#23). The value is treated | ||
| # as an opaque directory-name component downstream. | ||
| _gen_version = a.strip() |
Adds support for `name@version` syntax in `interface.yaml` imports so
consumers can declare which version of a producer they require, instead
of silently inheriting whatever happens to be on disk at
`<producer>/current/`.
Syntax:
imports:
- common@0.1.0.0 # pin to a released snapshot
- audiodecoder@current # in-development sibling (explicit)
- audiosink # bare name — deprecated, treated as @current
Resolver behaviour for the consumer's `current` build:
- `@<version>` — links to `lib<name>-v<version>-cpp.so`. The producer's
interface.yaml must list this version under
`versions_with_info`; otherwise the dependency-tree
generator fails fast with a clear, actionable error.
- `@current` — links to `lib<name>-vcurrent-cpp.so`.
- bare name — same as `@current`, plus a one-shot deprecation
warning per (consumer, import) pair. The warning is
suppressed for frozen-snapshot interface.yaml files
since they are historical artefacts and re-warning
every build would be noise.
Toolchain changes:
- host/aidl_interface.py — adds `parse_import_spec()`, populates a
per-interface `import_versions` map alongside the existing bare-name
`imports` list (preserved for back-compat with aidl_api.py), routes
pinned versions through the dependency tree builder, and replaces
the previous `KeyError: <target>` crash with a graceful RuntimeError
when a pin points to an unregistered target.
- host/aidl_ops.py — removes the `str(int(a))` coercion on `--version`
so dotted release strings (`0.1.0.0`) can be passed through.
- host/CMakeLists.inc — broadens the target-name version regex from
`-v([0-9]+)` to `-v([^-]+)-cpp$` to capture dotted release strings
alongside integer ordinals.
Verified against rdk-halif-aidl tree:
- bare-name imports build cleanly + emit one-shot deprecation warning
- `@current` pin builds cleanly + suppresses the warning
- `@<unregistered-version>` fails fast with the new error message
Closes #23. Related: rdkcentral/rdk-halif-aidl#538.
Ulrond
force-pushed
the
feature/23-versioned-imports
branch
from
3d59600 to
654c27e
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
Adds support for `name@version` syntax in `interface.yaml` `imports` so consumers can declare which version of a producer they require, instead of silently inheriting whatever happens to be on disk at `/current/`.
Implements #23. Used by rdkcentral/rdk-halif-aidl#538 — once this lands and tags as `2.2.0`, rdk-halif-aidl bumps its `binder_sdk.version` pin and migrates every in-tree `interface.yaml` to explicit pins.
Syntax
```yaml
imports:
```
Resolver behaviour (consumer's `current` build)
Files changed
Backward compatibility
Bare-name form keeps working through the deprecation window. The warning fires once per `(consumer, imported-name)` pair per build, not once per call — production logs stay readable. Plan: hard-remove the bare-name form in 3.0.0.
Verification
Tested against the rdk-halif-aidl tree (`develop` tip):
Why this is `!` (breaking)
The bare-name form is technically still working, so most existing consumers are unaffected. The break is the new failure mode for `name@version` pointing at an unregistered version — previously this would silently produce broken builds (`KeyError` from the topological sort, late in the cycle); now it fails fast at configure time. For consumers who were never using pinned imports (everyone, today), this is a no-op.
Closes