Re: [U-Boot] [PATCH v2 25/31] binman: Convert Image to a subclass of Entry

2019-07-17 Thread sjg
When support for sections (and thus hierarchical images) was added to
binman, the decision was made to create a new Section class which could
be used by both Image and an Entry_section class. The decision between
using inheritance and composition was tricky to make, but in the end it
was decided that Image was different enough from Entry that it made sense
to put the implementation of sections in an entirely separate class. It
also has the advantage that core Image code does have to rely on an entry
class in the etype directory.

This work was mostly completed in commit:

   8f1da50ccc "binman: Refactor much of the image code into 'section'

As a result of this, the Section class has its own version of things like
offset and size and these must be kept in sync with the parent
Entry_section class in some cases.

In the last year it has become apparent that the cost of keeping things in
sync is larger than expected, since more and more code wants to access
these properties.

An alternative approach, previously considered and rejected, now seems
better.

Adjust Image to be a subclass of Entry_section. Move the code from Section
(in bsection.py) to Entry_section and delete Section. Update all tests
accordingly.

This requires substantial changes to Image. Overall the changes reduce
code size by about 240 lines. While much of that is just boilerplate from
Section, there are quite a few functions in Entry_section which now do not
need to be overiden from Entry. This suggests the change is beneficial
even without further functionality being added.

A side benefit is that the properties of sections are now consistent with
other entries. This fixes a problem in testListCmd() where some properties
are missing for sections.

Unfortunately this is a very large commit since it is not feasible to do
the migration piecemeal. Given the substantial tests available and the
100% code coverage of binman, we should be able to do this safely.

Signed-off-by: Simon Glass 
---

Changes in v2: None

 tools/binman/README.entries   |  21 +-
 tools/binman/bsection.py  | 523 --
 tools/binman/entry.py |   8 +-
 tools/binman/etype/files.py   |   3 +-
 tools/binman/etype/fmap.py|   2 +-
 tools/binman/etype/section.py | 431 +---
 tools/binman/ftest.py |  29 +-
 tools/binman/image.py | 129 +++--
 tools/binman/image_test.py|  18 +-
 9 files changed, 462 insertions(+), 702 deletions(-)
 delete mode 100644 tools/binman/bsection.py

Applied to u-boot-dm, thanks!
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH v2 25/31] binman: Convert Image to a subclass of Entry

2019-07-08 Thread Simon Glass
When support for sections (and thus hierarchical images) was added to
binman, the decision was made to create a new Section class which could
be used by both Image and an Entry_section class. The decision between
using inheritance and composition was tricky to make, but in the end it
was decided that Image was different enough from Entry that it made sense
to put the implementation of sections in an entirely separate class. It
also has the advantage that core Image code does have to rely on an entry
class in the etype directory.

This work was mostly completed in commit:

   8f1da50ccc "binman: Refactor much of the image code into 'section'

As a result of this, the Section class has its own version of things like
offset and size and these must be kept in sync with the parent
Entry_section class in some cases.

In the last year it has become apparent that the cost of keeping things in
sync is larger than expected, since more and more code wants to access
these properties.

An alternative approach, previously considered and rejected, now seems
better.

Adjust Image to be a subclass of Entry_section. Move the code from Section
(in bsection.py) to Entry_section and delete Section. Update all tests
accordingly.

This requires substantial changes to Image. Overall the changes reduce
code size by about 240 lines. While much of that is just boilerplate from
Section, there are quite a few functions in Entry_section which now do not
need to be overiden from Entry. This suggests the change is beneficial
even without further functionality being added.

A side benefit is that the properties of sections are now consistent with
other entries. This fixes a problem in testListCmd() where some properties
are missing for sections.

Unfortunately this is a very large commit since it is not feasible to do
the migration piecemeal. Given the substantial tests available and the
100% code coverage of binman, we should be able to do this safely.

Signed-off-by: Simon Glass 
---

Changes in v2: None

 tools/binman/README.entries   |  21 +-
 tools/binman/bsection.py  | 523 --
 tools/binman/entry.py |   8 +-
 tools/binman/etype/files.py   |   3 +-
 tools/binman/etype/fmap.py|   2 +-
 tools/binman/etype/section.py | 431 +---
 tools/binman/ftest.py |  29 +-
 tools/binman/image.py | 129 +++--
 tools/binman/image_test.py|  18 +-
 9 files changed, 462 insertions(+), 702 deletions(-)
 delete mode 100644 tools/binman/bsection.py

diff --git a/tools/binman/README.entries b/tools/binman/README.entries
index 598d8278a70..7ce88ee5da8 100644
--- a/tools/binman/README.entries
+++ b/tools/binman/README.entries
@@ -521,16 +521,21 @@ Entry: section: Entry that contains other entries
 -
 
 Properties / Entry arguments: (see binman README for more information)
-- size: Size of section in bytes
-- align-size: Align size to a particular power of two
-- pad-before: Add padding before the entry
-- pad-after: Add padding after the entry
-- pad-byte: Pad byte to use when padding
-- sort-by-offset: Reorder the entries by offset
-- end-at-4gb: Used to build an x86 ROM which ends at 4GB (2^32)
-- name-prefix: Adds a prefix to the name of every entry in the section
+pad-byte: Pad byte to use when padding
+sort-by-offset: True if entries should be sorted by offset, False if
+they must be in-order in the device tree description
+end-at-4gb: Used to build an x86 ROM which ends at 4GB (2^32)
+skip-at-start: Number of bytes before the first entry starts. These
+effectively adjust the starting offset of entries. For example,
+if this is 16, then the first entry would start at 16. An entry
+with offset = 20 would in fact be written at offset 4 in the image
+file, since the first 16 bytes are skipped when writing.
+name-prefix: Adds a prefix to the name of every entry in the section
 when writing out the map
 
+Since a section is also an entry, it inherits all the properies of entries
+too.
+
 A section is an entry which can contain other entries, thus allowing
 hierarchical images to be created. See 'Sections and hierarchical images'
 in the binman README for more information.
diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py
deleted file mode 100644
index 082f424241c..000
--- a/tools/binman/bsection.py
+++ /dev/null
@@ -1,523 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0+
-# Copyright (c) 2018 Google, Inc
-# Written by Simon Glass 
-#
-# Base class for sections (collections of entries)
-#
-
-from __future__ import print_function
-
-from collections import OrderedDict
-import sys
-
-from entry import Entry
-import fdt_util
-import re
-import state
-import tools
-
-class Section(object):
-"""A section which contains multiple entries
-
-A section represents a collection of entries. There must be one or