From 948615083abd5e93ae920bfb893ff843270387d5 Mon Sep 17 00:00:00 2001 From: Anton Hansson Date: Wed, 12 Sep 2018 18:01:32 +0100 Subject: [PATCH] Re-write the module override logic. This makes the product-installed-files macro more accurately reflect the files installed for a given product, as well as fixing bugs in the previous implementation. Specifically, the complete list of overrides found so far is stripped in each round of expanding required modules. Previously, overrides were stripped out *after* expanding required modules. This meant that for a scenario where B depends on C, and A overrides B, C could get installed. It's unclear if this was a problem in practice. The other effect is that the offending artifacts txt is more accurate, since overridden modules are now correctly removed. Bug: 80410283 Test: build_test downstream Change-Id: I8bfc7c40bedd5cb2afba567bae4b998f51770793 --- core/definitions.mk | 35 ----------------------------------- core/main.mk | 36 ++++++++++++++++++------------------ 2 files changed, 18 insertions(+), 53 deletions(-) diff --git a/core/definitions.mk b/core/definitions.mk index 8393e4cc3..cf6e0a468 100644 --- a/core/definitions.mk +++ b/core/definitions.mk @@ -890,41 +890,6 @@ $(shell $(call echo-error,$(LOCAL_MODULE_MAKEFILE),$(LOCAL_MODULE): $(1))) $(error done) endef -########################################################### -## Package filtering -########################################################### - -# Given a list of installed modules (short or long names) -# return a list of the packages (yes, .apk packages, not -# modules in general) that are overridden by this list and, -# therefore, should not be installed. -# $(1): mixed list of installed modules -# TODO: This is fragile; find a reliable way to get this information. -define _get-package-overrides - $(eval ### Discard any words containing slashes, unless they end in .apk, \ - ### in which case trim off the directory component and the suffix. \ - ### If there are no slashes, keep the entire word.) - $(eval _gpo_names := $(subst /,@@@ @@@,$(1))) - $(eval _gpo_names := \ - $(filter %.apk,$(_gpo_names)) \ - $(filter-out %@@@ @@@%,$(_gpo_names))) - $(eval _gpo_names := $(patsubst %.apk,%,$(_gpo_names))) - $(eval _gpo_names := $(patsubst @@@%,%,$(_gpo_names))) - - $(eval ### Remove any remaining words that contain dots.) - $(eval _gpo_names := $(subst .,@@@ @@@,$(_gpo_names))) - $(eval _gpo_names := $(filter-out %@@@ @@@%,$(_gpo_names))) - - $(eval ### Now we have a list of any words that could possibly refer to \ - ### packages, although there may be words that do not. Only \ - ### real packages will be present under PACKAGES.*, though.) - $(foreach _gpo_name,$(_gpo_names),$(PACKAGES.$(_gpo_name).OVERRIDES)) -endef - -define get-package-overrides -$(sort $(strip $(call _get-package-overrides,$(1)))) -endef - ########################################################### ## Output the command lines, or not ########################################################### diff --git a/core/main.mk b/core/main.mk index df22f6911..54d7c23d6 100644 --- a/core/main.mk +++ b/core/main.mk @@ -902,18 +902,31 @@ $(foreach lt,$(ALL_LINK_TYPES),\ # Of the modules defined by the component makefiles, # determine what we actually want to build. + +# Expand a list of modules to the modules that they override (if any) +# $(1): The list of modules. +define module-overrides +$(foreach m,$(1),$(PACKAGES.$(m).OVERRIDES) $(EXECUTABLES.$(m).OVERRIDES)) +endef + ########################################################### ## Expand a module name list with REQUIRED modules ########################################################### # $(1): The variable name that holds the initial module name list. # the variable will be modified to hold the expanded results. # $(2): The initial module name list. +# $(3): The list of overridden modules. # Returns empty string (maybe with some whitespaces). define expand-required-modules $(eval _erm_req := $(foreach m,$(2),$(ALL_MODULES.$(m).REQUIRED))) \ -$(eval _erm_new_modules := $(sort $(filter-out $($(1)),$(_erm_req))))\ -$(if $(_erm_new_modules),$(eval $(1) += $(_erm_new_modules))\ - $(call expand-required-modules,$(1),$(_erm_new_modules))) +$(eval _erm_new_modules := $(sort $(filter-out $($(1)),$(_erm_req)))) \ +$(eval _erm_new_overrides := $(call module-overrides,$(_erm_new_modules))) \ +$(eval _erm_all_overrides := $(3) $(_erm_new_overrides)) \ +$(eval _erm_new_modules := $(filter-out $(_erm_all_overrides), $(_erm_new_modules))) \ +$(eval $(1) := $(filter-out $(_erm_new_overrides),$($(1)))) \ +$(eval $(1) += $(_erm_new_modules)) \ +$(if $(_erm_new_modules),\ + $(call expand-required-modules,$(1),$(_erm_new_modules),$(_erm_all_overrides))) endef # Transforms paths relative to PRODUCT_OUT to absolute paths. @@ -953,8 +966,7 @@ define product-installed-files $(if $(BOARD_VNDK_VERSION),vndk_package) \ ) \ $(eval ### Filter out the overridden packages and executables before doing expansion) \ - $(eval _pif_overrides := $(foreach p, $(_pif_modules), $(PACKAGES.$(p).OVERRIDES))) \ - $(eval _pif_overrides += $(foreach m, $(_pif_modules), $(EXECUTABLES.$(m).OVERRIDES))) \ + $(eval _pif_overrides := $(call module-overrides,$(_pif_modules))) \ $(eval _pif_modules := $(filter-out $(_pif_overrides), $(_pif_modules))) \ $(eval ### Resolve the :32 :64 module name) \ $(eval _pif_modules_32 := $(patsubst %:32,%,$(filter %:32, $(_pif_modules)))) \ @@ -966,7 +978,7 @@ define product-installed-files $(eval ### For the rest we add both) \ $(eval _pif_modules += $(call get-32-bit-modules, $(_pif_modules_rest))) \ $(eval _pif_modules += $(_pif_modules_rest)) \ - $(call expand-required-modules,_pif_modules,$(_pif_modules)) \ + $(call expand-required-modules,_pif_modules,$(_pif_modules),$(_pif_overrides)) \ $(call module-installed-files, $(_pif_modules)) \ $(call resolve-product-relative-paths,\ $(foreach cf,$(PRODUCTS.$(_mk).PRODUCT_COPY_FILES),$(call word-colon,2,$(cf)))) @@ -1056,18 +1068,6 @@ modules_to_install := $(sort \ $(CUSTOM_MODULES) \ ) -# Some packages may override others using LOCAL_OVERRIDES_PACKAGES. -# Filter out (do not install) any overridden packages. -overridden_packages := $(call get-package-overrides,$(modules_to_install)) -ifdef overridden_packages -# old_modules_to_install := $(modules_to_install) - modules_to_install := \ - $(filter-out $(foreach p,$(overridden_packages),$(p) %/$(p).apk %/$(p).odex %/$(p).vdex), \ - $(modules_to_install)) -endif -#$(error filtered out -# $(filter-out $(modules_to_install),$(old_modules_to_install))) - # Don't include any GNU General Public License shared objects or static # libraries in SDK images. GPL executables (not static/dynamic libraries) # are okay if they don't link against any closed source libraries (directly