From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Jackson Subject: Re: [PATCH v5] OSSTEST: introduce a raisin build test Date: Wed, 13 May 2015 17:15:00 +0100 Message-ID: <21843.30852.550437.285711@mariner.uk.xensource.com> References: <1431450790-28686-1-git-send-email-stefano.stabellini@eu.citrix.com> <21842.14869.478809.933823@mariner.uk.xensource.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: wei.liu2@citrix.com, ian.campbell@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org Stefano Stabellini writes ("Re: [PATCH v5] OSSTEST: introduce a raisin build test"): > On Tue, 12 May 2015, Ian Jackson wrote: > > I don't understand what the \\ are doing here. Perhaps you should use > > '' like in ts-xen-build ? > > I need to retain the " in the output raisin doesn't cope with '' in the config file ? > > This is very repetitive. In ts-xen-build, the names of the variables > > are irregular, but here they are regular. I think you should refactor > > this accordingly. > > I think that the cure here would be worse than the disease. In bash > would be fragile and difficult to read, but you probably would do it in > perl, right? In that case I don't know how it would look like; in fact I > wouldn't know how to write it. However if you are keen on it, feel free > to provide a snippet of code and I'll try to include it in the patch. Yes, it should be done in perl. Something like: foreach my $treename (qw(xen seabios blah blah)) { $buildcmd .= <>config \U$treename\E_URL='$r{"tree_$treename"}' echo >>config \U$treename\E_REVISION='$r{"revision_$treename"}' END } (Untested; adjust to taste.) > > Again, this is copied from ts-xen-build. > > Yes, it is. If this is not a descriptive comment, what would you have me > do? I could move trapping somewhere else common, but I don't think that > generalizing divide and stash is a good idea. divide should be moved into BuildSupport, probably. There, it would need a slightly longer name, and perhaps to take some more of its variables as parameters. It doesn't need to be "generalised", since you want it basically as-is. Ian.