Support optional prop assignments
This CL adds a number of changes to make the assignment of system
properties to be less confusing.
1. Added `a ?= b` syntax, which is called optional prop assignments. The
prop `a` gets the value `b` only when there is no non-optional prop
assignment for `a` such as `a = c`. This is useful for props that
provide some reasonable default values as fallback.
2. With the introduction of the optional prop assignment syntax,
duplicated non-optional assignments is prohibited; e.g., the follwing
now triggers a build-time error:
a = b
a = c
, but the following doesn't:
a ?= b
a = c
Note that the textual order between the optional and non-optional
assignments doesn't matter. The non-optional assignment eclipses the
optional assignment even when the former appears 'before' the latter.
a = c
a ?= b
In the above, `a` gets the value `c`
When there are multiple optional assignments without a non-optional
assignments as shown below, the last one wins:
a ?= b
a ?= c
`a` becomes `c`. Specifically, the former assignment is commented out
and the latter is converted to a non-optional assignment.
3. post_process_props.py is modified so that when a prop assignment is
deleted, changed, or added, the changes are recorded as comments. This
is to aid debugging. Previously, it was often difficult to find out why
a certain sysprop assignment is missing or is added.
4. post_process_prop.py now has a unittest
Bug: 117892318
Bug: 158735147
Test: atest --host post_process_prop_unittest
Exempt-From-Owner-Approval: cherry-pick from master
Merged-In: I9c073a21c8257987cf2378012cadaeeeb698a4fb
(cherry picked from commit 7aeb8de74e08eb2d305686aa8eff45353973e7d7)
Change-Id: I9c073a21c8257987cf2378012cadaeeeb698a4fb
diff --git a/tools/post_process_props.py b/tools/post_process_props.py
index 4fa15bc..397526f 100755
--- a/tools/post_process_props.py
+++ b/tools/post_process_props.py
@@ -16,8 +16,8 @@
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 +29,8 @@
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 +40,121 @@
# 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):
+ """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):
+ 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,25 +163,35 @@
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:
@@ -127,12 +206,15 @@
props = PropList(filename)
mangle_build_prop(props)
+ if not override_optional_props(props):
+ sys.exit(1)
if not validate(props):
sys.exit(1)
- # Drop any blacklisted keys
+ # Drop any disallowed keys
for key in argv[2:]:
- props.delete(key)
+ for p in props.get_props(key):
+ p.delete("%s is a disallowed key" % key)
props.write(filename)