[pacman-dev] [PATCH] makepkg: fix broken check for the fakeroot binary
In commit d8ee8d0c99c3820951e2e49dbdb71a5390bd1dc4 we made use of fakeroot absolutely mandatory, and disabled a lot of the code which checked to see if this now-defunct BUILDENV option was set, before setting up the environment to use fakeroot. Unfortunately, we missed one spot. The check_software routine still checked to see if fakeroot was enabled, but due to the option being removed, thought that it was in fact disabled, and as a result this check would never run. Fix by unconditionally checking for the fakeroot binary. While in theory, users could be using --verifysource or --packagelist or --printsrcinfo without, strictly speaking, needing fakeroot, they are sure to be building the software too, anyway, so this use case is not one we need to support. Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- scripts/makepkg.sh.in | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 8b39ca9a..db71d044 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1000,11 +1000,9 @@ check_software() { fi # fakeroot - correct package file permissions - if check_buildenv "fakeroot" "y" && (( EUID > 0 )); then - if ! type -p fakeroot >/dev/null; then - error "$(gettext "Cannot find the %s binary.")" "fakeroot" - ret=1 - fi + if ! type -p fakeroot >/dev/null; then + error "$(gettext "Cannot find the %s binary.")" "fakeroot" + ret=1 fi # gpg - package signing -- 2.19.1
On 05.11.18 01:06, Eli Schwartz wrote:
Just let me note here, that archlinux32's build master runs "makepkg --printsrcinfo" on a regular basis to determine properties of to-be-built packages, but does not build any packages itself. However, I see, that this is a rather cornercase which you indeed do not need to support: It is rather easy for us to provide all necessities for actually building. regards, Erich
On 11/5/18 3:09 AM, Erich Eckner wrote:
So, originally I asked allan whether I should try to do even more fancy things here, and he said not to bother due to the reasons I mentioned in the commit message. Your response inspired him to suggest I handle this use case anyway, and it also inspired me to do a bit more digging around in check_software(), and I'm now going submit a more targeted patchset, with some followups to handle some other issues nearby as well! -- Eli Schwartz Bug Wrangler and Trusted User
There are state variables for everything else, and we use them to do conditional checks on things, but it's currently a bit difficult to test whether a package is being built, as it's the default action if *no* options are specified. Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- This makes the next patch simpler, and will be reused in some patches I intend to submit in the future. scripts/makepkg.sh.in | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 3ac03d11..be8b761e 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -59,6 +59,7 @@ known_hash_algos=('md5' 'sha1' 'sha224' 'sha256' 'sha384' 'sha512') # Options ASDEPS=0 BUILDFUNC=0 +BUILDPKG=1 CHECKFUNC=0 CLEANBUILD=0 CLEANUP=0 @@ -1256,7 +1257,7 @@ while true; do --noprogressbar) PACMAN_OPTS+=("--noprogressbar") ;; # Makepkg Options - --allsource) SOURCEONLY=2 ;; + --allsource) BUILDPKG=0 SOURCEONLY=2 ;; -A|--ignorearch) IGNOREARCH=1 ;; -c|--clean) CLEANUP=1 ;; -C|--cleanbuild) CLEANBUILD=1 ;; @@ -1267,7 +1268,7 @@ while true; do -f|--force) FORCE=1 ;; -F) INFAKEROOT=1 ;; # generating integrity checks does not depend on architecture - -g|--geninteg) GENINTEG=1 IGNOREARCH=1;; + -g|--geninteg) BUILDPKG=0 GENINTEG=1 IGNOREARCH=1;; --holdver) HOLDVER=1 ;; -i|--install) INSTALL=1 ;; --key) shift; GPGKEY=$1 ;; @@ -1279,8 +1280,8 @@ while true; do --nosign) SIGNPKG='n' ;; -o|--nobuild) NOBUILD=1 ;; -p) shift; BUILDFILE=$1 ;; - --packagelist) PACKAGELIST=1 IGNOREARCH=1;; - --printsrcinfo) PRINTSRCINFO=1 IGNOREARCH=1;; + --packagelist) BUILDPKG=0 PACKAGELIST=1 IGNOREARCH=1;; + --printsrcinfo) BUILDPKG=0 PRINTSRCINFO=1 IGNOREARCH=1;; -r|--rmdeps) RMDEPS=1 ;; -R|--repackage) REPKG=1 ;; --sign) SIGNPKG='y' ;; @@ -1289,7 +1290,7 @@ while true; do --skippgpcheck) SKIPPGPCHECK=1;; -s|--syncdeps) DEP_BIN=1 ;; -S|--source) SOURCEONLY=1 ;; - --verifysource) VERIFYSOURCE=1 ;; + --verifysource) BUILDPKG=0 VERIFYSOURCE=1 ;; -h|--help) usage; exit $E_OK ;; -V|--version) version; exit $E_OK ;; -- 2.19.1
In commit d8ee8d0c99c3820951e2e49dbdb71a5390bd1dc4 we made use of fakeroot absolutely mandatory, and disabled a lot of the code which checked to see if this now-defunct BUILDENV option was set, before setting up the environment to use fakeroot. Unfortunately, we missed one spot. The check_software routine still checked to see if fakeroot was enabled, but due to the option being removed, thought that it was in fact disabled, and as a result this check would never run. Fix by checking to see if we are trying to build either a package or a source package, and if so, checking for fakeroot. These are the only two situations where fakeroot is needed. Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- v2: add check to see if we need fakeroot scripts/makepkg.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index be8b761e..21d59814 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -993,7 +993,7 @@ check_software() { fi # fakeroot - correct package file permissions - if check_buildenv "fakeroot" "y" && (( EUID > 0 )); then + if (( SOURCEONLY || BUILDPKG )); then if ! type -p fakeroot >/dev/null; then error "$(gettext "Cannot find the %s binary.")" "fakeroot" ret=1 -- 2.19.1
On 11/27/18 6:33 AM, Allan McRae wrote:
My rationale here was that running source extraction, prepare() and pkgver() are part of the general category of building a package -- and if you use --nobuild, I expect you're likely going to use --noextract shortly after. e.g. in followup patches I would like to check whether (( BUILDPKG || SOURCEONLY == 2 || VERIFYSOURCE )) to determine if we should check for vcs software, or || ( BUILDPKG && !SKIPCHECKSUMS ) as a modification to checking for the checksum binaries.
Will fix once I'm sure how to handle the above case.
-- Eli Schwartz Bug Wrangler and Trusted User
On 28/11/18 1:46 am, Eli Schwartz wrote:
The variable name is wrong if --nobuild does not imply BUILDPKG=0.
In commit d8ee8d0c99c3820951e2e49dbdb71a5390bd1dc4 we made use of fakeroot absolutely mandatory, and disabled a lot of the code which checked to see if this now-defunct BUILDENV option was set, before setting up the environment to use fakeroot. Unfortunately, we missed one spot. The check_software routine still checked to see if fakeroot was enabled, but due to the option being removed, thought that it was in fact disabled, and as a result this check would never run. Fix by checking to see if we are trying to build either a package or a source package, and if so, checking for fakeroot. These are the only two situations where fakeroot is needed. Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- v3: as originally reviewed, set BUILDPKG=0 when not actually building package artifacts. scripts/libmakepkg/executable/fakeroot.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/libmakepkg/executable/fakeroot.sh.in b/scripts/libmakepkg/executable/fakeroot.sh.in index 56c1b3fd..09064543 100644 --- a/scripts/libmakepkg/executable/fakeroot.sh.in +++ b/scripts/libmakepkg/executable/fakeroot.sh.in @@ -28,7 +28,7 @@ source "$LIBRARY/util/option.sh" executable_functions+=('executable_fakeroot') executable_fakeroot() { - if check_buildenv "fakeroot" "y" && (( EUID > 0 )); then + if (( SOURCEONLY || BUILDPKG )); then if ! type -p fakeroot >/dev/null; then error "$(gettext "Cannot find the %s binary.")" "fakeroot" return 1 -- 2.20.1
There are state variables for everything else, and we use them to do conditional checks on things, but it's currently a bit difficult to test whether a package is being built, as it's the default action if *no* options are specified. Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- v3: as originally reviewed, set BUILDPKG=0 when not actually building package artifacts. scripts/makepkg.sh.in | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 4449ccf7..47255b7f 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -58,6 +58,7 @@ known_hash_algos=('md5' 'sha1' 'sha224' 'sha256' 'sha384' 'sha512') # Options ASDEPS=0 BUILDFUNC=0 +BUILDPKG=1 CHECKFUNC=0 CLEANBUILD=0 CLEANUP=0 @@ -1042,7 +1043,7 @@ while true; do --noprogressbar) PACMAN_OPTS+=("--noprogressbar") ;; # Makepkg Options - --allsource) SOURCEONLY=2 ;; + --allsource) BUILDPKG=0 SOURCEONLY=2 ;; -A|--ignorearch) IGNOREARCH=1 ;; -c|--clean) CLEANUP=1 ;; -C|--cleanbuild) CLEANBUILD=1 ;; @@ -1053,7 +1054,7 @@ while true; do -f|--force) FORCE=1 ;; -F) INFAKEROOT=1 ;; # generating integrity checks does not depend on architecture - -g|--geninteg) GENINTEG=1 IGNOREARCH=1;; + -g|--geninteg) BUILDPKG=0 GENINTEG=1 IGNOREARCH=1;; --holdver) HOLDVER=1 ;; -i|--install) INSTALL=1 ;; --key) shift; GPGKEY=$1 ;; @@ -1063,10 +1064,10 @@ while true; do --nocheck) RUN_CHECK='n' ;; --noprepare) RUN_PREPARE='n' ;; --nosign) SIGNPKG='n' ;; - -o|--nobuild) NOBUILD=1 ;; + -o|--nobuild) BUILDPKG=0 NOBUILD=1 ;; -p) shift; BUILDFILE=$1 ;; - --packagelist) PACKAGELIST=1 IGNOREARCH=1;; - --printsrcinfo) PRINTSRCINFO=1 IGNOREARCH=1;; + --packagelist) BUILDPKG=0 PACKAGELIST=1 IGNOREARCH=1;; + --printsrcinfo) BUILDPKG=0 PRINTSRCINFO=1 IGNOREARCH=1;; -r|--rmdeps) RMDEPS=1 ;; -R|--repackage) REPKG=1 ;; --sign) SIGNPKG='y' ;; @@ -1074,8 +1075,8 @@ while true; do --skipinteg) SKIPCHECKSUMS=1; SKIPPGPCHECK=1 ;; --skippgpcheck) SKIPPGPCHECK=1;; -s|--syncdeps) DEP_BIN=1 ;; - -S|--source) SOURCEONLY=1 ;; - --verifysource) VERIFYSOURCE=1 ;; + -S|--source) BUILDPKG=0 SOURCEONLY=1 ;; + --verifysource) BUILDPKG=0 VERIFYSOURCE=1 ;; -h|--help) usage; exit $E_OK ;; -V|--version) version; exit $E_OK ;; -- 2.20.1
participants (3)
-
Allan McRae
-
Eli Schwartz
-
Erich Eckner