Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ac92659e authored by Dan Willemsen's avatar Dan Willemsen
Browse files

Restrict / from module names 

Since this is a directory separator, it causes a good deal of
strangeness in the build whenever we include a module name in a path.

It becomes particularly problematic if used together with ".."

Test: build_test on downstream branches
Change-Id: I344eca0db3346cd6ffabff767c34159c85ebc051
parent 500fcceb

Changes.md

+47 −1
Original line number Diff line number Diff line 
# Build System Changes for Android.mk Writers



## Removing '/' from Valid Module Names {#name_slash}



The build system uses module names in path names in many places. Having an

extra '/' or '../' being inserted can cause problems -- and not just build

breaks, but stranger invalid behavior.



In every case we've seen, the fix is relatively simple: move the directory into

`LOCAL_MODULE_RELATIVE_PATH` (or `LOCAL_MODULE_PATH` if you're still using it).

If this causes multiple modules to be named the same, use unique module names

and `LOCAL_MODULE_STEM` to change the installed file name:



``` make

include $(CLEAR_VARS)

LOCAL_MODULE := ver1/code.bin

LOCAL_MODULE_PATH := $(TARGET_OUT_ETC)/firmware

...

include $(BUILD_PREBUILT)



include $(CLEAR_VARS)

LOCAL_MODULE := ver2/code.bin

LOCAL_MODULE_PATH := $(TARGET_OUT_ETC)/firmware

...

include $(BUILD_PREBUILT)

```



Can be rewritten as:



```

include $(CLEAR_VARS)

LOCAL_MODULE := ver1_code.bin

LOCAL_MODULE_STEM := code.bin

LOCAL_MODULE_PATH := $(TARGET_OUT_VENDOR)/firmware/ver1

...

include $(BUILD_PREBUILT)



include $(CLEAR_VARS)

LOCAL_MODULE := ver2_code.bin

LOCAL_MODULE_STEM := code.bin

LOCAL_MODULE_PATH := $(TARGET_OUT_VENDOR)/firmware/ver2

...

include $(BUILD_PREBUILT)

```



You just need to make sure that any other references (`PRODUCT_PACKAGES`,

`LOCAL_REQUIRED_MODULES`, etc) are converted to the new names.



## Valid Module Names {#name}



We've adopted lexical requirements very similar to [Bazel's

requirements](https://docs.bazel.build/versions/master/build-ref.html#name) for

target names. Valid characters are `a-z`, `A-Z`, `0-9`, and the special

characters `_.+-=,@~/`. This currently applies to `LOCAL_PACKAGE_NAME`,

characters `_.+-=,@~`. This currently applies to `LOCAL_PACKAGE_NAME`,

`LOCAL_MODULE`, and `LOCAL_MODULE_SUFFIX`, and `LOCAL_MODULE_STEM*`.



Many other characters already caused problems if you used them, so we don't

core/definitions.mk

+9 −5
Original line number Diff line number Diff line
@@ -3488,24 +3488,26 @@ upper := 
###########################################################

## Verify module name meets character requirements:

##   a-z A-Z 0-9

##   _.+-=,@~/

##   _.+-=,@~

##

## This is equivalent to bazel's target name restrictions:

## This is a subset of bazel's target name restrictions:

##   https://docs.bazel.build/versions/master/build-ref.html#name

###########################################################

define verify-module-name

$(if $(filter-out $(LOCAL_MODULE),$(subst /,,$(LOCAL_MODULE))), \

  $(call pretty-warning,Module name contains a /$(comma) use LOCAL_MODULE_STEM and LOCAL_MODULE_RELATIVE_PATH instead)) \

$(if $(call _invalid-name-chars,$(LOCAL_MODULE)), \

  $(call pretty-error,Invalid characters in module name: $(call _invalid-name-chars,$(LOCAL_MODULE))))

endef

define _invalid-name-chars

$(subst /,,$(subst _,,$(subst .,,$(subst +,,$(subst -,,$(subst =,,$(subst $(comma),,$(subst @,,$(subst ~,,$(subst 0,,$(subst 1,,$(subst 2,,$(subst 3,,$(subst 4,,$(subst 5,,$(subst 6,,$(subst 7,,$(subst 8,,$(subst 9,,$(subst a,,$(subst b,,$(subst c,,$(subst d,,$(subst e,,$(subst f,,$(subst g,,$(subst h,,$(subst i,,$(subst j,,$(subst k,,$(subst l,,$(subst m,,$(subst n,,$(subst o,,$(subst p,,$(subst q,,$(subst r,,$(subst s,,$(subst t,,$(subst u,,$(subst v,,$(subst w,,$(subst x,,$(subst y,,$(subst z,,$(call to-lower,$(1)))))))))))))))))))))))))))))))))))))))))))))))

$(subst _,,$(subst .,,$(subst +,,$(subst -,,$(subst =,,$(subst $(comma),,$(subst @,,$(subst ~,,$(subst 0,,$(subst 1,,$(subst 2,,$(subst 3,,$(subst 4,,$(subst 5,,$(subst 6,,$(subst 7,,$(subst 8,,$(subst 9,,$(subst a,,$(subst b,,$(subst c,,$(subst d,,$(subst e,,$(subst f,,$(subst g,,$(subst h,,$(subst i,,$(subst j,,$(subst k,,$(subst l,,$(subst m,,$(subst n,,$(subst o,,$(subst p,,$(subst q,,$(subst r,,$(subst s,,$(subst t,,$(subst u,,$(subst v,,$(subst w,,$(subst x,,$(subst y,,$(subst z,,$(call to-lower,$(1))))))))))))))))))))))))))))))))))))))))))))))

endef

.KATI_READONLY := verify-module-name _invalid-name-chars



###########################################################

## Verify module stem meets character requirements:

##   a-z A-Z 0-9

##   _.+-=,@~/

##   _.+-=,@~

##

## This is a subset of bazel's target name restrictions:

##   https://docs.bazel.build/versions/master/build-ref.html#name
@@ -3513,7 +3515,9 @@ endef 
## $(1): The module stem variable to check

###########################################################

define verify-module-stem

$(if $(filter-out $($(1)),$(subst /,,$($(1)))), \

  $(call pretty-warning,Module stem \($(1)\) contains a /$(comma) use LOCAL_MODULE_RELATIVE_PATH instead)) \

$(if $(call _invalid-name-chars,$($(1))), \

  $(call pretty-error,Invalid characters in module stem ($(1)): $(call _invalid-name-chars,$($(1)))))

  $(call pretty-error,Invalid characters in module stem \($(1)\): $(call _invalid-name-chars,$($(1)))))

endef

.KATI_READONLY := verify-module-stem