build: cleanup build log, avoid user's CFLAGS, avoid too many include of Config.mk

[XEN PATCH 04/15] build: hide policy.bin commands

Posted by Anthony PERARD 1 year, 6 months ago

Instead, show only when "policy.bin" is been updated.

We still have the full command from the flask/policy Makefile, but we
can't change that Makefile.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/xsm/flask/Makefile | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index 3fdcf7727e..fc44ad684f 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -48,10 +48,15 @@ targets += flask-policy.S
 FLASK_BUILD_DIR := $(abs_objtree)/$(obj)
 POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
 
+policy_chk = \
+    $(Q)if ! cmp -s $(POLICY_SRC) $@; then \
+        $(kecho) '  UPD     $@'; \
+        cp $(POLICY_SRC) $@; \
+    fi
 $(obj)/policy.bin: FORCE
-	$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \
+	$(Q)$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \
 	        -C $(XEN_ROOT)/tools/flask/policy \
 	        FLASK_BUILD_DIR=$(FLASK_BUILD_DIR) POLICY_FILENAME=$(POLICY_SRC)
-	cmp -s $(POLICY_SRC) $@ || cp $(POLICY_SRC) $@
+	$(call policy_chk)
 
 clean-files := policy.* $(POLICY_SRC)
-- 
Anthony PERARD

Re: [XEN PATCH 04/15] build: hide policy.bin commands

Posted by Daniel P. Smith 1 year, 6 months ago

On 5/23/23 12:38, Anthony PERARD wrote:
> Instead, show only when "policy.bin" is been updated.
> 
> We still have the full command from the flask/policy Makefile, but we
> can't change that Makefile.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>   xen/xsm/flask/Makefile | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
> index 3fdcf7727e..fc44ad684f 100644
> --- a/xen/xsm/flask/Makefile
> +++ b/xen/xsm/flask/Makefile
> @@ -48,10 +48,15 @@ targets += flask-policy.S
>   FLASK_BUILD_DIR := $(abs_objtree)/$(obj)
>   POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
>   
> +policy_chk = \
> +    $(Q)if ! cmp -s $(POLICY_SRC) $@; then \
> +        $(kecho) '  UPD     $@'; \
> +        cp $(POLICY_SRC) $@; \
> +    fi
>   $(obj)/policy.bin: FORCE
> -	$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \
> +	$(Q)$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \
>   	        -C $(XEN_ROOT)/tools/flask/policy \
>   	        FLASK_BUILD_DIR=$(FLASK_BUILD_DIR) POLICY_FILENAME=$(POLICY_SRC)
> -	cmp -s $(POLICY_SRC) $@ || cp $(POLICY_SRC) $@
> +	$(call policy_chk)
>   
>   clean-files := policy.* $(POLICY_SRC)

Outside the suggestion and nit(s) from Jan, all else looks good to me.

Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>

Re: [XEN PATCH 04/15] build: hide policy.bin commands

Posted by Jan Beulich 1 year, 6 months ago

On 23.05.2023 18:38, Anthony PERARD wrote:
> --- a/xen/xsm/flask/Makefile
> +++ b/xen/xsm/flask/Makefile
> @@ -48,10 +48,15 @@ targets += flask-policy.S
>  FLASK_BUILD_DIR := $(abs_objtree)/$(obj)
>  POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
>  
> +policy_chk = \
> +    $(Q)if ! cmp -s $(POLICY_SRC) $@; then \
> +        $(kecho) '  UPD     $@'; \
> +        cp $(POLICY_SRC) $@; \

Wouldn't this better use move-if-changed? Which, if "UPD ..." output is
desired, would then need overriding from what Config.mk supplies?

In any event, much like move-if-changed itself - please avoid underscores
in names when dashes are fine to use.

> +    fi
>  $(obj)/policy.bin: FORCE

Nit: Blank line above here please.

Jan

> -	$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \
> +	$(Q)$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \
>  	        -C $(XEN_ROOT)/tools/flask/policy \
>  	        FLASK_BUILD_DIR=$(FLASK_BUILD_DIR) POLICY_FILENAME=$(POLICY_SRC)
> -	cmp -s $(POLICY_SRC) $@ || cp $(POLICY_SRC) $@
> +	$(call policy_chk)
>  
>  clean-files := policy.* $(POLICY_SRC)

Re: [XEN PATCH 04/15] build: hide policy.bin commands

Posted by Anthony PERARD 1 year, 6 months ago

On Wed, May 24, 2023 at 09:11:10AM +0200, Jan Beulich wrote:
> On 23.05.2023 18:38, Anthony PERARD wrote:
> > --- a/xen/xsm/flask/Makefile
> > +++ b/xen/xsm/flask/Makefile
> > @@ -48,10 +48,15 @@ targets += flask-policy.S
> >  FLASK_BUILD_DIR := $(abs_objtree)/$(obj)
> >  POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
> >  
> > +policy_chk = \
> > +    $(Q)if ! cmp -s $(POLICY_SRC) $@; then \
> > +        $(kecho) '  UPD     $@'; \
> > +        cp $(POLICY_SRC) $@; \
> 
> Wouldn't this better use move-if-changed? Which, if "UPD ..." output is
> desired, would then need overriding from what Config.mk supplies?

I don't like move-if-changed, because it remove the original target. On
incremental build, make will keep building the original target even
when not needed. So we keep seeing the `checkpolicy` command line when
there's otherwise nothing to do.

I could introduce a new generic macro instead, copy-if-changed, which
will do compare and copy (like policy_chk is doing here).

Thanks,

-- 
Anthony PERARD

Re: [XEN PATCH 04/15] build: hide policy.bin commands

Posted by Jan Beulich 1 year, 6 months ago

On 25.05.2023 15:34, Anthony PERARD wrote:
> On Wed, May 24, 2023 at 09:11:10AM +0200, Jan Beulich wrote:
>> On 23.05.2023 18:38, Anthony PERARD wrote:
>>> --- a/xen/xsm/flask/Makefile
>>> +++ b/xen/xsm/flask/Makefile
>>> @@ -48,10 +48,15 @@ targets += flask-policy.S
>>>  FLASK_BUILD_DIR := $(abs_objtree)/$(obj)
>>>  POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
>>>  
>>> +policy_chk = \
>>> +    $(Q)if ! cmp -s $(POLICY_SRC) $@; then \
>>> +        $(kecho) '  UPD     $@'; \
>>> +        cp $(POLICY_SRC) $@; \
>>
>> Wouldn't this better use move-if-changed? Which, if "UPD ..." output is
>> desired, would then need overriding from what Config.mk supplies?
> 
> I don't like move-if-changed, because it remove the original target. On
> incremental build, make will keep building the original target even
> when not needed. So we keep seeing the `checkpolicy` command line when
> there's otherwise nothing to do.
> 
> I could introduce a new generic macro instead, copy-if-changed, which
> will do compare and copy (like policy_chk is doing here).

Ah, yes, I think I see what you mean. I'd be fine with copy-if-changed,
ideally accompanied by some rule of thumb when to prefer it over
move-if-changed.

Jan