Add more suggestions on converting Makefile conditionals

Test: none
Change-Id: I5ccf5824c6a85d881070d8e0ae16d87d3ee6cee2

2 files changed, 159 insertions, 10 deletions

diff --git a/README.md b/README.md
index 18422eaa..73aa1ed6 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=<listen addr>` 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 546e19e2..93be3192 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.