Skip to content

Commit

Permalink
Reland "First pass at creating PRODUCT_HOST_PACKAGES"
Browse files Browse the repository at this point in the history
Adds icu-data_host_runtime_apex to fix unbundled builds which pull it in
via PRODUCT_PACKAGES, but are missing packages that would pull it in via
PRODUCT_HOST_PACKAGES.

Test: build/soong/build_test.bash
Test: in ub-timezonedata-master; tapas TimeZoneData; m
Change-Id: I1583c7582b386c3e8478711cb1df340518d763c1
  • Loading branch information
danw committed Mar 5, 2019
1 parent 9d5ca5d commit 4a70a19
Show file tree
Hide file tree
Showing 11 changed files with 177 additions and 6 deletions.
30 changes: 30 additions & 0 deletions Changes.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,35 @@
# Build System Changes for Android.mk Writers

## `PRODUCT_HOST_PACKAGES` split from `PRODUCT_PACKAGES` {#PRODUCT_HOST_PACKAGES}

Previously, adding a module to `PRODUCT_PACKAGES` that supported both the host
and the target (`host_supported` in Android.bp; two modules with the same name
in Android.mk) would cause both to be built and installed. In many cases you
only want either the host or target versions to be built/installed by default,
and would be over-building with both. So `PRODUCT_PACKAGES` will be changing to
just affect target modules, while `PRODUCT_HOST_PACKAGES` is being added for
host modules.

Functional differences between `PRODUCT_PACKAGES` and `PRODUCT_HOST_PACKAGES`:

* `PRODUCT_HOST_PACKAGES` does not have `_ENG`/`_DEBUG` variants, as that's a
property of the target, not the host.
* `PRODUCT_HOST_PACKAGES` does not support `LOCAL_MODULE_OVERRIDES`.
* `PRODUCT_HOST_PACKAGES` requires listed modules to exist, and be host
modules. (Unless `ALLOW_MISSING_DEPENDENCIES` is set)

This is still an active migration, so currently it still uses
`PRODUCT_PACKAGES` to make installation decisions, but verifies that if we used
`PRODUCT_HOST_PACKAGES`, it would trigger installation for all of the same host
packages. This check ignores shared libraries, as those are not normally
necessary in `PRODUCT_*PACKAGES`, and tended to be over-built (especially the
32-bit variants).

Future changes will switch installation decisions to `PRODUCT_HOST_PACKAGES`
for host modules, error when there's a host-only module in `PRODUCT_PACKAGES`,
and do some further cleanup where `LOCAL_REQUIRED_MODULES` are still merged
between host and target modules with the same name.

## `*.c.arm` / `*.cpp.arm` deprecation {#file_arm}

In Android.mk files, you used to be able to change LOCAL_ARM_MODE for each
Expand Down
87 changes: 81 additions & 6 deletions core/main.mk
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,22 @@ $(strip $(1))
endef
endif # TARGET_TRANSLATE_2ND_ARCH

# TODO: we can probably check to see if these modules are actually host
# modules
define get-host-32-bit-modules
$(sort $(foreach m,$(1),\
$(if $(ALL_MODULES.$(m)$(HOST_2ND_ARCH_MODULE_SUFFIX).CLASS),\
$(m)$(HOST_2ND_ARCH_MODULE_SUFFIX))))
endef
# Get a list of corresponding 32-bit module names, if one exists;
# otherwise return the original module name
define get-host-32-bit-modules-if-we-can
$(sort $(foreach m,$(1),\
$(if $(ALL_MODULES.$(m)$(HOST_2ND_ARCH_MODULE_SUFFIX).CLASS),\
$(m)$(HOST_2ND_ARCH_MODULE_SUFFIX),\
$(m))))
endef

# If a module is for a cross host os, the required modules must be for
# that OS too.
# If a module is built for 32-bit, the required modules must be 32-bit too;
Expand Down Expand Up @@ -1015,6 +1031,16 @@ $(if $(_erm_new_modules),\
$(call expand-required-modules,$(1),$(_erm_new_modules),$(_erm_all_overrides)))
endef

# Same as expand-required-modules above, but does not handle module overrides, as
# we don't intend to support them on the host.
define expand-required-host-modules
$(eval _erm_req := $(foreach m,$(2),$(ALL_MODULES.$(m).REQUIRED))) \
$(eval _erm_new_modules := $(sort $(filter-out $($(1)),$(_erm_req)))) \
$(eval $(1) += $(_erm_new_modules)) \
$(if $(_erm_new_modules),\
$(call expand-required-host-modules,$(1),$(_erm_new_modules)))
endef

# Transforms paths relative to PRODUCT_OUT to absolute paths.
# $(1): list of relative paths
# $(2): optional suffix to append to paths
Expand Down Expand Up @@ -1080,6 +1106,23 @@ define product-installed-files
$(foreach cf,$(PRODUCTS.$(_mk).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 := $(PRODUCTS.$(strip $(1)).PRODUCT_HOST_PACKAGES)) \
$(eval ### Resolve the :32 :64 module name) \
$(eval _hif_modules_32 := $(patsubst %:32,%,$(filter %:32, $(_hif_modules)))) \
$(eval _hif_modules_64 := $(patsubst %:64,%,$(filter %:64, $(_hif_modules)))) \
$(eval _hif_modules_rest := $(filter-out %:32 %:64,$(_hif_modules))) \
$(eval _hif_modules := $(call get-host-32-bit-modules-if-we-can, $(_hif_modules_32))) \
$(eval _hif_modules += $(_hif_modules_64)) \
$(eval ### For the rest we add both) \
$(eval _hif_modules += $(call get-host-32-bit-modules, $(_hif_modules_rest))) \
$(eval _hif_modules += $(_hif_modules_rest)) \
$(call expand-required-host-modules,_hif_modules,$(_hif_modules)) \
$(filter $(HOST_OUT_ROOT)/%,$(call module-installed-files, $(_hif_modules)))
endef

# Fails the build if the given list is non-empty, and prints it entries (stripping PRODUCT_OUT).
# $(1): list of files to print
# $(2): heading to print on failure
Expand All @@ -1106,11 +1149,43 @@ ifeq (true|,$(PRODUCTS.$(INTERNAL_PRODUCT).PRODUCT_ENFORCE_PACKAGES_EXIST)|$(fil
$(INTERNAL_PRODUCT) includes redundant whitelist entries for nonexistant PRODUCT_PACKAGES)
endif

# Check to ensure that all modules in PRODUCT_HOST_PACKAGES exist
#
# Many host modules are Linux-only, so skip this check on Mac. If we ever have Mac-only modules,
# maybe it would make sense to have PRODUCT_HOST_PACKAGES_LINUX/_DARWIN?
ifneq ($(HOST_OS),darwin)
ifneq (true,$(ALLOW_MISSING_DEPENDENCIES))
_modules := $(PRODUCTS.$(INTERNAL_PRODUCT).PRODUCT_HOST_PACKAGES)
_nonexistant_modules := $(foreach m,$(_modules),\
$(if $(filter FAKE,$(ALL_MODULES.$(m).CLASS))$(filter $(HOST_OUT_ROOT)/%,$(ALL_MODULES.$(m).INSTALLED)),,$(m)))
$(call maybe-print-list-and-error,$(_nonexistant_modules),\
$(INTERNAL_PRODUCT) includes non-existant modules in PRODUCT_HOST_PACKAGES)
endif
endif

ifdef FULL_BUILD
product_FILES := $(call product-installed-files, $(INTERNAL_PRODUCT))
product_host_FILES := $(call host-installed-files,$(INTERNAL_PRODUCT))
product_target_FILES := $(call product-installed-files, $(INTERNAL_PRODUCT))
# WARNING: The product_MODULES variable is depended on by external files.
product_MODULES := $(_pif_modules)

# Verify that PRODUCT_HOST_PACKAGES is complete
# This is a temporary requirement during migration
# Ignore libraries, since they shouldn't need to be in PRODUCT_PACKAGES for the most part anyway.
host_files_in_target_FILES := $(filter-out \
$(HOST_OUT_SHARED_LIBRARIES)/% \
$($(HOST_2ND_ARCH_VAR_PREFIX)HOST_OUT_SHARED_LIBRARIES)/%,\
$(filter $(HOST_OUT_ROOT)/%,$(product_target_FILES)))
ifneq (,$(filter-out $(product_host_FILES),$(host_files_in_target_FILES)))
packages := $(foreach f,$(filter-out $(product_host_FILES),$(host_files_in_target_FILES)), \
$(or $(INSTALLABLE_FILES.$(f).MODULE),$(f)))
$(warning Missing modules from PRODUCT_HOST_PACKAGES)
$(warning See $(CHANGES_URL)#PRODUCT_HOST_PACKAGES for more information)
$(foreach f,$(sort $(packages)),$(warning _ $(f)))
$(error stop)
endif
host_files_in_target_FILES :=

# Verify the artifact path requirements made by included products.
is_asan := $(if $(filter address,$(SANITIZE_TARGET)),true)
ifneq (true,$(or $(is_asan),$(DISABLE_ARTIFACT_PATH_REQUIREMENTS)))
Expand Down Expand Up @@ -1153,7 +1228,7 @@ $(call dist-for-goals,droidcore,$(CERTIFICATE_VIOLATION_MODULES_FILENAME))
$(eval unused_whitelist := $(filter-out $(files),$(whitelist_patterns))) \
$(call maybe-print-list-and-error,$(unused_whitelist),$(makefile) includes redundant whitelist entries in its artifact path requirement.) \
$(eval ### Optionally verify that nothing else produces files inside this artifact path requirement.) \
$(eval extra_files := $(filter-out $(files) $(HOST_OUT)/%,$(product_FILES))) \
$(eval extra_files := $(filter-out $(files) $(HOST_OUT)/%,$(product_target_FILES))) \
$(eval files_in_requirement := $(filter $(path_patterns),$(extra_files))) \
$(eval all_offending_files += $(files_in_requirement)) \
$(eval whitelist := $(PRODUCTS.$(INTERNAL_PRODUCT).PRODUCT_ARTIFACT_PATH_REQUIREMENT_WHITELIST)) \
Expand All @@ -1178,14 +1253,14 @@ else
# a subset of the module makefiles. Don't try to build any modules
# requested by the product, because we probably won't have rules
# to build them.
product_FILES :=
product_target_FILES :=
endif

# TODO: Remove the 3 places in the tree that use ALL_DEFAULT_INSTALLED_MODULES
# and get rid of it from this list.
modules_to_install := $(sort \
$(ALL_DEFAULT_INSTALLED_MODULES) \
$(product_FILES) \
$(product_target_FILES) \
$(call get-tagged-modules,$(tags_to_install)) \
$(CUSTOM_MODULES) \
)
Expand Down Expand Up @@ -1591,8 +1666,8 @@ modules:

.PHONY: dump-files
dump-files:
$(info product_FILES for $(TARGET_DEVICE) ($(INTERNAL_PRODUCT)):)
$(foreach p,$(sort $(product_FILES)),$(info : $(p)))
$(info product_target_FILES for $(TARGET_DEVICE) ($(INTERNAL_PRODUCT)):)
$(foreach p,$(sort $(product_target_FILES)),$(info : $(p)))
@echo Successfully dumped product file list

.PHONY: nothing
Expand Down
1 change: 1 addition & 0 deletions core/product.mk
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ _product_var_list := \
PRODUCT_AAPT_CONFIG \
PRODUCT_AAPT_PREF_CONFIG \
PRODUCT_AAPT_PREBUILT_DPI \
PRODUCT_HOST_PACKAGES \
PRODUCT_PACKAGES \
PRODUCT_PACKAGES_DEBUG \
PRODUCT_PACKAGES_DEBUG_ASAN \
Expand Down
4 changes: 4 additions & 0 deletions target/board/generic_x86/device.mk
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,7 @@ endif
PRODUCT_PACKAGES += \
bios.bin \
vgabios-cirrus.bin \

PRODUCT_HOST_PACKAGES += \
bios.bin \
vgabios-cirrus.bin \
4 changes: 4 additions & 0 deletions target/board/generic_x86_64/device.mk
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,7 @@ endif
PRODUCT_PACKAGES += \
bios.bin \
vgabios-cirrus.bin \

PRODUCT_HOST_PACKAGES += \
bios.bin \
vgabios-cirrus.bin \
37 changes: 37 additions & 0 deletions target/product/base_system.mk
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,43 @@ PRODUCT_PACKAGES += \
framework_manifest.xml \
system_compatibility_matrix.xml \

# Host tools to install
PRODUCT_HOST_PACKAGES += \
BugReport \
adb \
adbd \
atest \
bcc \
bit \
e2fsck \
fastboot \
flags_health_check \
icu-data_host_runtime_apex \
idmap2 \
incident_report \
ld.mc \
lpdump \
mdnsd \
minigzip \
mke2fs \
resize2fs \
selinux_policy_system \
sgdisk \
shell_and_utilities_system \
sqlite3 \
tinyplay \
tune2fs \
tzdatacheck \
unwind_info \
unwind_reg_info \
unwind_symbols \
viewcompiler \
tzdata_host \
tzdata_host_runtime_apex \
tzlookup.xml_host_runtime_apex \
tz_version_host \
tz_version_host_runtime_apex \

ifeq ($(TARGET_CORE_JARS),)
$(error TARGET_CORE_JARS is empty; cannot initialize PRODUCT_BOOT_JARS variable)
endif
Expand Down
10 changes: 10 additions & 0 deletions target/product/base_vendor.mk
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ PRODUCT_PACKAGES += \
shell_and_utilities_recovery \
watchdogd.recovery \

# These had been pulled in via init_second_stage.recovery, but may not be needed.
PRODUCT_HOST_PACKAGES += \
e2fsdroid \
mke2fs \
sload_f2fs \
make_f2fs \

PRODUCT_HOST_PACKAGES += \
icu-data_host_runtime_apex

# Base modules and settings for the vendor partition.
PRODUCT_PACKAGES += \
[email protected] \
Expand Down
4 changes: 4 additions & 0 deletions target/product/full_x86.mk
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ PRODUCT_PACKAGES += \
bios.bin \
vgabios-cirrus.bin \

PRODUCT_HOST_PACKAGES += \
bios.bin \
vgabios-cirrus.bin \

# Enable dynamic partition size
PRODUCT_USE_DYNAMIC_PARTITION_SIZE := true

Expand Down
3 changes: 3 additions & 0 deletions target/product/mainline_system.mk
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ PRODUCT_PACKAGES_DEBUG += \
tinypcminfo \
update_engine_client \

PRODUCT_HOST_PACKAGES += \
tinyplay

# Enable stats logging in LMKD
TARGET_LMKD_STATS_LOG := true
PRODUCT_SYSTEM_DEFAULT_PROPERTIES += \
Expand Down
2 changes: 2 additions & 0 deletions target/product/media_system.mk
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ PRODUCT_PACKAGES += \
StatementService \
vndk_snapshot_package \

PRODUCT_HOST_PACKAGES += \
fsck.f2fs \

PRODUCT_COPY_FILES += \
frameworks/native/data/etc/android.software.webview.xml:system/etc/permissions/android.software.webview.xml
Expand Down
1 change: 1 addition & 0 deletions target/product/runtime_libart.mk
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ PRODUCT_PACKAGES += \

# Android Runtime APEX module.
PRODUCT_PACKAGES += com.android.runtime
PRODUCT_HOST_PACKAGES += com.android.runtime

# Certificates.
PRODUCT_PACKAGES += \
Expand Down

0 comments on commit 4a70a19

Please sign in to comment.