On Sat, Apr 02, 2016 at 08:14:16AM +0900, Que Quotion wrote:
Sorry to double-post. This will be my second attempt to get this right, with yet another email client (gmail can't; geary failed; thunderbird?).
How exactly is this related to BUILDENV? This seems to be allowing arbitrary code to run before executing the build() function.
You can use the word "arbitrary" to make it sound like a bad idea if you like, but it very simply does for BUILDENV exactly what tidy.sh does for OPTIONS: allow for supplemental scripts to add to makepkg's functionality at this stage.
In fact, it does it in exactly the same way--because nearly all of this code was copied and pasted from tidy.sh.
Just as tidy.sh allows any kind of anything to be run, it could allow any bash script to be run here--but the intention is for scripts that serve build-environment altering functions to be run here (as the intention for tidy.sh is to run cleanup and compression scripts).
Your concern about cleanup seems misplaced. It will be up to the authors of supplemental scripts to make sure they work properly, and users who trust them to install and enable those features. This patch--in and of itself--will not cause the problems you are worried about.
This line does nothing, even if correctly spelled.
What is misspelled? This is intended to serve the same purpose as the same line serves in tidy.sh:
You don't see the problem? I suppose you might not because your mail client is completely shit and you've again dropped the thread. Consider: declare -a exta_buildopts vs. readonly -a extra_buildopts and: for func in ${extra_buildopts[@]}; do Do I really need to point out "exta" vs "extra" ?
declare -a packaging_options tidy_remove tidy_modify
The tidy scripts have names like upx.sh, the buildenv_ext scripts will have names like pgo.sh; these variables store the names of those options as they will be known in makepkg(-optimize).conf (upx, pgo).
Since nullglob isn't set, an empty directory will cause this to throw an error about a missing file (as the glob will turn into a literal). As there's no "buildenv_ext" files provided by makepkg, this means we error by default.
That could be fixed very easily, but again: copy pasta from tidy.sh. If you have a problem with this code--you should have a problem with that code too. Furthermore, as I have suggested in the forum (and posted a patch for), the first thing I would do with this if it were up to me is move ccache and distcc out of makepkg and ship them as builenv_ext scripts. Then you'd be shipping something that makes this unsafe code work just the way the same unsafe code works in tidy.sh.
Repeatedly using the excuse "but that's what tidy.sh does" isn't endearing, nor convincing me that you care about your work.
quoting
Sure, no problem. That also needs to be fixed in tidy.sh