From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH 7/8] raisin: Rework component specification Date: Mon, 20 Apr 2015 10:17:32 +0100 Message-ID: <5534C42C.8020208@eu.citrix.com> References: <1429201182-1044-1-git-send-email-george.dunlap@eu.citrix.com> <1429201182-1044-7-git-send-email-george.dunlap@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: Stefano Stabellini , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 04/17/2015 11:37 AM, Stefano Stabellini wrote: > 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 > > Good idea, I wanted to fix the way we disable components for a while now > > >> CC: Stefano Stabellini >> --- >> 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. I think the problem here is that if you just say "COMPONENTS=" in the config file, then it will override the COMPONENTS= on the command-line (i.e., 'COMPONENTS="xen" ./raise build' no longer works as expected). We could use one of the "set if not set already" bashisms, but I think those are all pretty ugly, and probably easy to accidentally forget when editing. Since the config file is part of the public interface, I thought it important to keep it simple and clean. >> # 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 > } I'll do the 'if' first, and maybe look at making a function later. >> +function get_components() { >> + if [[ -z "$COMPONENTS" ]] >> + then >> + COMPONENTS="$DEFAULT_COMPONENTS" > > Stray tab. Weird... I thought I had emacs set to use no tabs. I'll take a look... > I would call our internal components variable RAISIN_COMPONENTS and the > one in the config and exposed to users COMPONENTS. Any revision to this given my answer above? Off the top of my head I don't see a reason to have three variables. -George