diff --git a/host/CMakeLists.inc b/host/CMakeLists.inc index 33c2ef6..6634c5c 100644 --- a/host/CMakeLists.inc +++ b/host/CMakeLists.inc @@ -162,10 +162,14 @@ function(AddInterfaceLibrary aidl_target) endif() string(REGEX MATCH "^[^-]+" interface_name "${aidl_target}") - string(REGEX MATCH "-v([0-9]+)" version_part "${aidl_target}") - string(REGEX REPLACE "-v" "" interface_version "${version_part}") - - if (NOT interface_version) + # Match "-v-cpp" where is any non-dash token. This + # accepts both integer ordinals ("-v1-cpp") and dotted release strings + # ("-v0.1.0.0-cpp") used by repos that pin imports via "name@X.Y.Z.W" + # in interface.yaml (linux_binder_idl#23). + string(REGEX MATCH "-v([^-]+)-cpp$" _aidl_version_match "${aidl_target}") + set(interface_version "${CMAKE_MATCH_1}") + + if (interface_version STREQUAL "" OR interface_version STREQUAL "current") set(interface_version "current") set(interface_version_arg "") else() diff --git a/host/aidl_interface.py b/host/aidl_interface.py index 0027341..02cbdd7 100644 --- a/host/aidl_interface.py +++ b/host/aidl_interface.py @@ -47,6 +47,42 @@ def get_host_dir(): LANG_CPP = "cpp" CURRENT_VERSION = "current" +# Sentinel used by `get_imports()` to mean "no pin — caller picks the version +# from latest_version()/next_version()". Preserved for backward compatibility +# with bare-name imports during the deprecation window for `name@version`. +IMPORT_VERSION_UNPINNED = "unknown" + + +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) + + +# Track which (consumer, imported-name) pairs we've already warned about +# so the deprecation message fires once per build, not once per call. +_bare_import_warned = set() + TARGET_UPDATE_API = "update-api" TARGET_FREEZE_API = "freeze-api" @@ -92,7 +128,22 @@ def topological_sort(dependencies): raise ValueError("Cycle detected in dependencies") -def generate_libraries_dependencies(interfaces, out_dir): +def _resolve_lib_deps_for_current(interface): + """Helper: translate an interface's `current` imports into linker + target names (`-v-cpp`). Honours version pins; falls + back to `-vcurrent-cpp` for unpinned entries. + """ + imports = interface.get_imports(interface.next_version()) + lib_deps = [] + 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)) + return lib_deps + + +def generate_libraries_dependencies(interfaces, out_dir, all_loaded=None): """ Generates list of versioned interface libraries and thier dependencies as per versions mentioned from all available interfaces. The list will be in a topological order so that the dependent libraries are @@ -102,15 +153,24 @@ def generate_libraries_dependencies(interfaces, out_dir): in the build Args: - interfaces: List of all AIDL interfaces - out_dir: Location of the directory where the list must be placed - + interfaces: dict of base_name → AidlInterface (the canonical + "current" entry that overrides any same-name snapshot). + out_dir: Location of the directory where the list must be placed. + all_loaded: optional list of every AidlInterface seen by + `load_interfaces`, including frozen snapshots. When provided, + each snapshot directory `//interface.yaml` + is auto-registered as a buildable `-v-cpp` + target with its own pinned imports resolved. This lets + consumers reference frozen producer versions + (`name@`) without each producer having to enumerate + every snapshot under `versions_with_info` in its current + interface.yaml. """ dep_file = path.join(out_dir, LIBRARIES_DEPENDENCIES_FILE) if path.exists(dep_file): os.remove(dep_file) - + # Ensure output directory exists os.makedirs(out_dir, exist_ok=True) @@ -123,10 +183,71 @@ def generate_libraries_dependencies(interfaces, out_dir): for lib in interface.versions_with_info[ver]] lib_deps_dict["%s-v%s-cpp" %(interface_name, ver)] = lib_deps # For Current version - imports = interface.get_imports(interface.next_version()) - lib_deps = [imprt + "-vcurrent-cpp" - for imprt in list(imports.keys())] - lib_deps_dict["%s-vcurrent-cpp" %(interface_name)] = lib_deps + # `get_imports()` returns `{name: version_pin_or_UNPINNED}`. When + # a pin is present (e.g. "0.1.0.0") the consumer links against the + # corresponding versioned library (e.g. `audiodecoder-v0.1.0.0-cpp`). + # An UNPINNED entry falls back to the current sibling so bare-name + # imports keep working through the deprecation window. + lib_deps_dict["%s-vcurrent-cpp" %(interface_name)] = \ + _resolve_lib_deps_for_current(interface) + + # On-disk snapshot auto-discovery (linux_binder_idl#23). + # Walk every loaded AidlInterface; for snapshot directories + # (`//interface.yaml`), register a + # `-v-cpp` target whose deps come from the snapshot's + # own pinned imports. First-write-wins — entries already added via + # `versions_with_info` above are preserved. + if all_loaded is not None: + for snap in all_loaded: + version_label = path.basename(snap.interface_root) + if version_label == CURRENT_VERSION: + continue + target_name = "%s-v%s-cpp" % (snap.base_name, version_label) + if target_name in lib_deps_dict: + continue + snap_deps = [] + for imprt, pin in snap.import_versions.items(): + if pin is None or pin == CURRENT_VERSION: + # A snapshot importing without an explicit pin (or + # @current) is a recipe for silent drift — the + # snapshot would link against its dependency's + # in-development version. We honour the existing + # behaviour for back-compat but log a clear warning + # so the snapshot can be migrated. + logger.warning( + "snapshot %s v%s imports '%s' without a " + "concrete version pin. Frozen snapshots " + "should pin to a concrete producer version " + "(e.g. '%s@%s') to avoid silent drift when " + "the producer's current sibling changes." + % (snap.base_name, version_label, imprt, + imprt, version_label)) + snap_deps.append("%s-vcurrent-cpp" % imprt) + else: + snap_deps.append("%s-v%s-cpp" % (imprt, pin)) + lib_deps_dict[target_name] = snap_deps + + # Validate every referenced dependency target is registered. If a consumer + # pins to a producer version that has not been registered (e.g. + # `videodecoder@current` imports `common@0.1.0.0` but `common`'s + # interface.yaml does not declare 0.1.0.0 in `versions_with_info`), the + # topological sort below would fail with a confusing KeyError. Surface a + # clear, actionable message instead. + for consumer, deps in lib_deps_dict.items(): + for dep in deps: + if dep not in lib_deps_dict: + logger.error( + "Dependency target '%s' required by '%s' is not " + "registered. The producer module's interface.yaml " + "must list this version under `versions_with_info` " + "for the toolchain to build it. Either correct the " + "pin in the consumer's interface.yaml, register the " + "version on the producer side, or remove the pin to " + "fall back to the current sibling." + % (dep, consumer)) + raise RuntimeError( + "Unresolved versioned-import target '%s' (consumer: %s)" + % (dep, consumer)) libs_sorted = topological_sort(lib_deps_dict) @@ -189,16 +310,23 @@ def _interface_sort_key(loc): return (1 if parent == "current" else 0, parent, loc) interface_locations.sort(key=_interface_sort_key) aidl_interfaces = {} + all_loaded = [] for interface_loc in interface_locations: aidl_interface = AidlInterface(interfaces_roots, path.dirname(interface_loc), out_dir, gen_dir) if aidl_interface is not None: aidl_interfaces[aidl_interface.base_name] = aidl_interface + # Retain a reference to every AidlInterface loaded — including + # frozen-snapshot entries that the `aidl_interfaces[base_name]` + # assignment above would otherwise overwrite. Required for + # on-disk snapshot target auto-discovery in + # `generate_libraries_dependencies`. + all_loaded.append(aidl_interface) # TODO: Do this only if interfaces are updated. # Need to find a way to check if interfaces are updated if gen_lib_deps: - generate_libraries_dependencies(aidl_interfaces, out_dir) + generate_libraries_dependencies(aidl_interfaces, out_dir, all_loaded=all_loaded) return aidl_interfaces @@ -477,7 +605,7 @@ def process_imports(interface, interfaces, api_deps): imp_interface = interfaces[imp] # Set version as per thier frozen versions # in case of unknown(case of direct dependencies), set it to the latest - if imports[imp] == "unknown": + if imports[imp] == IMPORT_VERSION_UNPINNED: # Check if the latest version is "0" (never frozen) # If so, use the next version (current) ver = imp_interface.latest_version() @@ -989,9 +1117,39 @@ def _load_interface(self, interfaces_roots, out_dir, gen_dir): self.interface_gen_dir = path.join(interfaces_roots[0], "stable/generated", self.base_name) self.srcs = data.get("aidl_interface").get("srcs") - self.imports = data.get("aidl_interface").get("imports") - if self.imports is None: - self.imports = [] + + # imports may be: + # - bare name : "audiodecoder" (deprecated) + # - versioned name : "audiodecoder@0.1.0.0" + # - explicit current : "audiodecoder@current" + # + # `self.imports` stays as a flat list of bare names for back-compat + # with code that iterates it (aidl_api.py). The pinned version, if + # any, is captured per-name in `self.import_versions`. + raw_imports = data.get("aidl_interface").get("imports") or [] + self.imports = [] + self.import_versions = {} + # Deprecation warnings only fire for the in-development interface + # (located in `/current/`). Frozen-snapshot interface.yaml + # files are historical artefacts — re-warning on every build would + # be noise and is not actionable since the snapshot is immutable. + is_current = path.basename(self.interface_root) == CURRENT_VERSION + for raw in raw_imports: + name, version = parse_import_spec(raw) + self.imports.append(name) + self.import_versions[name] = version + if version is None and is_current: + key = (self.base_name, name) + if key not in _bare_import_warned: + _bare_import_warned.add(key) + logger.warning( + "interface.yaml: '%s' imports '%s' without a " + "version pin. Bare-name imports are deprecated " + "and will be removed in a future major release. " + "Use '%s@current' for the in-development version " + "or '%s@' to pin to a released snapshot." + % (self.base_name, name, name, name)) + self.stability = data.get("aidl_interface").get("stability") versions = [] versions_with_info = {} @@ -1049,9 +1207,18 @@ def get_imports(self, version): assert False, "Unknown version provided" for imp in self.imports: - # let the called decide whether to use the next version or current version - # for imports. - imports[imp] = "unknown" + # Resolve the per-import version pin (if any) declared in + # interface.yaml. A bare-name import leaves `pin` as None, + # which falls back to IMPORT_VERSION_UNPINNED — the existing + # contract that lets the caller pick latest/next/current. + # An explicit `@current` is treated the same as the unpinned + # form so the toolchain always builds against the consumer's + # current sibling. + pin = self.import_versions.get(imp) + if pin is None or pin == CURRENT_VERSION: + imports[imp] = IMPORT_VERSION_UNPINNED + else: + imports[imp] = pin return imports diff --git a/host/aidl_ops.py b/host/aidl_ops.py index 85a4b4c..21d4e99 100755 --- a/host/aidl_ops.py +++ b/host/aidl_ops.py @@ -200,9 +200,12 @@ def main(argv): _gen_dir = a 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() elif o == "-o" or o == "--out_dir": _out_dir = a