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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.01.2020 15:01, Anthony PERARD wrote:
> On Fri, Jan 03, 2020 at 05:42:18PM +0100, Jan Beulich wrote:
>> On 17.12.2019 11:58, Anthony PERARD wrote:
>>> --- a/xen/Kconfig
>>> +++ b/xen/Kconfig
>>> @@ -4,9 +4,26 @@
>>>  #
>>>  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)) if CC_IS_GCC
>>> +   default 0
>>
>> Why "if" and a 2nd "default" line here but ...
>>
>>> +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))
>>
>> ... just a single, unconditional one here?
> 
> clang-version.sh returns 0 when CC isn't clang, but gcc-version.sh
> doesn't check if CC is gcc, and returns a bogus values instead.
> 
> e.g.:
> 
> $ ./clang-version.sh clang
> 90000
> $ ./gcc-version.sh clang
> 40201

Imo both scripts should behave identically in this regard (and in fact
in all usage related ones). If adjusting the scripts is entirely
unacceptable for some reason, then the description should highlight
the need for the differences.

>> Wouldn't both better
>> have a "depends on CC_IS_*" line instead? This would then also
>> result (afaict) in no CONFIG_CLANG_VERSION in .config if building
>> with gcc (and vice versa), instead of a bogus CONFIG_CLANG_VERSION=0.
> 
> It sounds attracting to remove variables from .config, but it is equally
> attracting to always have a variable set. It can be used
> unconditionally when always set (without risking invalid syntax for
> example).

Hmm, yes, as long as we don't have (by mechanical conversion) or gain
constructs like

#if CONFIG_GCC_VERSION < 50000 /* must be gcc 4.x */

Plus - what's CONFIG_CC_IS_{GCC,CLANG} good for then? The same can
then be achieved by comparing CONFIG_{GCC,CLANG}_VERSION against zero.

Jan

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