Package: live-build
Version: 1:20170807
Severity: normal
Tags: patch

Hi,

I've run into some problems running live-build with
LB_BUILD_WITH_CHROOT. I understand that this is not a recommended or
supported configuration (though I'm not entirely sure why not, given the
amount of code for it), but it's being used in the Webconverger project
since quite some time and it mostly works.

When running lb `lb config --build-with-chroot=false` followed by `lb
build`, the build fails with:

[2017-08-28 15:52:15] lb binary_syslinux
P: Begin installing syslinux...
E: /usr/share/ISOLINUX

It seems the check for ISOLINUX is using an old path, which is
corrected for chroot builds only.

I'm attaching a patch that unifies the dependency handling for with and
without chroot, which fixes this problem as well as adds some dependency
checks with LB_BUILD_WITH_CHROOT=false and simplifies some code. There
is a second patch (the first in the series), that makes sure that these
dependency checks actually give a proper error message for missing
packages on the host.

With these patches applied, the commands mentioned above produce a
working image.

Gr.

Matthijs

-- Package-specific info:

-- System Information:
Debian Release: stretch/sid
  APT prefers stable
  APT policy: (990, 'stable'), (500, 'testing'), (500, 'oldstable'), (50, 
'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.11.0-1-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages live-build depends on:
ii  debootstrap  1.0.87

Versions of packages live-build recommends:
ii  apt-utils                       1.3.1
ii  cpio                            2.11+dfsg-5
ii  live-boot-doc                   1:20160511
ii  live-config-doc                 5.20160608
ii  live-manual-html [live-manual]  2:20151217.1
ii  wget                            1.18-4

live-build suggests no packages.

-- no debconf information
>From 406ed00e806abb84252033ac609fccffff9bcd83 Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matth...@stdin.nl>
Date: Mon, 28 Aug 2017 15:14:34 +0200
Subject: [PATCH 1/2] Error out when needed packages are missing on the host

Previously, Check_package would only show an error when host packages
are missing on a non-apt system. On apt system, the packages would be
added to _LB_PACKAGES, which causes them to be installed in the chroot,
not in the host (or not at all if Install_package is not called). This
behaviour could break the build.

This applies to either packages that must be present in the host (as
checked with `Check_package host ...`), as well as packages that can be
either in the chroot or host (as checked with `Check_package chroot`)
when LB_BUILD_WITH_CHROOT=false.
---
 functions/packages.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/functions/packages.sh b/functions/packages.sh
index c2f7cfabf..7c3592dac 100755
--- a/functions/packages.sh
+++ b/functions/packages.sh
@@ -16,16 +16,16 @@ Check_package ()
 
 	Check_installed "${CHROOT}" "${FILE}" "${PACKAGE}"
 
-	case "${INSTALL_STATUS}" in
-		1)
+	if [ "${INSTALL_STATUS}" -ne 0 ]
+	then
+		if [ "${LB_BUILD_WITH_CHROOT}" != "false" ]
+		then
 			_LB_PACKAGES="${_LB_PACKAGES} ${PACKAGE}"
-			;;
-
-		2)
+		else
 			Echo_error "You need to install %s on your host system." "${PACKAGE}"
 			exit 1
-			;;
-	esac
+		fi
+	fi
 }
 
 Install_package ()
-- 
2.11.0

>From 034bb0c00788f0927a068d151c53a8f341e215fa Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matth...@stdin.nl>
Date: Mon, 28 Aug 2017 11:29:54 +0200
Subject: [PATCH 2/2] Check all dependencies independent of
 LB_BUILD_WITH_CHROOT

Since commit fdc9250bc (Changing package dependency checks within chroot
to work outside as well), Check_package automatically checks for
LB_BUILD_WITH_CHROOT and works inside as well as outside of the chroot,
so no need to check LB_BUILD_WITH_CHROOT before calling them.
Install_package and Remove_package are just a no-op when building
without chroot, so they can also be called unconditionally.
Restore_cache and Save_cache do not check LB_BUILD_WITH_CHROOT but it
it should not hurt to call them when not needed (which already happened
in some cases).

This commit makes all Check_package calls unconditional on
LB_BUILD_WITH_CHROOT.

