On Thu, Oct 03, 2013 at 09:06:28PM +0800, 郑文辉(Techlive Zheng) wrote: > 2013/10/3 Dave Reisner <[email protected]>: > > 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.
