Merge lp:~salgado/linaro-image-tools/hwpack-install into lp:linaro-image-tools/11.11

[Guilherme Salgado]

This adds a linaro-hwpack-install script which must be given a path to a tarball containing the hwpack. This version of the script can be run on a ubuntu-minimal chroot, and I plan to provide a second script to be run outside of the chroot, which is able to download a tarball from a remote location (via wget/curl) and delegates to this one for the actual installation.

I'm mostly looking for feedback as to whether or not this approach is sensible, although a proper review would be great.

[James Westby (james-w)]

30 +TEMP_DIR="/var/tmp/hwpack"

it would be good to have that not be static, and create a tempdir
with an unpredictable name, using e.g. mktemp.

64 + sudo rm -f $HWPACK_PKGS_SOURCE
65 + sudo apt-get update -qq

I don't see why sudo would be needed? This will run as root won't it?

101 +for file in `ls "${HWPACK_DIR}"/sources.list.d.gpg/`; do

This can use a * expansion as the sources.list.d code does couldn't it?

Otherwise this looks good. Short and sweet and we should try and keep
it that way.

Thanks,

James

review: Approve

[James Westby (james-w)]

Hi,

I've changed the spec and the creation code to use "package=version"
in the manifest as requested, so that will need to be changed
here too.

Thanks,

James

[lp:~salgado/linaro-image-tools/hwpack-install]

62. By Guilherme Salgado on 2010-09-13

merge trunk

[Guilherme Salgado (salgado)]

On Wed, 2010-09-08 at 21:38 +0000, James Westby wrote:
> Review: Approve
> 30 +TEMP_DIR="/var/tmp/hwpack"
>
> it would be good to have that not be static, and create a tempdir
> with an unpredictable name, using e.g. mktemp.

Sure thing; changed.

>
> 64 + sudo rm -f $HWPACK_PKGS_SOURCE
> 65 + sudo apt-get update -qq
>
> I don't see why sudo would be needed? This will run as root won't it?

Not necessarily. When you run it inside the chroot you might run it as
any user, and that's how I've been testing it.

>
> 101 +for file in `ls "${HWPACK_DIR}"/sources.list.d.gpg/`; do
>
> This can use a * expansion as the sources.list.d code does couldn't it?

At first I thought I could but after trying it I realized that may not
be desirable as it would cause the file variable to point to
'/.../sources.list.d.gpg/*' when the directory is empty, and that will
cause apt-key to fail. Unless we can assume hwpacks will always include
a non-empty sources.list.d.gpg, I think we might want to keep it that
way. And maybe even change the other loop for sources.list.d.

>
> Otherwise this looks good. Short and sweet and we should try and keep
> it that way.

Thanks!

--
Guilherme Salgado <https://launchpad.net/~salgado>

[lp:~salgado/linaro-image-tools/hwpack-install]

63. By Guilherme Salgado on 2010-09-13

Use mktemp as suggested by James

64. By Guilherme Salgado on 2010-09-13

Change parsing of the manifest file in linaro-hwpack-install as it now uses an equal sign to separate packages from their version

[James Westby (james-w)]

On Mon, 13 Sep 2010 13:31:15 -0000, Guilherme Salgado <email address hidden> wrote:
> > 64 + sudo rm -f $HWPACK_PKGS_SOURCE
> > 65 + sudo apt-get update -qq
> >
> > I don't see why sudo would be needed? This will run as root won't it?
>
> Not necessarily. When you run it inside the chroot you might run it as
> any user, and that's how I've been testing it.

Hmm, are you using schroot or manually changing user in the chroot, or
something else?

> > 101 +for file in `ls "${HWPACK_DIR}"/sources.list.d.gpg/`; do
> >
> > This can use a * expansion as the sources.list.d code does couldn't it?
>
> At first I thought I could but after trying it I realized that may not
> be desirable as it would cause the file variable to point to
> '/.../sources.list.d.gpg/*' when the directory is empty, and that will
> cause apt-key to fail. Unless we can assume hwpacks will always include
> a non-empty sources.list.d.gpg, I think we might want to keep it that
> way. And maybe even change the other loop for sources.list.d.

Ah, good point. sources.list.d.gpg will be empty with our current
hwpacks, as I haven't implemented anything to put keys there. In general
though the specification doesn't state that there must be files there,
so I think your current code is good.

As for sources.list.d, the specification is equally quiet on that front,
and Alexander has a use case where it would be empty ("private" archives
of some sense), so I think that changing that loop would be good.

Thanks,

James

[Guilherme Salgado (salgado)]

On Mon, 2010-09-13 at 14:25 +0000, James Westby wrote:
> On Mon, 13 Sep 2010 13:31:15 -0000, Guilherme Salgado <email address hidden> wrote:
> > > 64 + sudo rm -f $HWPACK_PKGS_SOURCE
> > > 65 + sudo apt-get update -qq
> > >
> > > I don't see why sudo would be needed? This will run as root won't it?
> >
> > Not necessarily. When you run it inside the chroot you might run it as
> > any user, and that's how I've been testing it.
>
> Hmm, are you using schroot or manually changing user in the chroot, or
> something else?

schroot, but the main reason why I think it'd be nice (although
certainly not necessary) to run it as non-root is when you're logged
into the system and want to upgrade a hwpack. I'm not sure that's an
important use case and it'd be trivial to run the script as root in that
case anyway, so I'm happy to change this if you see any advantages in
requiring the script to run as root.

>
> > > 101 +for file in `ls "${HWPACK_DIR}"/sources.list.d.gpg/`; do
> > >
> > > This can use a * expansion as the sources.list.d code does couldn't it?
> >
> > At first I thought I could but after trying it I realized that may not
> > be desirable as it would cause the file variable to point to
> > '/.../sources.list.d.gpg/*' when the directory is empty, and that will
> > cause apt-key to fail. Unless we can assume hwpacks will always include
> > a non-empty sources.list.d.gpg, I think we might want to keep it that
> > way. And maybe even change the other loop for sources.list.d.
>
> Ah, good point. sources.list.d.gpg will be empty with our current
> hwpacks, as I haven't implemented anything to put keys there. In general
> though the specification doesn't state that there must be files there,
> so I think your current code is good.
>
> As for sources.list.d, the specification is equally quiet on that front,
> and Alexander has a use case where it would be empty ("private" archives
> of some sense), so I think that changing that loop would be good.

I've changed that.

--
Guilherme Salgado <https://launchpad.net/~salgado>

[lp:~salgado/linaro-image-tools/hwpack-install]

65. By Guilherme Salgado on 2010-09-13

Make sure the loops to add sources.list.d entries and gpg keys don't fail when there's no sources.list.d entries or gpg keys

[James Westby (james-w)]

On Mon, 13 Sep 2010 14:49:43 -0000, Guilherme Salgado <email address hidden> wrote:
> schroot, but the main reason why I think it'd be nice (although
> certainly not necessary) to run it as non-root is when you're logged
> into the system and want to upgrade a hwpack. I'm not sure that's an
> important use case and it'd be trivial to run the script as root in that
> case anyway, so I'm happy to change this if you see any advantages in
> requiring the script to run as root.

Good point, I don't think it's too important.

> I've changed that.

Thanks,

James

[lp:~salgado/linaro-image-tools/hwpack-install]

66. By Guilherme Salgado on 2010-09-14

merge trunk