Merge lp:~salgado/linaro-image-tools/bug-814671 into lp:linaro-image-tools/11.11

[Guilherme Salgado]

This makes lmc fail as soon as we detect we cannot run anything in the chroot. That way we can give a clearer error message. Testing this properly is rather tricky, but you can do it manually by unregistering the qemu-arm-static interpreter and then running lmc:

  sudo update-binfmts --package qemu-user-static --remove qemu-arm /usr/bin/qemu-arm-static
  linaro-media-create ...

After that you can register qemu-arm-static again by running the following:

  sudo update-binfmts --import qemu-arm

[Mattias Backman (mabac)]

Looks good to me. A minor nitpick about the test below.

I think we're fine without unit tests for this change since it's
really straight forward. I have tested it manually according to your
instructions and it works fine.

> === modified file 'linaro_image_tools/media_create/tests/test_media_create.py'
> --- linaro_image_tools/media_create/tests/test_media_create.py  2011-11-15 08:07:46 +0000
> +++ linaro_image_tools/media_create/tests/test_media_create.py  2011-11-16 21:17:32 +0000
> @@ -2682,6 +2682,7 @@
>             'prepare_chroot %(chroot_dir)s %(tmp_dir)s',
>             'cp %(linaro_hwpack_install)s %(chroot_dir)s/usr/bin',
>             'mount proc %(chroot_dir)s/proc -t proc',
> +            'chroot chroot_dir true',

Maybe that could be
                'chroot %(chroot_dir)s true',

just in case someone changes the chroot_dir variable?

>             'cp hwpack1.tgz %(chroot_dir)s',
>             ('%(chroot_args)s %(chroot_dir)s linaro-hwpack-install '
>              '--force-yes /hwpack1.tgz'),
>
>
>

[Mattias Backman (mabac)]

review: Approve

[Guilherme Salgado (salgado)]

On Thu, 2011-11-17 at 11:02 +0000, Mattias Backman wrote:
> Looks good to me. A minor nitpick about the test below.
>
> I think we're fine without unit tests for this change since it's
> really straight forward. I have tested it manually according to your
> instructions and it works fine.
>
> > === modified file 'linaro_image_tools/media_create/tests/test_media_create.py'
> > --- linaro_image_tools/media_create/tests/test_media_create.py 2011-11-15 08:07:46 +0000
> > +++ linaro_image_tools/media_create/tests/test_media_create.py 2011-11-16 21:17:32 +0000
> > @@ -2682,6 +2682,7 @@
> > 'prepare_chroot %(chroot_dir)s %(tmp_dir)s',
> > 'cp %(linaro_hwpack_install)s %(chroot_dir)s/usr/bin',
> > 'mount proc %(chroot_dir)s/proc -t proc',
> > + 'chroot chroot_dir true',
>
> Maybe that could be
> 'chroot %(chroot_dir)s true',
>
> just in case someone changes the chroot_dir variable?

Oh, I don't even consider this a nitpick; we should hard-code as little
as possible in our tests and I should know that. Consider it done, and
thanks for the quick review. :)

[Guilherme Salgado (salgado)]

I'm also wondering whether or not it's a good idea to merge this before we cut the 2011.11 release. I think it may be better to wait and land it after the release as it doesn't fix any defects anyway. Opinions?

[Mattias Backman (mabac)]

On Thu, Nov 17, 2011 at 2:24 PM, Guilherme Salgado
<email address hidden> wrote:
> I'm also wondering whether or not it's a good idea to merge this before we cut the 2011.11 release. I think it may be better to wait and land it after the release as it doesn't fix any defects anyway. Opinions?

You're right that it isn't urgent since no one is blocked by it so we
might as well land it after the release.

> --
> https://code.launchpad.net/~salgado/linaro-image-tools/bug-814671/+merge/82454
> You are reviewing the proposed merge of lp:~salgado/linaro-image-tools/bug-814671 into lp:linaro-image-tools.
>

[lp:~salgado/linaro-image-tools/bug-814671]

462. By Guilherme Salgado on 2011-11-17

Change a test to not hard-code a value

=== modified file 'linaro_image_tools/media_create/chroot_utils.py'
--- linaro_image_tools/media_create/chroot_utils.py	2011-07-25 16:48:37 +0000
+++ linaro_image_tools/media_create/chroot_utils.py	2011-11-17 14:06:23 +0000
@@ -57,6 +57,21 @@
 
     try:
         mount_chroot_proc(chroot_dir)
+        try:
+            # Sometimes the host will have qemu-user-static installed but
+            # another package (i.e. scratchbox) will have mangled its config
+            # and thus we won't be able to chroot and install the hwpack, so
+            # we fail here and tell the user to ensure qemu-arm-static is
+            # setup before trying again.
+            cmd_runner.run(['true'], as_root=True, chroot=chroot_dir).wait()
+        except:
+            print ("Cannot proceed with hwpack installation because "
+                   "there doesn't seem to be a binfmt interpreter registered "
+                   "to execute armel binaries in the chroot. Please check "
+                   "that qemu-user-static is installed and properly "
+                   "configured before trying again.")
+            raise
+
         for hwpack_file in hwpack_files:
             hwpack_verified = False
             if os.path.basename(hwpack_file) in verified_files:

=== modified file 'linaro_image_tools/media_create/tests/test_media_create.py'
--- linaro_image_tools/media_create/tests/test_media_create.py	2011-11-17 09:03:31 +0000
+++ linaro_image_tools/media_create/tests/test_media_create.py	2011-11-17 14:06:23 +0000
@@ -2716,6 +2716,7 @@
             'prepare_chroot %(chroot_dir)s %(tmp_dir)s',
             'cp %(linaro_hwpack_install)s %(chroot_dir)s/usr/bin',
             'mount proc %(chroot_dir)s/proc -t proc',
+            'chroot %(chroot_dir)s true',
             'cp hwpack1.tgz %(chroot_dir)s',
             ('%(chroot_args)s %(chroot_dir)s linaro-hwpack-install '
              '--force-yes /hwpack1.tgz'),