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

Commit 598ea498 authored by Jiyong Park's avatar Jiyong Park
Browse files

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 7aeb8de7)
Change-Id: I9c073a21c8257987cf2378012cadaeeeb698a4fb
parent 578b2ba0
Loading
Loading
Loading
Loading
+4 −3
Original line number Diff line number Diff line
@@ -71,10 +71,11 @@ 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),=))\
)

$(2): $(POST_PROCESS_PROPS) $(INTERNAL_BUILD_ID_MAKEFILE) $(API_FINGERPRINT) $(3)
+19 −0
Original line number Diff line number Diff line
@@ -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"],
}
+110 −28
Original line number Diff line number Diff line
@@ -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 @@ 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 +40,121 @@ 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):
  """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 @@ 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:
@@ -127,12 +206,15 @@ def main(argv):

  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)

+6 −0
Original line number Diff line number Diff line
<configuration description="Config to run post_process_props_unittest">
    <test class="com.android.tradefed.testtype.python.PythonBinaryHostTest" >
        <option name="par-file-name" value="post_process_props_unittest" />
        <option name="test-timeout" value="1m" />
    </test>
</configuration>
+230 −0
Original line number Diff line number Diff line
#!/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")

        # we have duplicated foo=true and foo=true, but that's allowed
        # since they have the same value
        self.assertTrue(override_optional_props(props))

if __name__ == '__main__':
    unittest.main(verbosity=2)