From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH v5] OSSTEST: introduce a raisin build test Date: Wed, 17 Jun 2015 16:12:33 +0100 Message-ID: References: <1431450790-28686-1-git-send-email-stefano.stabellini@eu.citrix.com> <21842.14869.478809.933823@mariner.uk.xensource.com> <21843.30852.550437.285711@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <21843.30852.550437.285711@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: xen-devel@lists.xen.org, wei.liu2@citrix.com, ian.campbell@citrix.com, Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On Wed, 13 May 2015, Ian Jackson wrote: > 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 .= < echo >>config \U$treename\E_URL='$r{"tree_$treename"}' > echo >>config \U$treename\E_REVISION='$r{"revision_$treename"}' > END > } > I see what you mean now, but the variable names are not that regular; this wouldn't work. In particular we have: QEMU_URL, QEMU_REVISION and tree_qemuu, revision_qemuu QEMU_TRADITIONAL_URL, QEMU_TRADITIONAL_REVISION and tree_qemu, revision_qemu > > > 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. OK, I should be able to do that.