| Message ID | 20160607043443.3111-1-Wei.Chen@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 07, 2016 at 12:34:43PM +0800, Wei Chen wrote: > In current Xen build rules, the build system will only check the > dependencies in current folder and obj-y generated dependencies > in other folder. > > But Makefile may add some objects to ALL_OBJS. These objects may > be not in the same folder as Makefile. Use arch/arm/Makefile as > an example: > ALL_OBJS := $(TARGET_SUBARCH)/head.o > > The head.o is not in the same folder as Makefile and is not listed > in obj-y. So, when we update the header files that had been included > in head.S. The build system would not check the dependency of head.S. > The head.S would not be re-compiled. > > In this patch, we will get objects added by Makefile and add their > dependencies to check list. > > Signed-off-by: Wei Chen <Wei.Chen@linaro.org> > --- > v1 -> v2: > - Use a generic method instead of changing Makefiles > --- > xen/Rules.mk | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/xen/Rules.mk b/xen/Rules.mk > index 961d533..fb3087c 100644 > --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -95,12 +95,21 @@ include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk > include Makefile > > DEPS = .*.d > + > +# In obj-y and ALL_OBJS, there may have some objects are not in the You are missing a verb here. I think you mean to say 'there may be some objects which are not in the' > +# same folder as Makefile, so we have to use gendep to generate > +# dependencies for them. > +DEP_OBJS = $(obj-y) > + > +# Extract objects from ALL_OBJS added by Makefile. > +DEP_OBJS += $(filter-out %built_in.o,$(ALL_OBJS)) > + > define gendep > ifneq ($(1),$(subst /,:,$(1))) > DEPS += $(dir $(1)).$(notdir $(1)).d > endif > endef > -$(foreach o,$(filter-out %/,$(obj-y)),$(eval $(call gendep,$(o)))) > +$(foreach o,$(filter-out %/,$(DEP_OBJS)),$(eval $(call gendep,$(o)))) > > # Ensure each subdirectory has exactly one trailing slash. > subdir-n := $(patsubst %,%/,$(patsubst %/,%,$(subdir-n) $(subdir-))) > -- > 2.7.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Wei Chen writes ("[RFC v2] xen/arm: build: add missed dependency for head.S"):
> In current Xen build rules, the build system will only check the
> dependencies in current folder and obj-y generated dependencies
> in other folder.
>
> But Makefile may add some objects to ALL_OBJS. These objects may
> be not in the same folder as Makefile. Use arch/arm/Makefile as
> an example:
> ALL_OBJS := $(TARGET_SUBARCH)/head.o
Thanks for the v2.
But I think I actually somewhat prefer the v1 approach, for these
reasons:
On 03/06/16 11:07, Wei Chen wrote:
> In my mind, the better way to fix this bug is converting the DEPS from
> ALL_OBJS. But I am afraid of the impact. I am not sure whether there
> are some dependencies are not generated from obj files.
>
Ian Jackson writes ("Re: [RFC] xen/arm: build: add missed dependency for head.S"):
> I do like this idea but I have the same worry. It might be possible
> to dump ALL_OBJS out somehow and check this, but it might be
> arch-dependent.
>
> Wei Chen's patch is IMO a straightforward fix.
I won't NAK the v2 but I think if Wei Chen is still happy for us to do
so, we should commit the v1.
Ian.
On Tue, Jun 07, 2016 at 05:01:35PM +0100, Ian Jackson wrote: > Wei Chen writes ("[RFC v2] xen/arm: build: add missed dependency for head.S"): > > In current Xen build rules, the build system will only check the > > dependencies in current folder and obj-y generated dependencies > > in other folder. > > > > But Makefile may add some objects to ALL_OBJS. These objects may > > be not in the same folder as Makefile. Use arch/arm/Makefile as > > an example: > > ALL_OBJS := $(TARGET_SUBARCH)/head.o > > Thanks for the v2. > > But I think I actually somewhat prefer the v1 approach, for these > reasons: > > On 03/06/16 11:07, Wei Chen wrote: > > In my mind, the better way to fix this bug is converting the DEPS from > > ALL_OBJS. But I am afraid of the impact. I am not sure whether there > > are some dependencies are not generated from obj files. > > > Ian Jackson writes ("Re: [RFC] xen/arm: build: add missed dependency for head.S"): > > I do like this idea but I have the same worry. It might be possible > > to dump ALL_OBJS out somehow and check this, but it might be > > arch-dependent. > > > > Wei Chen's patch is IMO a straightforward fix. > > I won't NAK the v2 but I think if Wei Chen is still happy for us to do > so, we should commit the v1. > > Ian. We shall take either v1 or v2. I don't really have an opinion which one. Either one: Release-acked-by: Wei Liu <wei.liu2@citrix.com>
On 7 June 2016 at 21:18, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > On Tue, Jun 07, 2016 at 12:34:43PM +0800, Wei Chen wrote: >> In current Xen build rules, the build system will only check the >> dependencies in current folder and obj-y generated dependencies >> in other folder. >> >> But Makefile may add some objects to ALL_OBJS. These objects may >> be not in the same folder as Makefile. Use arch/arm/Makefile as >> an example: >> ALL_OBJS := $(TARGET_SUBARCH)/head.o >> >> The head.o is not in the same folder as Makefile and is not listed >> in obj-y. So, when we update the header files that had been included >> in head.S. The build system would not check the dependency of head.S. >> The head.S would not be re-compiled. >> >> In this patch, we will get objects added by Makefile and add their >> dependencies to check list. >> >> Signed-off-by: Wei Chen <Wei.Chen@linaro.org> >> --- >> v1 -> v2: >> - Use a generic method instead of changing Makefiles >> --- >> xen/Rules.mk | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/xen/Rules.mk b/xen/Rules.mk >> index 961d533..fb3087c 100644 >> --- a/xen/Rules.mk >> +++ b/xen/Rules.mk >> @@ -95,12 +95,21 @@ include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk >> include Makefile >> >> DEPS = .*.d >> + >> +# In obj-y and ALL_OBJS, there may have some objects are not in the > > You are missing a verb here. > > I think you mean to say 'there may be some objects which are not in the' Thank you! it's my mistake. >> +# same folder as Makefile, so we have to use gendep to generate >> +# dependencies for them. >> +DEP_OBJS = $(obj-y) >> + >> +# Extract objects from ALL_OBJS added by Makefile. >> +DEP_OBJS += $(filter-out %built_in.o,$(ALL_OBJS)) >> + >> define gendep >> ifneq ($(1),$(subst /,:,$(1))) >> DEPS += $(dir $(1)).$(notdir $(1)).d >> endif >> endef >> -$(foreach o,$(filter-out %/,$(obj-y)),$(eval $(call gendep,$(o)))) >> +$(foreach o,$(filter-out %/,$(DEP_OBJS)),$(eval $(call gendep,$(o)))) >> >> # Ensure each subdirectory has exactly one trailing slash. >> subdir-n := $(patsubst %,%/,$(patsubst %/,%,$(subdir-n) $(subdir-))) >> -- >> 2.7.4 >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> http://lists.xen.org/xen-devel
Hi, On 8 June 2016 at 00:24, Wei Liu <wei.liu2@citrix.com> wrote: > On Tue, Jun 07, 2016 at 05:01:35PM +0100, Ian Jackson wrote: >> Wei Chen writes ("[RFC v2] xen/arm: build: add missed dependency for head.S"): >> > In current Xen build rules, the build system will only check the >> > dependencies in current folder and obj-y generated dependencies >> > in other folder. >> > >> > But Makefile may add some objects to ALL_OBJS. These objects may >> > be not in the same folder as Makefile. Use arch/arm/Makefile as >> > an example: >> > ALL_OBJS := $(TARGET_SUBARCH)/head.o >> >> Thanks for the v2. >> >> But I think I actually somewhat prefer the v1 approach, for these >> reasons: >> >> On 03/06/16 11:07, Wei Chen wrote: >> > In my mind, the better way to fix this bug is converting the DEPS from >> > ALL_OBJS. But I am afraid of the impact. I am not sure whether there >> > are some dependencies are not generated from obj files. >> > >> Ian Jackson writes ("Re: [RFC] xen/arm: build: add missed dependency for head.S"): >> > I do like this idea but I have the same worry. It might be possible >> > to dump ALL_OBJS out somehow and check this, but it might be >> > arch-dependent. >> > >> > Wei Chen's patch is IMO a straightforward fix. >> >> I won't NAK the v2 but I think if Wei Chen is still happy for us to do >> so, we should commit the v1. >> >> Ian. > > We shall take either v1 or v2. I don't really have an opinion which one. > Thank you. I will be happy if anyone would be committed. IMO, please commit the v1 patch. The v1 patch gives the arch Makefile more right to add dependencies not just only in ALL_OBJS. > Either one: > > Release-acked-by: Wei Liu <wei.liu2@citrix.com>
On Wed, Jun 08, 2016 at 11:14:14AM +0800, Wei Chen wrote: > Hi, > > On 8 June 2016 at 00:24, Wei Liu <wei.liu2@citrix.com> wrote: > > On Tue, Jun 07, 2016 at 05:01:35PM +0100, Ian Jackson wrote: > >> Wei Chen writes ("[RFC v2] xen/arm: build: add missed dependency for head.S"): > >> > In current Xen build rules, the build system will only check the > >> > dependencies in current folder and obj-y generated dependencies > >> > in other folder. > >> > > >> > But Makefile may add some objects to ALL_OBJS. These objects may > >> > be not in the same folder as Makefile. Use arch/arm/Makefile as > >> > an example: > >> > ALL_OBJS := $(TARGET_SUBARCH)/head.o > >> > >> Thanks for the v2. > >> > >> But I think I actually somewhat prefer the v1 approach, for these > >> reasons: > >> > >> On 03/06/16 11:07, Wei Chen wrote: > >> > In my mind, the better way to fix this bug is converting the DEPS from > >> > ALL_OBJS. But I am afraid of the impact. I am not sure whether there > >> > are some dependencies are not generated from obj files. > >> > > >> Ian Jackson writes ("Re: [RFC] xen/arm: build: add missed dependency for head.S"): > >> > I do like this idea but I have the same worry. It might be possible > >> > to dump ALL_OBJS out somehow and check this, but it might be > >> > arch-dependent. > >> > > >> > Wei Chen's patch is IMO a straightforward fix. > >> > >> I won't NAK the v2 but I think if Wei Chen is still happy for us to do > >> so, we should commit the v1. > >> > >> Ian. > > > > We shall take either v1 or v2. I don't really have an opinion which one. > > > > Thank you. I will be happy if anyone would be committed. IMO, please > commit the v1 patch. Thanks for confirming. Since both you and Ian prefer v1, I've queued up v1 for both xen-unstable and 4.7 branch. Wei.
diff --git a/xen/Rules.mk b/xen/Rules.mk index 961d533..fb3087c 100644 --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -95,12 +95,21 @@ include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk include Makefile DEPS = .*.d + +# In obj-y and ALL_OBJS, there may have some objects are not in the +# same folder as Makefile, so we have to use gendep to generate +# dependencies for them. +DEP_OBJS = $(obj-y) + +# Extract objects from ALL_OBJS added by Makefile. +DEP_OBJS += $(filter-out %built_in.o,$(ALL_OBJS)) + define gendep ifneq ($(1),$(subst /,:,$(1))) DEPS += $(dir $(1)).$(notdir $(1)).d endif endef -$(foreach o,$(filter-out %/,$(obj-y)),$(eval $(call gendep,$(o)))) +$(foreach o,$(filter-out %/,$(DEP_OBJS)),$(eval $(call gendep,$(o)))) # Ensure each subdirectory has exactly one trailing slash. subdir-n := $(patsubst %,%/,$(patsubst %/,%,$(subdir-n) $(subdir-)))
In current Xen build rules, the build system will only check the dependencies in current folder and obj-y generated dependencies in other folder. But Makefile may add some objects to ALL_OBJS. These objects may be not in the same folder as Makefile. Use arch/arm/Makefile as an example: ALL_OBJS := $(TARGET_SUBARCH)/head.o The head.o is not in the same folder as Makefile and is not listed in obj-y. So, when we update the header files that had been included in head.S. The build system would not check the dependency of head.S. The head.S would not be re-compiled. In this patch, we will get objects added by Makefile and add their dependencies to check list. Signed-off-by: Wei Chen <Wei.Chen@linaro.org> --- v1 -> v2: - Use a generic method instead of changing Makefiles --- xen/Rules.mk | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)