Standardize invocations of emerge in build_image

build_image

Index: build_image
diff --git a/build_image b/build_image
index ba9ac9180954e8bf224fd74b261415e75eaa3642..96bb4b225d0eab3f2fa1ce092fa2ed7ea4693743 100755
--- a/build_image
+++ b/build_image
@@ -145,17 +145,17 @@ check_blacklist
 # TODO(vlaviano): Validate command line flags. Check for conflicting flags and
 # reconcile them if possible. Exit with an error message otherwise.
 
-INSTALL_MASK=""
-if [ "${FLAGS_installmask}" -eq "${FLAGS_TRUE}" ] ; then
+export INSTALL_MASK=""

[kliegs, 2011/04/07 14:34:24]

Is this export needed?  I thought export wasn't needed for variables declared at
file scope in scripts and prevents the risk of environment pollution if the
script is accidentally called wrong.

+if [ ${FLAGS_installmask} -eq ${FLAGS_TRUE} ] ; then
   INSTALL_MASK="${DEFAULT_INSTALL_MASK}"
 fi
 
 # Reduce the size of factory install shim.
-if [ "${FLAGS_factory_install}" -eq "${FLAGS_TRUE}" ]; then
+if [ ${FLAGS_factory_install} -eq ${FLAGS_TRUE} ]; then
   # Disable --withdev flag when --factory_install is set to True. Otherwise, the
   # dev image produced will be based on install shim, rather than a pristine
   # image
-  if [ "${FLAGS_withdev}" -eq "${FLAGS_TRUE}" ]; then
+  if [ ${FLAGS_withdev} -eq ${FLAGS_TRUE} ]; then
     info "Incompatible flags: --withdev and --factory_install cannot both be \
 set to True. Resetting --withdev to False."
     FLAGS_withdev=${FLAGS_FALSE}
@@ -171,14 +171,14 @@ set to True. Resetting --withdev to False."
   FLAGS_statefulfs_size=140
 fi
 
-if [ "$((FLAGS_rootfs_size + FLAGS_rootfs_hash_pad))" -gt \
-     "${FLAGS_rootfs_partition_size}" ] ; then
+if [ $((FLAGS_rootfs_size + FLAGS_rootfs_hash_pad)) -gt \
+     ${FLAGS_rootfs_partition_size} ] ; then
   die "rootfs ($((FLAGS_rootfs_size + FLAGS_rootfs_hash_pad)) MiB) is \
 bigger than partition (${FLAGS_rootfs_partition_size} MiB)."
 fi
 
 EMERGE_BOARD_CMD="emerge-${FLAGS_board}"
-if [ "${FLAGS_fast}" -eq "${FLAGS_TRUE}" ]; then
+if [ ${FLAGS_fast} -eq ${FLAGS_TRUE} ]; then
   echo "Using alternate emerge"
   EMERGE_CMD="${GCLIENT_ROOT}/chromite/bin/parallel_emerge"
   EMERGE_BOARD_CMD="${EMERGE_CMD} --board=${FLAGS_board}"
@@ -191,21 +191,24 @@ OVERLAY_CHROMEOS_DIR="${SRC_ROOT}/third_party/chromiumos-overlay/chromeos/"
 
 # Configure extra USE or packages for this type of build.
 EXTRA_PACKAGES=""
-EXTRA_USE=""
-if [ "${FLAGS_factory_install}" -eq "${FLAGS_TRUE}" ] ; then
+if [ ${FLAGS_factory_install} -eq ${FLAGS_TRUE} ] ; then
   # Factory install needs to have the kernel initrmafs enabled,
   # and the factory installer added.
   EXTRA_PACKAGES="${EXTRA_PACKAGES} chromeos-base/chromeos-factoryinstall"
-  EXTRA_USE="${EXTRA_USE} initramfs"
+  export USE="${USE} initramfs"

[kliegs, 2011/04/07 14:34:24]

Same as above - is export actually needed here?

 fi
 
+emerge_to_image() {
+  sudo -E ${EMERGE_BOARD_CMD} --root-deps=rdeps --usepkgonly \
+    "$@" ${EMERGE_JOBS}
+}
+
 # Freshen kernel with correct USE flags. This is a noop if we have
 # the right kernel prebuilt. Factory install uses USE="initramfs".
 # We don't allow building from source with the image as a target,
 # and it's not possible to store prebuilts for the same package
 # with different use flags.
