Merge lp:~milo/linaro-image-tools/bug1054422 into lp:linaro-image-tools/11.11

Proposed by Milo Casagrande
Status: Merged
Approved by: Данило Шеган
Approved revision: 568
Merged at revision: 565
Proposed branch: lp:~milo/linaro-image-tools/bug1054422
Merge into: lp:linaro-image-tools/11.11
Diff against target: 99 lines (+36/-6)
4 files modified
linaro_image_tools/hwpack/hardwarepack.py (+3/-3)
linaro_image_tools/hwpack/hwpack_fields.py (+1/-1)
linaro_image_tools/hwpack/tests/test_config_v3.py (+29/-0)
linaro_image_tools/utils.py (+3/-2)
To merge this branch: bzr merge lp:~milo/linaro-image-tools/bug1054422
Reviewer Review Type Date Requested Status
Данило Шеган (community) Approve
Paul Sokolovsky Pending
Linaro Infrastructure Pending
Review via email: mp+125995@code.launchpad.net

Description of the change

Changes in this MP fixes problem with bug 1054422. Added also some tests for Samsung specific fields.
There are also two minor fixes found while looking at the code.

To post a comment you must log in.
Revision history for this message
Данило Шеган (danilo) wrote :

Looks good, thanks for the nice fix.

However small the other fixes are, we should generally strive to keep them in separate branches, especially for things where we plan to do a last-minute re-rollout of the tarball: it would be much better to include only relevant changes (it's not going to be obvious what's going on in the commit log if you lump these three changes together, no matter how verbose you make the commit message; it's neither going to be clear why were the other two changes critical and that they needed a .1 release :).

review: Approve
Revision history for this message
Milo Casagrande (milo) wrote :

Hi Danilo,

thanks for the review.

On Mon, Sep 24, 2012 at 9:18 PM, Данило Шеган <email address hidden> wrote:
>
> However small the other fixes are, we should generally strive to keep them in separate branches, especially for things where we plan to do a last-minute re-rollout of the tarball: it would be much better to include only relevant changes (it's not going to be obvious what's going on in the commit log if you lump these three changes together, no matter how verbose you make the commit message; it's neither going to be clear why were the other two changes critical and that they needed a .1 release :).

Yeah, I know, I was not sure to include them here.
I can merge only the two commits for the bug fix, and after the
rollout merge the remaining two with the small fixes.

--
Milo Casagrande
Infrastructure Engineer
Linaro.org <www.linaro.org> │ Open source software for ARM SoCs

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'linaro_image_tools/hwpack/hardwarepack.py'
--- linaro_image_tools/hwpack/hardwarepack.py 2012-08-29 10:31:19 +0000
+++ linaro_image_tools/hwpack/hardwarepack.py 2012-09-24 12:06:24 +0000
@@ -166,14 +166,14 @@
166 self.samsung_bl2_len = samsung_bl2_len166 self.samsung_bl2_len = samsung_bl2_len
167167
168 @classmethod168 @classmethod
169 def add_v3_config(self, boards=None, bootloaders=None):169 def add_v3_config(cls, boards=None, bootloaders=None):
170 """Add fields that are specific to the v3 config format.170 """Add fields that are specific to the v3 config format.
171 These fields are not present in the earlier config files.171 These fields are not present in the earlier config files.
172172
173 :param boards: The boards section of the hwpack.173 :param boards: The boards section of the hwpack.
174 :param bootloaders: The bootloaders section of the hwpack."""174 :param bootloaders: The bootloaders section of the hwpack."""
175 self.boards = boards175 cls.boards = boards
176 self.bootloaders = bootloaders176 cls.bootloaders = bootloaders
177177
178 @classmethod178 @classmethod
179 def from_config(cls, config, version, architecture):179 def from_config(cls, config, version, architecture):
180180
=== modified file 'linaro_image_tools/hwpack/hwpack_fields.py'
--- linaro_image_tools/hwpack/hwpack_fields.py 2012-08-28 06:02:43 +0000
+++ linaro_image_tools/hwpack/hwpack_fields.py 2012-09-24 12:06:24 +0000
@@ -129,7 +129,7 @@
129 ROOT_MIN_SIZE_FIELD: None,129 ROOT_MIN_SIZE_FIELD: None,
130 LOADER_MIN_SIZE_FIELD: None,130 LOADER_MIN_SIZE_FIELD: None,
131 SAMSUNG_BL1_LEN_FIELD: None,131 SAMSUNG_BL1_LEN_FIELD: None,
132 SAMSUNG_BL1_LEN_FIELD: None,132 SAMSUNG_BL1_START_FIELD: None,
133 SAMSUNG_ENV_LEN_FIELD: None,133 SAMSUNG_ENV_LEN_FIELD: None,
134 SAMSUNG_BL2_LEN_FIELD: None,134 SAMSUNG_BL2_LEN_FIELD: None,
135 SNOWBALL_STARTUP_FILES_CONFIG_FIELD: None,135 SNOWBALL_STARTUP_FILES_CONFIG_FIELD: None,
136136
=== modified file 'linaro_image_tools/hwpack/tests/test_config_v3.py'
--- linaro_image_tools/hwpack/tests/test_config_v3.py 2012-08-28 17:34:16 +0000
+++ linaro_image_tools/hwpack/tests/test_config_v3.py 2012-09-24 12:06:24 +0000
@@ -26,6 +26,10 @@
26from linaro_image_tools.hwpack.config import Config, HwpackConfigError26from linaro_image_tools.hwpack.config import Config, HwpackConfigError
27from linaro_image_tools.hwpack.hwpack_fields import (27from linaro_image_tools.hwpack.hwpack_fields import (
28 DEFINED_PARTITION_LAYOUTS,28 DEFINED_PARTITION_LAYOUTS,
29 SAMSUNG_BL1_LEN_FIELD,
30 SAMSUNG_BL1_START_FIELD,
31 SAMSUNG_BL2_LEN_FIELD,
32 SAMSUNG_ENV_LEN_FIELD,
29)33)
3034
3135
@@ -796,3 +800,28 @@
796 self.assertValidationError("Unknown key in metadata: "800 self.assertValidationError("Unknown key in metadata: "
797 "'boards: snowball: foo'",801 "'boards: snowball: foo'",
798 config._validate_keys)802 config._validate_keys)
803
804 def test_valid_samsung_bl1_len_field(self):
805 config = self.get_config(self.valid_start_v3 +
806 SAMSUNG_BL1_LEN_FIELD + ': 1\n')
807 self.assertEqual(None, config._validate_keys())
808
809 def test_valid_samsung_bl1_start_field(self):
810 config = self.get_config(self.valid_start_v3 +
811 SAMSUNG_BL1_START_FIELD + ': 1\n')
812 self.assertEqual(None, config._validate_keys())
813
814 def test_valid_samsung_bl2_len_field(self):
815 config = self.get_config(self.valid_start_v3 +
816 SAMSUNG_BL2_LEN_FIELD + ': 1\n')
817 self.assertEqual(None, config._validate_keys())
818
819 def test_valid_samsung_env_len_field(self):
820 config = self.get_config(self.valid_start_v3 +
821 SAMSUNG_ENV_LEN_FIELD + ': 1\n')
822 self.assertEqual(None, config._validate_keys())
823
824 def test_samsung_field_wrong(self):
825 config = self.get_config(self.valid_start_v3 +
826 'samsung_wrong_field: 1\n')
827 self.assertRaises(HwpackConfigError, config._validate_keys)
799828
=== modified file 'linaro_image_tools/utils.py'
--- linaro_image_tools/utils.py 2012-07-26 19:49:43 +0000
+++ linaro_image_tools/utils.py 2012-09-24 12:06:24 +0000
@@ -76,13 +76,14 @@
7676
7777
78def path_in_tarfile_exists(path, tar_file):78def path_in_tarfile_exists(path, tar_file):
79 tarinfo = tarfile.open(tar_file, 'r:gz')
80 try:79 try:
80 tarinfo = tarfile.open(tar_file, 'r:gz')
81 tarinfo.getmember(path)81 tarinfo.getmember(path)
82 return True82 return True
83 except KeyError:83 except KeyError:
84 return False84 return False
85 tarinfo.close()85 finally:
86 tarinfo.close()
8687
8788
88def verify_file_integrity(sig_file_list):89def verify_file_integrity(sig_file_list):

Subscribers

People subscribed via source and target branches