Merge lp:~salgado/linaro-image-tools/save-plain-boot-script into lp:linaro-image-tools/11.11

[Guilherme Salgado]

l-m-c now stores the original boot script (before we feed it to mkimage)
in the boot partition

[James Westby (james-w)]

Hi,

Looks good to me, thanks.

Requesting a review from Loïc as I think that he had some opinions
about this.

Thanks,

James

review: Approve

[Loïc Minier (lool)]

Is this compatible with flash-kernel boot.script usage?

[Guilherme Salgado (salgado)]

On Thu, 2011-02-10 at 20:51 +0000, Loïc Minier wrote:
> Is this compatible with flash-kernel boot.script usage?

flash-kernel's seems to look for the boot.script file under /boot, so I
think the answer is no.

[Loïc Minier (lool)]

Maybe we should write it to /boot instead then?

In general, I think we should treat linaro-image-tools as an installer, and the installed system should allow upgrades etc., but this was also always a lower priority for Linaro and I don't know the specifics of how boot.script was designed/implemented; I lack a good understanding to make a good proposal to push this forward

[Guilherme Salgado (salgado)]

On Tue, 2011-02-15 at 21:06 +0000, Loïc Minier wrote:
> Maybe we should write it to /boot instead then?

We could do that, but it's not as trivial as writing it to the boot
partition -- the function which writes it doesn't have a reference to
the rootfs, so I'm not sure when I'll be able to do it.

> In general, I think we should treat linaro-image-tools as an installer, and the installed system should allow upgrades etc., but this was also always a lower priority for Linaro and I don't know the specifics of how boot.script was designed/implemented; I lack a good understanding to make a good proposal to push this forward

That makes sense, but it is a different use case to the one presented on
the bug. Since the change here supports the bug's use case, how would
you feel about landing it now and filing a separate bug for having the
boot.script written under /boot rather than on the boot partition?

[Loïc Minier (lool)]

I'm a bit concerned about having two "boot.script" files in different locations

Perhaps with another name; perhaps we can simply name it boot.txt and this will also mean that if u-boot moves to supporting text files it gets picked up?

[Loïc Minier (lool)]

http://article.gmane.org/gmane.comp.boot-loaders.u-boot/93267

It should be named uEnv.txt apparently, and should contain an environment, not a boot script

[Alexander Sack (asac)]

just my comment: the bug i filed definitely had in mind that flash-kernel picks this up. hence i wrote /boot/boot.script

[Guilherme Salgado (salgado)]

On Wed, 2011-02-16 at 14:41 +0000, Alexander Sack wrote:
> just my comment: the bug i filed definitely had in mind that flash-kernel picks this up. hence i wrote /boot/boot.script

Fair enough. I think it's better to drop this and and do a separate
change which places the file under /boot. It'd have been nicer if you
said so when I asked on the bug why the request was for the file to be
under /boot, though, but anyway..

[Loïc Minier (lool)]

So this should be fixed to either install a boot.script to root_partition/boot or to install boot.txt to boot_partition, but the current version with boot_partition/boot.script isn't ok.

review: Needs Fixing

[James Westby (james-w)]

I have no objection to the implementation, so I'll leave the decision up to Loïc,
as he is opinionated about the intent.

Thanks,

James

review: Abstain

[Loïc Minier (lool)]

As discussed earlier, I'm fine with a boot.txt in the boot partition; thanks!

review: Approve

=== modified file 'linaro_media_create/boards.py'
--- linaro_media_create/boards.py	2011-02-25 13:19:31 +0000
+++ linaro_media_create/boards.py	2011-02-28 17:22:01 +0000
@@ -498,12 +498,12 @@
 def make_boot_script(boot_script_data, boot_script):
     # Need to save the boot script data into a file that will be passed to
     # mkimage.
-    _, tmpfile = tempfile.mkstemp()
-    atexit.register(os.unlink, tmpfile)
-    with open(tmpfile, 'w') as fd:
+    plain_boot_script = os.path.join(
+        os.path.dirname(boot_script), 'boot.txt')
+    with open(plain_boot_script, 'w') as fd:
         fd.write(boot_script_data)
     return _run_mkimage(
-        'script', '0', '0', 'boot script', tmpfile, boot_script)
+        'script', '0', '0', 'boot script', plain_boot_script, boot_script)
 
 
 def install_mx5_boot_loader(imx_file, boot_device_or_file):
 
=== modified file 'linaro_media_create/tests/test_media_create.py'
--- linaro_media_create/tests/test_media_create.py	2011-02-25 13:19:31 +0000
+++ linaro_media_create/tests/test_media_create.py	2011-02-28 17:22:01 +0000
@@ -606,15 +606,16 @@
         self.assertEqual(expected, fixture.mock.calls)
 
     def test_make_boot_script(self):
-        self.useFixture(MockSomethingFixture(
-            tempfile, 'mkstemp', lambda: (-1, '/tmp/random-abxzr')))
+        tempdir = self.useFixture(CreateTempDirFixture()).tempdir
         self._mock_get_file_matching()
         fixture = self._mock_Popen()
-        make_boot_script('boot script data', 'boot_script')
+        boot_script_path = os.path.join(tempdir, 'boot.scr')
+        plain_boot_script_path = os.path.join(tempdir, 'boot.txt')
+        make_boot_script('boot script data', boot_script_path)
         expected = [
             'sudo', 'mkimage', '-A', 'arm', '-O', 'linux', '-T', 'script',
             '-C', 'none', '-a', '0', '-e', '0', '-n', 'boot script',
-            '-d', '/tmp/random-abxzr', 'boot_script']
+            '-d', plain_boot_script_path, boot_script_path]
         self.assertEqual([expected], fixture.mock.calls)
 
     def test_get_file_matching(self):