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
On 05.11.18 01:06, Eli Schwartz wrote:
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.
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:
On 05.11.18 01:06, Eli Schwartz wrote:
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.
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.
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!
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 ;;
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
On 14/11/18 11:55 am, Eli Schwartz wrote:
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 ;;@@ -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;;@@ -1279,8 +1280,8 @@ while true; do --nosign) SIGNPKG='n' ;; -o|--nobuild) NOBUILD=1 ;;
BUILDPKG=0
-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;;@@ -1289,7 +1290,7 @@ while true; do --skippgpcheck) SKIPPGPCHECK=1;; -s|--syncdeps) DEP_BIN=1 ;; -S|--source) SOURCEONLY=1 ;;
BUILDPKG=0
--verifysource) VERIFYSOURCE=1 ;;
--verifysource) BUILDPKG=0 VERIFYSOURCE=1 ;;-h|--help) usage; exit $E_OK ;; -V|--version) version; exit $E_OK ;;
On 11/27/18 6:33 AM, Allan McRae wrote:
On 14/11/18 11:55 am, Eli Schwartz wrote:
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 ;;@@ -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;;@@ -1279,8 +1280,8 @@ while true; do --nosign) SIGNPKG='n' ;; -o|--nobuild) NOBUILD=1 ;;
BUILDPKG=0
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.
-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;;@@ -1289,7 +1290,7 @@ while true; do --skippgpcheck) SKIPPGPCHECK=1;; -s|--syncdeps) DEP_BIN=1 ;; -S|--source) SOURCEONLY=1 ;;
BUILDPKG=0
Will fix once I'm sure how to handle the above case.
--verifysource) VERIFYSOURCE=1 ;;
--verifysource) BUILDPKG=0 VERIFYSOURCE=1 ;;-h|--help) usage; exit $E_OK ;; -V|--version) version; exit $E_OK ;;
On 28/11/18 1:46 am, Eli Schwartz wrote:
On 11/27/18 6:33 AM, Allan McRae wrote:
On 14/11/18 11:55 am, Eli Schwartz wrote:
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 ;;@@ -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;;@@ -1279,8 +1280,8 @@ while true; do --nosign) SIGNPKG='n' ;; -o|--nobuild) NOBUILD=1 ;;
BUILDPKG=0
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.
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
On 1/4/19 12:26 AM, Eli Schwartz wrote:
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.
I am apparently super tired tonight...
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
pacman-dev@lists.archlinux.org