-USE="${EXTRA_USE} ${USE}" emerge-${FLAGS_board} \
-    -uNDvg --binpkg-respect-use=y virtual/kernel
+emerge-${FLAGS_board} -uNDvg --binpkg-respect-use=y virtual/kernel

[davidjames, 2011/04/07 06:32:18]

--binpkg-respect-use=y is implied by -N so we can get rid of that.

-uDNvg would be more consistent with the rest of the file than -uNDvg

[jrbarnette, 2011/04/07 19:13:45]

I'll update the options.

Should the command use ${EMERGE_BOARD_CMD} instead of
emerge-${FLAGS_board}?

[davidjames, 2011/04/07 19:26:16]

Yes -- parallel_emerge used to have a bug where it didn't support -N correctly,
but that's fixed now, so we can use ${EMERGE_BOARD_CMD} again.

 
 # Use canonical path since some tools (e.g. mount) do not like symlinks.
 # Append build attempt to output directory.
@@ -218,11 +221,11 @@ OUTSIDE_OUTPUT_DIR="../build/images/${FLAGS_board}/${IMAGE_SUBDIR}"
 # different name.
 DEVELOPER_IMAGE_NAME=
 PRISTINE_IMAGE_NAME=chromiumos_image.bin
-if [ "${FLAGS_withdev}" -eq "${FLAGS_TRUE}" ]; then
+if [ ${FLAGS_withdev} -eq ${FLAGS_TRUE} ]; then
   PRISTINE_IMAGE_NAME=chromiumos_base_image.bin
   DEVELOPER_IMAGE_NAME=chromiumos_image.bin
 # Rename pristine image for factory install shim
-elif [ "${FLAGS_factory_install}" -eq "${FLAGS_TRUE}" ]; then
+elif [ ${FLAGS_factory_install} -eq ${FLAGS_TRUE} ]; then
   PRISTINE_IMAGE_NAME=factory_install_shim.bin
 fi
 
