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

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Wei Liu writes ("[PATCH v2] 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.
> 
> Now we have four options for ipxe:
> 
>   --enable-ipxe         enable building in-tree ipxe
>   --disable-ipxe        disable building in-tree ipxe
>   --with-system-ipxe    specify a path to be baked into code, disable
>                         building in-tree ipxe, this trumps --{en,dis}able-ipxe

Personally I would have preferred --with-system-ipxe=/PATH to be
spelled --with-ipxe=/PATH but I can live with what you have here.  On
irc you said you preferred it this way for consistency with other
options.  I think the other options are a bit of a mess too, but,
fine.

>   --without-system-ipxe no effect

This, I think, should probably be an error.  As a rule of thumb it is
nicer to bomb out when inconsistent or erroneous options are
specified, rather than letting the build continue and doing something
random.

That applies here too:

> -    # IPXE depends on Rombios
> -    if test "x$enable_rombios" = "xno"; then
> -        AC_MSG_ERROR([Rombios is required for using IPXE])
> -    fi

I think this check should be done after after and outside the
AC_ARG_ENABLE and AC_ARG_WITH so that --enable-ipxe --disable-rombios
is an error.  (And remove the part in the AC_ARG_ENABLE that squashes
this case, then.)

I hope that makes sense.

Thanks,
Ian.

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