[pacman-dev] [PATCH v2 0/1] libmakepkg: extension for additional BUILDENV options
From: Que Quotion <quequotion@gmail.com> Resubmitting as requested, in the format requested, by the means requested. buildenv_ext.sh does for BUILDENV what tidy.sh does for OPTIONS --- scripts/Makefile.am | 2 ++ scripts/libmakepkg/buildenv_ext.sh.in | 42 +++++++++++++++++++++++++++++++++++ scripts/makepkg.sh.in | 3 +++ 3 files changed, 47 insertions(+) create mode 100644 scripts/libmakepkg/buildenv_ext.sh.in -- 2.7.4
From: Que Quotion <quequotion@gmail.com> --- scripts/libmakepkg/buildenv_ext.sh.in | 42 +++++++++++++++++++++++++++++++++++ scripts/Makefile.am | 2 ++ scripts/makepkg.sh.in | 3 +++ 3 files changed, 47 insertions(+) create mode 100644 scripts/libmakepkg/buildenv_ext.sh.in diff --git a/scripts/libmakepkg/buildenv_ext.sh.in b/scripts/libmakepkg/buildenv_ext.sh.in new file mode 100644 index 0000000..a877b87 --- /dev/null +++ b/scripts/libmakepkg/buildenv_ext.sh.in @@ -0,0 +1,42 @@ +#!/usr/bin/bash +# +# buildenv_ext.sh - addional functions for altering the build environment +# before compiliation +# +# Copyright (c) 2015-2016 Pacman Development Team <pacman-dev@archlinux.org> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +[[ -n "$LIBMAKEPKG_BUILDENV_EXT_SH" ]] && return +LIBMAKEPKG_BUILDENV_EXT_SH=1 + +LIBRARY=${LIBRARY:-'@libmakepkgdir@'} + +declare -a exta_buildopts + +for lib in "$LIBRARY/buildenv_ext/"*.sh; do + source "$lib" +done + +readonly -a extra_buildopts + +buildenv_ext() { + + # options that alter compilation parameters + for func in ${extra_buildopts[@]}; do + $func + done + +} diff --git a/scripts/Makefile.am b/scripts/Makefile.am index 6f9abb8..a633827 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -45,6 +45,7 @@ LIBMAKEPKGDIRS = \ lint_pkgbuild \ source \ tidy \ + buildenv_ext \ util LIBMAKEPKG = \ @@ -82,6 +83,7 @@ LIBMAKEPKG_IN = \ libmakepkg/source/local.sh \ libmakepkg/source/svn.sh \ libmakepkg/tidy.sh \ + libmakepkg/buildenv_ext.sh \ libmakepkg/tidy/docs.sh \ libmakepkg/tidy/emptydirs.sh \ libmakepkg/tidy/libtool.sh \ -- 2.7.4 diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2efcc98..aafec5d 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -866,6 +866,9 @@ run_build() { export DISTCC_HOSTS fi + # Check for BUILDENV extensions, use any that are requested (check buildenv and PKGBUILD opts) + buildenv_ext + run_function_safe "build" } -- 2.7.4
On Sat, Apr 02, 2016 at 03:52:07AM +0900, quequotion@gmail.com wrote:
From: Que Quotion <quequotion@gmail.com>
---
How exactly is this related to BUILDENV? This seems to be allowing arbitrary code to run before executing the build() function. What about allowing for cleanup after build()? Otherwise, you leak side effects into check() and package() as well.
scripts/libmakepkg/buildenv_ext.sh.in | 42 +++++++++++++++++++++++++++++++++++ scripts/Makefile.am | 2 ++ scripts/makepkg.sh.in | 3 +++ 3 files changed, 47 insertions(+) create mode 100644 scripts/libmakepkg/buildenv_ext.sh.in
diff --git a/scripts/libmakepkg/buildenv_ext.sh.in b/scripts/libmakepkg/buildenv_ext.sh.in new file mode 100644 index 0000000..a877b87 --- /dev/null +++ b/scripts/libmakepkg/buildenv_ext.sh.in @@ -0,0 +1,42 @@ +#!/usr/bin/bash +# +# buildenv_ext.sh - addional functions for altering the build environment +# before compiliation +# +# Copyright (c) 2015-2016 Pacman Development Team <pacman-dev@archlinux.org> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +[[ -n "$LIBMAKEPKG_BUILDENV_EXT_SH" ]] && return +LIBMAKEPKG_BUILDENV_EXT_SH=1 + +LIBRARY=${LIBRARY:-'@libmakepkgdir@'} + +declare -a exta_buildopts
This line does nothing, even if correctly spelled.
+ +for lib in "$LIBRARY/buildenv_ext/"*.sh; do
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.
+ source "$lib" +done + +readonly -a extra_buildopts + +buildenv_ext() { + + # options that alter compilation parameters + for func in ${extra_buildopts[@]}; do + $func
quoting... for func in "${extra_buildopts[@]}"; do "$func" done
+ done + +} diff --git a/scripts/Makefile.am b/scripts/Makefile.am index 6f9abb8..a633827 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -45,6 +45,7 @@ LIBMAKEPKGDIRS = \ lint_pkgbuild \ source \ tidy \ + buildenv_ext \ util
LIBMAKEPKG = \ @@ -82,6 +83,7 @@ LIBMAKEPKG_IN = \ libmakepkg/source/local.sh \ libmakepkg/source/svn.sh \ libmakepkg/tidy.sh \ + libmakepkg/buildenv_ext.sh \ libmakepkg/tidy/docs.sh \ libmakepkg/tidy/emptydirs.sh \ libmakepkg/tidy/libtool.sh \ -- 2.7.4 diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2efcc98..aafec5d 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -866,6 +866,9 @@ run_build() { export DISTCC_HOSTS fi
+ # Check for BUILDENV extensions, use any that are requested (check buildenv and PKGBUILD opts) + buildenv_ext + run_function_safe "build" }
-- 2.7.4
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.
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
What is misspelled? This is intended to serve the same purpose as the same line serves in tidy.sh: 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). 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.
quoting
Sure, no problem. That also needs to be fixed in tidy.sh
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
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.
Repeatedly using the excuse "but that's what tidy.sh does" isn't endearing, nor convincing me that you care about your work. I only mean to point out that you are asking me to propose code that would be of a higher standard than what is in the repository. I get it,
You're just going to have to believe me that I really am trying to get this right. And no, I didn't see the problem; looked over it and over it. Somehow just could not process that an "r" was missing there... Is the "declare" line unnecessary? I know the scripts themselves will create the bash variable "extra_buildopts" but I put in this line mirroring tidy.sh--is it unnecessary there too? that's a good thing, but don't act like these problems were entirely my creation--I've imitated the work of the pacman devs in an earnest attempt to have this code integrated.
On 02/04/16 09:14, Que Quotion wrote:
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.
The tidy and {package,pkgbuild}_lint passes, all started by moving as much as possible out of makepkg itself and into libmakepkg. Once that was done, then the ability to extend those passes was added. That is the order things need to be done for the buildenv setup too. I have not commented on your patch because I require the time to look into whether moving all that outside of makepkg is even plausible. And if I spend that much time looking into whether it can be done, I might as well do it... Until that time, there is no need to bump your patch. It is not lost (we have a patchwork instance tracking all submitted patches), and the next release is a long time away. Allan
Allan Thank you for your consideration; it is encouraging.
I don't mean to be pushy, and I don't mind who gets the thing done or when--I really just want to see the idea implemented one way or another.
Reisner My reply to you went off thread again; I'm trying to get the hang of it.
From: Que Quotion <quequotion@gmail.com> Attempting to address certain concerns. --- scripts/libmakepkg/buildenv_ext.sh.in | 46 +++++++++++++++++++++++++++++++++++ scripts/Makefile.am | 2 ++ scripts/makepkg.sh.in | 3 +++ 3 files changed, 51 insertions(+) create mode 100644 scripts/libmakepkg/buildenv_ext.sh.in diff --git a/scripts/libmakepkg/buildenv_ext.sh.in b/scripts/libmakepkg/buildenv_ext.sh.in new file mode 100644 index 0000000..a877b87 --- /dev/null +++ b/scripts/libmakepkg/buildenv_ext.sh.in @@ -0,0 +1,46 @@ +#!/usr/bin/bash +# +# buildenv_ext.sh - addional functions for altering the build environment +# before compiliation +# +# Copyright (c) 2015-2016 Pacman Development Team <pacman-dev@archlinux.org> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +[[ -n "$LIBMAKEPKG_BUILDENV_EXT_SH" ]] && return +LIBMAKEPKG_BUILDENV_EXT_SH=1 + +LIBRARY=${LIBRARY:-'@libmakepkgdir@'} + +declare -a extra_buildopts + +shopt -s nullglob #in case of no buildenv_ext scripts + +for lib in "$LIBRARY/buildenv_ext/"*.sh; do + source "$lib" +done + +shopt -u nullglob #put this back where we found it + +readonly -a extra_buildopts + +buildenv_ext() { + + # options that alter compilation parameters + for func in "${extra_buildopts[@]}"; do + "$func" + done + +} diff --git a/scripts/Makefile.am b/scripts/Makefile.am index 6f9abb8..a633827 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -45,6 +45,7 @@ LIBMAKEPKGDIRS = \ lint_pkgbuild \ source \ tidy \ + buildenv_ext \ util LIBMAKEPKG = \ @@ -82,6 +83,7 @@ LIBMAKEPKG_IN = \ libmakepkg/source/local.sh \ libmakepkg/source/svn.sh \ libmakepkg/tidy.sh \ + libmakepkg/buildenv_ext.sh \ libmakepkg/tidy/docs.sh \ libmakepkg/tidy/emptydirs.sh \ libmakepkg/tidy/libtool.sh \ -- 2.7.4 diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2efcc98..aafec5d 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -866,6 +866,9 @@ run_build() { export DISTCC_HOSTS fi + # Check for BUILDENV extensions, use any that are requested (check buildenv and PKGBUILD opts) + buildenv_ext + run_function_safe "build" } -- 2.7.4
From: Que Quotion <quequotion@gmail.com> I really only mean these as an example, but it does allow for a little more simplification to makepkg. I like the idea that the only built-in build_options would be 'buildflags' and 'makeflags'. --- scripts/libmakepkg/buildenv_ext/ccache.sh.in | 46 ++++++++++++++++++++++++++++ scripts/libmakepkg/buildenv_ext/distcc.sh.in | 45 ++++++++++++++++++++++++++++ scripts/makepkg.sh.in | 36 +--------------------- 3 files changed, 92 insertions(+), 35 deletions(-) create mode 100644 scripts/libmakepkg/buildenv_ext/ccache.sh.in create mode 100644 scripts/libmakepkg/buildenv_ext/distcc.sh.in diff --git a/scripts/libmakepkg/buildenv_ext/ccache.sh.in b/scripts/libmakepkg/buildenv_ext/ccache.sh.in new file mode 100644 index 0000000..8d9e0b5 --- /dev/null +++ b/scripts/libmakepkg/buildenv_ext/ccache.sh.in @@ -0,0 +1,46 @@ +#!/usr/bin/bash +# +# ccache.sh - Cache compiliations and recycle them to save time on repititions +# +# Copyright (c) 2008-2016 Pacman Development Team <pacman-dev@archlinux.org> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +[[ -n "$LIBMAKEPKG_CCACHE_SH" ]] && return +LIBMAKEPKG_CCACHE_SH=1 + +LIBRARY=${LIBRARY:-'@libmakepkgdir@'} + +source "$LIBRARY/util/message.sh" +source "$LIBRARY/util/option.sh" + +extra_buildopts+=('ccache') + +local ccache=0 + +ccache() { + + # use ccache if it is requested (check buildenv and PKGBUILD opts) + if check_buildoption "ccache" "y"; then + if ! type -p ccache >/dev/null; then + error "$(gettext "Cannot find the %s binary required for compiler cache usage.")" "ccache" + return 1 + fi + if [ -d /usr/lib/ccache/bin ]; then + export PATH="/usr/lib/ccache/bin:$PATH" + ccache=1 + fi + fi +} diff --git a/scripts/libmakepkg/buildenv_ext/distcc.sh.in b/scripts/libmakepkg/buildenv_ext/distcc.sh.in new file mode 100644 index 0000000..248445a --- /dev/null +++ b/scripts/libmakepkg/buildenv_ext/distcc.sh.in @@ -0,0 +1,45 @@ +#!/usr/bin/bash +# +# distcc.sh - Distribute compliation to reduce compilation time +# +# Copyright (c) 2008-2016 Pacman Development Team <pacman-dev@archlinux.org> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +[[ -n "$LIBMAKEPKG_CCACHE_SH" ]] && return +LIBMAKEPKG_CCACHE_SH=1 + +LIBRARY=${LIBRARY:-'@libmakepkgdir@'} + +source "$LIBRARY/util/message.sh" +source "$LIBRARY/util/option.sh" + +extra_buildopts+=('distcc') + +distcc() { + if check_buildoption "distcc" "y"; then + if ! type -p distcc >/dev/null; then + error "$(gettext "Cannot find the %s binary required for distributed compilation.")" "distcc" + return 1 + fi + if (( ccache )); then + export CCACHE_PREFIX="${CCACHE_PREFIX:+$CCACHE_PREFIX }distcc" + export CCACHE_BASEDIR="$srcdir" + elif [[ -d /usr/lib/distcc/bin ]]; then + export PATH="/usr/lib/distcc/bin:$PATH" + fi + export DISTCC_HOSTS + fi +} diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index aafec5d..e5f2028 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -48,7 +48,7 @@ declare -r startdir="$PWD" LIBRARY=${LIBRARY:-'@libmakepkgdir@'} -build_options=('ccache' 'distcc' 'buildflags' 'makeflags') +build_options=('buildflags' 'makeflags') splitpkg_overrides=('pkgdesc' 'arch' 'url' 'license' 'groups' 'depends' 'optdepends' 'provides' 'conflicts' 'replaces' 'backup' 'options' 'install' 'changelog') @@ -847,24 +847,6 @@ run_prepare() { } run_build() { - local ccache=0 - - # use ccache if it is requested (check buildenv and PKGBUILD opts) - if check_buildoption "ccache" "y" && [[ -d /usr/lib/ccache/bin ]]; then - export PATH="/usr/lib/ccache/bin:$PATH" - ccache=1 - fi - - # use distcc if it is requested (check buildenv and PKGBUILD opts) - if check_buildoption "distcc" "y"; then - if (( ccache )); then - export CCACHE_PREFIX="${CCACHE_PREFIX:+$CCACHE_PREFIX }distcc" - export CCACHE_BASEDIR="$srcdir" - elif [[ -d /usr/lib/distcc/bin ]]; then - export PATH="/usr/lib/distcc/bin:$PATH" - fi - export DISTCC_HOSTS - fi # Check for BUILDENV extensions, use any that are requested (check buildenv and PKGBUILD opts) buildenv_ext @@ -1559,22 +1541,6 @@ check_software() { fi fi - # distcc - compilation with distcc - if check_buildoption "distcc" "y"; then - if ! type -p distcc >/dev/null; then - error "$(gettext "Cannot find the %s binary required for distributed compilation.")" "distcc" - ret=1 - fi - fi - - # ccache - compilation with ccache - if check_buildoption "ccache" "y"; then - if ! type -p ccache >/dev/null; then - error "$(gettext "Cannot find the %s binary required for compiler cache usage.")" "ccache" - ret=1 - fi - fi - # strip - strip symbols from binaries/libraries if check_option "strip" "y"; then if ! type -p strip >/dev/null; then -- 2.7.4
participants (4)
-
Allan McRae
-
Dave Reisner
-
Que Quotion
-
quequotion＠gmail.com