| Message ID | 20230120194431.55922-3-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Toolstack build system improvement, toward non-recursive makefiles | expand |
On 20/01/2023 7:44 pm, Anthony PERARD wrote: > No need for $(AUTOSRCS), GNU make can generate them as needed when > trying to build them as needed when trying to build the object. Also, > those two AUTOSRCS don't need to be a prerequisite of "all". As for > the "clean" target, those two files are already removed via "_*.c". > > We don't need $(AUTOINCS) either: > - As for both _libxl_save_msgs*.h headers, we are adding more > selective dependencies so the headers will still be generated as > needed. > - "clean" rule already delete the _*.h files, so AUTOINCS aren't needed > there. > > "libxl_internal_json.h" doesn't seems to have ever existed, so the > dependency is removed. > > Rework objects prerequisites, to have them dependents on either > "libxl.h" or "libxl_internal.h". "libxl.h" is not normally included > directly in the source code as "libxl_internal.h" is used instead, but > we have "libxl.h" as prerequisite of "libxl_internal.h", so generated > headers will still be generated as needed. > > Make doesn't need "libxl.h" to generate "testidl.c", "libxl.h" is only > needed later when building "testidl.o". This avoid the need to > regenerate "testidl.c" when only "libxl.h" changed. Also use automatic > variables $< and $@. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > > Notes: > v6: > - rebased, part of the patch commited as 4ff0811 > - reword commit message > > v4: > - new patch > > tools/libs/light/Makefile | 28 ++++++++++++++++------------ > 1 file changed, 16 insertions(+), 12 deletions(-) > > diff --git a/tools/libs/light/Makefile b/tools/libs/light/Makefile > index cd3fa855e1..b28447a2ae 100644 > --- a/tools/libs/light/Makefile > +++ b/tools/libs/light/Makefile > @@ -148,9 +148,6 @@ LIBXL_TEST_OBJS += $(foreach t, $(LIBXL_TESTS_INSIDE),libxl_test_$t.opic) > TEST_PROG_OBJS += $(foreach t, $(LIBXL_TESTS_PROGS),test_$t.o) test_common.o > TEST_PROGS += $(foreach t, $(LIBXL_TESTS_PROGS),test_$t) > > -AUTOINCS = _libxl_save_msgs_callout.h _libxl_save_msgs_helper.h > -AUTOSRCS = _libxl_save_msgs_callout.c _libxl_save_msgs_helper.c > - > CLIENTS = testidl libxl-save-helper > > SAVE_HELPER_OBJS = libxl_save_helper.o _libxl_save_msgs_helper.o > @@ -178,13 +175,13 @@ libxl_x86_acpi.o libxl_x86_acpi.opic: CFLAGS += -I$(XEN_ROOT)/tools > $(SAVE_HELPER_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenevtchn) $(CFLAGS_libxenguest) > > testidl.o: CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenlight) > -testidl.c: libxl_types.idl gentest.py $(XEN_INCLUDE)/libxl.h $(AUTOINCS) > - $(PYTHON) gentest.py libxl_types.idl testidl.c.new > - mv testidl.c.new testidl.c > +testidl.c: libxl_types.idl gentest.py > + $(PYTHON) gentest.py $< $@.new > + mv -f $@.new $@ Doesn't this want to be a mov-if-changed? We don't need to force a rebuild if testidl.c hasn't changed, I don't think. > > -all: $(CLIENTS) $(TEST_PROGS) $(AUTOSRCS) $(AUTOINCS) > +all: $(CLIENTS) $(TEST_PROGS) > > -$(OBJS-y) $(PIC_OBJS) $(SAVE_HELPER_OBJS) $(LIBXL_TEST_OBJS) $(TEST_PROG_OBJS): $(AUTOINCS) libxl.api-ok > +$(OBJS-y) $(PIC_OBJS) $(SAVE_HELPER_OBJS) $(LIBXL_TEST_OBJS) $(TEST_PROG_OBJS): libxl.api-ok > > $(DSDT_FILES-y): acpi > > @@ -196,7 +193,7 @@ libxl.api-ok: check-libxl-api-rules _libxl.api-for-check > $(PERL) $^ > touch $@ > > -_libxl.api-for-check: $(XEN_INCLUDE)/libxl.h $(AUTOINCS) > +_libxl.api-for-check: $(XEN_INCLUDE)/libxl.h > $(CC) $(CPPFLAGS) $(CFLAGS) -c -E $< $(APPEND_CFLAGS) \ > -DLIBXL_EXTERNAL_CALLERS_ONLY=LIBXL_EXTERNAL_CALLERS_ONLY \ > >$@.new > @@ -208,14 +205,22 @@ _libxl_save_msgs_helper.h _libxl_save_msgs_callout.h: \ > $(PERL) -w $< $@ >$@.new > $(call move-if-changed,$@.new,$@) > > +# > +# headers dependencies on generated headers > +# > $(XEN_INCLUDE)/libxl.h: $(XEN_INCLUDE)/_libxl_types.h > $(XEN_INCLUDE)/libxl_json.h: $(XEN_INCLUDE)/_libxl_types_json.h > libxl_internal.h: $(XEN_INCLUDE)/libxl.h $(XEN_INCLUDE)/libxl_json.h > libxl_internal.h: _libxl_types_internal.h _libxl_types_private.h _libxl_types_internal_private.h > -libxl_internal_json.h: _libxl_types_internal_json.h > +libxl_internal.h: _libxl_save_msgs_callout.h > > -$(OBJS-y) $(PIC_OBJS) $(LIBXL_TEST_OBJS) $(TEST_PROG_OBJS) $(SAVE_HELPER_OBJS): $(XEN_INCLUDE)/libxl.h > +# > +# objects dependencies on headers that depends on generated headers > +# > +$(TEST_PROG_OBJS): $(XEN_INCLUDE)/libxl.h > $(OBJS-y) $(PIC_OBJS) $(LIBXL_TEST_OBJS): libxl_internal.h > +$(SAVE_HELPER_OBJS): $(XEN_INCLUDE)/libxl.h _libxl_save_msgs_helper.h > +testidl.o: $(XEN_INCLUDE)/libxl.h I know you're just rearranging existing the existing logic, but why doesn't `#include <libxl.h>` not generate suitable dependences ? ~Andrew
On Mon, Feb 06, 2023 at 06:02:51PM +0000, Andrew Cooper wrote: > On 20/01/2023 7:44 pm, Anthony PERARD wrote: > > diff --git a/tools/libs/light/Makefile b/tools/libs/light/Makefile > > index cd3fa855e1..b28447a2ae 100644 > > --- a/tools/libs/light/Makefile > > +++ b/tools/libs/light/Makefile > > @@ -178,13 +175,13 @@ libxl_x86_acpi.o libxl_x86_acpi.opic: CFLAGS += -I$(XEN_ROOT)/tools > > $(SAVE_HELPER_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenevtchn) $(CFLAGS_libxenguest) > > > > testidl.o: CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenlight) > > -testidl.c: libxl_types.idl gentest.py $(XEN_INCLUDE)/libxl.h $(AUTOINCS) > > - $(PYTHON) gentest.py libxl_types.idl testidl.c.new > > - mv testidl.c.new testidl.c > > +testidl.c: libxl_types.idl gentest.py > > + $(PYTHON) gentest.py $< $@.new > > + mv -f $@.new $@ > > Doesn't this want to be a mov-if-changed? > > We don't need to force a rebuild if testidl.c hasn't changed, I don't think. I don't like move-if-changed, as when the prerequisites happens to be newer than the target, the rules keeps been executed on incremental builds. Also in this case, only two targets depends on this one, "testidl.o" and "testidl", so move-if-changed would not be very useful. > > @@ -208,14 +205,22 @@ _libxl_save_msgs_helper.h _libxl_save_msgs_callout.h: \ > > $(PERL) -w $< $@ >$@.new > > $(call move-if-changed,$@.new,$@) > > > > +# > > +# headers dependencies on generated headers > > +# > > $(XEN_INCLUDE)/libxl.h: $(XEN_INCLUDE)/_libxl_types.h > > $(XEN_INCLUDE)/libxl_json.h: $(XEN_INCLUDE)/_libxl_types_json.h > > libxl_internal.h: $(XEN_INCLUDE)/libxl.h $(XEN_INCLUDE)/libxl_json.h > > libxl_internal.h: _libxl_types_internal.h _libxl_types_private.h _libxl_types_internal_private.h > > -libxl_internal_json.h: _libxl_types_internal_json.h > > +libxl_internal.h: _libxl_save_msgs_callout.h > > > > -$(OBJS-y) $(PIC_OBJS) $(LIBXL_TEST_OBJS) $(TEST_PROG_OBJS) $(SAVE_HELPER_OBJS): $(XEN_INCLUDE)/libxl.h > > +# > > +# objects dependencies on headers that depends on generated headers > > +# > > +$(TEST_PROG_OBJS): $(XEN_INCLUDE)/libxl.h > > $(OBJS-y) $(PIC_OBJS) $(LIBXL_TEST_OBJS): libxl_internal.h > > +$(SAVE_HELPER_OBJS): $(XEN_INCLUDE)/libxl.h _libxl_save_msgs_helper.h > > +testidl.o: $(XEN_INCLUDE)/libxl.h > > I know you're just rearranging existing the existing logic, but why > doesn't `#include <libxl.h>` not generate suitable dependences ? Make doesn't know how to read *.c files ;-). But more importantly, we don't have any logic to generate dependencies ahead of building the object, so make doesn't know about it. Thanks,
On 20.01.23 20:44, Anthony PERARD wrote: > No need for $(AUTOSRCS), GNU make can generate them as needed when > trying to build them as needed when trying to build the object. Also, > those two AUTOSRCS don't need to be a prerequisite of "all". As for > the "clean" target, those two files are already removed via "_*.c". > > We don't need $(AUTOINCS) either: > - As for both _libxl_save_msgs*.h headers, we are adding more > selective dependencies so the headers will still be generated as > needed. > - "clean" rule already delete the _*.h files, so AUTOINCS aren't needed > there. > > "libxl_internal_json.h" doesn't seems to have ever existed, so the > dependency is removed. > > Rework objects prerequisites, to have them dependents on either > "libxl.h" or "libxl_internal.h". "libxl.h" is not normally included > directly in the source code as "libxl_internal.h" is used instead, but > we have "libxl.h" as prerequisite of "libxl_internal.h", so generated > headers will still be generated as needed. > > Make doesn't need "libxl.h" to generate "testidl.c", "libxl.h" is only > needed later when building "testidl.o". This avoid the need to > regenerate "testidl.c" when only "libxl.h" changed. Also use automatic > variables $< and $@. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen
diff --git a/tools/libs/light/Makefile b/tools/libs/light/Makefile index cd3fa855e1..b28447a2ae 100644 --- a/tools/libs/light/Makefile +++ b/tools/libs/light/Makefile @@ -148,9 +148,6 @@ LIBXL_TEST_OBJS += $(foreach t, $(LIBXL_TESTS_INSIDE),libxl_test_$t.opic) TEST_PROG_OBJS += $(foreach t, $(LIBXL_TESTS_PROGS),test_$t.o) test_common.o TEST_PROGS += $(foreach t, $(LIBXL_TESTS_PROGS),test_$t) -AUTOINCS = _libxl_save_msgs_callout.h _libxl_save_msgs_helper.h -AUTOSRCS = _libxl_save_msgs_callout.c _libxl_save_msgs_helper.c - CLIENTS = testidl libxl-save-helper SAVE_HELPER_OBJS = libxl_save_helper.o _libxl_save_msgs_helper.o @@ -178,13 +175,13 @@ libxl_x86_acpi.o libxl_x86_acpi.opic: CFLAGS += -I$(XEN_ROOT)/tools $(SAVE_HELPER_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenevtchn) $(CFLAGS_libxenguest) testidl.o: CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenlight) -testidl.c: libxl_types.idl gentest.py $(XEN_INCLUDE)/libxl.h $(AUTOINCS) - $(PYTHON) gentest.py libxl_types.idl testidl.c.new - mv testidl.c.new testidl.c +testidl.c: libxl_types.idl gentest.py + $(PYTHON) gentest.py $< $@.new + mv -f $@.new $@ -all: $(CLIENTS) $(TEST_PROGS) $(AUTOSRCS) $(AUTOINCS) +all: $(CLIENTS) $(TEST_PROGS) -$(OBJS-y) $(PIC_OBJS) $(SAVE_HELPER_OBJS) $(LIBXL_TEST_OBJS) $(TEST_PROG_OBJS): $(AUTOINCS) libxl.api-ok +$(OBJS-y) $(PIC_OBJS) $(SAVE_HELPER_OBJS) $(LIBXL_TEST_OBJS) $(TEST_PROG_OBJS): libxl.api-ok $(DSDT_FILES-y): acpi @@ -196,7 +193,7 @@ libxl.api-ok: check-libxl-api-rules _libxl.api-for-check $(PERL) $^ touch $@ -_libxl.api-for-check: $(XEN_INCLUDE)/libxl.h $(AUTOINCS) +_libxl.api-for-check: $(XEN_INCLUDE)/libxl.h $(CC) $(CPPFLAGS) $(CFLAGS) -c -E $< $(APPEND_CFLAGS) \ -DLIBXL_EXTERNAL_CALLERS_ONLY=LIBXL_EXTERNAL_CALLERS_ONLY \ >$@.new @@ -208,14 +205,22 @@ _libxl_save_msgs_helper.h _libxl_save_msgs_callout.h: \ $(PERL) -w $< $@ >$@.new $(call move-if-changed,$@.new,$@) +# +# headers dependencies on generated headers +# $(XEN_INCLUDE)/libxl.h: $(XEN_INCLUDE)/_libxl_types.h $(XEN_INCLUDE)/libxl_json.h: $(XEN_INCLUDE)/_libxl_types_json.h libxl_internal.h: $(XEN_INCLUDE)/libxl.h $(XEN_INCLUDE)/libxl_json.h libxl_internal.h: _libxl_types_internal.h _libxl_types_private.h _libxl_types_internal_private.h -libxl_internal_json.h: _libxl_types_internal_json.h +libxl_internal.h: _libxl_save_msgs_callout.h -$(OBJS-y) $(PIC_OBJS) $(LIBXL_TEST_OBJS) $(TEST_PROG_OBJS) $(SAVE_HELPER_OBJS): $(XEN_INCLUDE)/libxl.h +# +# objects dependencies on headers that depends on generated headers +# +$(TEST_PROG_OBJS): $(XEN_INCLUDE)/libxl.h $(OBJS-y) $(PIC_OBJS) $(LIBXL_TEST_OBJS): libxl_internal.h +$(SAVE_HELPER_OBJS): $(XEN_INCLUDE)/libxl.h _libxl_save_msgs_helper.h +testidl.o: $(XEN_INCLUDE)/libxl.h # This exploits the 'multi-target pattern rule' trick. # gentypes.py should be executed only once to make all the targets. @@ -262,5 +267,4 @@ clean:: $(RM) testidl.c.new testidl.c *.api-ok $(RM) $(TEST_PROGS) libxenlight_test.so libxl_test_*.opic $(RM) -r __pycache__ - $(RM) $(AUTOSRCS) $(AUTOINCS) $(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(CURDIR) clean
No need for $(AUTOSRCS), GNU make can generate them as needed when trying to build them as needed when trying to build the object. Also, those two AUTOSRCS don't need to be a prerequisite of "all". As for the "clean" target, those two files are already removed via "_*.c". We don't need $(AUTOINCS) either: - As for both _libxl_save_msgs*.h headers, we are adding more selective dependencies so the headers will still be generated as needed. - "clean" rule already delete the _*.h files, so AUTOINCS aren't needed there. "libxl_internal_json.h" doesn't seems to have ever existed, so the dependency is removed. Rework objects prerequisites, to have them dependents on either "libxl.h" or "libxl_internal.h". "libxl.h" is not normally included directly in the source code as "libxl_internal.h" is used instead, but we have "libxl.h" as prerequisite of "libxl_internal.h", so generated headers will still be generated as needed. Make doesn't need "libxl.h" to generate "testidl.c", "libxl.h" is only needed later when building "testidl.o". This avoid the need to regenerate "testidl.c" when only "libxl.h" changed. Also use automatic variables $< and $@. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Notes: v6: - rebased, part of the patch commited as 4ff0811 - reword commit message v4: - new patch tools/libs/light/Makefile | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-)