Dan Willemsen | 7733862 | 2017-11-08 16:39:18 -0800 | [diff] [blame] | 1 | # Build System Changes for Android.mk Writers |
| 2 | |
Ulya Trafimovich | 4b4fd16 | 2021-03-31 11:49:53 +0100 | [diff] [blame] | 3 | ## Dexpreopt starts enforcing `<uses-library>` checks (for Java modules) |
| 4 | |
| 5 | In order to construct correct class loader context for dexpreopt, build system |
| 6 | needs to know about the shared library dependencies of Java modules listed in |
| 7 | the `<uses-library>` tags in the manifest. Since the build system does not have |
| 8 | access to the manifest contents, that information must be present in the build |
| 9 | files. In simple cases Soong is able to infer it from its knowledge of Java SDK |
| 10 | libraries and the `libs` property in Android.bp, but in more complex cases it is |
| 11 | necessary to add the missing information in Android.bp/Android.mk manually. |
| 12 | |
| 13 | To specify a list of libraries for a given modules, use: |
| 14 | |
| 15 | * Android.bp properties: `uses_libs`, `optional_uses_libs` |
| 16 | * Android.mk variables: `LOCAL_USES_LIBRARIES`, `LOCAL_OPTIONAL_USES_LIBRARIES` |
| 17 | |
| 18 | If a library is in `libs`, it usually should *not* be added to the above |
| 19 | properties, and Soong should be able to infer the `<uses-library>` tag. But |
| 20 | sometimes a library also needs additional information in its |
| 21 | Android.bp/Android.mk file (e.g. when it is a `java_library` rather than a |
| 22 | `java_sdk_library`, or when the library name is different from its module name, |
| 23 | or when the module is defined in Android.mk rather than Android.bp). In such |
| 24 | cases it is possible to tell the build system that the library provides a |
| 25 | `<uses-library>` with a given name (however, this is discouraged and will be |
| 26 | deprecated in the future, and it is recommended to fix the underlying problem): |
| 27 | |
| 28 | * Android.bp property: `provides_uses_lib` |
| 29 | * Android.mk variable: `LOCAL_PROVIDES_USES_LIBRARY` |
| 30 | |
| 31 | It is possible to disable the check on a per-module basis. When doing that it is |
| 32 | also recommended to disable dexpreopt, as disabling a failed check will result |
| 33 | in incorrect class loader context recorded in the .odex file, which will cause |
| 34 | class loader context mismatch and dexopt at first boot. |
| 35 | |
| 36 | * Android.bp property: `enforce_uses_lib` |
| 37 | * Android.mk variable: `LOCAL_ENFORCE_USES_LIBRARIES` |
| 38 | |
| 39 | Finally, it is possible to globally disable the check: |
| 40 | |
| 41 | * For a given product: `PRODUCT_BROKEN_VERIFY_USES_LIBRARIES := true` |
| 42 | * On the command line: `RELAX_USES_LIBRARY_CHECK=true` |
| 43 | |
| 44 | The environment variable overrides the product variable, so it is possible to |
| 45 | disable the check for a product, but quickly re-enable it for a local build. |
| 46 | |
Yo Chiang | 64faf88 | 2020-07-15 18:35:12 +0800 | [diff] [blame] | 47 | ## `LOCAL_REQUIRED_MODULES` requires listed modules to exist {#BUILD_BROKEN_MISSING_REQUIRED_MODULES} |
| 48 | |
| 49 | Modules listed in `LOCAL_REQUIRED_MODULES`, `LOCAL_HOST_REQUIRED_MODULES` and |
| 50 | `LOCAL_TARGET_REQUIRED_MODULES` need to exist unless `ALLOW_MISSING_DEPENDENCIES` |
| 51 | is set. |
| 52 | |
| 53 | To temporarily relax missing required modules check, use: |
| 54 | |
| 55 | `BUILD_BROKEN_MISSING_REQUIRED_MODULES := true` |
| 56 | |
Jiyong Park | 0b4fccb | 2020-06-26 17:38:00 +0900 | [diff] [blame] | 57 | ## Changes in system properties settings |
| 58 | |
| 59 | ### Product variables |
| 60 | |
| 61 | System properties for each of the partition is supposed to be set via following |
| 62 | product config variables. |
| 63 | |
Donghyun Jo | 8c65ef6 | 2021-03-02 08:51:03 +0900 | [diff] [blame] | 64 | For system partition, |
Jiyong Park | 0b4fccb | 2020-06-26 17:38:00 +0900 | [diff] [blame] | 65 | |
Donghyun Jo | 8c65ef6 | 2021-03-02 08:51:03 +0900 | [diff] [blame] | 66 | * `PRODUCT_SYSTEM_PROPERTIES` |
Jiyong Park | 0b4fccb | 2020-06-26 17:38:00 +0900 | [diff] [blame] | 67 | * `PRODUCT_SYSTEM_DEFAULT_PROPERTIES` is highly discouraged. Will be deprecated. |
| 68 | |
| 69 | For vendor partition, |
| 70 | |
| 71 | * `PRODUCT_VENDOR_PROPERTIES` |
| 72 | * `PRODUCT_PROPERTY_OVERRIDES` is highly discouraged. Will be deprecated. |
| 73 | * `PRODUCT_DEFAULT_PROPERTY_OVERRIDES` is also discouraged. Will be deprecated. |
| 74 | |
| 75 | For odm partition, |
| 76 | |
| 77 | * `PRODUCT_ODM_PROPERTIES` |
| 78 | |
| 79 | For system_ext partition, |
| 80 | |
| 81 | * `PRODUCT_SYSTEM_EXT_PROPERTIES` |
| 82 | |
| 83 | For product partition, |
| 84 | |
| 85 | * `PRODUCT_PRODUCT_PROPERTIES` |
| 86 | |
| 87 | ### Duplication is not allowed within a partition |
| 88 | |
| 89 | For each partition, having multiple sysprop assignments for the same name is |
| 90 | prohibited. For example, the following will now trigger an error: |
| 91 | |
| 92 | `PRODUCT_VENDOR_PROPERTIES += foo=true foo=false` |
| 93 | |
| 94 | Having duplication across partitions are still allowed. So, the following is |
| 95 | not an error: |
| 96 | |
| 97 | `PRODUCT_VENDOR_PROPERTIES += foo=true` |
| 98 | `PRODUCT_SYSTEM_PROPERTIES += foo=false` |
| 99 | |
| 100 | In that case, the final value is determined at runtime. The precedence is |
| 101 | |
| 102 | * product |
| 103 | * odm |
| 104 | * vendor |
| 105 | * system_ext |
| 106 | * system |
| 107 | |
| 108 | So, `foo` becomes `true` because vendor has higher priority than system. |
| 109 | |
| 110 | To temporarily turn the build-time restriction off, use |
| 111 | |
| 112 | `BUILD_BROKEN_DUP_SYSPROP := true` |
| 113 | |
| 114 | ### Optional assignments |
| 115 | |
| 116 | System properties can now be set as optional using the new syntax: |
| 117 | |
| 118 | `name ?= value` |
| 119 | |
| 120 | Then the system property named `name` gets the value `value` only when there |
| 121 | is no other non-optional assignments having the same name. For example, the |
| 122 | following is allowed and `foo` gets `true` |
| 123 | |
| 124 | `PRODUCT_VENDOR_PROPERTIES += foo=true foo?=false` |
| 125 | |
| 126 | Note that the order between the optional and the non-optional assignments |
| 127 | doesn't matter. The following gives the same result as above. |
| 128 | |
| 129 | `PRODUCT_VENDOR_PROPERTIES += foo?=false foo=true` |
| 130 | |
| 131 | Optional assignments can be duplicated and in that case their order matters. |
| 132 | Specifically, the last one eclipses others. |
| 133 | |
| 134 | `PRODUCT_VENDOR_PROPERTIES += foo?=apple foo?=banana foo?=mango` |
| 135 | |
| 136 | With above, `foo` becomes `mango` since its the last one. |
| 137 | |
| 138 | Note that this behavior is different from the previous behavior of preferring |
| 139 | the first one. To go back to the original behavior for compatability reason, |
| 140 | use: |
| 141 | |
| 142 | `BUILD_BROKEN_DUP_SYSPROP := true` |
| 143 | |
Yo Chiang | d474181 | 2020-07-14 15:15:08 +0800 | [diff] [blame] | 144 | ## ELF prebuilts in `PRODUCT_COPY_FILES` {#BUILD_BROKEN_ELF_PREBUILT_PRODUCT_COPY_FILES} |
Yo Chiang | 73d3148 | 2020-04-07 15:59:59 +0800 | [diff] [blame] | 145 | |
Yo Chiang | d474181 | 2020-07-14 15:15:08 +0800 | [diff] [blame] | 146 | ELF prebuilts in `PRODUCT_COPY_FILES` that are installed into these paths are an |
Yo Chiang | 73d3148 | 2020-04-07 15:59:59 +0800 | [diff] [blame] | 147 | error: |
| 148 | |
| 149 | * `<partition>/bin/*` |
| 150 | * `<partition>/lib/*` |
| 151 | * `<partition>/lib64/*` |
| 152 | |
Yo Chiang | d474181 | 2020-07-14 15:15:08 +0800 | [diff] [blame] | 153 | Define prebuilt modules and add them to `PRODUCT_PACKAGES` instead. |
Yo Chiang | 73d3148 | 2020-04-07 15:59:59 +0800 | [diff] [blame] | 154 | To temporarily relax this check and restore the behavior prior to this change, |
| 155 | set `BUILD_BROKEN_ELF_PREBUILT_PRODUCT_COPY_FILES := true` in `BoardConfig.mk`. |
| 156 | |
Dan Willemsen | 66d21d4 | 2020-01-27 19:26:02 -0800 | [diff] [blame] | 157 | ## COPY_HEADERS usage now produces warnings {#copy_headers} |
| 158 | |
| 159 | We've considered `BUILD_COPY_HEADERS`/`LOCAL_COPY_HEADERS` to be deprecated for |
| 160 | a long time, and the places where it's been able to be used have shrinked over |
| 161 | the last several releases. Equivalent functionality is not available in Soong. |
| 162 | |
| 163 | See the [build/soong/docs/best_practices.md#headers] for more information about |
| 164 | how best to handle headers in Android. |
| 165 | |
Dan Willemsen | 2bfffb9 | 2020-01-14 10:56:53 -0800 | [diff] [blame] | 166 | ## `m4` is not available on `$PATH` |
| 167 | |
| 168 | There is a prebuilt of it available in prebuilts/build-tools, and a make |
| 169 | variable `M4` that contains the path. |
| 170 | |
| 171 | Beyond the direct usage, whenever you use bison or flex directly, they call m4 |
| 172 | behind the scene, so you must set the M4 environment variable (and depend upon |
| 173 | it for incremental build correctness): |
| 174 | |
| 175 | ``` |
| 176 | $(intermediates)/foo.c: .KATI_IMPLICIT_OUTPUTS := $(intermediates)/foo.h |
| 177 | $(intermediates)/foo.c: $(LOCAL_PATH)/foo.y $(M4) $(BISON) $(BISON_DATA) |
| 178 | M4=$(M4) $(BISON) ... |
| 179 | ``` |
| 180 | |
Dan Willemsen | 2607625 | 2020-01-02 20:08:10 -0800 | [diff] [blame] | 181 | ## Rules executed within limited environment |
| 182 | |
| 183 | With `ALLOW_NINJA_ENV=false` (soon to be the default), ninja, and all the |
| 184 | rules/actions executed within it will only have access to a limited number of |
| 185 | environment variables. Ninja does not track when environment variables change |
| 186 | in order to trigger rebuilds, so changing behavior based on arbitrary variables |
| 187 | is not safe with incremental builds. |
| 188 | |
| 189 | Kati and Soong can safely use environment variables, so the expectation is that |
| 190 | you'd embed any environment variables that you need to use within the command |
| 191 | line generated by those tools. See the [export section](#export_keyword) below |
| 192 | for examples. |
| 193 | |
| 194 | For a temporary workaround, you can set `ALLOW_NINJA_ENV=true` in your |
| 195 | environment to restore the previous behavior, or set |
| 196 | `BUILD_BROKEN_NINJA_USES_ENV_VAR := <var> <var2> ...` in your `BoardConfig.mk` |
| 197 | to allow specific variables to be passed through until you've fixed the rules. |
| 198 | |
Dan Willemsen | 0cb422f | 2019-11-25 17:51:18 -0800 | [diff] [blame] | 199 | ## LOCAL_C_INCLUDES outside the source/output trees are an error {#BUILD_BROKEN_OUTSIDE_INCLUDE_DIRS} |
| 200 | |
| 201 | Include directories are expected to be within the source tree (or in the output |
| 202 | directory, generated during the build). This has been checked in some form |
| 203 | since Oreo, but now has better checks. |
| 204 | |
| 205 | There's now a `BUILD_BROKEN_OUTSIDE_INCLUDE_DIRS` variable, that when set, will |
| 206 | turn these errors into warnings temporarily. I don't expect this to last more |
| 207 | than a release, since they're fairly easy to clean up. |
| 208 | |
| 209 | Neither of these cases are supported by Soong, and will produce errors when |
| 210 | converting your module. |
| 211 | |
| 212 | ### Absolute paths |
| 213 | |
| 214 | This has been checked since Oreo. The common reason to hit this is because a |
| 215 | makefile is calculating a path, and ran abspath/realpath/etc. This is a problem |
| 216 | because it makes your build non-reproducible. It's very unlikely that your |
| 217 | source path is the same on every machine. |
| 218 | |
| 219 | ### Using `../` to leave the source/output directories |
| 220 | |
| 221 | This is the new check that has been added. In every case I've found, this has |
| 222 | been a mistake in the Android.mk -- assuming that `LOCAL_C_INCLUDES` (which is |
| 223 | relative to the top of the source tree) acts like `LOCAL_SRC_FILES` (which is |
| 224 | relative to `LOCAL_PATH`). |
| 225 | |
| 226 | Since this usually isn't a valid path, you can almost always just remove the |
| 227 | offending line. |
| 228 | |
| 229 | |
Dan Willemsen | 2607625 | 2020-01-02 20:08:10 -0800 | [diff] [blame] | 230 | ## `BOARD_HAL_STATIC_LIBRARIES` and `LOCAL_HAL_STATIC_LIBRARIES` are obsolete {#BOARD_HAL_STATIC_LIBRARIES} |
Yifan Hong | 88adfc6 | 2019-10-11 15:52:44 -0700 | [diff] [blame] | 231 | |
| 232 | Define proper HIDL / Stable AIDL HAL instead. |
| 233 | |
| 234 | * For libhealthd, use health HAL. See instructions for implementing |
| 235 | health HAL: |
| 236 | |
| 237 | * [hardware/interfaces/health/2.1/README.md] for health 2.1 HAL (recommended) |
| 238 | * [hardware/interfaces/health/1.0/README.md] for health 1.0 HAL |
| 239 | |
| 240 | * For libdumpstate, use at least Dumpstate HAL 1.0. |
| 241 | |
Tao Bao | 84f88a4 | 2019-05-28 16:30:18 -0700 | [diff] [blame] | 242 | ## PRODUCT_STATIC_BOOT_CONTROL_HAL is obsolete {#PRODUCT_STATIC_BOOT_CONTROL_HAL} |
| 243 | |
| 244 | `PRODUCT_STATIC_BOOT_CONTROL_HAL` was the workaround to allow sideloading with |
| 245 | statically linked boot control HAL, before shared library HALs were supported |
| 246 | under recovery. Android Q has added such support (HALs will be loaded in |
| 247 | passthrough mode), and the workarounds are being removed. Targets should build |
| 248 | and install the recovery variant of boot control HAL modules into recovery |
| 249 | image, similar to the ones installed for normal boot. See the change to |
| 250 | crosshatch for example of this: |
| 251 | |
| 252 | * [device/google/crosshatch/bootctrl/Android.bp] for `bootctrl.sdm845` building |
| 253 | rules |
| 254 | * [device/google/crosshatch/device.mk] for installing `bootctrl.sdm845.recovery` |
| 255 | and `android.hardware.boot@1.0-impl.recovery` into recovery image |
| 256 | |
| 257 | [device/google/crosshatch/bootctrl/Android.bp]: https://android.googlesource.com/device/google/crosshatch/+/master/bootctrl/Android.bp |
| 258 | [device/google/crosshatch/device.mk]: https://android.googlesource.com/device/google/crosshatch/+/master/device.mk |
| 259 | |
Dan Willemsen | 695849e | 2019-04-17 12:25:09 -0700 | [diff] [blame] | 260 | ## Deprecation of `BUILD_*` module types |
| 261 | |
| 262 | See [build/make/Deprecation.md](Deprecation.md) for the current status. |
| 263 | |
Dan Willemsen | 3d05a08 | 2019-02-22 23:06:03 +0000 | [diff] [blame] | 264 | ## `PRODUCT_HOST_PACKAGES` split from `PRODUCT_PACKAGES` {#PRODUCT_HOST_PACKAGES} |
| 265 | |
| 266 | Previously, adding a module to `PRODUCT_PACKAGES` that supported both the host |
| 267 | and the target (`host_supported` in Android.bp; two modules with the same name |
| 268 | in Android.mk) would cause both to be built and installed. In many cases you |
| 269 | only want either the host or target versions to be built/installed by default, |
| 270 | and would be over-building with both. So `PRODUCT_PACKAGES` will be changing to |
| 271 | just affect target modules, while `PRODUCT_HOST_PACKAGES` is being added for |
| 272 | host modules. |
| 273 | |
| 274 | Functional differences between `PRODUCT_PACKAGES` and `PRODUCT_HOST_PACKAGES`: |
| 275 | |
| 276 | * `PRODUCT_HOST_PACKAGES` does not have `_ENG`/`_DEBUG` variants, as that's a |
| 277 | property of the target, not the host. |
| 278 | * `PRODUCT_HOST_PACKAGES` does not support `LOCAL_MODULE_OVERRIDES`. |
| 279 | * `PRODUCT_HOST_PACKAGES` requires listed modules to exist, and be host |
| 280 | modules. (Unless `ALLOW_MISSING_DEPENDENCIES` is set) |
| 281 | |
| 282 | This is still an active migration, so currently it still uses |
| 283 | `PRODUCT_PACKAGES` to make installation decisions, but verifies that if we used |
| 284 | `PRODUCT_HOST_PACKAGES`, it would trigger installation for all of the same host |
| 285 | packages. This check ignores shared libraries, as those are not normally |
| 286 | necessary in `PRODUCT_*PACKAGES`, and tended to be over-built (especially the |
| 287 | 32-bit variants). |
| 288 | |
| 289 | Future changes will switch installation decisions to `PRODUCT_HOST_PACKAGES` |
| 290 | for host modules, error when there's a host-only module in `PRODUCT_PACKAGES`, |
| 291 | and do some further cleanup where `LOCAL_REQUIRED_MODULES` are still merged |
| 292 | between host and target modules with the same name. |
| 293 | |
Dan Willemsen | 46267cb | 2019-01-25 14:35:58 -0800 | [diff] [blame] | 294 | ## `*.c.arm` / `*.cpp.arm` deprecation {#file_arm} |
| 295 | |
| 296 | In Android.mk files, you used to be able to change LOCAL_ARM_MODE for each |
| 297 | source file by appending `.arm` to the end of the filename in |
| 298 | `LOCAL_SRC_FILES`. |
| 299 | |
| 300 | Soong does not support this uncommonly used behavior, instead expecting those |
| 301 | files to be split out into a separate static library that chooses `arm` over |
| 302 | `thumb` for the entire library. This must now also be done in Android.mk files. |
| 303 | |
Dan Willemsen | f264690 | 2019-01-25 16:54:37 -0800 | [diff] [blame] | 304 | ## Windows cross-compiles no longer supported in Android.mk |
| 305 | |
| 306 | Modules that build for Windows (our only `HOST_CROSS` OS currently) must now be |
| 307 | defined in `Android.bp` files. |
| 308 | |
Dan Willemsen | e048f7e | 2019-02-09 18:58:36 -0800 | [diff] [blame] | 309 | ## `LOCAL_MODULE_TAGS := eng debug` are obsolete {#LOCAL_MODULE_TAGS} |
Dan Willemsen | 9569ddd | 2019-01-22 19:38:56 -0800 | [diff] [blame] | 310 | |
Dan Willemsen | e048f7e | 2019-02-09 18:58:36 -0800 | [diff] [blame] | 311 | `LOCAL_MODULE_TAGS` value `eng` and `debug` are now obsolete. They allowed |
Dan Willemsen | 9569ddd | 2019-01-22 19:38:56 -0800 | [diff] [blame] | 312 | modules to specify that they should always be installed on `-eng`, or `-eng` |
| 313 | and `-userdebug` builds. This conflicted with the ability for products to |
| 314 | specify which modules should be installed, effectively making it impossible to |
| 315 | build a stripped down product configuration that did not include those modules. |
| 316 | |
| 317 | For the equivalent functionality, specify the modules in `PRODUCT_PACKAGES_ENG` |
| 318 | or `PRODUCT_PACKAGES_DEBUG` in the appropriate product makefiles. |
| 319 | |
| 320 | Core android packages like `su` got added to the list in |
| 321 | `build/make/target/product/base_system.mk`, but for device-specific modules |
| 322 | there are often better base product makefiles to use instead. |
| 323 | |
Dan Willemsen | 0636428 | 2019-01-02 14:32:54 -0800 | [diff] [blame] | 324 | ## `USER` deprecation {#USER} |
| 325 | |
| 326 | `USER` will soon be `nobody` in many cases due to the addition of a sandbox |
| 327 | around the Android build. Most of the time you shouldn't need to know the |
| 328 | identity of the user running the build, but if you do, it's available in the |
| 329 | make variable `BUILD_USERNAME` for now. |
| 330 | |
| 331 | Similarly, the `hostname` tool will also be returning a more consistent value |
| 332 | of `android-build`. The real value is available as `BUILD_HOSTNAME`. |
| 333 | |
Dan Willemsen | 6dbb33d | 2018-10-21 19:41:49 -0700 | [diff] [blame] | 334 | ## `BUILD_NUMBER` removal from Android.mk {#BUILD_NUMBER} |
| 335 | |
| 336 | `BUILD_NUMBER` should not be used directly in Android.mk files, as it would |
| 337 | trigger them to be re-read every time the `BUILD_NUMBER` changes (which it does |
| 338 | on every build server build). If possible, just remove the use so that your |
| 339 | builds are more reproducible. If you do need it, use `BUILD_NUMBER_FROM_FILE`: |
| 340 | |
| 341 | ``` make |
| 342 | $(LOCAL_BUILT_MODULE): |
| 343 | mytool --build_number $(BUILD_NUMBER_FROM_FILE) -o $@ |
| 344 | ``` |
| 345 | |
| 346 | That will expand out to a subshell that will read the current `BUILD_NUMBER` |
| 347 | whenever it's run. It will not re-run your command if the build number has |
| 348 | changed, so incremental builds will have the build number from the last time |
| 349 | the particular output was rebuilt. |
| 350 | |
Dan Willemsen | 78c40be | 2018-10-17 16:50:49 -0700 | [diff] [blame] | 351 | ## `DIST_DIR`, `dist_goal`, and `dist-for-goals` {#dist} |
| 352 | |
| 353 | `DIST_DIR` and `dist_goal` are no longer available when reading Android.mk |
| 354 | files (or other build tasks). Always use `dist-for-goals` instead, which takes |
| 355 | a PHONY goal, and a list of files to copy to `$DIST_DIR`. Whenever `dist` is |
| 356 | specified, and the goal would be built (either explicitly on the command line, |
| 357 | or as a dependency of something on the command line), that file will be copied |
| 358 | into `$DIST_DIR`. For example, |
| 359 | |
| 360 | ``` make |
| 361 | $(call dist-for-goals,foo,bar/baz) |
| 362 | ``` |
| 363 | |
| 364 | will copy `bar/baz` into `$DIST_DIR/baz` when `m foo dist` is run. |
| 365 | |
| 366 | #### Renames during copy |
| 367 | |
| 368 | Instead of specifying just a file, a destination name can be specified, |
| 369 | including subdirectories: |
| 370 | |
| 371 | ``` make |
| 372 | $(call dist-for-goals,foo,bar/baz:logs/foo.log) |
| 373 | ``` |
| 374 | |
| 375 | will copy `bar/baz` into `$DIST_DIR/logs/foo.log` when `m foo dist` is run. |
| 376 | |
Dan Willemsen | 5fb16a6 | 2018-09-04 16:23:14 -0700 | [diff] [blame] | 377 | ## `.PHONY` rule enforcement {#phony_targets} |
| 378 | |
| 379 | There are several new warnings/errors meant to ensure the proper use of |
| 380 | `.PHONY` targets in order to improve the speed and reliability of incremental |
| 381 | builds. |
| 382 | |
| 383 | `.PHONY`-marked targets are often used as shortcuts to provide "friendly" names |
| 384 | for real files to be built, but any target marked with `.PHONY` is also always |
| 385 | considered dirty, needing to be rebuilt every build. This isn't a problem for |
| 386 | aliases or one-off user-requested operations, but if real builds steps depend |
| 387 | on a `.PHONY` target, it can get quite expensive for what should be a tiny |
| 388 | build. |
| 389 | |
| 390 | ``` make |
| 391 | ...mk:42: warning: PHONY target "out/.../foo" looks like a real file (contains a "/") |
| 392 | ``` |
| 393 | |
| 394 | Between this warning and the next, we're requiring that `.PHONY` targets do not |
| 395 | have "/" in them, and real file targets do have a "/". This makes it more |
| 396 | obvious when reading makefiles what is happening, and will help the build |
| 397 | system differentiate these in the future too. |
| 398 | |
| 399 | ``` make |
| 400 | ...mk:42: warning: writing to readonly directory: "kernel-modules" |
| 401 | ``` |
| 402 | |
| 403 | This warning will show up for one of two reasons: |
| 404 | |
| 405 | 1. The target isn't intended to be a real file, and should be marked with |
| 406 | `.PHONY`. This would be the case for this example. |
| 407 | 2. The target is a real file, but it's outside the output directories. All |
| 408 | outputs from the build system should be within the output directory, |
| 409 | otherwise `m clean` is unable to clean the build, and future builds may not |
| 410 | work properly. |
| 411 | |
| 412 | ``` make |
| 413 | ...mk:42: warning: real file "out/.../foo" depends on PHONY target "buildbins" |
| 414 | ``` |
| 415 | |
| 416 | If the first target isn't intended to be a real file, then it should be marked |
| 417 | with `.PHONY`, which will satisfy this warning. This isn't the case for this |
| 418 | example, as we require `.PHONY` targets not to have '/' in them. |
| 419 | |
| 420 | If the second (PHONY) target is a real file, it may unnecessarily be marked |
| 421 | with `.PHONY`. |
| 422 | |
| 423 | ### `.PHONY` and calling other build systems |
| 424 | |
| 425 | One common pattern (mostly outside AOSP) that we've seen hit these warning is |
| 426 | when building with external build systems (firmware, bootloader, kernel, etc). |
| 427 | Those are often marked as `.PHONY` because the Android build system doesn't |
| 428 | have enough dependencies to know when to run the other build system again |
| 429 | during an incremental build. |
| 430 | |
| 431 | We recommend to build these outside of Android, and deliver prebuilts into the |
| 432 | Android tree instead of decreasing the speed and reliability of the incremental |
| 433 | Android build. |
| 434 | |
| 435 | In cases where that's not desired, to preserve the speed of Android |
| 436 | incrementals, over-specifying dependencies is likely a better option than |
| 437 | marking it with `.PHONY`: |
| 438 | |
| 439 | ``` make |
| 440 | out/target/.../zImage: $(sort $(shell find -L $(KERNEL_SRCDIR))) |
| 441 | ... |
| 442 | ``` |
| 443 | |
| 444 | For reliability, many of these other build systems do not guarantee the same |
| 445 | level of incremental build assurances as the Android Build is attempting to do |
| 446 | -- without custom checks, Make doesn't rebuild objects when CFLAGS change, etc. |
| 447 | In order to fix this, our recommendation is to do clean builds for each of |
| 448 | these external build systems every time anything they rely on changes. For |
| 449 | relatively smaller builds (like the kernel), this may be reasonable as long as |
| 450 | you're not trying to actively debug the kernel. |
| 451 | |
| 452 | ## `export` and `unexport` deprecation {#export_keyword} |
Dan Willemsen | 8b9c3cc | 2018-02-27 02:15:32 -0800 | [diff] [blame] | 453 | |
Dan Willemsen | 3a1072a | 2019-05-14 22:03:58 -0700 | [diff] [blame] | 454 | The `export` and `unexport` keywords are obsolete, and will throw errors when |
| 455 | used. |
Dan Willemsen | 8b9c3cc | 2018-02-27 02:15:32 -0800 | [diff] [blame] | 456 | |
Dan Willemsen | 8b9c3cc | 2018-02-27 02:15:32 -0800 | [diff] [blame] | 457 | Device specific configuration should not be able to affect common core build |
| 458 | steps -- we're looking at triggering build steps to be invalidated if the set |
| 459 | of environment variables they can access changes. If device specific |
| 460 | configuration is allowed to change those, switching devices with the same |
| 461 | output directory could become significantly more expensive than it already can |
| 462 | be. |
| 463 | |
Dan Willemsen | 3a1072a | 2019-05-14 22:03:58 -0700 | [diff] [blame] | 464 | If used during Android.mk files, and later tasks, it is increasingly likely |
| 465 | that they are being used incorrectly. Attempting to change the environment for |
| 466 | a single build step, and instead setting it for hundreds of thousands. |
Dan Willemsen | 8b9c3cc | 2018-02-27 02:15:32 -0800 | [diff] [blame] | 467 | |
| 468 | It is not recommended to just move the environment variable setting outside of |
| 469 | the build (in vendorsetup.sh, or some other configuration script or wrapper). |
| 470 | We expect to limit the environment variables that the build respects in the |
| 471 | future, others will be cleared. (There will be methods to get custom variables |
| 472 | into the build, just not to every build step) |
| 473 | |
| 474 | Instead, write the export commands into the rule command lines themselves: |
| 475 | |
| 476 | ``` make |
| 477 | $(intermediates)/generated_output.img: |
| 478 | rm -rf $@ |
| 479 | export MY_ENV_A="$(MY_A)"; make ... |
| 480 | ``` |
| 481 | |
| 482 | If you want to set many environment variables, and/or use them many times, |
| 483 | write them out to a script and source the script: |
| 484 | |
| 485 | ``` make |
| 486 | envsh := $(intermediates)/env.sh |
| 487 | $(envsh): |
| 488 | rm -rf $@ |
| 489 | echo 'export MY_ENV_A="$(MY_A)"' >$@ |
| 490 | echo 'export MY_ENV_B="$(MY_B)"' >>$@ |
| 491 | |
| 492 | $(intermediates)/generated_output.img: PRIVATE_ENV := $(envsh) |
| 493 | $(intermediates)/generated_output.img: $(envsh) a/b/c/package.sh |
| 494 | rm -rf $@ |
| 495 | source $(PRIVATE_ENV); make ... |
| 496 | source $(PRIVATE_ENV); a/b/c/package.sh ... |
| 497 | ``` |
| 498 | |
Dan Willemsen | 5f76fc0 | 2018-06-21 21:42:29 -0700 | [diff] [blame] | 499 | ## Implicit make rules are obsolete {#implicit_rules} |
Dan Willemsen | 62db0f0 | 2018-06-16 09:37:13 -0700 | [diff] [blame] | 500 | |
| 501 | Implicit rules look something like the following: |
| 502 | |
| 503 | ``` make |
| 504 | $(TARGET_OUT_SHARED_LIBRARIES)/%_vendor.so: $(TARGET_OUT_SHARED_LIBRARIES)/%.so |
| 505 | ... |
| 506 | |
| 507 | %.o : %.foo |
| 508 | ... |
| 509 | ``` |
| 510 | |
Dan Willemsen | 5f76fc0 | 2018-06-21 21:42:29 -0700 | [diff] [blame] | 511 | These can have wide ranging effects across unrelated modules, so they're now obsolete. Instead, use static pattern rules, which are similar, but explicitly match the specified outputs: |
Dan Willemsen | 62db0f0 | 2018-06-16 09:37:13 -0700 | [diff] [blame] | 512 | |
| 513 | ``` make |
| 514 | libs := $(foreach lib,libfoo libbar,$(TARGET_OUT_SHARED_LIBRARIES)/$(lib)_vendor.so) |
| 515 | $(libs): %_vendor.so: %.so |
| 516 | ... |
| 517 | |
| 518 | files := $(wildcard $(LOCAL_PATH)/*.foo) |
| 519 | gen := $(patsubst $(LOCAL_PATH)/%.foo,$(intermediates)/%.o,$(files)) |
| 520 | $(gen): %.o : %.foo |
| 521 | ... |
| 522 | ``` |
| 523 | |
Dan Willemsen | ac92659 | 2018-06-11 22:28:00 -0700 | [diff] [blame] | 524 | ## Removing '/' from Valid Module Names {#name_slash} |
| 525 | |
| 526 | The build system uses module names in path names in many places. Having an |
| 527 | extra '/' or '../' being inserted can cause problems -- and not just build |
| 528 | breaks, but stranger invalid behavior. |
| 529 | |
| 530 | In every case we've seen, the fix is relatively simple: move the directory into |
| 531 | `LOCAL_MODULE_RELATIVE_PATH` (or `LOCAL_MODULE_PATH` if you're still using it). |
| 532 | If this causes multiple modules to be named the same, use unique module names |
| 533 | and `LOCAL_MODULE_STEM` to change the installed file name: |
| 534 | |
| 535 | ``` make |
| 536 | include $(CLEAR_VARS) |
| 537 | LOCAL_MODULE := ver1/code.bin |
| 538 | LOCAL_MODULE_PATH := $(TARGET_OUT_ETC)/firmware |
| 539 | ... |
| 540 | include $(BUILD_PREBUILT) |
| 541 | |
| 542 | include $(CLEAR_VARS) |
| 543 | LOCAL_MODULE := ver2/code.bin |
| 544 | LOCAL_MODULE_PATH := $(TARGET_OUT_ETC)/firmware |
| 545 | ... |
| 546 | include $(BUILD_PREBUILT) |
| 547 | ``` |
| 548 | |
| 549 | Can be rewritten as: |
| 550 | |
| 551 | ``` |
| 552 | include $(CLEAR_VARS) |
| 553 | LOCAL_MODULE := ver1_code.bin |
| 554 | LOCAL_MODULE_STEM := code.bin |
| 555 | LOCAL_MODULE_PATH := $(TARGET_OUT_VENDOR)/firmware/ver1 |
| 556 | ... |
| 557 | include $(BUILD_PREBUILT) |
| 558 | |
| 559 | include $(CLEAR_VARS) |
| 560 | LOCAL_MODULE := ver2_code.bin |
| 561 | LOCAL_MODULE_STEM := code.bin |
| 562 | LOCAL_MODULE_PATH := $(TARGET_OUT_VENDOR)/firmware/ver2 |
| 563 | ... |
| 564 | include $(BUILD_PREBUILT) |
| 565 | ``` |
| 566 | |
| 567 | You just need to make sure that any other references (`PRODUCT_PACKAGES`, |
| 568 | `LOCAL_REQUIRED_MODULES`, etc) are converted to the new names. |
| 569 | |
Dan Willemsen | bbe6a02 | 2018-06-10 14:49:01 -0700 | [diff] [blame] | 570 | ## Valid Module Names {#name} |
| 571 | |
| 572 | We've adopted lexical requirements very similar to [Bazel's |
| 573 | requirements](https://docs.bazel.build/versions/master/build-ref.html#name) for |
| 574 | target names. Valid characters are `a-z`, `A-Z`, `0-9`, and the special |
Dan Willemsen | ac92659 | 2018-06-11 22:28:00 -0700 | [diff] [blame] | 575 | characters `_.+-=,@~`. This currently applies to `LOCAL_PACKAGE_NAME`, |
Dan Willemsen | bbe6a02 | 2018-06-10 14:49:01 -0700 | [diff] [blame] | 576 | `LOCAL_MODULE`, and `LOCAL_MODULE_SUFFIX`, and `LOCAL_MODULE_STEM*`. |
| 577 | |
| 578 | Many other characters already caused problems if you used them, so we don't |
| 579 | expect this to have a large effect. |
| 580 | |
Dan Willemsen | 5039ef4 | 2018-05-18 11:00:17 -0700 | [diff] [blame] | 581 | ## PATH Tools {#PATH_Tools} |
| 582 | |
| 583 | The build has started restricting the external host tools usable inside the |
| 584 | build. This will help ensure that build results are reproducible across |
| 585 | different machines, and catch mistakes before they become larger issues. |
| 586 | |
| 587 | To start with, this includes replacing the $PATH with our own directory of |
| 588 | tools, mirroring that of the host PATH. The only difference so far is the |
| 589 | removal of the host GCC tools. Anything that is not explicitly in the |
| 590 | configuration as allowed will continue functioning, but will generate a log |
| 591 | message. This is expected to become more restrictive over time. |
| 592 | |
| 593 | The configuration is located in build/soong/ui/build/paths/config.go, and |
| 594 | contains all the common tools in use in many builds. Anything not in that list |
| 595 | will currently print a warning in the `$OUT_DIR/soong.log` file, including the |
| 596 | command and arguments used, and the process tree in order to help locate the |
| 597 | usage. |
| 598 | |
| 599 | In order to fix any issues brought up by these checks, the best way to fix them |
| 600 | is to use tools checked into the tree -- either as prebuilts, or building them |
| 601 | as host tools during the build. |
| 602 | |
| 603 | As a temporary measure, you can set `TEMPORARY_DISABLE_PATH_RESTRICTIONS=true` |
| 604 | in your environment to temporarily turn off the error checks and allow any tool |
| 605 | to be used (with logging). Beware that GCC didn't work well with the interposer |
| 606 | used for logging, so this may not help in all cases. |
| 607 | |
Dan Willemsen | 79fd696 | 2017-11-28 22:32:05 -0800 | [diff] [blame] | 608 | ## Deprecating / obsoleting envsetup.sh variables in Makefiles |
Dan Willemsen | 7733862 | 2017-11-08 16:39:18 -0800 | [diff] [blame] | 609 | |
| 610 | It is not required to source envsetup.sh before running a build. Many scripts, |
| 611 | including a majority of our automated build systems, do not do so. Make will |
| 612 | transparently make every environment variable available as a make variable. |
| 613 | This means that relying on environment variables only set up in envsetup.sh will |
| 614 | produce different output for local users and scripted users. |
| 615 | |
| 616 | Many of these variables also include absolute path names, which we'd like to |
| 617 | keep out of the generated files, so that you don't need to do a full rebuild if |
| 618 | you move the source tree. |
| 619 | |
| 620 | To fix this, we're marking the variables that are set in envsetup.sh as |
| 621 | deprecated in the makefiles. This will trigger a warning every time one is read |
Dan Willemsen | 79fd696 | 2017-11-28 22:32:05 -0800 | [diff] [blame] | 622 | (or written) inside Kati. Once all the warnings have been removed for a |
| 623 | particular variable, we'll switch it to obsolete, and any references will become |
| 624 | errors. |
Dan Willemsen | 7733862 | 2017-11-08 16:39:18 -0800 | [diff] [blame] | 625 | |
| 626 | ### envsetup.sh variables with make equivalents |
| 627 | |
| 628 | | instead of | use | |
| 629 | |--------------------------------------------------------------|----------------------| |
Yasuhiro Kubota | cd301f6 | 2018-10-09 15:51:23 +0900 | [diff] [blame] | 630 | | OUT {#OUT} | PRODUCT_OUT | |
Dan Willemsen | 7733862 | 2017-11-08 16:39:18 -0800 | [diff] [blame] | 631 | | ANDROID_HOST_OUT {#ANDROID_HOST_OUT} | HOST_OUT | |
| 632 | | ANDROID_PRODUCT_OUT {#ANDROID_PRODUCT_OUT} | PRODUCT_OUT | |
| 633 | | ANDROID_HOST_OUT_TESTCASES {#ANDROID_HOST_OUT_TESTCASES} | HOST_OUT_TESTCASES | |
| 634 | | ANDROID_TARGET_OUT_TESTCASES {#ANDROID_TARGET_OUT_TESTCASES} | TARGET_OUT_TESTCASES | |
| 635 | |
| 636 | All of the make variables may be relative paths from the current directory, or |
| 637 | absolute paths if the output directory was specified as an absolute path. If you |
| 638 | need an absolute variable, convert it to absolute during a rule, so that it's |
| 639 | not expanded into the generated ninja file: |
| 640 | |
| 641 | ``` make |
| 642 | $(PRODUCT_OUT)/gen.img: my/src/path/gen.sh |
| 643 | export PRODUCT_OUT=$$(cd $(PRODUCT_OUT); pwd); cd my/src/path; ./gen.sh -o $${PRODUCT_OUT}/gen.img |
| 644 | ``` |
| 645 | |
| 646 | ### ANDROID_BUILD_TOP {#ANDROID_BUILD_TOP} |
| 647 | |
| 648 | In Android.mk files, you can always assume that the current directory is the |
| 649 | root of the source tree, so this can just be replaced with '.' (which is what |
| 650 | $TOP is hardcoded to), or removed entirely. If you need an absolute path, see |
| 651 | the instructions above. |
| 652 | |
| 653 | ### Stop using PATH directly {#PATH} |
| 654 | |
| 655 | This isn't only set by envsetup.sh, but it is modified by it. Due to that it's |
| 656 | rather easy for this to change between different shells, and it's not ideal to |
| 657 | reread the makefiles every time this changes. |
| 658 | |
| 659 | In most cases, you shouldn't need to touch PATH at all. When you need to have a |
| 660 | rule reference a particular binary that's part of the source tree or outputs, |
| 661 | it's preferrable to just use the path to the file itself (since you should |
| 662 | already be adding that as a dependency). |
| 663 | |
| 664 | Depending on the rule, passing the file path itself may not be feasible due to |
| 665 | layers of unchangable scripts/binaries. In that case, be sure to add the |
| 666 | dependency, but modify the PATH within the rule itself: |
| 667 | |
| 668 | ``` make |
| 669 | $(TARGET): myscript my/path/binary |
| 670 | PATH=my/path:$$PATH myscript -o $@ |
| 671 | ``` |
| 672 | |
| 673 | ### Stop using PYTHONPATH directly {#PYTHONPATH} |
| 674 | |
| 675 | Like PATH, this isn't only set by envsetup.sh, but it is modified by it. Due to |
| 676 | that it's rather easy for this to change between different shells, and it's not |
| 677 | ideal to reread the makefiles every time. |
| 678 | |
| 679 | The best solution here is to start switching to Soong's python building support, |
| 680 | which packages the python interpreter, libraries, and script all into one file |
| 681 | that no longer needs PYTHONPATH. See fontchain_lint for examples of this: |
| 682 | |
| 683 | * [external/fonttools/Lib/fontTools/Android.bp] for python_library_host |
| 684 | * [frameworks/base/Android.bp] for python_binary_host |
| 685 | * [frameworks/base/data/fonts/Android.mk] to execute the python binary |
| 686 | |
| 687 | If you still need to use PYTHONPATH, do so within the rule itself, just like |
| 688 | path: |
| 689 | |
| 690 | ``` make |
| 691 | $(TARGET): myscript.py $(sort $(shell find my/python/lib -name '*.py')) |
| 692 | PYTHONPATH=my/python/lib:$$PYTHONPATH myscript.py -o $@ |
| 693 | ``` |
Yifan Hong | 97de88c | 2017-12-12 18:01:09 -0800 | [diff] [blame] | 694 | ### Stop using PRODUCT_COMPATIBILITY_MATRIX_LEVEL_OVERRIDE directly {#PRODUCT_COMPATIBILITY_MATRIX_LEVEL_OVERRIDE} |
| 695 | |
| 696 | Specify Framework Compatibility Matrix Version in device manifest by adding a `target-level` |
| 697 | attribute to the root element `<manifest>`. If `PRODUCT_COMPATIBILITY_MATRIX_LEVEL_OVERRIDE` |
| 698 | is 26 or 27, you can add `"target-level"="1"` to your device manifest instead. |
Dan Willemsen | 7733862 | 2017-11-08 16:39:18 -0800 | [diff] [blame] | 699 | |
Stephen Hines | 178cf8e | 2018-01-11 11:54:48 -0800 | [diff] [blame] | 700 | ### Stop using USE_CLANG_PLATFORM_BUILD {#USE_CLANG_PLATFORM_BUILD} |
| 701 | |
| 702 | Clang is the default and only supported Android compiler, so there is no reason |
| 703 | for this option to exist. |
| 704 | |
Dan Willemsen | 7733862 | 2017-11-08 16:39:18 -0800 | [diff] [blame] | 705 | ### Other envsetup.sh variables {#other_envsetup_variables} |
| 706 | |
| 707 | * ANDROID_TOOLCHAIN |
| 708 | * ANDROID_TOOLCHAIN_2ND_ARCH |
| 709 | * ANDROID_DEV_SCRIPTS |
| 710 | * ANDROID_EMULATOR_PREBUILTS |
| 711 | * ANDROID_PRE_BUILD_PATHS |
| 712 | |
| 713 | These are all exported from envsetup.sh, but don't have clear equivalents within |
| 714 | the makefile system. If you need one of them, you'll have to set up your own |
| 715 | version. |
| 716 | |
| 717 | |
| 718 | [build/soong/Changes.md]: https://android.googlesource.com/platform/build/soong/+/master/Changes.md |
Dan Willemsen | 66d21d4 | 2020-01-27 19:26:02 -0800 | [diff] [blame] | 719 | [build/soong/docs/best_practices.md#headers]: https://android.googlesource.com/platform/build/soong/+/master/docs/best_practices.md#headers |
Dan Willemsen | 7733862 | 2017-11-08 16:39:18 -0800 | [diff] [blame] | 720 | [external/fonttools/Lib/fontTools/Android.bp]: https://android.googlesource.com/platform/external/fonttools/+/master/Lib/fontTools/Android.bp |
| 721 | [frameworks/base/Android.bp]: https://android.googlesource.com/platform/frameworks/base/+/master/Android.bp |
| 722 | [frameworks/base/data/fonts/Android.mk]: https://android.googlesource.com/platform/frameworks/base/+/master/data/fonts/Android.mk |
Yifan Hong | 88adfc6 | 2019-10-11 15:52:44 -0700 | [diff] [blame] | 723 | [hardware/interfaces/health/1.0/README.md]: https://android.googlesource.com/platform/hardware/interfaces/+/master/health/1.0/README.md |
| 724 | [hardware/interfaces/health/2.1/README.md]: https://android.googlesource.com/platform/hardware/interfaces/+/master/health/2.1/README.md |