[pacman-dev] [PATCH 1/2] makepkg: allow make-style environment vars to override BUILDSCRIPT vars

[Dave Reisner]

On Thu, Oct 03, 2013 at 09:06:28PM +0800, 郑文辉(Techlive Zheng) wrote:
> 2013/10/3 Dave Reisner <d at falconindy.com>:
> > On Thu, Oct 03, 2013 at 03:49:33PM +0800, Techlive Zheng wrote:
> >> This allows for VAR=value and VAR+=value variable declarations in
> >> command line to override variables in BUILDSCRIPT.
> >> ---
> >>  scripts/makepkg.sh.in | 9 ++++-----
> >>  1 file changed, 4 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> >> index 1ef2af2..f6d884f 100644
> >> --- a/scripts/makepkg.sh.in
> >> +++ b/scripts/makepkg.sh.in
> >> @@ -2712,11 +2712,6 @@ if [[ ! -w $BUILDDIR ]]; then
> >>       exit 1
> >>  fi
> >>
> >> -# override settings from extra variables on commandline, if any
> >> -if (( ${#extra_environment[*]} )); then
> >> -     export "${extra_environment[@]}"
> >> -fi
> >> -
> >>  PKGDEST=${_PKGDEST:-$PKGDEST}
> >>  PKGDEST=${PKGDEST:-$startdir} #default to $startdir if undefined
> >>  if (( ! (NOBUILD || GENINTEG) )) && [[ ! -w $PKGDEST ]]; then
> >> @@ -2810,7 +2805,11 @@ if [[ $BUILDDIR = "$startdir" ]]; then
> >>  else
> >>       srcdir="$BUILDDIR/$pkgbase/src"
> >>       pkgdirbase="$BUILDDIR/$pkgbase/pkg"
> >> +fi
> >>
> >> +# override settings from extra variables on commandline, if any
> >> +if (( ${#extra_environment[*]} )); then
> >> +     export "${extra_environment[@]}"
> >
> > Doing this is dangerous, as it lets you do things like:
> >
> >   makepkg pkgver=this-is-not-a-valid-version
> >
> > Allowing a documented feature to override basic sanity checks is not a
> > good idea, imo.
> >
> >>  fi
> >>
> >>  # set pkgdir to something "sensible" for (not recommended) use during build()
> >> --
> >> 1.8.4
> >>
> >>
> >
> 
> Yes, that is a risk, but people should know what they are doing.

And what are the rules? Where have you documented this?

Regardless, I don't think we should require that people have a
strong understanding of shell quoting rules and hand escape their
values. It's going to make for a really crappy user experience,
especially given the failure modes. I realize that this probably isn't
going to be a widely used feature, but it should still make sense and
have easily understood behavior.

> My main purpose to move this section here is to use the variables from
> BUILDSCRIPT in the command line vars assignment in the following
> patch.

Yes, I surmised that from your other posts. I'd strongly prefer that you
find another way. Sorry.