[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN PATCH for-4.17 v6 2/5] libs/light: Rework targets prerequisites


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
  • Date: Tue, 7 Feb 2023 11:37:12 +0000
  • Authentication-results: esa1.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Juergen Gross <jgross@xxxxxxxx>
  • Delivery-date: Tue, 07 Feb 2023 11:37:37 +0000
  • Ironport-data: A9a23:TbkC0apdLqPdEOCITZ222Hkxqv9eBmI+ZRIvgKrLsJaIsI4StFCzt garIBnQM6zcYGamKN4jbIu3pk9Vv5WEyYBkG1c9qyhhE3sQ8ZuZCYyVIHmrMnLJJKUvbq7FA +Y2MYCccZ9uHhcwgj/3b9ANeFEljfngqoLUUbKCYWYpA1c/Ek/NsDo788YhmIlknNOlNA2Ev NL2sqX3NUSsnjV5KQr40YrawP9UlKm06WxwUmAWP6gR5weEzyZNVfrzGInqR5fGatgMdgKFb 76rIIGRpgvx4xorA9W5pbf3GmVirmn6ZFXmZtJ+AsBOszAazsAA+v9T2Mk0MC+7vw6hjdFpo OihgLTrIesf0g8gr8xGO/VQO3kW0aSrY9YrK1Dn2SCY5xWun3cBX5yCpaz5VGEV0r8fPI1Ay RAXAGEHPhPT2c2s+4+2UbQ32PoSc9vSLqpK7xmMzRmBZRonaZXKQqGM7t5ExjYgwMtJGJ4yZ eJAN2ApNk6ZJUQSZBFOUslWcOSA3xETdxVRrk6VoqwmpXDe1gVr3JDmMcbPe8zMTsJQ9qqdj jOYozypW0hAXDCZ4Wbcylf8pOrMpircXZoySq+e6KBA2ELGkwT/DzVJDADm8JFVkHWWS99Zb kAZ5Ccqhawz71CwCMnwWQWip3yJtQJaXMBfe8Ul7Cmdx6yS5ByWbkAGQSRGc8cOr9ItSHoh0 Vrht9rxCCZmqrG9VXOX/bDSpjS3URX5NkdbO3VCF1FcpYC+/sdq1Emnostf/LCd39elGmGu7 CqxkSUl2u5Ns/9UxZyp1AWS696znaThQgkw7wTRe2uq6AJleYKoD7CVBUjnAeVod9jAEATY1 JQQs43Htb1VU8nR/MCYaL9VdIxF8cppJ9E1bbRHO5A6vwqg9He4FWy7yGEvfRw5WirolNKAX aMyhe+zzMULVJdJRfUtC25UNyjN5faIKDgdfqqIBueim7AoHON9wAlgZFSLw0fmm1U2nKc0N P+zKJjzUClCWfg/k2bsHY/xNIPHIQhnlAvuqW3TlUz7gdJymlbFIVv6DLd+Rr9gt/7VyOkk2 91eK9GL231ivB7WO0HqHXooBQlSdxATXMmmw/G7g8bfemKK7kl9Ua6OqV7gEqQ595loehDgo ijnBR8AmAKh3hUq62yiMxheVV8mZr4nxVpTAMDmFQ/AN6QLCWp30JoiSg==
  • Ironport-hdrordr: A9a23:WD7fC65FdlX33dJLQQPXwYOCI+orL9Y04lQ7vn2ZFiY6TiXIra +TdaoguSMc6QxhP03I/OrrBEDuewK5yXcY2/hoAV7BZnibhILYFvAe0WKK+VSJcECelp856U p5SdkdNDSaNykAsS+V2njDLz8V+qj5zEnkv5ao815dCSVRL41w5QZwDQiWVmVwWQl9HJI8UL aM+8ZdoDKkWHIPKuC2HGMMUeTvr8DC0MuOW29OOzcXrC21yR+44r/zFBaVmj8YTjN02L8ntU TVjgDj4a2nkvejjjvRzXXa4Zh6kMbojvFDGMuPoM4ILSiEsHffWK1RH5m5+BwlquCm71gn1P HKvhcbJsx2r0jce2mkyCGdqTUJhFwVmhzf4G7dpUGmjd3yRTo8BcYEr5leaAHl500pu8w5+L 5X3kqC3qAnQi/orWDY3ZzlRhtqnk27rT4JiugIlUFSVoMYdft4sZEfxkVIC50NdRiKprzPKN MeTf002cwmPG9yLkqp9FWH+ebcFUjbyy32DnTruaSuoktrdT5CvgslLfck7wc9HaIGOud5Dt v/Q9VVfZF1P7orhPFGdZM8qI2MeyTwaCOJCXmVJ1v/EqEBJjbil77biY9Fpt2CSdgw1501l4 3GUFRE8UgIW2yrJ/Gv8fRwg1PwqEPUZ0Wo9iib3ek9hpTMAIDGC2mobncAs+WdmN0jIuv9H8 yeBfttcpneBGHzA5tO2wHke7Q6EwhmbPEo
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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,

-- 
Anthony PERARD



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.