diff --git a/Changes.md b/Changes.md index 453ea6c7e0..61e6bb66c5 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 e0bc2f4e5c..a6e586d2ea 100644 --- a/core/board_config.mk +++ b/core/board_config.mk @@ -93,6 +93,7 @@ _build_broken_var_list := \ 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 fdf92b7b52..f1311ed81d 100644 --- a/core/sysprop.mk +++ b/core/sysprop.mk @@ -71,10 +71,22 @@ endef define build-properties ALL_DEFAULT_INSTALLED_MODULES += $(2) -# TODO(b/117892318): eliminate the call to uniq-pairs-by-first-component when -# it is guaranteed that there is no dup. +$(eval # Properties can be assigned using `prop ?= value` or `prop = value` syntax.) +$(eval # Eliminate spaces around the ?= and = separators.) $(foreach name,$(strip $(4)),\ - $(eval _resolved_$(name) := $$(call collapse-pairs, $$(call uniq-pairs-by-first-component,$$($(name)),=)))\ + $(eval _temp := $$(call collapse-pairs,$$($(name)),?=))\ + $(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) @@ -99,7 +111,7 @@ $(2): $(POST_PROCESS_PROPS) $(INTERNAL_BUILD_ID_MAKEFILE) $(API_FINGERPRINT) $(3 echo "$$(line)" >> $$@;\ )\ ) - $(hide) $(POST_PROCESS_PROPS) $$@ $(5) + $(hide) $(POST_PROCESS_PROPS) $$(_option) $$@ $(5) $(hide) echo "# end of file" >> $$@ endef diff --git a/target/board/emulator_arm64/device.mk b/target/board/emulator_arm64/device.mk index 57675d02de..73dc2f4c4f 100644 --- a/target/board/emulator_arm64/device.mk +++ b/target/board/emulator_arm64/device.mk @@ -26,7 +26,3 @@ endif PRODUCT_COPY_FILES += \ $(LOCAL_KERNEL):kernel - -# Adjust the Dalvik heap to be appropriate for a tablet. -$(call inherit-product-if-exists, frameworks/base/build/tablet-dalvik-heap.mk) -$(call inherit-product-if-exists, frameworks/native/build/tablet-dalvik-heap.mk) diff --git a/target/board/generic_arm64/device.mk b/target/board/generic_arm64/device.mk index 3b7cd44acb..b34004fd76 100644 --- a/target/board/generic_arm64/device.mk +++ b/target/board/generic_arm64/device.mk @@ -18,7 +18,3 @@ PRODUCT_COPY_FILES += \ device/google/cuttlefish_kernel/5.4-arm64/kernel-5.4:kernel-5.4 \ device/google/cuttlefish_kernel/5.4-arm64/kernel-5.4-gz:kernel-5.4-gz \ device/google/cuttlefish_kernel/5.4-arm64/kernel-5.4-lz4:kernel-5.4-lz4 - -# Adjust the Dalvik heap to be appropriate for a tablet. -$(call inherit-product-if-exists, frameworks/base/build/tablet-dalvik-heap.mk) -$(call inherit-product-if-exists, frameworks/native/build/tablet-dalvik-heap.mk) diff --git a/target/product/aosp_product.mk b/target/product/aosp_product.mk index f22c3a31f2..a3da1c9319 100644 --- a/target/product/aosp_product.mk +++ b/target/product/aosp_product.mk @@ -23,9 +23,9 @@ $(call inherit-product-if-exists, frameworks/base/data/sounds/AllAudio.mk) # Additional settings used in all AOSP builds PRODUCT_PRODUCT_PROPERTIES += \ - ro.config.ringtone=Ring_Synth_04.ogg \ - ro.config.notification_sound=pixiedust.ogg \ - ro.com.android.dataroaming=true \ + ro.config.ringtone?=Ring_Synth_04.ogg \ + ro.config.notification_sound?=pixiedust.ogg \ + ro.com.android.dataroaming?=true \ # More AOSP packages PRODUCT_PACKAGES += \ diff --git a/target/product/base_system.mk b/target/product/base_system.mk index bec550bc24..f6770fb69b 100644 --- a/target/product/base_system.mk +++ b/target/product/base_system.mk @@ -348,7 +348,7 @@ PRODUCT_BOOT_JARS += android.test.base endif PRODUCT_COPY_FILES += system/core/rootdir/init.zygote32.rc:system/etc/init/hw/init.zygote32.rc -PRODUCT_SYSTEM_PROPERTIES += ro.zygote=zygote32 +PRODUCT_SYSTEM_PROPERTIES += ro.zygote?=zygote32 PRODUCT_SYSTEM_PROPERTIES += debug.atrace.tags.enableflags=0 diff --git a/target/product/full_base.mk b/target/product/full_base.mk index dfb22047ee..64f61ffbcb 100644 --- a/target/product/full_base.mk +++ b/target/product/full_base.mk @@ -43,8 +43,8 @@ PRODUCT_PACKAGES += \ # Additional settings used in all AOSP builds PRODUCT_VENDOR_PROPERTIES := \ - ro.config.ringtone=Ring_Synth_04.ogg \ - ro.config.notification_sound=pixiedust.ogg + ro.config.ringtone?=Ring_Synth_04.ogg \ + ro.config.notification_sound?=pixiedust.ogg # Put en_US first in the list, so make it default. PRODUCT_LOCALES := en_US diff --git a/target/product/full_base_telephony.mk b/target/product/full_base_telephony.mk index 5e18c058c7..d8a54cd7f3 100644 --- a/target/product/full_base_telephony.mk +++ b/target/product/full_base_telephony.mk @@ -20,8 +20,8 @@ # entirely appropriate to inherit from for on-device configurations. PRODUCT_VENDOR_PROPERTIES := \ - keyguard.no_require_sim=true \ - ro.com.android.dataroaming=true + keyguard.no_require_sim?=true \ + ro.com.android.dataroaming?=true PRODUCT_COPY_FILES := \ device/sample/etc/apns-full-conf.xml:system/etc/apns-conf.xml \ diff --git a/target/product/handheld_system.mk b/target/product/handheld_system.mk index 22c817e54b..e2c91b666b 100644 --- a/target/product/handheld_system.mk +++ b/target/product/handheld_system.mk @@ -84,6 +84,6 @@ PRODUCT_COPY_FILES += \ frameworks/av/media/libeffects/data/audio_effects.conf:system/etc/audio_effects.conf PRODUCT_VENDOR_PROPERTIES += \ - ro.carrier=unknown \ - ro.config.notification_sound=OnTheHunt.ogg \ - ro.config.alarm_alert=Alarm_Classic.ogg + ro.carrier?=unknown \ + ro.config.notification_sound?=OnTheHunt.ogg \ + ro.config.alarm_alert?=Alarm_Classic.ogg diff --git a/target/product/media_system.mk b/target/product/media_system.mk index a3fafb3426..7a2dd73e76 100644 --- a/target/product/media_system.mk +++ b/target/product/media_system.mk @@ -74,7 +74,7 @@ PRODUCT_COPY_FILES += $(call add-to-product-copy-files-if-exists,\ # On userdebug builds, collect more tombstones by default. ifneq (,$(filter userdebug eng,$(TARGET_BUILD_VARIANT))) PRODUCT_VENDOR_PROPERTIES += \ - tombstoned.max_tombstone_count=50 + tombstoned.max_tombstone_count?=50 endif PRODUCT_VENDOR_PROPERTIES += \ diff --git a/target/product/runtime_libart.mk b/target/product/runtime_libart.mk index 78a4af09cc..b96601ddd3 100644 --- a/target/product/runtime_libart.mk +++ b/target/product/runtime_libart.mk @@ -57,23 +57,23 @@ PRODUCT_SYSTEM_PROPERTIES += \ # On eng builds, make "boot" reasons only extract for faster turnaround. ifeq (eng,$(TARGET_BUILD_VARIANT)) PRODUCT_SYSTEM_PROPERTIES += \ - pm.dexopt.first-boot=extract \ - pm.dexopt.boot=extract + pm.dexopt.first-boot?=extract \ + pm.dexopt.boot?=extract else PRODUCT_SYSTEM_PROPERTIES += \ - pm.dexopt.first-boot=quicken \ - pm.dexopt.boot=verify + pm.dexopt.first-boot?=quicken \ + pm.dexopt.boot?=verify endif # The install filter is speed-profile in order to enable the use of # profiles from the dex metadata files. Note that if a profile is not provided # or if it is empty speed-profile is equivalent to (quicken + empty app image). PRODUCT_SYSTEM_PROPERTIES += \ - pm.dexopt.install=speed-profile \ - pm.dexopt.bg-dexopt=speed-profile \ - pm.dexopt.ab-ota=speed-profile \ - pm.dexopt.inactive=verify \ - pm.dexopt.shared=speed + pm.dexopt.install?=speed-profile \ + pm.dexopt.bg-dexopt?=speed-profile \ + pm.dexopt.ab-ota?=speed-profile \ + pm.dexopt.inactive?=verify \ + pm.dexopt.shared?=speed # Pass file with the list of updatable boot class path packages to dex2oat. PRODUCT_SYSTEM_PROPERTIES += \ diff --git a/tools/Android.bp b/tools/Android.bp index 159890ce49..149d06df4c 100644 --- a/tools/Android.bp +++ b/tools/Android.bp @@ -37,3 +37,22 @@ python_binary_host { }, }, } + +python_test_host { + name: "post_process_props_unittest", + main: "test_post_process_props.py", + srcs: [ + "post_process_props.py", + "test_post_process_props.py", + ], + version: { + py2: { + enabled: false, + }, + py3: { + enabled: true, + }, + }, + test_config: "post_process_props_unittest.xml", + test_suites: ["general-tests"], +} diff --git a/tools/post_process_props.py b/tools/post_process_props.py index 4fa15bcd61..d8c9cb157f 100755 --- a/tools/post_process_props.py +++ b/tools/post_process_props.py @@ -14,10 +14,11 @@ # 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 [blacklist_key, ...] -# Blacklisted keys are removed from the property file, if present +# Usage: post_process_props.py file.prop [disallowed_key, ...] +# Disallowed keys are removed from the property file, if present # See PROP_VALUE_MAX in system_properties.h. # The constant in system_properties.h includes the terminating NUL, @@ -29,8 +30,8 @@ PROP_VALUE_MAX = 91 def mangle_build_prop(prop_list): # If ro.debuggable is 1, then enable adb on USB by default # (this is for userdebug builds) - if prop_list.get("ro.debuggable") == "1": - val = prop_list.get("persist.sys.usb.config") + if prop_list.get_value("ro.debuggable") == "1": + val = prop_list.get_value("persist.sys.usb.config") if "adb" not in val: if val == "": val = "adb" @@ -40,52 +41,132 @@ def mangle_build_prop(prop_list): # UsbDeviceManager expects a value here. If it doesn't get it, it will # default to "adb". That might not the right policy there, but it's better # to be explicit. - if not prop_list.get("persist.sys.usb.config"): + if not prop_list.get_value("persist.sys.usb.config"): prop_list.put("persist.sys.usb.config", "none"); def validate(prop_list): """Validate the properties. + If the value of a sysprop exceeds the max limit (91), it's an error, unless + the sysprop is a read-only one. + + Checks if there is no optional prop assignments. + Returns: True if nothing is wrong. """ check_pass = True - for p in prop_list.get_all(): + for p in prop_list.get_all_props(): if len(p.value) > PROP_VALUE_MAX and not p.name.startswith("ro."): check_pass = False sys.stderr.write("error: %s cannot exceed %d bytes: " % (p.name, PROP_VALUE_MAX)) sys.stderr.write("%s (%d)\n" % (p.value, len(p.value))) + + if p.is_optional(): + check_pass = False + sys.stderr.write("error: found unresolved optional prop assignment:\n") + sys.stderr.write(str(p) + "\n") + return check_pass +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 + When there are a?=b and a?=c, then only the last one survives + When there are a=b and a=c, then it's an error. + + Returns: + True if the override was successful + """ + success = True + for name in prop_list.get_all_names(): + props = prop_list.get_props(name) + optional_props = [p for p in props if p.is_optional()] + overriding_props = [p for p in props if not p.is_optional()] + if len(overriding_props) > 1: + # duplicated props are allowed when the all have the same value + if all(overriding_props[0].value == p.value for p in overriding_props): + for p in optional_props: + p.delete("overridden by %s" % str(overriding_props[0])) + 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: + sys.stderr.write("%s\n" % str(p)) + elif len(overriding_props) == 1: + for p in optional_props: + p.delete("overridden by %s" % str(overriding_props[0])) + else: + if len(optional_props) > 1: + for p in optional_props[:-1]: + p.delete("overridden by %s" % str(optional_props[-1])) + # Make the last optional one as non-optional + optional_props[-1].optional = False + + return success + class Prop: - def __init__(self, name, value, comment=None): + def __init__(self, name, value, optional=False, comment=None): self.name = name.strip() self.value = value.strip() - self.comment = comment + if comment != None: + self.comments = [comment] + else: + self.comments = [] + self.optional = optional @staticmethod def from_line(line): line = line.rstrip('\n') if line.startswith("#"): - return Prop("", "", line) + return Prop("", "", comment=line) + elif "?=" in line: + name, value = line.split("?=", 1) + return Prop(name, value, optional=True) elif "=" in line: name, value = line.split("=", 1) - return Prop(name, value) + return Prop(name, value, optional=False) else: # don't fail on invalid line # TODO(jiyong) make this a hard error - return Prop("", "", line) + return Prop("", "", comment=line) def is_comment(self): - return self.comment != None + return bool(self.comments and not self.name) + + def is_optional(self): + return (not self.is_comment()) and self.optional + + def make_as_comment(self): + # Prepend "#" to the last line which is the prop assignment + if not self.is_comment(): + assignment = str(self).rsplit("\n", 1)[-1] + self.comments.append("#" + assignment) + self.name = "" + self.value = "" + + def delete(self, reason): + self.comments.append("# Removed by post_process_props.py because " + reason) + self.make_as_comment() def __str__(self): - if self.is_comment(): - return self.comment - else: - return self.name + "=" + self.value + assignment = [] + if not self.is_comment(): + operator = "?=" if self.is_optional() else "=" + assignment.append(self.name + operator + self.value) + return "\n".join(self.comments + assignment) class PropList: @@ -94,47 +175,65 @@ class PropList: self.props = [Prop.from_line(l) for l in f.readlines() if l.strip() != ""] - def get_all(self): + def get_all_props(self): return [p for p in self.props if not p.is_comment()] - def get(self, name): + def get_all_names(self): + return set([p.name for p in self.get_all_props()]) + + def get_props(self, name): + return [p for p in self.get_all_props() if p.name == name] + + def get_value(self, name): + # Caution: only the value of the first sysprop having the name is returned. return next((p.value for p in self.props if p.name == name), "") def put(self, name, value): - index = next((i for i,p in enumerate(self.props) if p.name == name), -1) + # Note: when there is an optional prop for the name, its value isn't changed. + # Instead a new non-optional prop is appended, which will override the + # optional prop. Otherwise, the new value might be overridden by an existing + # non-optional prop of the same name. + index = next((i for i,p in enumerate(self.props) + if p.name == name and not p.is_optional()), -1) if index == -1: - self.props.append(Prop(name, value)) + self.props.append(Prop(name, value, + comment="# Auto-added by post_process_props.py")) else: + self.props[index].comments.append( + "# Value overridden by post_process_props.py. Original value: %s" % + self.props[index].value) self.props[index].value = value - def delete(self, name): - index = next((i for i,p in enumerate(self.props) if p.name == name), -1) - if index != -1: - new_comment = "# removed by post_process_props.py\n#" + str(self.props[index]) - self.props[index] = Prop.from_line(new_comment) - def write(self, filename): with open(filename, 'w+') as f: for p in self.props: 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, args.allow_dup): + sys.exit(1) if not validate(props): sys.exit(1) - # Drop any blacklisted keys - for key in argv[2:]: - props.delete(key) + # Drop any disallowed keys + 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/post_process_props_unittest.xml b/tools/post_process_props_unittest.xml new file mode 100644 index 0000000000..4a6ecc2c6a --- /dev/null +++ b/tools/post_process_props_unittest.xml @@ -0,0 +1,6 @@ + + + + diff --git a/tools/test_post_process_props.py b/tools/test_post_process_props.py new file mode 100644 index 0000000000..12d52e566d --- /dev/null +++ b/tools/test_post_process_props.py @@ -0,0 +1,255 @@ +#!/usr/bin/env python3 +# +# Copyright (C) 2020 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import contextlib +import io +import unittest + +from unittest.mock import * +from post_process_props import * + +class PropTestCase(unittest.TestCase): + def test_createFromLine(self): + p = Prop.from_line("# this is comment") + self.assertTrue(p.is_comment()) + self.assertEqual("", p.name) + self.assertEqual("", p.value) + self.assertFalse(p.is_optional()) + self.assertEqual("# this is comment", str(p)) + + for line in ["a=b", "a = b", "a= b", "a =b", " a=b "]: + p = Prop.from_line(line) + self.assertFalse(p.is_comment()) + self.assertEqual("a", p.name) + self.assertEqual("b", p.value) + self.assertFalse(p.is_optional()) + self.assertEqual("a=b", str(p)) + + for line in ["a?=b", "a ?= b", "a?= b", "a ?=b", " a?=b "]: + p = Prop.from_line(line) + self.assertFalse(p.is_comment()) + self.assertEqual("a", p.name) + self.assertEqual("b", p.value) + self.assertTrue(p.is_optional()) + self.assertEqual("a?=b", str(p)) + + def test_makeAsComment(self): + p = Prop.from_line("a=b") + p.comments.append("# a comment") + self.assertFalse(p.is_comment()) + + p.make_as_comment() + self.assertTrue(p.is_comment()) + self.assertTrue("# a comment\n#a=b", str(p)) + +class PropListTestcase(unittest.TestCase): + def setUp(self): + content = """ + # comment + foo=true + bar=false + qux?=1 + # another comment + foo?=false + """ + self.patcher = patch("post_process_props.open", mock_open(read_data=content)) + self.mock_open = self.patcher.start() + self.props = PropList("file") + + def tearDown(self): + self.patcher.stop() + self.props = None + + def test_readFromFile(self): + self.assertEqual(4, len(self.props.get_all_props())) + expected = [ + ("foo", "true", False), + ("bar", "false", False), + ("qux", "1", True), + ("foo", "false", True) + ] + for i,p in enumerate(self.props.get_all_props()): + self.assertEqual(expected[i][0], p.name) + self.assertEqual(expected[i][1], p.value) + self.assertEqual(expected[i][2], p.is_optional()) + self.assertFalse(p.is_comment()) + + self.assertEqual(set(["foo", "bar", "qux"]), self.props.get_all_names()) + + self.assertEqual("true", self.props.get_value("foo")) + self.assertEqual("false", self.props.get_value("bar")) + self.assertEqual("1", self.props.get_value("qux")) + + # there are two assignments for 'foo' + self.assertEqual(2, len(self.props.get_props("foo"))) + + def test_putNewProp(self): + self.props.put("new", "30") + + self.assertEqual(5, len(self.props.get_all_props())) + last_prop = self.props.get_all_props()[-1] + self.assertEqual("new", last_prop.name) + self.assertEqual("30", last_prop.value) + self.assertFalse(last_prop.is_optional()) + + def test_putExistingNonOptionalProp(self): + self.props.put("foo", "NewValue") + + self.assertEqual(4, len(self.props.get_all_props())) + foo_prop = self.props.get_props("foo")[0] + self.assertEqual("foo", foo_prop.name) + self.assertEqual("NewValue", foo_prop.value) + self.assertFalse(foo_prop.is_optional()) + self.assertEqual("# Value overridden by post_process_props.py. " + + "Original value: true\nfoo=NewValue", str(foo_prop)) + + def test_putExistingOptionalProp(self): + self.props.put("qux", "2") + + self.assertEqual(5, len(self.props.get_all_props())) + last_prop = self.props.get_all_props()[-1] + self.assertEqual("qux", last_prop.name) + self.assertEqual("2", last_prop.value) + self.assertFalse(last_prop.is_optional()) + self.assertEqual("# Auto-added by post_process_props.py\nqux=2", + str(last_prop)) + + def test_deleteNonOptionalProp(self): + props_to_delete = self.props.get_props("foo")[0] + props_to_delete.delete(reason="testing") + + self.assertEqual(3, len(self.props.get_all_props())) + self.assertEqual("# Removed by post_process_props.py because testing\n" + + "#foo=true", str(props_to_delete)) + + def test_deleteOptionalProp(self): + props_to_delete = self.props.get_props("qux")[0] + props_to_delete.delete(reason="testing") + + self.assertEqual(3, len(self.props.get_all_props())) + self.assertEqual("# Removed by post_process_props.py because testing\n" + + "#qux?=1", str(props_to_delete)) + + def test_overridingNonOptional(self): + props_to_be_overridden = self.props.get_props("foo")[1] + self.assertTrue("true", props_to_be_overridden.value) + + self.assertTrue(override_optional_props(self.props)) + + # size reduced to 3 because foo?=false was overridden by foo=true + self.assertEqual(3, len(self.props.get_all_props())) + + self.assertEqual(1, len(self.props.get_props("foo"))) + self.assertEqual("true", self.props.get_props("foo")[0].value) + + self.assertEqual("# Removed by post_process_props.py because " + + "overridden by foo=true\n#foo?=false", + str(props_to_be_overridden)) + + def test_overridingOptional(self): + content = """ + # comment + qux?=2 + foo=true + bar=false + qux?=1 + # another comment + foo?=false + """ + with patch('post_process_props.open', mock_open(read_data=content)) as m: + props = PropList("hello") + + props_to_be_overridden = props.get_props("qux")[0] + self.assertEqual("2", props_to_be_overridden.value) + + self.assertTrue(override_optional_props(props)) + + self.assertEqual(1, len(props.get_props("qux"))) + self.assertEqual("1", props.get_props("qux")[0].value) + # the only left optional assignment becomes non-optional + self.assertFalse(props.get_props("qux")[0].is_optional()) + + self.assertEqual("# Removed by post_process_props.py because " + + "overridden by qux?=1\n#qux?=2", + str(props_to_be_overridden)) + + def test_overridingDuplicated(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") + + # fails due to duplicated foo=true and foo=false + self.assertFalse(override_optional_props(props)) + + self.assertEqual("error: found duplicate sysprop assignments:\n" + + "foo=true\nfoo=false\n", stderr_redirect.getvalue()) + + def test_overridingDuplicatedWithSameValue(self): + content = """ + # comment + foo=true + bar=false + qux?=1 + foo=true + # 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") + optional_prop = props.get_props("foo")[2] # the last foo?=false one + + # we have duplicated foo=true and foo=true, but that's allowed + # since they have the same value + self.assertTrue(override_optional_props(props)) + + # foo?=false should be commented out + self.assertEqual("# Removed by post_process_props.py because " + + "overridden by foo=true\n#foo?=false", + str(optional_prop)) + + 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)