| Message ID | 1478586541-8528-1-git-send-email-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 08, 2016 at 07:29:01AM +0100, Juergen Gross wrote: > The dependency for setting up the links for ioemu is wrong: it is > depending on tools/qemu-xen-traditional-dir which is being modified by > each "make tools" call. This leads to rebuilds of several stubdom > libraries for each call of "make stubdom" as those depend on the ioemu > linkfarm stamp file. > > Modify the dependency to the qemu Makefile instead which is a better > indicator whether we need to create the qemu directory before setting > up the links. > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > stubdom/Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/stubdom/Makefile b/stubdom/Makefile > index 2921f30..0b33e23 100644 > --- a/stubdom/Makefile > +++ b/stubdom/Makefile > @@ -281,10 +281,10 @@ IOEMU_LINKFARM_TARGET := ioemu/linkfarm.stamp > endif > > ifeq ($(QEMU_ROOT),.) > -$(XEN_ROOT)/tools/qemu-xen-traditional-dir: > +$(XEN_ROOT)/tools/qemu-xen-traditional-dir/Makefile: > $(MAKE) DESTDIR= -C $(XEN_ROOT)/tools qemu-xen-traditional-dir-find > What happens if you modify qemu-trad source file(s) and then rebuild stubdom? Will that work correctly after this patch is applied? > -ioemu/linkfarm.stamp: $(XEN_ROOT)/tools/qemu-xen-traditional-dir > +ioemu/linkfarm.stamp: $(XEN_ROOT)/tools/qemu-xen-traditional-dir/Makefile > mkdir -p ioemu > set -e; \ > $(buildmakevars2shellvars); \ > -- > 2.6.6 >
On 08/11/16 11:41, Wei Liu wrote: > On Tue, Nov 08, 2016 at 07:29:01AM +0100, Juergen Gross wrote: >> The dependency for setting up the links for ioemu is wrong: it is >> depending on tools/qemu-xen-traditional-dir which is being modified by >> each "make tools" call. This leads to rebuilds of several stubdom >> libraries for each call of "make stubdom" as those depend on the ioemu >> linkfarm stamp file. >> >> Modify the dependency to the qemu Makefile instead which is a better >> indicator whether we need to create the qemu directory before setting >> up the links. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> stubdom/Makefile | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/stubdom/Makefile b/stubdom/Makefile >> index 2921f30..0b33e23 100644 >> --- a/stubdom/Makefile >> +++ b/stubdom/Makefile >> @@ -281,10 +281,10 @@ IOEMU_LINKFARM_TARGET := ioemu/linkfarm.stamp >> endif >> >> ifeq ($(QEMU_ROOT),.) >> -$(XEN_ROOT)/tools/qemu-xen-traditional-dir: >> +$(XEN_ROOT)/tools/qemu-xen-traditional-dir/Makefile: >> $(MAKE) DESTDIR= -C $(XEN_ROOT)/tools qemu-xen-traditional-dir-find >> > > What happens if you modify qemu-trad source file(s) and then rebuild > stubdom? Will that work correctly after this patch is applied? I think so, yes. Make is using the file modification times of the link targets to figure out what to rebuild. The only problem which could occur is that adding a file to qemu would lead to a link missing. This could be caught by always creating the links. Question is whether this rare case is to be handled properly or not. Especially when taking the status of qemu-trad into account: I wouldn't expect new source files to show up very often. Juergen
On 08/11/16 07:29, Juergen Gross wrote: > The dependency for setting up the links for ioemu is wrong: it is > depending on tools/qemu-xen-traditional-dir which is being modified by > each "make tools" call. This leads to rebuilds of several stubdom > libraries for each call of "make stubdom" as those depend on the ioemu > linkfarm stamp file. > > Modify the dependency to the qemu Makefile instead which is a better > indicator whether we need to create the qemu directory before setting > up the links. > > Signed-off-by: Juergen Gross <jgross@suse.com> Ping? Juergen > --- > stubdom/Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/stubdom/Makefile b/stubdom/Makefile > index 2921f30..0b33e23 100644 > --- a/stubdom/Makefile > +++ b/stubdom/Makefile > @@ -281,10 +281,10 @@ IOEMU_LINKFARM_TARGET := ioemu/linkfarm.stamp > endif > > ifeq ($(QEMU_ROOT),.) > -$(XEN_ROOT)/tools/qemu-xen-traditional-dir: > +$(XEN_ROOT)/tools/qemu-xen-traditional-dir/Makefile: > $(MAKE) DESTDIR= -C $(XEN_ROOT)/tools qemu-xen-traditional-dir-find > > -ioemu/linkfarm.stamp: $(XEN_ROOT)/tools/qemu-xen-traditional-dir > +ioemu/linkfarm.stamp: $(XEN_ROOT)/tools/qemu-xen-traditional-dir/Makefile > mkdir -p ioemu > set -e; \ > $(buildmakevars2shellvars); \ >
On Tue, Nov 08, 2016 at 12:18:05PM +0100, Juergen Gross wrote: > On 08/11/16 11:41, Wei Liu wrote: > > On Tue, Nov 08, 2016 at 07:29:01AM +0100, Juergen Gross wrote: > >> The dependency for setting up the links for ioemu is wrong: it is > >> depending on tools/qemu-xen-traditional-dir which is being modified by > >> each "make tools" call. This leads to rebuilds of several stubdom > >> libraries for each call of "make stubdom" as those depend on the ioemu > >> linkfarm stamp file. > >> > >> Modify the dependency to the qemu Makefile instead which is a better > >> indicator whether we need to create the qemu directory before setting > >> up the links. > >> > >> Signed-off-by: Juergen Gross <jgross@suse.com> > >> --- > >> stubdom/Makefile | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/stubdom/Makefile b/stubdom/Makefile > >> index 2921f30..0b33e23 100644 > >> --- a/stubdom/Makefile > >> +++ b/stubdom/Makefile > >> @@ -281,10 +281,10 @@ IOEMU_LINKFARM_TARGET := ioemu/linkfarm.stamp > >> endif > >> > >> ifeq ($(QEMU_ROOT),.) > >> -$(XEN_ROOT)/tools/qemu-xen-traditional-dir: > >> +$(XEN_ROOT)/tools/qemu-xen-traditional-dir/Makefile: > >> $(MAKE) DESTDIR= -C $(XEN_ROOT)/tools qemu-xen-traditional-dir-find > >> > > > > What happens if you modify qemu-trad source file(s) and then rebuild > > stubdom? Will that work correctly after this patch is applied? > > I think so, yes. Make is using the file modification times of the link > targets to figure out what to rebuild. > I'm afraid I'm still not convinced this patch is the right approach. The original approach is slower but it always produce the correct result. The approach in this patch makes things faster but it is not always correct. I think I value correctness over speed. > The only problem which could occur is that adding a file to qemu would > lead to a link missing. This could be caught by always creating the > links. Maybe this is the right approach? Wei. > Question is whether this rare case is to be handled properly or > not. Especially when taking the status of qemu-trad into account: I > wouldn't expect new source files to show up very often. > > > Juergen
diff --git a/stubdom/Makefile b/stubdom/Makefile index 2921f30..0b33e23 100644 --- a/stubdom/Makefile +++ b/stubdom/Makefile @@ -281,10 +281,10 @@ IOEMU_LINKFARM_TARGET := ioemu/linkfarm.stamp endif ifeq ($(QEMU_ROOT),.) -$(XEN_ROOT)/tools/qemu-xen-traditional-dir: +$(XEN_ROOT)/tools/qemu-xen-traditional-dir/Makefile: $(MAKE) DESTDIR= -C $(XEN_ROOT)/tools qemu-xen-traditional-dir-find -ioemu/linkfarm.stamp: $(XEN_ROOT)/tools/qemu-xen-traditional-dir +ioemu/linkfarm.stamp: $(XEN_ROOT)/tools/qemu-xen-traditional-dir/Makefile mkdir -p ioemu set -e; \ $(buildmakevars2shellvars); \
The dependency for setting up the links for ioemu is wrong: it is depending on tools/qemu-xen-traditional-dir which is being modified by each "make tools" call. This leads to rebuilds of several stubdom libraries for each call of "make stubdom" as those depend on the ioemu linkfarm stamp file. Modify the dependency to the qemu Makefile instead which is a better indicator whether we need to create the qemu directory before setting up the links. Signed-off-by: Juergen Gross <jgross@suse.com> --- stubdom/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)