From 2322c4dc9bc510c0e6ef767158636f4b9b46d937 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 12 Nov 2019 14:39:17 -0800 Subject: [PATCH] Add more suggestions on converting Makefile conditionals Test: none Change-Id: I5ccf5824c6a85d881070d8e0ae16d87d3ee6cee2 --- README.md | 27 +++++--- docs/best_practices.md | 142 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 18422eaad..73aa1ed61 100644 --- a/README.md +++ b/README.md @@ -303,12 +303,19 @@ written to a [ninja](http://ninja-build.org) build file. ### How do I write conditionals? -Soong deliberately does not support conditionals in Android.bp files. -Instead, complexity in build rules that would require conditionals are handled -in Go, where high level language features can be used and implicit dependencies -introduced by conditionals can be tracked. Most conditionals are converted -to a map property, where one of the values in the map will be selected and -appended to the top level properties. +Soong deliberately does not support conditionals in Android.bp files. We +suggest removing most conditionals from the build. See +[Best Practices](docs/best_practices.md#removing-conditionals) for some +examples on how to remove conditionals. + +In cases where build time conditionals are unavoidable, complexity in build +rules that would require conditionals are handled in Go through Soong plugins. +This allows Go language features to be used for better readability and +testability, and implicit dependencies introduced by conditionals can be +tracked. Most conditionals supported natively by Soong are converted to a map +property. When building the module one of the properties in the map will be +selected, and its values appended to the property with the same name at the +top level of the module. For example, to support architecture specific files: ``` @@ -326,9 +333,9 @@ cc_library { } ``` -See [art/build/art.go](https://android.googlesource.com/platform/art/+/master/build/art.go) -or [external/llvm/soong/llvm.go](https://android.googlesource.com/platform/external/llvm/+/master/soong/llvm.go) -for examples of more complex conditionals on product variables or environment variables. +When building the module for arm the `generic.cpp` and `arm.cpp` sources will +be built. When building for x86 the `generic.cpp` and 'x86.cpp' sources will +be built. ## Developing for Soong @@ -346,7 +353,7 @@ the IDE. To run the soong_build process in a debugger, install `dlv` and then start the build with `SOONG_DELVE=` in the environment. -For examle: +For example: ```bash SOONG_DELVE=:1234 m nothing ``` diff --git a/docs/best_practices.md b/docs/best_practices.md index 546e19e23..93be3192e 100644 --- a/docs/best_practices.md +++ b/docs/best_practices.md @@ -146,3 +146,145 @@ they're used dynamically via `dlopen`. If they're only used via `LOCAL_SHARED_LIBRARIES` / `shared_libs`, then those dependencies will trigger them to be installed when necessary. Adding unnecessary libraries into `PRODUCT_PACKAGES` will force them to always be installed, wasting space. + +## Removing conditionals + +Over-use of conditionals in the build files results in an untestable number +of build combinations, leading to more build breakages. It also makes the +code less testable, as it must be built with each combination of flags to +be tested. + +### Conditionally compiled module + +Conditionally compiling a module can generally be replaced with conditional +installation: + +``` +ifeq (some condition) +# body of the Android.mk file +LOCAL_MODULE:= bt_logger +include $(BUILD_EXECUTABLE) +endif +``` + +Becomes: + +``` +cc_binary { + name: "bt_logger", + // body of the module +} +``` + +And in a product Makefile somewhere (something included with +`$(call inherit-product, ...)`: + +``` +ifeq (some condition) # Or no condition +PRODUCT_PACKAGES += bt_logger +endif +``` + +If the condition was on a type of board or product, it can often be dropped +completely by putting the `PRODUCT_PACKAGES` entry in a product makefile that +is included only by the correct products or boards. + +### Conditionally compiled module with multiple implementations + +If there are multiple implementations of the same module with one selected +for compilation via a conditional, the implementations can sometimes be renamed +to unique values. + +For example, the name of the gralloc HAL module can be overridden by the +`ro.hardware.gralloc` system property: + +``` +# In hardware/acme/soc_a/gralloc/Android.mk: +ifeq ($(TARGET_BOARD_PLATFORM),soc_a) +LOCAL_MODULE := gralloc.acme +... +include $(BUILD_SHARED_LIBRARY) +endif + +# In hardware/acme/soc_b/gralloc/Android.mk: +ifeq ($(TARGET_BOARD_PLATFORM),soc_b) +LOCAL_MODULE := gralloc.acme +... +include $(BUILD_SHARED_LIBRARY) +endif +``` + +Becomes: +``` +# In hardware/acme/soc_a/gralloc/Android.bp: +cc_library { + name: "gralloc.soc_a", + ... +} + +# In hardware/acme/soc_b/gralloc/Android.bp: +cc_library { + name: "gralloc.soc_b", + ... +} +``` + +Then to select the correct gralloc implementation, a product makefile inherited +by products that use soc_a should contain: + +``` +PRODUCT_PACKAGES += gralloc.soc_a +PRODUCT_PROPERTY_OVERRIDES += ro.hardware.gralloc=soc_a +``` + +In cases where the names cannot be made unique a `soong_namespace` should be +used to partition a set of modules so that they are built only when the +namespace is listed in `PRODUCT_SOONG_NAMESPACES`. See the +[Name resolution](../README.md#name-resolution) section of the Soong README.md +for more on namespaces. + +### Module with name based on variable + +HAL modules sometimes use variables like `$(TARGET_BOARD_PLATFORM)` in their +module name. These can be renamed to a fixed name. + +For example, the name of the gralloc HAL module can be overridden by the +`ro.hardware.gralloc` system property: + +``` +LOCAL_MODULE := gralloc.$(TARGET_BOARD_PLATFORM) +... +include $(BUILD_SHARED_LIBRARY) +``` + +Becomes: +``` +cc_library { + name: "gralloc.acme", + ... +} +``` + +Then to select the correct gralloc implementation, a product makefile should +contain: + +``` +PRODUCT_PACKAGES += gralloc.acme +PRODUCT_PROPERTY_OVERRIDES += ro.hardware.gralloc=acme +``` + +### Conditionally used source files, libraries or flags + +The preferred solution is to convert the conditional to runtime, either by +autodetecting the correct value or loading the value from a system property +or a configuration file. + +As a last resort, if the conditional cannot be removed, a Soong plugin can +be written in Go that can implement additional features for specific module +types. Soong plugins are inherently tightly coupled to the build system +and will require ongoing maintenance as the build system is changed; so +plugins should be used only when absolutely required. + +See [art/build/art.go](https://android.googlesource.com/platform/art/+/master/build/art.go) +or [external/llvm/soong/llvm.go](https://android.googlesource.com/platform/external/llvm/+/master/soong/llvm.go) +for examples of more complex conditionals on product variables or environment variables.