For binary_syslinux, this fixes the check (which used outdated paths
outside the chroot since 7b6dfd9d1), for binary_grub-efi,
binary_package-lists and chroot_package-lists this simplifies the code
(but also causes the check to become package-based instead of file-based
on apt-based systems), and for binary_loadlin and binary_win32-loader
this adds the check outside the chroot which was previously missing.
---
 scripts/build/binary_grub-efi      | 60 +++++++-------------------------------
 scripts/build/binary_loadlin       | 29 +++++++++---------
 scripts/build/binary_package-lists | 32 ++++++--------------
 scripts/build/binary_syslinux      | 41 ++++----------------------
 scripts/build/binary_win32-loader  | 16 +++++-----
 scripts/build/chroot_package-lists | 45 ++++++++--------------------
 6 files changed, 58 insertions(+), 165 deletions(-)

diff --git a/scripts/build/binary_grub-efi b/scripts/build/binary_grub-efi
index 6d158cd4f..d24532d50 100755
--- a/scripts/build/binary_grub-efi
+++ b/scripts/build/binary_grub-efi
@@ -45,56 +45,11 @@ Check_architectures amd64 i386
 Check_crossarchitectures
 
 # Checking depends
-case "${LB_BUILD_WITH_CHROOT}" in
-	true)
-		_CHROOT_DIR=""
-
-		Check_package chroot /usr/lib/grub/x86_64-efi/configfile.mod grub-efi-amd64-bin
-		Check_package chroot /usr/lib/grub/i386-efi/configfile.mod grub-efi-ia32-bin
-		Check_package chroot /usr/bin/grub-mkimage grub-common
-		Check_package chroot /usr/bin/mcopy mtools
-		Check_package chroot /sbin/mkfs.msdos dosfstools
-		;;
-
-	false)
-		_CHROOT_DIR="chroot"
-
-		if [ ! -e /usr/lib/grub/x86_64-efi ]
-		then
-			# grub-efi-amd64-bin
-			Echo_error "/usr/lib/grub/x86_64-efi - no such directory"
-			exit 1
-		fi
-
-		if [ ! -e /usr/lib/grub/i386-efi ]
-		then
-			# grub-efi-ia32-bin
-			Echo_error "/usr/lib/grub/i386-efi - no such directory"
-			exit 1
-		fi
-
-		if [ ! -e /usr/bin/grub-mkimage ]
-		then
-			# grub-common
-			Echo_error "/usr/bin/grub-mkimage - no such file."
-			exit 1
-		fi
-
-		if [ ! -e /usr/bin/mcopy ]
-		then
-			# mtools
-			Echo_error "/usr/bin/mcopy - no such file."
-			exit 1
-		fi
-
-		if [ ! -e /sbin/mkfs.msdos ]
-		then
-			# dosfstools
-			Echo_error "/sbin/mkfs.msdos - no such file."
-			exit 1
-		fi
-		;;
-esac
+Check_package chroot /usr/lib/grub/x86_64-efi/configfile.mod grub-efi-amd64-bin
+Check_package chroot /usr/lib/grub/i386-efi/configfile.mod grub-efi-ia32-bin
+Check_package chroot /usr/bin/grub-mkimage grub-common
+Check_package chroot /usr/bin/mcopy mtools
+Check_package chroot /sbin/mkfs.msdos dosfstools
 
 # Setting destination directory
 case "${LIVE_IMAGE_TYPE}" in
@@ -125,6 +80,11 @@ case "${LB_BUILD_WITH_CHROOT}" in
 		mkdir -p chroot/${LIVE_BUILD_PATH}
 		cp "${LIVE_BUILD_PATH}/efi-image" "chroot/${LIVE_BUILD_PATH}"
 		cp "${LIVE_BUILD_PATH}/grub-cpmodules" "chroot/${LIVE_BUILD_PATH}"
+
+		_CHROOT_DIR=""
+	;;
+	false)
+		_CHROOT_DIR="chroot"
         ;;
 esac
 #####
diff --git a/scripts/build/binary_loadlin b/scripts/build/binary_loadlin
index 69f740beb..d74a47021 100755
--- a/scripts/build/binary_loadlin
+++ b/scripts/build/binary_loadlin
@@ -43,19 +43,20 @@ Check_lockfile .lock
 # Creating lock file
 Create_lockfile .lock
 
