diff --git a/host/CMakeLists.inc b/host/CMakeLists.inc index d4ceb5c..33c2ef6 100644 --- a/host/CMakeLists.inc +++ b/host/CMakeLists.inc @@ -53,6 +53,24 @@ else() message(STATUS "LINUX_BINDER_AIDL_ROOT_OUT explicitly set to ${LINUX_BINDER_AIDL_ROOT_OUT}") endif() +# Base directory under which generated //{src,include} live. +# Decoupled from LINUX_BINDER_AIDL_ROOT_OUT (the intermediate out/ dir) so that +# a consumer using module-local generation can point this at its repo root +# while keeping intermediate artefacts in a separate out/ tree. Defaults to +# LINUX_BINDER_AIDL_ROOT_OUT for backwards compatibility. +# +# Contract: this must resolve to the directory whose generated output the +# generator actually produces. For module-local layout the generator emits +# into ///, so this must be set to that +# interfaces root. A mismatch is caught loudly by the post-generation source +# re-check below (FATAL_ERROR "no C++ files found"), never silently. +if (NOT DEFINED LINUX_BINDER_AIDL_GENERATED_ROOT) + set(LINUX_BINDER_AIDL_GENERATED_ROOT "${LINUX_BINDER_AIDL_ROOT_OUT}") + message(STATUS "LINUX_BINDER_AIDL_GENERATED_ROOT not set, defaulting to ${LINUX_BINDER_AIDL_GENERATED_ROOT}") +else() + message(STATUS "LINUX_BINDER_AIDL_GENERATED_ROOT explicitly set to ${LINUX_BINDER_AIDL_GENERATED_ROOT}") +endif() + # Derive host AIDL compiler root if not provided if (NOT DEFINED HOST_AIDL_DIR) set(HOST_AIDL_DIR "${LINUX_BINDER_AIDL_ROOT_OUT}/host/aidl_compiler") @@ -151,7 +169,9 @@ function(AddInterfaceLibrary aidl_target) set(interface_version "current") set(interface_version_arg "") else() - set(interface_version_arg "-v ${interface_version}") + # CMake list so execute_process expands this to two separate argv entries. + # aidl_ops.py uses getopt and rejects "-v " as a single token. + set(interface_version_arg -v ${interface_version}) endif() if (NOT "${${aidl_target}_deps}" STREQUAL "") @@ -166,29 +186,81 @@ function(AddInterfaceLibrary aidl_target) list(GET interfaces_dir 0 first_interfaces_root) - # Use LINUX_BINDER_AIDL_ROOT_OUT directly - parent project provides the complete path - set(source_dir ${LINUX_BINDER_AIDL_ROOT_OUT}/${interface_name}/${interface_version}/src) - set(include_dir ${LINUX_BINDER_AIDL_ROOT_OUT}/${interface_name}/${interface_version}/include) - - # Check if generated sources already exist (pre-committed in repo) + # Generated sources live under LINUX_BINDER_AIDL_GENERATED_ROOT (which + # defaults to LINUX_BINDER_AIDL_ROOT_OUT). For module-local consumers this + # is the repo root, so the path resolves to ///. + set(source_dir ${LINUX_BINDER_AIDL_GENERATED_ROOT}/${interface_name}/${interface_version}/src) + set(include_dir ${LINUX_BINDER_AIDL_GENERATED_ROOT}/${interface_name}/${interface_version}/include) + + # Regeneration policy (#530, Option A): + # - "current" version: ALWAYS regenerate. The AIDL under /current/ + # is mutable and the previous "skip if include/src already populated" + # guard silently shipped stale C++ when the AIDL changed but the + # generated tree happened to exist (e.g. from a prior build or a + # stale checkout). The AIDL compiler overwrites in place, so a + # successful regen brings the tree to byte-equivalent state with + # the AIDL; if a method or type is removed, its primary file is + # overwritten with the new content. Regen is cheap relative to + # compile and eliminates the "ship stale generated code" class of + # bug entirely. + # - Frozen versions (e.g. v1, v2): pre-committed C++ is part of the + # release contract - if pre-committed sources exist on disk, reuse + # them as-is and skip regen entirely. The first build of a freshly + # checked-out frozen version that ships no generated tree (rare, + # but possible for in-flight versions before sources are committed) + # will generate once to bootstrap; subsequent builds reuse what's + # on disk. file(GLOB_RECURSE existing_sources "${source_dir}/*.cpp") - - if(NOT existing_sources) - # Generated code doesn't exist - need to run API update first + + if (interface_version STREQUAL "current") + message(STATUS "Regenerating sources for ${interface_name} v${interface_version} (always-regen, #530)") + + # Generate Sources + message(VERBOSE "Command: ${LINUX_BINDER_AIDL_ROOT_HOST}/aidl_ops.py -g ${interface_version_arg} -r ${interfaces_dir} -o ${LINUX_BINDER_AIDL_ROOT_OUT} ${interface_name}") + execute_process( + COMMAND ${LINUX_BINDER_AIDL_ROOT_HOST}/aidl_ops.py -g ${interface_version_arg} -r "${interfaces_dir}" -o "${LINUX_BINDER_AIDL_ROOT_OUT}" ${interface_name} + RESULT_VARIABLE gen_result + ) + + if(NOT gen_result EQUAL 0) + message(FATAL_ERROR "AIDL generation failed for ${interface_name}; check the aidl_ops.py output above.") + endif() + + # Re-glob after generation; the generator writes /current/{src,include}. + file(GLOB_RECURSE source_files "${source_dir}/*.cpp") + if(NOT source_files) + # Defensive fallback: if regen ran cleanly but produced no files in + # the expected location (e.g. a component whose interface loader + # maps to a different versioned directory than CMake's "current" + # path), fall back to any pre-committed sources so the build + # remains usable. This preserves the previous behaviour for those + # components without losing the new always-regen guarantee for the + # common case. + if(existing_sources) + list(LENGTH existing_sources list_length) + message(WARNING "Regeneration produced no files in ${source_dir}; " + "falling back to ${list_length} pre-committed source(s). " + "Generated C++ may be stale relative to AIDL.") + set(source_files ${existing_sources}) + else() + message(FATAL_ERROR "Source generation completed but no C++ files found in ${source_dir}") + endif() + endif() + elseif(NOT existing_sources) + # Frozen version with no pre-committed sources - generate once. message(STATUS "No pre-generated sources found for ${interface_name} v${interface_version}") message(STATUS "Generating sources using aidl_ops...") - - # Generate Sources + message(VERBOSE "Command: ${LINUX_BINDER_AIDL_ROOT_HOST}/aidl_ops.py -g ${interface_version_arg} -r ${interfaces_dir} -o ${LINUX_BINDER_AIDL_ROOT_OUT} ${interface_name}") execute_process( COMMAND ${LINUX_BINDER_AIDL_ROOT_HOST}/aidl_ops.py -g ${interface_version_arg} -r "${interfaces_dir}" -o "${LINUX_BINDER_AIDL_ROOT_OUT}" ${interface_name} RESULT_VARIABLE gen_result ) - + if(NOT gen_result EQUAL 0) - message(FATAL_ERROR "Failed to generate sources for ${interface_name}. Run './build_interfaces.sh ${interface_name}' first.") + message(FATAL_ERROR "AIDL generation failed for ${interface_name}; check the aidl_ops.py output above.") endif() - + # Re-check after generation file(GLOB_RECURSE source_files "${source_dir}/*.cpp") if(NOT source_files) diff --git a/host/aidl_api.py b/host/aidl_api.py index cd0c370..d4ba8d4 100644 --- a/host/aidl_api.py +++ b/host/aidl_api.py @@ -206,6 +206,14 @@ def handle_update_api(interface_name, interfaces): logger.verbose("Running %s-%s" %(interface.base_name, "update_api")) + # In module-local layout the AIDL sources already live in their versioned + # directory - there is no separate central stable/aidl tree to copy into. + # update-api would rsync the directory onto itself, so it is a no-op here. + if interface.layout == "module-local": + logger.info("update-api skipped for %s - module-local layout, AIDL is " + "already in place" %(interface.base_name)) + return + # Copy AIDL source files from {module}/current/ to stable/aidl/{module}/current/ current_api_dir = aidl_interface.get_versioned_dir(interface, CURRENT_VERSION) diff --git a/host/aidl_gen_rule.py b/host/aidl_gen_rule.py index c3b549a..a2837d9 100644 --- a/host/aidl_gen_rule.py +++ b/host/aidl_gen_rule.py @@ -200,7 +200,11 @@ def gen_cpp_sources(interface_name, gen_version = interface.next_version() version_dir = "" - if gen_version == interface.next_version(): + if interface.layout == "module-local": + # In module-local layout the version IS the interface's own + # directory name (e.g. "current", "0.1.0.0"). + version_dir = path.basename(interface.interface_root) + elif gen_version == interface.next_version(): version_dir = CURRENT_VERSION else: version_dir = gen_version @@ -330,17 +334,24 @@ def handle_source_gen(interface_name, # Instruct user to create one by executing # `aidl_ops -u `. logger.error("API dump for the current version of AIDL \ - interface %s does not exist." %(aidlInterface.baseName)) + interface %s does not exist." %(interface.base_name)) logger.fatal("Run \"make %s-update-api\", or add \"stability: \ unstable\" to the build rule for the interface if it \ does not need to be versioned" \ - %(aidlInterface.baseName)) + %(interface.base_name)) if interface.stability == "unstable": # TODO: Implement return if gen_version is not None: + if interface.layout == "module-local": + # In module-local layout the version is selected by which + # interface directory is built, not by a -v flag. + logger.fatal("-v/--version is not applicable to module-local " + "interface %s; the version is its own directory (%s)" + %(interface.base_name, + path.basename(interface.interface_root))) if int(gen_version) < 1 or \ int(gen_version) > int(interface.next_version()): logger.error("%s doesn have version %s, latest Available: %s" \ diff --git a/host/aidl_interface.py b/host/aidl_interface.py index b9d3e83..0027341 100644 --- a/host/aidl_interface.py +++ b/host/aidl_interface.py @@ -176,6 +176,18 @@ def load_interfaces(interfaces_roots, out_dir, gen_dir=None, gen_lib_deps=False) interface_locations.append(path.join(root, INTERFACE_DEF_YAML)) logger.debug("Interfaces found at %s" %(interface_locations)) + # Stable ordering: walk order from os.walk is filesystem-dependent, but + # when a module ships both /current/interface.yaml AND a frozen + # snapshot //interface.yaml under the same base_name, + # the LAST one inserted into the aidl_interfaces dict wins - which + # decides where the generator writes its output. Sort with frozen + # versions first and "current" last so current always wins, matching + # the "build current" intent of the rest of the toolchain (#530). + def _interface_sort_key(loc): + parent = path.basename(path.dirname(loc)) + # "current" sorts last; everything else by name + return (1 if parent == "current" else 0, parent, loc) + interface_locations.sort(key=_interface_sort_key) aidl_interfaces = {} for interface_loc in interface_locations: aidl_interface = AidlInterface(interfaces_roots, path.dirname(interface_loc), @@ -348,12 +360,11 @@ def get_preprocessed_dir(interface, version): Returns: Path to preprocessed directory: out/.preprocessed/{module}/{module}_interface/{version}/ """ - # Use a dedicated .preprocessed directory in out/ for clarity - # Extract workspace root from interface_root_stable path - # interface_root_stable is typically: /stable/aidl/{module} - stable_path = interface.interface_root_stable - workspace_root = path.dirname(path.dirname(path.dirname(stable_path))) # Go up 3 levels - + # Use a dedicated .preprocessed directory in out/ for clarity. + # Preprocessed AIDL is a shared build artefact - anchor it at the + # workspace root regardless of the interface's layout. + workspace_root = interface.workspace_root + preprocessed_root = path.join(workspace_root, "out", ".preprocessed", interface.base_name) return path.join(preprocessed_root, interface.base_name + AIDL_PREPROCESSED_SUFFIX, version) @@ -887,6 +898,12 @@ class AidlInterface: # Subdirectory name under stable/ for AIDL files (default: "aidl") # Can be overridden in interface.yaml via stable_aidl_subdir field stable_aidl_subdir = "aidl" + # Artefact layout: "central" (shared stable/ tree) or "module-local" + # (versioned AIDL + generated C++ co-located with the interface). + # Overridable via the interface.yaml `layout` field. + layout = "central" + # First interfaces root - used to locate shared build artefacts. + workspace_root = "" # Locations of directories required for intermediatory operations interface_root_out = "" interface_api_dir_out = "" @@ -944,16 +961,32 @@ def _load_interface(self, interfaces_roots, out_dir, gen_dir): # Read stable_aidl_subdir from interface.yaml (optional field, default: "aidl") self.stable_aidl_subdir = data.get("aidl_interface").get("stable_aidl_subdir", "aidl") + # Workspace root - the first interfaces root. Used to locate shared + # build artefacts (e.g. preprocessed AIDL) independently of layout. + self.workspace_root = path.realpath(interfaces_roots[0]) + + # layout: where versioned AIDL sources and generated C++ are located. + # central - shared stable/ tree at the workspace root (legacy default) + # module-local - co-located with the interface, in its own directory + # The older `frozen_location` field is still honoured for back-compat. self.frozen_location = data.get("aidl_interface").get("frozen_location", "stable") - if self.frozen_location == "stable": + layout = data.get("aidl_interface").get("layout") + if layout is None: + layout = "module-local" if self.frozen_location == "interface" else "central" + self.layout = layout + + if self.layout == "module-local": + # Versioned AIDL and generated C++ live in the interface's own + # directory tree: //. interface_root is the + # / directory, so its parent is the module dir. + module_dir = path.dirname(self.interface_root) + self.interface_root_stable = module_dir + self.interface_gen_dir = module_dir + else: # First directory in the list of interface root directory should # have the stable directory self.interface_root_stable = path.join(interfaces_roots[0], f"stable/{self.stable_aidl_subdir}", self.base_name) self.interface_gen_dir = path.join(interfaces_roots[0], "stable/generated", self.base_name) - elif self.frozen_location == "interface": - self.interface_root_stable = path.join(self.interface_root, "aidl_api") - # TODO Needs to be updated - 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")