BUILD_BROKEN_DUP_SYSPROP as escape hatch for the new sysprop restriction
As the final step for the refactoring of sysprop configuration, this
change adds BUILD_BROKEN_DUP_SYSPROP which is the escape hatch for
the new restriction. When it is turned on, the new syntax `a ?= b`
collapses to the old syntax `a = b`, duplicated assignments are allowed,
and the dups are resolved following the legacy rule of preferring the
first.
This change also summarizes all the user-facing changes to the Change.md
file.
Lastly, post_process_prop.py is refactored to accept new argument
'--allow-dup' which when turned on allowes duplicated sysprops.
Bug: 117892318
Bug: 158735147
Test: atest --host post_process_prop_unittest
Exempt-From-Owner-Approval: cherry-pick from master
Merged-In: I7bdfffd47d50aad66a78e28a30c3dad7ebac080c
(cherry picked from commit b302cdf6a417b9c8eaedee713187735a74707d6a)
Change-Id: I7bdfffd47d50aad66a78e28a30c3dad7ebac080c
diff --git a/Changes.md b/Changes.md
index 453ea6c..61e6bb6 100644
--- a/Changes.md
+++ b/Changes.md
@@ -1,5 +1,92 @@
# Build System Changes for Android.mk Writers
+## Changes in system properties settings
+
+### Product variables
+
+System properties for each of the partition is supposed to be set via following
+product config variables.
+
+For system partititon,
+
+* `PRODUCT_SYSTEM_PROPERITES`
+* `PRODUCT_SYSTEM_DEFAULT_PROPERTIES` is highly discouraged. Will be deprecated.
+
+For vendor partition,
+
+* `PRODUCT_VENDOR_PROPERTIES`
+* `PRODUCT_PROPERTY_OVERRIDES` is highly discouraged. Will be deprecated.
+* `PRODUCT_DEFAULT_PROPERTY_OVERRIDES` is also discouraged. Will be deprecated.
+
+For odm partition,
+
+* `PRODUCT_ODM_PROPERTIES`
+
+For system_ext partition,
+
+* `PRODUCT_SYSTEM_EXT_PROPERTIES`
+
+For product partition,
+
+* `PRODUCT_PRODUCT_PROPERTIES`
+
+### Duplication is not allowed within a partition
+
+For each partition, having multiple sysprop assignments for the same name is
+prohibited. For example, the following will now trigger an error:
+
+`PRODUCT_VENDOR_PROPERTIES += foo=true foo=false`
+
+Having duplication across partitions are still allowed. So, the following is
+not an error:
+
+`PRODUCT_VENDOR_PROPERTIES += foo=true`
+`PRODUCT_SYSTEM_PROPERTIES += foo=false`
+
+In that case, the final value is determined at runtime. The precedence is
+
+* product
+* odm
+* vendor
+* system_ext
+* system
+
+So, `foo` becomes `true` because vendor has higher priority than system.
+
+To temporarily turn the build-time restriction off, use
+
+`BUILD_BROKEN_DUP_SYSPROP := true`
+
+### Optional assignments
+
+System properties can now be set as optional using the new syntax:
+
+`name ?= value`
+
+Then the system property named `name` gets the value `value` only when there
+is no other non-optional assignments having the same name. For example, the
+following is allowed and `foo` gets `true`
+
+`PRODUCT_VENDOR_PROPERTIES += foo=true foo?=false`
+
+Note that the order between the optional and the non-optional assignments
+doesn't matter. The following gives the same result as above.
+
+`PRODUCT_VENDOR_PROPERTIES += foo?=false foo=true`
+
+Optional assignments can be duplicated and in that case their order matters.
+Specifically, the last one eclipses others.
+
+`PRODUCT_VENDOR_PROPERTIES += foo?=apple foo?=banana foo?=mango`
+
+With above, `foo` becomes `mango` since its the last one.
+
+Note that this behavior is different from the previous behavior of preferring
+the first one. To go back to the original behavior for compatability reason,
+use:
+
+`BUILD_BROKEN_DUP_SYSPROP := true`
+
## ELF prebuilts in PRODUCT_COPY_FILES
ELF prebuilts in PRODUCT_COPY_FILES that are installed into these paths are an
diff --git a/core/board_config.mk b/core/board_config.mk
index ae1614f..1fbd989 100644
--- a/core/board_config.mk
+++ b/core/board_config.mk
@@ -93,6 +93,7 @@
BUILD_BROKEN_TREBLE_SYSPROP_NEVERALLOW \
BUILD_BROKEN_USES_NETWORK \
BUILD_BROKEN_VINTF_PRODUCT_COPY_FILES \
+ BUILD_BROKEN_DUP_SYSPROP \
_build_broken_var_list += \
$(foreach m,$(AVAILABLE_BUILD_MODULE_TYPES) \
diff --git a/core/sysprop.mk b/core/sysprop.mk
index 3806a96..aa4fe91 100644
--- a/core/sysprop.mk
+++ b/core/sysprop.mk
@@ -78,6 +78,17 @@
$(eval _resolved_$(name) := $$(call collapse-pairs,$$(_temp),=))\
)
+$(eval # Implement the legacy behavior when BUILD_BROKEN_DUP_SYSPROP is on.)
+$(eval # Optional assignments are all converted to normal assignments and)
+$(eval # when their duplicates the first one wins)
+$(if $(filter true,$(BUILD_BROKEN_DUP_SYSPROP)),\
+ $(foreach name,$(strip $(4)),\
+ $(eval _temp := $$(subst ?=,=,$$(_resolved_$(name))))\
+ $(eval _resolved_$(name) := $$(call uniq-pairs-by-first-component,$$(_resolved_$(name)),=))\
+ )\
+ $(eval _option := --allow-dup)\
+)
+
$(2): $(POST_PROCESS_PROPS) $(INTERNAL_BUILD_ID_MAKEFILE) $(API_FINGERPRINT) $(3)
$(hide) echo Building $$@
$(hide) mkdir -p $$(dir $$@)
@@ -100,7 +111,7 @@
echo "$$(line)" >> $$@;\
)\
)
- $(hide) $(POST_PROCESS_PROPS) $$@ $(5)
+ $(hide) $(POST_PROCESS_PROPS) $$(_option) $$@ $(5)
$(hide) echo "# end of file" >> $$@
endef
diff --git a/tools/post_process_props.py b/tools/post_process_props.py
index 397526f..78a23fb 100755
--- a/tools/post_process_props.py
+++ b/tools/post_process_props.py
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+import argparse
import sys
# Usage: post_process_props.py file.prop [disallowed_key, ...]
@@ -69,7 +70,7 @@
return check_pass
-def override_optional_props(prop_list):
+def override_optional_props(prop_list, allow_dup=False):
"""Override a?=b with a=c, if the latter exists
Overriding is done by deleting a?=b
@@ -88,6 +89,15 @@
# duplicated props are allowed when the all have the same value
if all(overriding_props[0].value == p.value for p in overriding_props):
continue
+ # or if dup is explicitly allowed for compat reason
+ if allow_dup:
+ # this could left one or more optional props unresolved.
+ # Convert them into non-optional because init doesn't understand ?=
+ # syntax
+ for p in optional_props:
+ p.optional = False
+ continue
+
success = False
sys.stderr.write("error: found duplicate sysprop assignments:\n")
for p in overriding_props:
@@ -198,25 +208,30 @@
f.write(str(p) + "\n")
def main(argv):
- filename = argv[1]
+ parser = argparse.ArgumentParser(description="Post-process build.prop file")
+ parser.add_argument("--allow-dup", dest="allow_dup", action="store_true",
+ default=False)
+ parser.add_argument("filename")
+ parser.add_argument("disallowed_keys", metavar="KEY", type=str, nargs="*")
+ args = parser.parse_args()
- if not filename.endswith("/build.prop"):
+ if not args.filename.endswith("/build.prop"):
sys.stderr.write("bad command line: " + str(argv) + "\n")
sys.exit(1)
- props = PropList(filename)
+ props = PropList(args.filename)
mangle_build_prop(props)
- if not override_optional_props(props):
+ if not override_optional_props(props, args.allow_dup):
sys.exit(1)
if not validate(props):
sys.exit(1)
# Drop any disallowed keys
- for key in argv[2:]:
+ for key in args.disallowed_keys:
for p in props.get_props(key):
p.delete("%s is a disallowed key" % key)
- props.write(filename)
+ props.write(args.filename)
if __name__ == "__main__":
main(sys.argv)
diff --git a/tools/test_post_process_props.py b/tools/test_post_process_props.py
index 8a9c3ed..44fe957 100644
--- a/tools/test_post_process_props.py
+++ b/tools/test_post_process_props.py
@@ -226,5 +226,24 @@
# since they have the same value
self.assertTrue(override_optional_props(props))
+ def test_allowDuplicates(self):
+ content = """
+ # comment
+ foo=true
+ bar=false
+ qux?=1
+ foo=false
+ # another comment
+ foo?=false
+ """
+ with patch("post_process_props.open", mock_open(read_data=content)) as m:
+ stderr_redirect = io.StringIO()
+ with contextlib.redirect_stderr(stderr_redirect):
+ props = PropList("hello")
+
+ # we have duplicated foo=true and foo=false, but that's allowed
+ # because it's explicitly allowed
+ self.assertTrue(override_optional_props(props, allow_dup=True))
+
if __name__ == '__main__':
unittest.main(verbosity=2)