Re: [Xen-devel] [PATCH] tools: fix dependency for ipxe and rombios

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Wei Liu writes ("[PATCH] tools: fix dependency for ipxe and rombios"):
> It appears that the test in 01d631028 for ipxe's dependency on rombios
> is not good enough. Configuring with --disable-rombios doesn't disable
> ipxe.
> 
> Fix it by testing the dependency in AC_ARG_ENABLE and AC_ARG_WITH at
> the same time. We intentionally omit help string in AC_ARG_ENABLE so
> that it is not visible to users.

But there is already
   AX_ARG_DEFAULT_ENABLE([ipxe], [Disable IPXE])
so now you are defining this argument twice.


Looking at the code for seabios as an example, this is quite tangled.

My reading of the autoconf FM is that the design intent is that one
should support *either* --with/without, *or* --enable/disable.  So I
think what we ought to support is:
   --with-seabios=/PATH
   --with-seabios          use our version
   --without-seabios       disable support for seabios

But, actually, we support these:
   --enable-seabios
   --disable-seabios                
   --with-system-seabios=/path
   --without-system-seabios
And their interactions are weird.  I haven't conclusively analysed the
combinations of these things, but one very odd thing is that
--without-system-seabios disables building our own seabios!


So I'm sorry to say that I think the answer is to revert 01d631028a02
and to replace it with a patch which introduces
   --with-ipxe=/PATH
   --with-ipxe          like --enable-ipxe, but also sets path to ""
   --without-ipxe       same as --disable-ipxe

Sorry for not spotting earlier that we had --enable-ipxe already.


If you want to keep your current approach then certainly you mustn't
duplicate the definition of the --enable-ipxe argument.  But you also
have to think clearly about how --enable-ipxe ought to interact with
--with-system-ipxe and the corresponding rombios options.


What do think ?

ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel