| Message ID | 20230523163811.30792-5-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | build: cleanup build log, avoid user's CFLAGS, avoid too many include of Config.mk | expand |
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)
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>
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,
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
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)
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(-)