[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH v3] xen: rework `checkpolicy` detection when using "randconfig"
On Thu, Sep 16, 2021 at 05:34:00PM +0200, Jan Beulich wrote: > On 08.09.2021 13:17, Anthony PERARD wrote: > > --- a/Config.mk > > +++ b/Config.mk > > @@ -137,12 +137,6 @@ export XEN_HAS_BUILD_ID=y > > build_id_linker := --build-id=sha1 > > endif > > > > -ifndef XEN_HAS_CHECKPOLICY > > - CHECKPOLICY ?= checkpolicy > > - XEN_HAS_CHECKPOLICY := $(shell $(CHECKPOLICY) -h 2>&1 | grep -q xen && > > echo y || echo n) > > - export XEN_HAS_CHECKPOLICY > > -endif > > Is there a particular reason to go from XEN_HAS_CHECKPOLICY to ... > > > --- a/xen/Makefile > > +++ b/xen/Makefile > > @@ -17,6 +17,8 @@ export XEN_BUILD_HOST ?= $(shell hostname) > > PYTHON_INTERPRETER := $(word 1,$(shell which python3 python python2 > > 2>/dev/null) python) > > export PYTHON ?= $(PYTHON_INTERPRETER) > > > > +export CHECKPOLICY ?= checkpolicy > > + > > export BASEDIR := $(CURDIR) > > export XEN_ROOT := $(BASEDIR)/.. > > > > @@ -178,6 +180,8 @@ CFLAGS += $(CLANG_FLAGS) > > export CLANG_FLAGS > > endif > > > > +export HAS_CHECKPOLICY := $(call success,$(CHECKPOLICY) -h 2>&1 | grep -q > > xen) > > ... HAS_CHECKPOLICY? As soon as things get put in the environment, Not really anymore, it's just left over from having put this in Kconfig in previous version of the patch. > I'm always suspecting possible name collisions ... Yes, it's probably better to keep the XEN_ prefix. > > @@ -189,14 +193,24 @@ ifeq ($(config-build),y) > > # *config targets only - make sure prerequisites are updated, and descend > > # in tools/kconfig to make the *config target > > > > +# Create a file for KCONFIG_ALLCONFIG which depends on the environment. > > +# This will be use by kconfig targets > > allyesconfig/allmodconfig/allnoconfig/randconfig > > +filechk_kconfig_allconfig = \ > > + $(if $(findstring n,$(HAS_CHECKPOLICY)),echo > > 'CONFIG_XSM_FLASK_POLICY=n';) \ > > + $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG), :) > > Nit: It would be nice if you were consistent with the blanks after > commas in $(if ...). Personally I'm also considering $(if ...)s the > more difficult to follow the longer they are. Hence for the 2nd one > I wonder whether > > $(if $(KCONFIG_ALLCONFIG),cat,:) $(KCONFIG_ALLCONFIG) > > wouldn't be easier to read. How about: $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG);) \ : .. instead, as that would be more consistent with the previous line, that is there would be only one branch to the $(if ) and no else, and thus probably easier to read. > > +.allconfig.tmp: FORCE > > + set -e; { $(call filechk_kconfig_allconfig); } > $@ > > Is there a particular reason for the .tmp suffix? Yes, .*.tmp are already ignored via .gitignore. Thanks, -- Anthony PERARD
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |