On 12.07.2021 18:32, Anthony PERARD wrote: > On Wed, Jul 07, 2021 at 05:48:57PM +0200, Jan Beulich wrote: >> On 01.07.2021 16:09, Anthony PERARD wrote: >>> --- a/xen/common/Makefile >>> +++ b/xen/common/Makefile >>> @@ -80,8 +80,12 @@ config.gz: $(CONF_FILE) >>> >>> config_data.o: config.gz >>> >>> -config_data.S: $(BASEDIR)/tools/binfile >>> - $(SHELL) $(BASEDIR)/tools/binfile $@ config.gz xen_config_data >>> +quiet_cmd_binfile = BINFILE $@ >>> +cmd_binfile = $(SHELL) $< $@ config.gz xen_config_data >> >> This is an abuse of $< which I consider overly confusing: >> $(BASEDIR)/tools/binfile is not the input file to the rule. Instead >> the script generates an assembly file "out of thin air", with not >> input files at all. The rule and ... >> >>> +config_data.S: $(BASEDIR)/tools/binfile FORCE >> >> ... dependency shouldn't give a different impression. What would >> be nice (without having checked how difficult this might be) would >> be if quiet_cmd_binfile and cmd_binfile could move to xen/Rules.mk >> and merely be used from here (and the other location, where the >> same concern obviously applies). > > I've though of having cmd_binfile in Rules.mk, but it's made more > complicated by having a "-i" flag used in flask/. > > So one things I've writen was: > > config_data.S: $(BASEDIR)/tools/binfile FORCE > $(call if_changed,binfile,,config.gz xen_config_data) > flask-policy.S: $(BASEDIR)/tools/binfile FORCE > $(call if_changed,binfile,-i,policy.bin xsm_flask_init_policy) > > with: > cmd_binfile = $(SHELL) $(BASEDIR)/tools/binfile $(2) $@ $(3) > > I thought this would be confusing, so I avoid it.
Indeed that's why I did write "without having checked how difficult this might be", because I definitely didn't want to suggest such anomalies to get introduced. It's unhelpful that options have to come first. > But maybe with the lists of flags at the end, it would be better: > $(call if_changed,binfile,policy.bin xsm_flask_init_policy,-i) Yes, this is a little better imo, but still not pretty. > Still want to move cmd_binfile to Rules.mk? I'd still like it to be moved, but without resulting in a rule that's not consistent with others. Maybe we should have a BINFILE_FLAGS variable (paralleling e.g. CFLAGS)? Jan
