[buildd-tools-devel] Bug#500746: Bug#500746: new patch, with environment export

[Kees Cook]

Hi,

On Sun, May 10, 2009 at 02:34:08AM +0100, Roger Leigh wrote:
> - Is the hook script present on the host or in the chroot?  From the
>   pipe_command CHROOT=>1 it looks like it runs inside the chroot, but
>   is this the intention?  This can just the documented in the manpage
>   if it is the case.
> 
>   One possibility to allow clean separation would be to pipe the
>   script to schroot which could just invoke /bin/sh to read the
>   script from stdin.  This would require the script to be Bourne
>   shell only, however.

Ah, good point.  I was using scripts that existed in both places (i.e. in a
bind mount).  For now I'd like to avoid doing stdin piping as that feels
too complex.  I think either we need document it as such, or have sbuild
copy the command into the chroot before executing it.  Again, I think just
documenting it is the cleanest approach.

> - Should get_env be in Sbuild::Base rather than Sbuild::Build?  This
>   is the base class for all objects in the Sbuild:: and Buildd::
>   hierarchies.  Is it something that's going to be generally usable
>   in other contexts or does it just have the one use?
> 
>   If this is the only intended use, I'd prefer it to be a Sbuild::Build
>   method.  It could also be a general function in Sbuild.pm.

Ah!  Okay, that's easy enough to move.  I figured that since Base had the
knowledge of 'Config', it seemed like the right level.  Simple to move,
though.

> - The Config object can contain scalars, arrayrefs and hashrefs
>   (and other references such as filehandles).  I think it only makes
>   sense to export simple scalars into the environment.  The code does
>   this which is great.  Do you see any potential use for exporting
>   arrays and hashes as comma- and key=value comma-separated lists
>   for example.  I can't myself, I'm just thinking if there's any
>   reasonable use for those variables, and if so, can they be easily
>   used by scripts in this format.

Yeah, you'd said in earlier emails "scalar and array", and when I looked at
the non-scalar values, they seemed pretty un-useful, so I opted to go the
route of avoiding all non-scalars.  Exporting all these values into the
environment already feels like overkill.  ;)

I'll send a git patch shortly...

-Kees

-- 
Kees Cook                                            @debian.org