+
 case "${LB_ARCHITECTURES}" in
 	amd64|i386)
-		case "${LB_BUILD_WITH_CHROOT}" in
-			true)
-				# Checking depends
-				Check_package chroot /usr/lib/loadlin/loadlin.exe.gz loadlin
+		# Checking depends
+		Check_package chroot /usr/lib/loadlin/loadlin.exe.gz loadlin
 
-				# Restoring cache
-				Restore_cache cache/packages.binary
+		# Restoring cache
+		Restore_cache cache/packages.binary
 
-				# Installing depends
-				Install_package
+		# Installing depends
+		Install_package
 
+		case "${LB_BUILD_WITH_CHROOT}" in
+			true)
 				_PREFIX="chroot"
 				;;
 
@@ -69,15 +70,11 @@ case "${LB_ARCHITECTURES}" in
 		gunzip -c "${_PREFIX}/usr/lib/loadlin/loadlin.exe.gz" > binary/tools/loadlin.exe
 		gunzip -c "${_PREFIX}/usr/share/doc/loadlin/manual.txt.gz" > binary/tools/loadlin.txt
 
-		case "${LB_BUILD_WITH_CHROOT}" in
-			true)
-				# Saving cache
-				Save_cache cache/packages.binary
+		# Saving cache
+		Save_cache cache/packages.binary
 
-				# Removing depends
-				Remove_package
-				;;
-		esac
+		# Removing depends
+		Remove_package
 
 cat > binary/install/install.bat << EOF
 \tools\loadlin.exe vmlinuz initrd=initrd.gz
