|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 8/8] Refactor package dependency checking and installation
On Thu, 9 Apr 2015, George Dunlap wrote:
> First, create a new global variable, PKGTYPE. At the moment support "deb"
> and "rpm".
>
> Define _check-package-$PKGTYPE which returns true if the package is
> installed, false otherwise, and _install-package-$PKGTYPE which will
> install a list of packages.
>
> Define check-package(), which will take a list of packages, and check
> to see if they're installed. Any missing packages will be added to an
> array called "missing".
>
> Change _${COMPONENT}_install_dependencies to
> ${COMPONENT}_check_package. Have these call check-package.
>
> Don't call _${COMPONENT}_install_dependencies from ${COMPONENT}_build.
>
> Define check-builddeps(). Define an empty "missing" array. Call
> check-package for "raisin" dependincies (like git and rpmbuild). Then
> call for_each_component check_package.
>
> At this point we have an array with all missing packages. If it's
> empty, be happy. If it's non-empty, and deps=true, try to install the
> packages; otherwise print the missing packages and exit.
>
> Add install-builddeps(), which is basically check-builddeps() with
> deps=true by default.
>
> Call check-builddeps from build() to close the loop.
I think that this is a good idea. It is nice to be able to check whether
the deps are already installed before going ahead and installing them. I
also like the new command to install dependencies and do nothing else.
It needs to be changed a bit to work without core.sh but it is mostly
OK.
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> ---
> CC: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> ---
> components/grub | 6 ++--
> components/libvirt | 6 ++--
> components/xen | 10 +++---
> lib/build.sh | 87 +++++++++++++++++++++++++++++++++++------------
> lib/common-functions.sh | 89
> +++++++++++++++++++++++++++++++++++++++----------
> 5 files changed, 148 insertions(+), 50 deletions(-)
>
> diff --git a/components/grub b/components/grub
> index a5aa27d..839e001 100644
> --- a/components/grub
> +++ b/components/grub
> @@ -1,6 +1,6 @@
> #!/usr/bin/env bash
>
> -function _grub_install_dependencies() {
> +function grub_check_package() {
> local DEP_Debian_common="build-essential tar autoconf bison flex"
> local DEP_Debian_x86_32="$DEP_Debian_common"
> local DEP_Debian_x86_64="$DEP_Debian_common libc6-dev-i386"
> @@ -18,8 +18,8 @@ function _grub_install_dependencies() {
> echo grub is only supported on x86_32 and x86_64
> return
> fi
> - echo installing Grub dependencies
> - eval install_dependencies \$DEP_"$DISTRO"_"$ARCH"
> + echo checking Grub dependencies
> + eval check-package \$DEP_"$DISTRO"_"$ARCH"
> }
>
Need to change build not to call _grub_install_dependencies
> diff --git a/components/libvirt b/components/libvirt
> index 6602dcf..b106970 100644
> --- a/components/libvirt
> +++ b/components/libvirt
> @@ -1,6 +1,6 @@
> #!/usr/bin/env bash
>
> -function _libvirt_install_dependencies() {
> +function libvirt_check_package() {
> local DEP_Debian_common="build-essential libtool autoconf autopoint \
> xsltproc libxml2-utils pkg-config python-dev \
> libxml-xpath-perl libyajl-dev libxml2-dev \
> @@ -18,8 +18,8 @@ function _libvirt_install_dependencies() {
> local DEP_Fedora_x86_32="$DEP_Fedora_common"
> local DEP_Fedora_x86_64="$DEP_Fedora_common"
>
> - echo installing Libvirt dependencies
> - eval install_dependencies \$DEP_"$DISTRO"_"$ARCH"
> + echo checking Libvirt dependencies
> + eval check-package \$DEP_"$DISTRO"_"$ARCH"
> }
Same here
> function libvirt_build() {
> diff --git a/components/xen b/components/xen
> index 7a9f22d..ce46e3d 100644
> --- a/components/xen
> +++ b/components/xen
> @@ -1,6 +1,8 @@
> #!/usr/bin/env bash
>
> -function _xen_install_dependencies() {
> +function xen_check_package() {
> + $requireargs DISTRO ARCH
> +
> local DEP_Debian_common="build-essential python-dev gettext uuid-dev \
> libncurses5-dev libyajl-dev libaio-dev pkg-config
> libglib2.0-dev \
> libssl-dev libpixman-1-dev bridge-utils wget"
> @@ -15,13 +17,11 @@ function _xen_install_dependencies() {
> local DEP_Fedora_x86_32="$DEP_Fedora_common dev86 acpica-tools texinfo"
> local DEP_Fedora_x86_64="$DEP_Fedora_x86_32 glibc-devel.i686"
>
> - echo installing Xen dependencies
> - eval install_dependencies \$DEP_"$DISTRO"_"$ARCH"
> + echo Checking Xen dependencies
> + eval check-package \$DEP_"$DISTRO"_"$ARCH"
> }
>
> function xen_build() {
> - _xen_install_dependencies
> -
> cd "$BASEDIR"
> git-checkout $XEN_UPSTREAM_URL $XEN_UPSTREAM_REVISION xen-dir
> cd xen-dir
> diff --git a/lib/build.sh b/lib/build.sh
> index ab1e087..a453874 100755
> --- a/lib/build.sh
> +++ b/lib/build.sh
> @@ -2,32 +2,72 @@
>
> set -e
>
> -_help() {
> - echo "Usage: ./build.sh <options> <command>"
> +RAISIN_HELP+=("check-builddep Check to make sure we have all dependencies
> installed")
> +function check-builddep() {
> + local -a missing
> +
> + $arg_parse
> +
> + default deps "false" ; $default_post
> +
> + $requireargs PKGTYPE DISTRO
> +
> + check-package git
> +
> + if [[ $DISTRO = "Fedora" ]] ; then
> + check-package rpm-build
> + fi
> +
> + for_each_component check_package
> +
> + if [[ -n "${missing[@]}" ]] ; then
> + echo "Missing packages: ${missing[@]}"
> + if $deps ; then
> + echo "Installing..."
> + install-package "${missing[@]}"
> + else
> + echo "Please install, or run ./raise install-builddep"
> + exit 1
> + fi
> + fi
> +}
indentation is wrong
> +RAISIN_HELP+=("install-builddep Install all packages required for build.
> Uses sudo.")
> +function install-builddep() {
> + local pkgtype
> +
> + $arg_parse
> +
> + check-builddep deps=true
> +}
> +
> +build-help() {
> + echo "Usage: $0 build opt=val"
> echo "where options are:"
> - echo " -n | --no-deps Do no install build-time dependencies"
> - echo " -v | --verbose Verbose"
> - echo " -y | --yes Do not ask questions and continue"
> - echo "where commands are:"
> - echo " build Build the components enabled in config"
> - echo " install Install binaries under / (requires sudo)"
> - echo " configure Configure the system (requires sudo)"
> + echo " yes=(true|false) Don't ask before doing anything
> dangerous. Defaults to false."
> + echo " deps=(true|false) Attempt to install dependencies.
> Defaults to false."
> + echo " verbose=(true|false) Increased verbosity. Defaults to false."
> }
I would keep the old style help and options for simplicity. Removing the
deps option entirely is OK I think.
> +RAISIN_HELP+=("build Clone and build components enabled in config")
> build() {
> $arg_parse
>
> - if [[ $YES != "y" ]]
> - then
> - echo "Do you want Raisin to automatically install build time
> dependencies for you? (y/n)"
> + default deps "false" ; $default_post
> + default verbose "false" ; $default_post
> + default yes "false" ; $default_post
> +
> + if $deps && ! $yes ; then
> + echo "WARNING: You have chosen to automatically install software as
> root."
> + echo -n "Are you sure that you want to continue? (y/n)"
> while read answer
> do
> if [[ "$answer" = "n" ]]
> then
> - NO_DEPS=1
> - break
> + exit 0
> elif [[ "$answer" = "y" ]]
> then
> + yes="true"
> break
> else
> echo "Reply y or n"
I would keep the warning as it was before
> @@ -35,12 +75,10 @@ build() {
> done
> fi
>
> + check-builddep
> +
> mkdir -p "$INST_DIR" &>/dev/null
> - install_dependencies git
> - if [[ $DISTRO = "Fedora" ]]
> - then
> - install_dependencies rpm-build
> - fi
> +
> # build and install under $DESTDIR
> for_each_component clean
> for_each_component build
> @@ -59,6 +97,7 @@ unraise() {
> rm -rf "$INST_DIR"
> }
>
> +RAISIN_HELP+=("install Install binaries under / (requires sudo)")
> install() {
> $arg_parse
>
> @@ -71,12 +110,15 @@ install() {
> install_package xen-system
> }
>
> +RAISIN_HELP+=("configure Configure the system (requires sudo)")
> configure() {
> $arg_parse
>
> - if [[ $YES != "y" ]]
> - then
> - echo "Proceeding we'll make changes to the running system,"
> + default verbose "false" ; $default_post
> + default yes "false" ; $default_post
> +
> + if $deps && ! $yes ; then
> + echo "WARNING: Proceeding we'll make changes to the running system,"
> echo "are you sure that you want to continue? (y/n)"
> while read answer
> do
> @@ -85,6 +127,7 @@ configure() {
> exit 0
> elif [[ "$answer" = "y" ]]
> then
> + yes="true"
> break
> else
> echo "Reply y or n"
> diff --git a/lib/common-functions.sh b/lib/common-functions.sh
> index cfa0c7f..fd73f08 100644
> --- a/lib/common-functions.sh
> +++ b/lib/common-functions.sh
> @@ -97,15 +97,23 @@ function get_distro() {
> case "$os_VENDOR" in
> "Debian"* | "Ubuntu"* | "LinuxMint"* )
> DISTRO="Debian"
> + PKGTYPE="deb"
> + ;;
> + "Fedora" )
> + DISTRO="Fedora"
> + PKGTYPE="rpm"
> ;;
> "SUSE"* )
> DISTRO="SUSE"
> + PKGTYPE="rpm"
> ;;
> "OpenSUSE"* | "openSUSE"* )
> DISTRO="openSUSE"
> + PKGTYPE="rpm"
> ;;
> "Red"* | "CentOS"* )
> DISTRO="CentOS"
> + PKGTYPE="rpm"
> ;;
> *)
> DISTRO=$os_VENDOR
> @@ -114,6 +122,7 @@ function get_distro() {
>
> export os_VENDOR os_RELEASE os_UPDATE os_CODENAME
> export DISTRO
> + export PKGTYPE
> }
>
> function get_arch() {
> @@ -122,24 +131,70 @@ function get_arch() {
> -e s/aarch64/arm64/`
> }
>
> -function install_dependencies() {
> - if [[ "$NO_DEPS" && "$NO_DEPS" -eq 1 ]]
> - then
> - echo "Not installing any dependencies, as requested."
> - echo "Depency list: $*"
> - return 0
> +function _check-package-deb() {
> + $arg_parse
> +
> + $verbose && echo "Checking for package ${args[0]}"
> +
> + if dpkg -s "${args[0]}" 2>/dev/null | grep -q "Status:.*installed" ; then
> + return 0
> + else
> + return 1
> fi
> - case $DISTRO in
> - "Debian" )
> - $SUDO apt-get install -y $*
> - ;;
> - "Fedora" )
> - $SUDO yum install -y $*
> - ;;
> - * )
> - echo "I don't know how to install dependencies on $DISTRO"
> - ;;
> - esac
> +}
> +
> +function _install-package-deb() {
> + $arg_parse
> +
> + $requireargs SUDO
> +
> + $SUDO apt-get install -y "${args[@]}"
> +}
> +
> +function _check-package-rpm() {
> + local pstatus
> +
> + $arg_parse
> +
> + $verbose && echo "Checking for package ${args[0]}"
> +
> + if rpm -q "${args[0]}" 2>&1 >/dev/null ; then
> + return 0
> + else
> + return 1
> + fi
> +}
Indentation and coding style
> +function _install-package-rpm() {
> + $arg_parse
> +
> + $requireargs SUDO
> +
> + $SUDO yum install -y "${args[@]}"
> +}
> +
> +# Modifies inherited variable "missing"
> +function check-package() {
> + local p
> +
> + $arg_parse
> +
> + $requireargs PKGTYPE
> +
> + for p in "${args[@]}" ; do
> + if ! _check-package-${PKGTYPE} $p ; then
> + missing+=("$p")
> + fi
> + done
same here
> +}
> +
> +function install-package() {
> + $arg_parse
> +
> + $requireargs PKGTYPE
> +
> + _install-package-${PKGTYPE} "${args[@]}"
> }
>
> function start_initscripts() {
> --
> 1.9.1
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |