From 81f1e92ed8a7216025d7940646ac7a36a7d4ac76 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Fri, 8 Apr 2022 11:12:10 -0700 Subject: [PATCH] Remove usages of long-form variables The long-form variables (PRODUCTS..) are used to get information about multiple products. However, they've never really worked correctly, and so importing multiple products is deprecated behavior. Remove as many usages of the long-form variables and multi-product imports as possible. Bug: 228518445 Test: Manually Change-Id: I0b67f16360ff8bdcdb39638de739440472bccf76 --- core/config.mk | 3 +- core/main.mk | 20 ++++------ core/ninja_config.mk | 1 - core/product-graph.mk | 53 +++++++++---------------- core/product.mk | 75 +++++------------------------------ core/product_config.mk | 62 +++++++++-------------------- tools/filter-product-graph.py | 68 ------------------------------- 7 files changed, 56 insertions(+), 226 deletions(-) delete mode 100755 tools/filter-product-graph.py diff --git a/core/config.mk b/core/config.mk index bd3d1a083b..7af600f3d5 100644 --- a/core/config.mk +++ b/core/config.mk @@ -1235,8 +1235,7 @@ endef # These goals don't need to collect and include Android.mks/CleanSpec.mks # in the source tree. -dont_bother_goals := out \ - product-graph dump-products +dont_bother_goals := out product-graph # Make ANDROID Soong config variables visible to Android.mk files, for # consistency with those defined in BoardConfig.mk files. diff --git a/core/main.mk b/core/main.mk index 7a12bf3107..e0efdad61e 100644 --- a/core/main.mk +++ b/core/main.mk @@ -1235,18 +1235,14 @@ endef # See the select-bitness-of-required-modules definition. # $(1): product makefile -define _product-var - $(call get-product-var,$(1),$(2)) -endef - define product-installed-files $(eval _pif_modules := \ - $(call _product-var,$(1),PRODUCT_PACKAGES) \ - $(if $(filter eng,$(tags_to_install)),$(call _product-var,$(1),PRODUCT_PACKAGES_ENG)) \ - $(if $(filter debug,$(tags_to_install)),$(call _product-var,$(1),PRODUCT_PACKAGES_DEBUG)) \ - $(if $(filter tests,$(tags_to_install)),$(call _product-var,$(1),PRODUCT_PACKAGES_TESTS)) \ - $(if $(filter asan,$(tags_to_install)),$(call _product-var,$(1),PRODUCT_PACKAGES_DEBUG_ASAN)) \ - $(if $(filter java_coverage,$(tags_to_install)),$(call _product-var,$(1),PRODUCT_PACKAGES_DEBUG_JAVA_COVERAGE)) \ + $(call get-product-var,$(1),PRODUCT_PACKAGES) \ + $(if $(filter eng,$(tags_to_install)),$(call get-product-var,$(1),PRODUCT_PACKAGES_ENG)) \ + $(if $(filter debug,$(tags_to_install)),$(call get-product-var,$(1),PRODUCT_PACKAGES_DEBUG)) \ + $(if $(filter tests,$(tags_to_install)),$(call get-product-var,$(1),PRODUCT_PACKAGES_TESTS)) \ + $(if $(filter asan,$(tags_to_install)),$(call get-product-var,$(1),PRODUCT_PACKAGES_DEBUG_ASAN)) \ + $(if $(filter java_coverage,$(tags_to_install)),$(call get-product-var,$(1),PRODUCT_PACKAGES_DEBUG_JAVA_COVERAGE)) \ $(call auto-included-modules) \ ) \ $(eval ### Filter out the overridden packages and executables before doing expansion) \ @@ -1257,13 +1253,13 @@ define product-installed-files $(call expand-required-modules,_pif_modules,$(_pif_modules),$(_pif_overrides)) \ $(filter-out $(HOST_OUT_ROOT)/%,$(call module-installed-files, $(_pif_modules))) \ $(call resolve-product-relative-paths,\ - $(foreach cf,$(call _product-var,$(1),PRODUCT_COPY_FILES),$(call word-colon,2,$(cf)))) + $(foreach cf,$(call get-product-var,$(1),PRODUCT_COPY_FILES),$(call word-colon,2,$(cf)))) endef # Similar to product-installed-files above, but handles PRODUCT_HOST_PACKAGES instead # This does support the :32 / :64 syntax, but does not support module overrides. define host-installed-files - $(eval _hif_modules := $(call _product-var,$(1),PRODUCT_HOST_PACKAGES)) \ + $(eval _hif_modules := $(call get-product-var,$(1),PRODUCT_HOST_PACKAGES)) \ $(eval ### Split host vs host cross modules) \ $(eval _hcif_modules := $(filter host_cross_%,$(_hif_modules))) \ $(eval _hif_modules := $(filter-out host_cross_%,$(_hif_modules))) \ diff --git a/core/ninja_config.mk b/core/ninja_config.mk index 2157c9ea4a..e436b2cc69 100644 --- a/core/ninja_config.mk +++ b/core/ninja_config.mk @@ -25,7 +25,6 @@ PARSE_TIME_MAKE_GOALS := \ cts \ custom_images \ dicttool_aosp \ - dump-products \ eng \ oem_image \ online-system-api-sdk-docs \ diff --git a/core/product-graph.mk b/core/product-graph.mk index 63e904086a..379110e902 100644 --- a/core/product-graph.mk +++ b/core/product-graph.mk @@ -15,12 +15,12 @@ # # the sort also acts as a strip to remove the single space entries that creep in because of the evals -define gather-all-products +define gather-all-makefiles-for-current-product $(eval _all_products_visited := )\ -$(sort $(call all-products-inner, $(PRODUCTS))) +$(sort $(call gather-all-makefiles-for-current-product-inner,$(INTERNAL_PRODUCT))) endef -define all-products-inner +define gather-all-makefiles-for-current-product-inner $(foreach p,$(1),\ $(if $(filter $(p),$(_all_products_visited)),, \ $(p) \ @@ -30,30 +30,12 @@ define all-products-inner ) endef -this_makefile := build/make/core/product-graph.mk - -products_graph := $(OUT_DIR)/products.dot -ifeq ($(strip $(ANDROID_PRODUCT_GRAPH)),) -products_list := $(INTERNAL_PRODUCT) -else -ifeq ($(strip $(ANDROID_PRODUCT_GRAPH)),--all) -products_list := --all -else -products_list := $(foreach prod,$(ANDROID_PRODUCT_GRAPH),$(call resolve-short-product-name,$(prod))) -endif -endif - -all_products := $(call gather-all-products) - -open_parethesis := ( -close_parenthesis := ) - node_color_target := orange node_color_common := beige node_color_vendor := lavenderblush node_color_default := white define node-color -$(if $(filter $(1),$(PRIVATE_PRODUCTS_FILTER)),\ +$(if $(filter $(1),$(PRIVATE_TOP_LEVEL_MAKEFILE)),\ $(node_color_target),\ $(if $(filter build/make/target/product/%,$(1)),\ $(node_color_common),\ @@ -62,30 +44,33 @@ $(if $(filter $(1),$(PRIVATE_PRODUCTS_FILTER)),\ ) endef +open_parethesis := ( +close_parenthesis := ) + # Emit properties of a product node to a file. # $(1) the product # $(2) the output file define emit-product-node-props $(hide) echo \"$(1)\" [ \ -label=\"$(dir $(1))\\n$(notdir $(1))\\n\\n$(subst $(close_parenthesis),,$(subst $(open_parethesis),,$(call get-product-var,$(1),PRODUCT_MODEL)))\\n$(call get-product-var,$(1),PRODUCT_DEVICE)\" \ +label=\"$(dir $(1))\\n$(notdir $(1))$(if $(filter $(1),$(PRIVATE_TOP_LEVEL_MAKEFILE)),$(subst $(open_parethesis),,$(subst $(close_parenthesis),,\\n\\n$(PRODUCT_MODEL)\\n$(PRODUCT_DEVICE))))\" \ style=\"filled\" fillcolor=\"$(strip $(call node-color,$(1)))\" \ colorscheme=\"svg\" fontcolor=\"darkblue\" \ ] >> $(2) endef -$(products_graph): PRIVATE_PRODUCTS := $(all_products) -$(products_graph): PRIVATE_PRODUCTS_FILTER := $(products_list) +products_graph := $(OUT_DIR)/products.dot -$(products_graph): $(this_makefile) - @echo Product graph DOT: $@ for $(PRIVATE_PRODUCTS_FILTER) - $(hide) echo 'digraph {' > $@.in - $(hide) echo 'graph [ ratio=.5 ];' >> $@.in - $(hide) $(foreach p,$(PRIVATE_PRODUCTS), \ - $(foreach d,$(PRODUCTS.$(strip $(p)).INHERITS_FROM), echo \"$(d)\" -\> \"$(p)\" >> $@.in;)) - $(foreach p,$(PRIVATE_PRODUCTS),$(call emit-product-node-props,$(p),$@.in)) - $(hide) echo '}' >> $@.in - $(hide) build/make/tools/filter-product-graph.py $(PRIVATE_PRODUCTS_FILTER) < $@.in > $@ +$(products_graph): PRIVATE_ALL_MAKEFILES_FOR_THIS_PRODUCT := $(call gather-all-makefiles-for-current-product) +$(products_graph): PRIVATE_TOP_LEVEL_MAKEFILE := $(INTERNAL_PRODUCT) +$(products_graph): + @echo Product graph DOT: $@ for $(PRIVATE_TOP_LEVEL_MAKEFILE) + $(hide) echo 'digraph {' > $@ + $(hide) echo 'graph [ ratio=.5 ];' >> $@ + $(hide) $(foreach p,$(PRIVATE_ALL_MAKEFILES_FOR_THIS_PRODUCT), \ + $(foreach d,$(PRODUCTS.$(strip $(p)).INHERITS_FROM), echo \"$(d)\" -\> \"$(p)\" >> $@;)) + $(foreach p,$(PRIVATE_ALL_MAKEFILES_FOR_THIS_PRODUCT),$(call emit-product-node-props,$(p),$@)) + $(hide) echo '}' >> $@ .PHONY: product-graph product-graph: $(products_graph) diff --git a/core/product.mk b/core/product.mk index f31611443c..53fee1c54f 100644 --- a/core/product.mk +++ b/core/product.mk @@ -377,17 +377,6 @@ _product_single_value_vars += PRODUCT_MODULE_BUILD_FROM_SOURCE .KATI_READONLY := _product_single_value_vars _product_list_vars _product_var_list :=$= $(_product_single_value_vars) $(_product_list_vars) -define dump-product -$(warning ==== $(1) ====)\ -$(foreach v,$(_product_var_list),\ -$(warning PRODUCTS.$(1).$(v) := $(call get-product-var,$(1),$(v))))\ -$(warning --------) -endef - -define dump-products -$(foreach p,$(PRODUCTS),$(call dump-product,$(p))) -endef - # # Functions for including product makefiles # @@ -464,64 +453,18 @@ endef # -# Does various consistency checks on all of the known products. +# Does various consistency checks on the current product. # Takes no parameters, so $(call ) is not necessary. # -define check-all-products +define check-current-product $(if ,, \ - $(eval _cap_names :=) \ - $(foreach p,$(PRODUCTS), \ - $(eval pn := $(strip $(PRODUCTS.$(p).PRODUCT_NAME))) \ - $(if $(pn),,$(error $(p): PRODUCT_NAME must be defined.)) \ - $(if $(filter $(pn),$(_cap_names)), \ - $(error $(p): PRODUCT_NAME must be unique; "$(pn)" already used by $(strip \ - $(foreach \ - pp,$(PRODUCTS), - $(if $(filter $(pn),$(PRODUCTS.$(pp).PRODUCT_NAME)), \ - $(pp) \ - ))) \ - ) \ - ) \ - $(eval _cap_names += $(pn)) \ - $(if $(call is-c-identifier,$(pn)),, \ - $(error $(p): PRODUCT_NAME must be a valid C identifier, not "$(pn)") \ - ) \ - $(eval pb := $(strip $(PRODUCTS.$(p).PRODUCT_BRAND))) \ - $(if $(pb),,$(error $(p): PRODUCT_BRAND must be defined.)) \ - $(foreach cf,$(strip $(PRODUCTS.$(p).PRODUCT_COPY_FILES)), \ - $(if $(filter 2 3,$(words $(subst :,$(space),$(cf)))),, \ - $(error $(p): malformed COPY_FILE "$(cf)") \ - ) \ - ) \ - ) \ -) -endef - - -# -# Returns the product makefile path for the product with the provided name -# -# $(1): short product name like "generic" -# -define _resolve-short-product-name - $(eval pn := $(strip $(1))) - $(eval p := \ - $(foreach p,$(PRODUCTS), \ - $(if $(filter $(pn),$(PRODUCTS.$(p).PRODUCT_NAME)), \ - $(p) \ - )) \ - ) - $(eval p := $(sort $(p))) - $(if $(filter 1,$(words $(p))), \ - $(p), \ - $(if $(filter 0,$(words $(p))), \ - $(error No matches for product "$(pn)"), \ - $(error Product "$(pn)" ambiguous: matches $(p)) \ - ) \ - ) -endef -define resolve-short-product-name -$(strip $(call _resolve-short-product-name,$(1))) + $(if $(call is-c-identifier,$(PRODUCT_NAME)),, \ + $(error $(INTERNAL_PRODUCT): PRODUCT_NAME must be a valid C identifier, not "$(pn)")) \ + $(if $(PRODUCT_BRAND),, \ + $(error $(INTERNAL_PRODUCT): PRODUCT_BRAND must be defined.)) \ + $(foreach cf,$(strip $(PRODUCT_COPY_FILES)), \ + $(if $(filter 2 3,$(words $(subst :,$(space),$(cf)))),, \ + $(error $(p): malformed COPY_FILE "$(cf)")))) endef # BoardConfig variables that are also inherited in product mks. Should ideally diff --git a/core/product_config.mk b/core/product_config.mk index 939a02206a..1e74fa9562 100644 --- a/core/product_config.mk +++ b/core/product_config.mk @@ -208,35 +208,26 @@ $(foreach f,$(android_products_makefiles), \ ) # Dedup, extract product names, etc. -product_paths :=$(sort $(product_paths)) -all_named_products := $(call _first,$(product_paths),:) -all_product_makefiles := $(call _second,$(product_paths),:) +product_paths := $(sort $(product_paths)) +all_named_products := $(sort $(call _first,$(product_paths),:)) +all_product_makefiles := $(sort $(call _second,$(product_paths),:)) current_product_makefile := $(call _second,$(filter $(TARGET_PRODUCT):%,$(product_paths)),:) COMMON_LUNCH_CHOICES := $(sort $(common_lunch_choices)) -load_all_product_makefiles := -ifneq (,$(filter product-graph, $(MAKECMDGOALS))) -ifeq ($(ANDROID_PRODUCT_GRAPH),--all) -load_all_product_makefiles := true -endif -endif -ifneq (,$(filter dump-products,$(MAKECMDGOALS))) -ifeq ($(ANDROID_DUMP_PRODUCTS),all) -load_all_product_makefiles := true -endif -endif +# Check that there are no duplicate product names +$(foreach p,$(all_named_products), \ + $(if $(filter 1,$(words $(filter $(p):%,$(product_paths)))),, \ + $(error Product name must be unique, "$(p)" used by $(call _second,$(filter $(p):%,$(product_paths)),:)))) ifneq ($(ALLOW_RULES_IN_PRODUCT_CONFIG),) _product_config_saved_KATI_ALLOW_RULES := $(.KATI_ALLOW_RULES) .KATI_ALLOW_RULES := $(ALLOW_RULES_IN_PRODUCT_CONFIG) endif -ifeq ($(load_all_product_makefiles),true) -# Import all product makefiles. -$(call import-products, $(all_product_makefiles)) -else -# Import just the current product. -$(if $(current_product_makefile),,$(error Can not locate config makefile for product "$(TARGET_PRODUCT)")) +ifeq (,$(current_product_makefile)) + $(error Can not locate config makefile for product "$(TARGET_PRODUCT)") +endif + ifneq (,$(filter $(TARGET_PRODUCT),$(products_using_starlark_config))) RBC_PRODUCT_CONFIG := true RBC_BOARD_CONFIG := true @@ -258,44 +249,29 @@ else endif include $(OUT_DIR)/rbc/rbc_product_config_results.mk endif -endif # Import all or just the current product makefile - -# Quick check -$(check-all-products) # This step was already handled in the RBC product configuration. -# Since the equivalent starlark code will not add the partial products to -# the PRODUCTS variable, it's ok for them to be set before check-all-products ifeq ($(RBC_PRODUCT_CONFIG)$(SKIP_ARTIFACT_PATH_REQUIREMENT_PRODUCTS_CHECK),) # Import all the products that have made artifact path requirements, so that we can verify -# the artifacts they produce. -# These are imported after check-all-products because some of them might not be real products. +# the artifacts they produce. They might be intermediate makefiles instead of real products. $(foreach makefile,$(ARTIFACT_PATH_REQUIREMENT_PRODUCTS),\ $(if $(filter-out $(makefile),$(PRODUCTS)),$(eval $(call import-products,$(makefile))))\ ) endif +INTERNAL_PRODUCT := $(current_product_makefile) +# Strip and assign the PRODUCT_ variables. +$(call strip-product-vars) + +# Quick check +$(check-current-product) + ifneq ($(ALLOW_RULES_IN_PRODUCT_CONFIG),) .KATI_ALLOW_RULES := $(_saved_KATI_ALLOW_RULES) _product_config_saved_KATI_ALLOW_RULES := endif -ifneq ($(filter dump-products, $(MAKECMDGOALS)),) -$(dump-products) -endif - -# Convert a short name like "sooner" into the path to the product -# file defining that product. -# -INTERNAL_PRODUCT := $(call resolve-short-product-name, $(TARGET_PRODUCT)) -ifneq ($(current_product_makefile),$(INTERNAL_PRODUCT)) -$(error PRODUCT_NAME inconsistent in $(current_product_makefile) and $(INTERNAL_PRODUCT)) -endif - - ############################################################################ -# Strip and assign the PRODUCT_ variables. -$(call strip-product-vars) current_product_makefile := all_product_makefiles := diff --git a/tools/filter-product-graph.py b/tools/filter-product-graph.py deleted file mode 100755 index b3a5b42c39..0000000000 --- a/tools/filter-product-graph.py +++ /dev/null @@ -1,68 +0,0 @@ -#!/usr/bin/env python -# vim: ts=2 sw=2 nocindent - -import re -import sys - -def choose_regex(regs, line): - for func,reg in regs: - m = reg.match(line) - if m: - return (func,m) - return (None,None) - -def gather(included, deps): - result = set() - for inc in included: - result.add(inc) - for d in deps: - if inc == d[1]: - result.add(d[0]) - return result - -def main(): - deps = [] - infos = [] - def dependency(m): - deps.append((m.group(1), m.group(2))) - def info(m): - infos.append((m.group(1), m.group(2))) - - REGS = [ - (dependency, re.compile(r'"(.*)"\s*->\s*"(.*)"')), - (info, re.compile(r'"(.*)"(\s*\[.*\])')), - ] - - lines = sys.stdin.readlines() - lines = [line.strip() for line in lines] - - for line in lines: - func,m = choose_regex(REGS, line) - if func: - func(m) - - # filter - sys.stderr.write("argv: " + str(sys.argv) + "\n") - if not (len(sys.argv) == 2 and sys.argv[1] == "--all"): - targets = sys.argv[1:] - - included = set(targets) - prevLen = -1 - while prevLen != len(included): - prevLen = len(included) - included = gather(included, deps) - - deps = [dep for dep in deps if dep[1] in included] - infos = [info for info in infos if info[0] in included] - - print "digraph {" - print "graph [ ratio=.5 ];" - for dep in deps: - print '"%s" -> "%s"' % dep - for info in infos: - print '"%s"%s' % info - print "}" - - -if __name__ == "__main__": - main()