diff --git a/scripts/build/binary_package-lists b/scripts/build/binary_package-lists
index dcbadccb5..a13491ee3 100755
--- a/scripts/build/binary_package-lists
+++ b/scripts/build/binary_package-lists
@@ -55,23 +55,13 @@ if ls config/package-lists/*.list > /dev/null 2>&1 || \
 then
 	# Check depends
 	Check_package host /usr/bin/apt-ftparchive apt-utils
+	Check_package chroot /usr/bin/grep-aptavail dctrl-tools
 
-	case "${LB_BUILD_WITH_CHROOT}" in
-		true)
-			# Restoring cache
-			Restore_cache cache/packages.chroot
+	# Restoring cache
+	Restore_cache cache/packages.chroot
 
-			# Check depends
-			Check_package chroot /usr/bin/grep-aptavail dctrl-tools
-
-			# Installing depends
-			Install_package
-			;;
-
-		false)
-			Check_package host /usr/bin/grep-aptavail dctrl-tools
-			;;
-	esac
+	# Installing depends
+	Install_package
 
 	if [ -e "${LIVE_BUILD}/share/bin/Packages" ]
 	then
@@ -163,15 +153,11 @@ then
 	rm -rf chroot/binary.deb
 	mv chroot/var/lib/dpkg/status.tmp chroot/var/lib/dpkg/status
 
-	case "${LB_BUILD_WITH_CHROOT}" in
-		true)
-			# Removing depends
-			Remove_package
+	# Removing depends
+	Remove_package
 
-			# Saving cache
-			Save_cache cache/packages.chroot
-			;;
-	esac
+	# Saving cache
+	Save_cache cache/packages.chroot
 
 	# Creating stage file
 	Create_stagefile .build/binary_package-lists
diff --git a/scripts/build/binary_syslinux b/scripts/build/binary_syslinux
index 1347aa20b..bb6658826 100755
--- a/scripts/build/binary_syslinux
+++ b/scripts/build/binary_syslinux
@@ -114,42 +114,13 @@ else
 fi
 
 # Checking depends
-case "${LB_BUILD_WITH_CHROOT}" in
-	true)
-		Check_package chroot /usr/lib/$(echo ${_BOOTLOADER} | tr [a-z] [A-Z]) ${_BOOTLOADER}
-		Check_package chroot /usr/lib/syslinux syslinux-common
-
-		if ls "${_SOURCE}"/*.svg* > /dev/null 2>&1
-		then
-			Check_package chroot /usr/bin/rsvg-convert librsvg2-bin
-		fi
-		;;
+Check_package chroot /usr/lib/$(echo ${_BOOTLOADER} | tr [a-z] [A-Z]) ${_BOOTLOADER}
+Check_package chroot /usr/lib/syslinux syslinux-common
 
-	false)
-		if [ ! -e "/usr/share/$(echo ${_BOOTLOADER} | tr [a-z] [A-Z])" ]
-		then
-			Echo_error "/usr/share/$(echo ${_BOOTLOADER} | tr [a-z] [A-Z])"
-			exit 1
-		fi
-
-		if [ ! -e /usr/lib/syslinux ]
-		then
-			# syslinux-common
-			Echo_error "/usr/lib/syslinux - no such directory"
-			exit 1
-		fi
-
-		if ls "${_SOURCE}"/*.svg* > /dev/null 2>&1
-		then
-			if [ ! -e /usr/bin/rsvg-convert ]
-			then
-				# librsvg2-bin
-				Echo_error "/usr/bin/rsvg-convert - no such file"
-				exit 1
-			fi
-		fi
-		;;
-esac
+if ls "${_SOURCE}"/*.svg* > /dev/null 2>&1
+then
+	Check_package chroot /usr/bin/rsvg-convert librsvg2-bin
+fi
 
 # Restoring cache
 Restore_cache cache/packages.binary
diff --git a/scripts/build/binary_win32-loader b/scripts/build/binary_win32-loader
index eecb6b263..710587283 100755
--- a/scripts/build/binary_win32-loader
+++ b/scripts/build/binary_win32-loader
@@ -55,17 +55,17 @@ esac
 
 case "${LB_ARCHITECTURES}" in
 	amd64|i386)
-		if [ "${LB_BUILD_WITH_CHROOT}" = "true" ]
-		then
-			# Checking depends
-			Check_package chroot /usr/share/win32/win32-loader.exe win32-loader
+		# Checking depends
+		Check_package chroot /usr/share/win32/win32-loader.exe win32-loader
 
-			# Restoring cache
-			Restore_cache cache/packages.binary
+		# Restoring cache
+		Restore_cache cache/packages.binary
 
-			# Installing depends
-			Install_package
+		# Installing depends
+		Install_package
 
+		if [ "${LB_BUILD_WITH_CHROOT}" = "true" ]
+		then
 			# Copying win32-loader
 			cp -r chroot/usr/share/win32/* binary
 		else
diff --git a/scripts/build/chroot_package-lists b/scripts/build/chroot_package-lists
index d64fe49e0..26a24061d 100755
--- a/scripts/build/chroot_package-lists
+++ b/scripts/build/chroot_package-lists
@@ -49,26 +49,14 @@ then
 	exit 0
 fi
 
-case "${LB_BUILD_WITH_CHROOT}" in
-	true)
-		# Checking depends
-		Check_package chroot /usr/bin/grep-aptavail dctrl-tools
-
-		# Restoring cache
-		Restore_cache cache/packages.chroot
-
-		# Installing depends
-		Install_package
-		;;
-
-	false)
-		if [ ! -e /usr/bin/grep-aptavail ]; then
-			# dctrl-tools
-			Echo_error "/usr/bin/grep-aptavail - no such file."
-			exit 1
-		fi
-		;;
-esac
+# Checking depends
+Check_package chroot /usr/bin/grep-aptavail dctrl-tools
+
+# Restoring cache
+Restore_cache cache/packages.chroot
+
+# Installing depends
+Install_package
 
 if [ -e "${LIVE_BUILD}/share/bin/Packages" ]
 then
@@ -102,19 +90,10 @@ done
 
 rm -f chroot/bin/Packages
 
-case "${LB_BUILD_WITH_CHROOT}" in
-	true)
-		# Removing dctrl-tools again if the user has not installed it
-		if ! grep -qs dctrl-tools chroot/root/packages.chroot
-		then
-			# Removing depends
-			Remove_package
-		fi
-
-		# Saving cache
-		Save_cache cache/packages.chroot
-		;;
-esac
+Remove_package
+
+# Saving cache
+Save_cache cache/packages.chroot
 
 # Creating stage file
 Create_stagefile .build/chroot_package-lists.${_PASS}
-- 
2.11.0

Reply via email to