Re: [Xen-devel] [PATCH 7/8] raisin: Rework component specification

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 16 Apr 2015, George Dunlap wrote:
> Allow COMPONENTS to be specified in the config (or on the command-line)
> 
> Now you can keep all components enabled in your config but build only
> one like so:
> 
> COMPONENTS="xen" ./raise build
> 
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>

Good idea, I wanted to fix the way we disable components for a while now


> CC: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> ---
>  defconfig               |  5 +++++
>  lib/common-functions.sh | 49 
> +++++++++++++++++++++++++++++++++++--------------
>  2 files changed, 40 insertions(+), 14 deletions(-)
> 
> diff --git a/defconfig b/defconfig
> index 23c76eb..4ec8a0b 100644
> --- a/defconfig
> +++ b/defconfig
> @@ -1,5 +1,10 @@
>  # Config variables for raisin
>  
> +# Components
> +# All components: xen grub libvirt
> +# Core xen functionality: xen
> +DEFAULT_COMPONENTS="xen grub libvirt"

I would just call this COMPONENTS, also see below.
Also please update the other comment in this file on how to disable components.


>  # Build config
>  ## Make command to run
>  MAKE="make -j2"
> diff --git a/lib/common-functions.sh b/lib/common-functions.sh
> index e66c6f4..42406e9 100644
> --- a/lib/common-functions.sh
> +++ b/lib/common-functions.sh
> @@ -31,13 +31,41 @@ function common_init() {
>  
>      get_distro
>      get_arch
> +    get_components
>  
> -    for f in `cat "$BASEDIR"/components/series`
> +    echo "Distro: $DISTRO"
> +    echo "Arch: $ARCH"
> +    echo "Components: $COMPONENTS"

if [[ $VERBOSE -eq 1 ]]
then
    echo stuff
fi

To make the code nicer to read, we could have a function called
verbose_echo:

function verbose_echo() {
    if [[ $VERBOSE -eq 1 ]]
    then
        echo $*
    fi
}


> +    for f in $COMPONENTS
>      do
>          source "$BASEDIR"/components/"$f"
>      done
>  }
>  
> +function get_components() {
> +    if [[ -z "$COMPONENTS" ]]
> +    then
> +     COMPONENTS="$DEFAULT_COMPONENTS"

Stray tab.
I would call our internal components variable RAISIN_COMPONENTS and the
one in the config and exposed to users COMPONENTS.


> +    fi
> +
> +    if [[ -z "$COMPONENTS" ]] 
> +    then 
> +     local component
> +     for component in `cat "$BASEDIR"/components/series`
> +     do
> +         local capital
> +            capital=`echo $component | tr '[:lower:]' '[:upper:]'`
> +            if eval [[ ! -z \$"$capital"_UPSTREAM_REVISION ]]

please rebase: _UPSTREAM is gone


> +            then
> +             COMPONENTS="$COMPONENTS $component"
> +             echo "Found component $component"

verbose_echo


> +            fi
> +     done
> +     export COMPONENTS

a bad mix of spaces and tabs in this function


> +    fi
> +}
> +
>  function get_distro() {
>      if [[ -x "`which lsb_release 2>/dev/null`" ]]
>      then
> @@ -222,20 +250,13 @@ function stop_initscripts() {
>  }
>  
>  function for_each_component () {
> -    for component in `cat "$BASEDIR"/components/series`
> +    local component
> +
> +    for component in $COMPONENTS
>      do
> -        capital=`echo $component | tr '[:lower:]' '[:upper:]'`
> -        if [[ $VERBOSE -eq 1 ]]
> -        then
> -            echo -n "$capital"_REVISION =" "
> -            eval echo \$"$capital"_REVISION
> -        fi
> -        if eval [[ ! -z \$"$capital"_REVISION ]]
> -        then
> -            [[ $VERBOSE -eq 0 ]] || echo calling "$component"_"$1"
> -            "$component"_"$1"
> -            [[ $VERBOSE -eq 0 ]] || echo "$component"_"$1" done
> -        fi
> +        [[ $VERBOSE -eq 0 ]] || echo calling "$component"_"$1"
> +        "$component"_"$1"
> +        [[ $VERBOSE -eq 0 ]] || echo "$component"_"$1" done
>      done
>  }
>  
> -- 
> 1.9.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel