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

[Roger Leigh]

On Sat, May 09, 2009 at 04:05:14PM -0700, Kees Cook wrote:

> Here's my proposed patch to implement --setup-hook, which exports the build
> settings, as well as the build Config into the environment.

It looks nice.  Some comments about a couple of nits:

- 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.

- 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.

And this one isn't a problem at all, just a thought:

- 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.


Regards,
Roger

-- 
  .''`.  Roger Leigh
 : :' :  Debian GNU/Linux             http://people.debian.org/~rleigh/
 `. `'   Printing on GNU/Linux?       http://gutenprint.sourceforge.net/
   `-    GPG Public Key: 0x25BFB848   Please GPG sign your mail.