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

On Thu, Oct 03, 2013 at 09:06:28PM +0800, 郑文辉(Techlive Zheng) wrote:

2013/10/3 Dave Reisner :

...

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.