Re: CVS commit: pkgsrc/mk

[Matthias Scheler <tron%netbsd.org@localhost>]

On Wed, Jun 05, 2013 at 07:42:24AM -0400, Greg Troxel wrote:
> > Committed By:       tron
> > Date:               Wed Jun  5 08:19:57 UTC 2013
> >
> > Modified Files:
> >     pkgsrc/mk: bsd.pkg.mk
> >
> > Log Message:
> > Revert change to "PKG_SETENV":
> 
> I think it's good to revert this until we have addressed most of the
> issues it will cause, but I aso think we should be heading for sanitization.
> 
> > 1.) It breaks the build of "www/firefox" which gets upset if "SHELL" is
> >     not defined in the environment. There are probably more packages
> >     which similar problems.
> 
> That sounds like a bug in www/firefox.  It absolutely should not behave
> differently based on the user's shell.  So probably it needs
> CONFIGURE_ENV of SHELL=/bin/sh.

I'm not convinced that this will to the job. There is a questionable
Python script which checks explicitely for the "SHELL" variable.

> (But I get it that it takes time to fix these, and I agree that it not
> being done yet is a good reason to revert.)

Thanks.

> > 2.) It breaks established use case like this one:
> >
> >     export ALLOW_VULNERABLE_PACKAGES=yes
> >     cd pkgsrc/multimedia/ffmpeg2theora
> >     bmake install
> >
> >     In this case the value of "ALLOW_VULNERABLE_PACKAGES" will not be
> >     passed to the build of "pkgsrc/multimedia/ffmpeg". And the build of
> >     this package will fail due to known vulnerabilities.
> 
> It may be reasonable to special-case a few variables, but they should
> get printed out, similar to BUILD_DEFS, to sort of guard against
> unintended leakage.

That sounds like a good plan. But I don't think I could come up with
that list. "SHELL" (which is controversial) and "ALLOW_VULNERABLE_PACKAGES"
are the only ones I've found so far.

> Or those variables should all start with PKGSRC_

That would work but break some user visible interfaces.

        Kind regards

-- 
Matthias Scheler                                  http://zhadum.org.uk/