@@ -418,15 +421,11 @@ update_dev_packages() {
     root_dev_dir="${ROOT_FS_DIR}/usr/local"
 
   # Install developer packages described in chromeos-dev.
-  sudo -E USE="${USE} ${EXTRA_USE}" INSTALL_MASK="${INSTALL_MASK}" \
-      ${EMERGE_BOARD_CMD} --root="${root_dev_dir}" --root-deps=rdeps \
-      --usepkgonly -uDNv chromeos-dev ${EMERGE_JOBS}
+  emerge_to_image --root="${root_dev_dir}" -uDNv chromeos-dev
 
   if [[ $FLAGS_preserve -eq ${FLAGS_TRUE} ]] ; then
     # Clean out unused packages
-    sudo -E USE="${USE} ${EXTRA_USE}" INSTALL_MASK="${INSTALL_MASK}" \
-      ${EMERGE_BOARD_CMD} --root="${ROOT_FS_DIR}" --root-deps=rdeps \
-      --usepkgonly --depclean ${EMERGE_JOBS}
+    emerge_to_image --root="${ROOT_FS_DIR}" --depclean
   fi
 
   # Install the bare necessary files so that the "emerge" command works
@@ -506,14 +505,10 @@ update_base_packages() {
     -s "${STATEFUL_FS_DIR}" -e "${ESP_FS_DIR}"
 
   # Emerge updated packages, exactly like when creating base image
-  sudo USE="${EXTRA_USE}" INSTALL_MASK="${INSTALL_MASK}" ${EMERGE_BOARD_CMD} \
-    --root="${ROOT_FS_DIR}" --root-deps=rdeps \
-    --usepkgonly -uDNv chromeos ${EXTRA_PACKAGES} ${EMERGE_JOBS}
+  emerge_to_image --root="${ROOT_FS_DIR}" -uDNv chromeos ${EXTRA_PACKAGES}
 
   # Clean out unused packages
-  sudo INSTALL_MASK="${INSTALL_MASK}" ${EMERGE_BOARD_CMD} \
-    --root="${ROOT_FS_DIR}" --root-deps=rdeps \
-    --usepkgonly --depclean ${EMERGE_JOBS}
+  emerge_to_image --root="${ROOT_FS_DIR}" --depclean
 
   trap - EXIT
   ${SCRIPTS_DIR}/mount_gpt_image.sh -u -r "${ROOT_FS_DIR}" \
@@ -638,9 +633,7 @@ create_base_image() {
   # runtime packages for chrome os. This builds up a chrome os image from
   # binary packages with runtime dependencies only.  We use INSTALL_MASK to
   # trim the image size as much as possible.
-  sudo USE="${EXTRA_USE}" INSTALL_MASK="${INSTALL_MASK}" ${EMERGE_BOARD_CMD} \
-    --root="${ROOT_FS_DIR}" --root-deps=rdeps \
-    --usepkgonly chromeos ${EXTRA_PACKAGES} ${EMERGE_JOBS}
+  emerge_to_image --root="${ROOT_FS_DIR}" chromeos ${EXTRA_PACKAGES}

[davidjames, 2011/04/07 06:32:18]

Could we add -uDNv here? That way, we won't ever install packages that don't
match the user's use flags -- they'll fail right away even if they're in
--nowithdev mode.

[davidjames, 2011/04/07 06:41:18]

On second thought, let's not do that  -- it'll probably sadly break the world as
I bet there are people and scripts that are still depending on not having to set
USE flags for build_image...

See the --nowithdebug flag for build_packages for an example. It sets a USE flag
in build_packages but that use flag will be nowhere to be found by the time
build_image comes around. Surprised that it works but I think it does...

[kliegs, 2011/04/07 14:34:24]

Agreed - I know up until this week I never passed extra USE flags to build_image
but did to build_packages so this would break that case.  Might be worth filing
a bug on for future consideration but should go in a different CL.

 
   # Set /etc/lsb-release on the image.
   "${OVERLAY_CHROMEOS_DIR}/scripts/cros_set_lsb_release" \
@@ -738,7 +731,7 @@ if [[ $FLAGS_preserve -eq ${FLAGS_TRUE} ]] ; then
 fi
 
 # Perform an eclean to remove packages which are not installed
-if [[ "${FLAGS_eclean}" -eq "${FLAGS_TRUE}" ]]; then
+if [[ ${FLAGS_eclean} -eq ${FLAGS_TRUE} ]]; then
   eclean-${FLAGS_board} -d packages
 fi
 
@@ -754,7 +747,7 @@ else
 fi
 
 USE_DEV_KEYS=
-if [ "${FLAGS_factory_install}" -eq "${FLAGS_TRUE}" ]; then
+if [ ${FLAGS_factory_install} -eq ${FLAGS_TRUE} ]; then
   USE_DEV_KEYS="--use_dev_keys"
 fi
 
@@ -768,9 +761,9 @@ ${SCRIPTS_DIR}/bin/cros_make_image_bootable "${OUTPUT_DIR}" \
 # should unconditionally test an image after crosbug12352 is fixed
 if [[ "${ARCH}" = "x86" ]] ||
    [[ "${ARCH}" = "arm" &&
-      "${FLAGS_crosbug12352_arm_kernel_signing}" -eq "${FLAGS_TRUE}" ]]; then
+      ${FLAGS_crosbug12352_arm_kernel_signing} -eq ${FLAGS_TRUE} ]]; then
   BOOT_FLAG=
-  if [ "${FLAGS_factory_install}" -eq "${FLAGS_TRUE}" ]; then
+  if [ ${FLAGS_factory_install} -eq ${FLAGS_TRUE} ]; then
     BOOT_FLAG="-b 1"  # BOOT_FLAG_DEVELOPER value defined in load_kernel_fw.h
     info "--factory_install set, pass BOOT_FLAG_DEVELOPER flag to \
 load_kernel_test"
@@ -782,7 +775,7 @@ load_kernel_test"
 fi
 
 # Create a developer image based on the chromium os base image.
-if [ "${FLAGS_withdev}" -eq "${FLAGS_TRUE}" ] ; then
+if [ ${FLAGS_withdev} -eq ${FLAGS_TRUE} ] ; then
   if [[ ! -f ${DEVELOPER_IMG} ]] ; then
     echo "Creating developer image from base image ${PRISTINE_IMAGE_NAME}"
     cp ${PRISTINE_IMG} ${DEVELOPER_IMG}
@@ -807,7 +800,7 @@ ln -s $(basename ${OUTPUT_DIR}) ${FLAGS_output_root}/${FLAGS_board}/latest
 
 echo "Done.  Image created in ${OUTPUT_DIR}"
 echo "Chromium OS image created as ${PRISTINE_IMAGE_NAME}"
-if [ "${FLAGS_withdev}" -eq "${FLAGS_TRUE}" ]; then
+if [ ${FLAGS_withdev} -eq ${FLAGS_TRUE} ]; then
   echo "Developer image created as ${DEVELOPER_IMAGE_NAME}"
 fi