Re: [PATCH 10/17] binman: Allow overriding BuildSectionData()

2021-12-02 Thread Simon Glass
This method is currently marked private. However it is useful to be able
to subclass it, since much of the entry_Section code can be reused. Rename
it.

Also document one confusing part of this code, so people can understand
how to add a test for this case.

Fix up a few pylint warnings to avoid regressing the score.

Signed-off-by: Simon Glass 
---

 tools/binman/etype/section.py | 16 
 tools/binman/ftest.py |  2 +-
 2 files changed, 13 insertions(+), 5 deletions(-)

Applied to u-boot-dm/next, thanks!


[PATCH 10/17] binman: Allow overriding BuildSectionData()

2021-11-23 Thread Simon Glass
This method is currently marked private. However it is useful to be able
to subclass it, since much of the entry_Section code can be reused. Rename
it.

Also document one confusing part of this code, so people can understand
how to add a test for this case.

Fix up a few pylint warnings to avoid regressing the score.

Signed-off-by: Simon Glass 
---

 tools/binman/etype/section.py | 16 
 tools/binman/ftest.py |  2 +-
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 952c01de186..334240384ea 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -182,7 +182,7 @@ class Entry_section(Entry):
 
 return data
 
-def _BuildSectionData(self, required):
+def BuildSectionData(self, required):
 """Build the contents of a section
 
 This places all entries at the right place, dealing with padding before
@@ -190,6 +190,9 @@ class Entry_section(Entry):
 pad-before and pad-after properties in the section items) since that is
 handled by the parent section.
 
+This should be overridden by subclasses which want to build their own
+data structure for the section.
+
 Args:
 required: True if the data must be present, False if it is OK to
 return None
@@ -201,6 +204,9 @@ class Entry_section(Entry):
 
 for entry in self._entries.values():
 entry_data = entry.GetData(required)
+
+# This can happen when this section is referenced from a collection
+# earlier in the image description. See testCollectionSection().
 if not required and entry_data is None:
 return None
 data = self.GetPaddedDataForEntry(entry, entry_data)
@@ -250,7 +256,7 @@ class Entry_section(Entry):
 This excludes any padding. If the section is compressed, the
 compressed data is returned
 """
-data = self._BuildSectionData(required)
+data = self.BuildSectionData(required)
 if data is None:
 return None
 self.SetContents(data)
@@ -278,7 +284,7 @@ class Entry_section(Entry):
 self._SortEntries()
 self._ExpandEntries()
 
-data = self._BuildSectionData(True)
+data = self.BuildSectionData(True)
 self.SetContents(data)
 
 self.CheckSize()
@@ -735,7 +741,9 @@ class Entry_section(Entry):
 nothing.
 
 Args:
-missing: List of missing properties / entry args, each a string
+entry (Entry): Entry to raise the error on
+missing (list of str): List of missing properties / entry args, 
each
+a string
 """
 if not self._ignore_missing:
 missing = ', '.join(missing)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 6a36e8f3158..3982560c47c 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -4533,7 +4533,7 @@ class TestFunctional(unittest.TestCase):
 def testCollectionSection(self):
 """Test a collection where a section must be built first"""
 # Sections never have their contents when GetData() is called, but when
-# _BuildSectionData() is called with required=True, a section will 
force
+# BuildSectionData() is called with required=True, a section will force
 # building the contents, producing an error is anything is still
 # missing.
 data = self._DoReadFile('199_collection_section.dts')
-- 
2.34.0.rc2.393.gf8c9666880-goog