From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Luis R. Rodriguez" Subject: Re: [PATCH v6 12/13] autoconf: xen: enable explicit preference option for xenstored preference Date: Thu, 3 Jul 2014 20:57:55 +0200 Message-ID: <20140703185755.GJ27687@wotan.suse.de> References: <1402622331-4282-1-git-send-email-mcgrof@do-not-panic.com> <1402622331-4282-13-git-send-email-mcgrof@do-not-panic.com> <1404309271.5562.96.camel@kazak.uk.xensource.com> <20140702200911.GY27687@wotan.suse.de> <1404379240.14865.15.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1X2mCj-0000z6-2R for xen-devel@lists.xenproject.org; Thu, 03 Jul 2014 18:57:57 +0000 Content-Disposition: inline In-Reply-To: <1404379240.14865.15.camel@kazak.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 Campbell Cc: Keir Fraser , "Luis R. Rodriguez" , Tim Deegan , Ian Jackson , Jan Beulich , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On Thu, Jul 03, 2014 at 10:20:40AM +0100, Ian Campbell wrote: > On Wed, 2014-07-02 at 22:09 +0200, Luis R. Rodriguez wrote: > > On Wed, Jul 02, 2014 at 02:54:31PM +0100, Ian Campbell wrote: > > > On Thu, 2014-06-12 at 18:18 -0700, Luis R. Rodriguez wrote: > > > > Since the xenstored preference > > > > is explicit now and since we require configure substitutions for it we > > > > make use of the AX_XEN_EXPAND_CONFIG() helpers as otherwise substitution > > > > for SBINDIR is not propagated from the top level configuration. > > > > > > I'm afraid I don't understand what is going on here. Why is SBINDIR not > > > propagated? > > > > Its saying that we need AX_XEN_EXPAND_CONFIG() in order to do substitutions > > for variables that include SBINDIR in this case for the xenstore full path. > > Specifically you mean that the autoconf/m4 code now uses these variables > in addition to them appearing in Paths.mk? Right. > Strictly speaking you don't > need the associated AC_SUBST's but they are harmless. Without the AC_SUBST() I do not believe substitiution is done by autoconf on Paths.mk. > > > > Since we are now parsing an entry within Paths.mk.in on tools we let > > > > the move the parsing of the file to be the tool's configure. > > > > > > I can't parse this. > > > > I meant to clarify that since we now have to do a substitution on tools > > we move the calls to AX_XEN_EXPAND_CONFIG() there. This patch should have > > also removed the AX_XEN_EXPAND_CONFIG() from the top level configure.ac. > > I think moving it would break stubdoms under ./configure > --disable-tools? > > I think I would suggest keeping AX_XEN_EXPAND_CONFIG and the generation > of config/Paths.mk in the top level configure but call > AX_XEN_EXPAND_CONFIG again in tools/configure.ac without the generation > of config/Paths.mk (I think the AC_SUBST's are benign for this second > call) AX_XEN_EXPAND_CONFIG() is needed in both as we rely on some variable substitutions like SBINDIR on the tools path as well. > > After testing this though and removing AX_XEN_EXPAND_CONFIG() from configure.ac > > I do see though that substitution doesn't work, it only works if we leave > > AX_XEN_EXPAND_CONFIG() on both places. > > I should have read this next para before commenting above! OK I take it this means I can keep it on both? > > This is the issue I mentioned that > > I ran into and you had proposed we could try to figure out a fix if > > this was happening. Well it is. Any ideas ? Or are you OK to live with > > calling AX_XEN_EXPAND_CONFIG() twice, once on configure.ac and another > > on tools/configure.ac ? > > Yes, so long as the generation of config/Paths.mk remains in the > toplevel configure only. OK so use AX_XEN_EXPAND_CONFIG() on both top level configure.ac and tools/configure.ac but only have config/Paths.mk be a AC_CONFIG_FILES() for the top level configure.ac ? I think that's how the v6 patch was then. Luis