[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [XEN PATCH v3 2/6] xen: Have Kconfig check $(CC)'s version



On Thu, Jan 16, 2020 at 12:30:49PM +0100, Jan Beulich wrote:
> On 15.01.2020 18:00, Anthony PERARD wrote:
> > --- a/xen/Kconfig
> > +++ b/xen/Kconfig
> > @@ -4,9 +4,25 @@
> >  #
> >  mainmenu "Xen/$(SRCARCH) $(XEN_FULLVERSION) Configuration"
> >  
> > +source "scripts/Kconfig.include"
> > +
> >  config BROKEN
> >     bool
> >  
> > +config CC_IS_GCC
> > +   def_bool $(success,$(CC) --version | head -n 1 | grep -q gcc)
> > +
> > +config GCC_VERSION
> > +   int
> > +   default $(shell,$(BASEDIR)/scripts/gcc-version.sh $(CC))
> > +
> > +config CC_IS_CLANG
> > +   def_bool $(success,$(CC) --version | head -n 1 | grep -q clang)
> > +
> > +config CLANG_VERSION
> > +   int
> > +   default $(shell,$(BASEDIR)/scripts/clang-version.sh $(CC))
> 
> I continue to be unhappy about the redundancy, but I will accept
> it if others indeed think this is helpful. However, I don't see
> then why the setting of CC_IS_* need another shell invocation
> each - this could just be *_VERSION > 0 then, seeing that the
> scripts already to a respective grep of the --version output.

From functionality point of view, replacing the macro by
"def_bool %_VERSION > 0" in "config CC_IS_%" would be fine, even so it
would be weird to read. I think that would need a comment saying:
  # %-version.sh is expected to return "0" when $(CC) isn't %

That could be done on commit.


> Even better would imo be, as suggested before, a "depends on
> CC_IS_*" on each *_VERSION.

Haven't we discussed this before?

> As a nit - common style elsewhere would suggest that there ought
> to be a blank after the commas in $(macro, ...) invocations.
> This would then extend to Kconfig.include as well, unless that's
> a largely verbatim inherited file.

That's not a good idea, it may not matter in this case but adding a
space after commas in some other cases will not do what one wants. make
and Kconfig keeps the spaces when expanding the macro.

-- 
Anthony PERARD

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.