Re: [Xen-devel] [XEN PATCH v3 2/6] xen: Have Kconfig check $(CC)'s version
On 17.01.2020 17:23, Anthony PERARD wrote: > On Thu, Jan 16, 2020 at 01:40:39PM +0100, Jan Beulich wrote: >> On 16.01.2020 13:29, Anthony PERARD wrote: >> Indeed, hence also my "as suggested before". I remain unconvinced >> that is it useful to have e.g. >> >> CONFIG_GCC_VERSION=80300 >> CONFIG_CLANG_VERSION=0 >> >> in xen/.config. This is at best confusing, because it may not >> represent what the system actually has installed (which I realize >> is also not the intention, but the variable naming suggests that >> this is what was found on the system; I have no better naming >> suggestion, to preempt a possible question to this effect). > > After a talk on #xendevel yesterday, I have George agreeing that we > should keep the same behavior as the one Linux have. And Ian saying that > we should copy entire files where we can. If we modify the behavior of > %_VERSION, it would make it more difficult to copy entire files from > %Linux later. > > So, now, can we finally commit the patch series, with both %_VERSION > set, and let this bikeshedding rest, and move on? Well, someone feel free to go ahead then. Every time I'll get to see this I'll be tempted to clean up the redundancy, but so be it. (I don't view this as bike shedding at all, btw. While every individual instance may not matter much, it is the sum of them that I'm worried about.) Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [XEN PATCH v3 2/6] xen: Have Kconfig check $(CC)'s version
On Thu, Jan 16, 2020 at 01:40:39PM +0100, Jan Beulich wrote: > On 16.01.2020 13:29, Anthony PERARD wrote: > Indeed, hence also my "as suggested before". I remain unconvinced > that is it useful to have e.g. > > CONFIG_GCC_VERSION=80300 > CONFIG_CLANG_VERSION=0 > > in xen/.config. This is at best confusing, because it may not > represent what the system actually has installed (which I realize > is also not the intention, but the variable naming suggests that > this is what was found on the system; I have no better naming > suggestion, to preempt a possible question to this effect). After a talk on #xendevel yesterday, I have George agreeing that we should keep the same behavior as the one Linux have. And Ian saying that we should copy entire files where we can. If we modify the behavior of %_VERSION, it would make it more difficult to copy entire files from %Linux later. So, now, can we finally commit the patch series, with both %_VERSION set, and let this bikeshedding rest, and move on? Thank you, -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [XEN PATCH v3 2/6] xen: Have Kconfig check $(CC)'s version
On 16.01.2020 13:29, Anthony PERARD wrote: > 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. Sure. >> Even better would imo be, as suggested before, a "depends on >> CC_IS_*" on each *_VERSION. > > Haven't we discussed this before? Indeed, hence also my "as suggested before". I remain unconvinced that is it useful to have e.g. CONFIG_GCC_VERSION=80300 CONFIG_CLANG_VERSION=0 in xen/.config. This is at best confusing, because it may not represent what the system actually has installed (which I realize is also not the intention, but the variable naming suggests that this is what was found on the system; I have no better naming suggestion, to preempt a possible question to this effect). >> 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. Where blanks matter they should of course be omitted. When they don't matter, personally I think common style guidelines should be honored. But this may be just me ... Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
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@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [XEN PATCH v3 2/6] xen: Have Kconfig check $(CC)'s version
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. Even better would imo be, as suggested before, a "depends on CC_IS_*" on each *_VERSION. 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. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH v3 2/6] xen: Have Kconfig check $(CC)'s version
This import several files from Linux v5.3 - scripts/Kconfig.include - scripts/clang-version.sh - scripts/gcc-version.sh and several config values from from Linux's init/Kconfig file. But gcc-version.sh have been modified to return "0" when $CC isn't GCC, like clang-version.sh do. Files are copied into scripts/ directory because that's were the files are found in Linux tree, and also because we are going to import more of Kbuild from Linux which is located in scripts/. CONFIG_GCC_VERSION and CONFIG_CC_IS_CLANG are going to be use in follow-up patches. Signed-off-by: Anthony PERARD Acked-by: Andrew Cooper --- Notes: v3: - Have gcc-version behave like clang-version and return 0 when the compiler tested isn't the expected one. v2: - move the export CC* earlier in xen/Makefile, and add CXX to the list. xen/Kconfig | 16 +++ xen/Makefile | 2 ++ xen/scripts/Kconfig.include | 39 xen/scripts/clang-version.sh | 19 ++ xen/scripts/gcc-version.sh | 25 +++ 5 files changed, 101 insertions(+) create mode 100644 xen/scripts/Kconfig.include create mode 100755 xen/scripts/clang-version.sh create mode 100755 xen/scripts/gcc-version.sh diff --git a/xen/Kconfig b/xen/Kconfig index 01067326b4e7..57427927abf0 100644 --- 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)) + source "arch/$(SRCARCH)/Kconfig" config DEFCONFIG_LIST diff --git a/xen/Makefile b/xen/Makefile index efbe9605e52b..c326fee5880e 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -18,6 +18,8 @@ export XEN_CONFIG_EXPERT ?= n PYTHON_INTERPRETER := $(word 1,$(shell which python3 python python2 2>/dev/null) python) export PYTHON ?= $(PYTHON_INTERPRETER) +export CC CXX LD + export BASEDIR := $(CURDIR) export XEN_ROOT := $(BASEDIR)/.. diff --git a/xen/scripts/Kconfig.include b/xen/scripts/Kconfig.include new file mode 100644 index ..8221095ca34b --- /dev/null +++ b/xen/scripts/Kconfig.include @@ -0,0 +1,39 @@ +# SPDX-License-Identifier: GPL-2.0-only +# Kconfig helper macros + +# Convenient variables +comma := , +quote := " +squote := ' +empty := +space := $(empty) $(empty) +dollar := $ +right_paren := ) +left_paren := ( + +# $(if-success,,,) +# Return if exits with 0, otherwise. +if-success = $(shell,{ $(1); } >/dev/null 2>&1 && echo "$(2)" || echo "$(3)") + +# $(success,) +# Return y if exits with 0, n otherwise +success = $(if-success,$(1),y,n) + +# $(failure,) +# Return n if exits with 0, y otherwise +failure = $(if-success,$(1),n,y) + +# $(cc-option,) +# Return y if the compiler supports , n otherwise +cc-option = $(success,$(CC) -Werror $(CLANG_FLAGS) $(1) -E -x c /dev/null -o /dev/null) + +# $(ld-option,) +# Return y if the linker supports , n otherwise +ld-option = $(success,$(LD) -v $(1)) + +# check if $(CC) and $(LD) exist +$(error-if,$(failure,command -v $(CC)),compiler '$(CC)' not found) +$(error-if,$(failure,command -v $(LD)),linker '$(LD)' not found) + +# gcc version including patch level +gcc-version := $(shell,$(BASEDIR)/scripts/gcc-version.sh $(CC)) diff --git a/xen/scripts/clang-version.sh b/xen/scripts/clang-version.sh new file mode 100755 index ..6fabf0695761 --- /dev/null +++ b/xen/scripts/clang-version.sh @@ -0,0 +1,19 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# +# clang-version clang-command +# +# Print the compiler version of `clang-command' in a 5 or 6-digit form +# such as `50001' for clang-5.0.1 etc. + +compiler="$*" + +if ! ( $compiler --version | grep -q clang) ; then + echo 0 + exit 1 +fi + +MAJOR=$(echo __clang_major__ | $compiler -E -x c - | tail -n 1) +MINOR=$(echo __clang_minor__ | $compiler -E -x c - | tail -n 1) +PATCHLEVEL=$(echo __clang_patchlevel__ | $compiler -E -x c - | tail -n 1) +printf "%d%02d%02d\\n" $MAJOR $MINOR $PATCHLEVEL diff --git a/xen/scripts/gcc-version.sh b/xen/scripts/gcc-version.sh new file mode 100755 index ..b3261949dea5 --- /dev/null +++ b/xen/scripts/gcc-version.sh @@ -0,0 +1,25 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# +# gcc-version gcc-command +# +# Print the gcc version of `gcc-command' in a 5 or 6-digit form +# such as `29503' for gcc-2.95.3, `30301' for gcc-3.3.1, etc. + +compiler="$*" + +if [ ${#compiler} -eq 0 ]; then + echo "Error: No compiler specified." >&2 + printf "Usage:\n\t$0 \n"