diff mbox

Makefile: Make "install" depend on "trace-events-all"

Message ID 20170204143245.15974-1-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng Feb. 4, 2017, 2:32 p.m. UTC
We install this file to data dir but since 0ab8ed18 it's no longer
required by any objects during "make". List it explicitly as a depended
target of install and fix the broken "make install" command.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Lluís Vilanova Feb. 4, 2017, 10:39 p.m. UTC | #1
Fam Zheng writes:

> We install this file to data dir but since 0ab8ed18 it's no longer
> required by any objects during "make". List it explicitly as a depended
> target of install and fix the broken "make install" command.

I'm probably wrong, but I remember someone worked on making traces
self-descriptive, so that simpletrace would no longer need access to the
generated trace-events-all file.

If the file is really never used, then all stray rules to generate it should be
removed, as well as its installation.

Cheers,
  Lluis
Daniel P. Berrangé Feb. 6, 2017, 10:35 a.m. UTC | #2
On Sat, Feb 04, 2017 at 10:32:45PM +0800, Fam Zheng wrote:
> We install this file to data dir but since 0ab8ed18 it's no longer
> required by any objects during "make". List it explicitly as a depended
> target of install and fix the broken "make install" command.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>


Regards,
Daniel
Daniel P. Berrangé Feb. 6, 2017, 10:36 a.m. UTC | #3
On Sun, Feb 05, 2017 at 12:39:45AM +0200, Lluís Vilanova wrote:
> Fam Zheng writes:
> 
> > We install this file to data dir but since 0ab8ed18 it's no longer
> > required by any objects during "make". List it explicitly as a depended
> > target of install and fix the broken "make install" command.
> 
> I'm probably wrong, but I remember someone worked on making traces
> self-descriptive, so that simpletrace would no longer need access to the
> generated trace-events-all file.
> 
> If the file is really never used, then all stray rules to generate it should be
> removed, as well as its installation.

It is used at runtime when analysing a simpletrace file. While it would
be nice if they were self-descriptive, that isn't the case today, so we
must install this file.

Regards,
Daniel
Christian Borntraeger Feb. 7, 2017, 10:17 a.m. UTC | #4
On 02/04/2017 03:32 PM, Fam Zheng wrote:
> We install this file to data dir but since 0ab8ed18 it's no longer
> required by any objects during "make". List it explicitly as a depended
> target of install and fix the broken "make install" command.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>

Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>


> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 4b72a4c..b993741 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -589,7 +589,7 @@ endif
>  endif
> 
> 
> -install: all $(if $(BUILD_DOCS),install-doc) \
> +install: all $(if $(BUILD_DOCS),install-doc) $(BUILD_DIR)/trace-events-all \
>  install-datadir install-localstatedir
>  ifneq ($(TOOLS),)
>  	$(call install-prog,$(subst qemu-ga,qemu-ga$(EXESUF),$(TOOLS)),$(DESTDIR)$(bindir))
>
Stefan Hajnoczi Feb. 7, 2017, 3:14 p.m. UTC | #5
On Sat, Feb 04, 2017 at 10:32:45PM +0800, Fam Zheng wrote:
> We install this file to data dir but since 0ab8ed18 it's no longer
> required by any objects during "make". List it explicitly as a depended
> target of install and fix the broken "make install" command.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan
Doug Gilmore Feb. 7, 2017, 10:25 p.m. UTC | #6
On 02/07/2017 07:14 AM, Stefan Hajnoczi wrote:
> On Sat, Feb 04, 2017 at 10:32:45PM +0800, Fam Zheng wrote:
>> We install this file to data dir but since 0ab8ed18 it's no longer
>> required by any objects during "make". List it explicitly as a depended
>> target of install and fix the broken "make install" command.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> ---
>>  Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Thanks, applied to my tracing tree:
> https://github.com/stefanha/qemu/commits/tracing
> 
> Stefan
> 
Are there plans to commit this change?

AFAICS, ToT builds are broken without this change.

Thanks,

Doug
Stefan Hajnoczi Feb. 13, 2017, 2:04 p.m. UTC | #7
On Tue, Feb 07, 2017 at 02:25:31PM -0800, Doug Gilmore wrote:
> On 02/07/2017 07:14 AM, Stefan Hajnoczi wrote:
> > On Sat, Feb 04, 2017 at 10:32:45PM +0800, Fam Zheng wrote:
> >> We install this file to data dir but since 0ab8ed18 it's no longer
> >> required by any objects during "make". List it explicitly as a depended
> >> target of install and fix the broken "make install" command.
> >>
> >> Signed-off-by: Fam Zheng <famz@redhat.com>
> >> ---
> >>  Makefile | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Thanks, applied to my tracing tree:
> > https://github.com/stefanha/qemu/commits/tracing
> > 
> > Stefan
> > 
> Are there plans to commit this change?
> 
> AFAICS, ToT builds are broken without this change.

Hi Doug,
I've been mostly offline due to illness but am back now.  The pull
request will be sent today.

Stefan
diff mbox

Patch

diff --git a/Makefile b/Makefile
index 4b72a4c..b993741 100644
--- a/Makefile
+++ b/Makefile
@@ -589,7 +589,7 @@  endif
 endif
 
 
-install: all $(if $(BUILD_DOCS),install-doc) \
+install: all $(if $(BUILD_DOCS),install-doc) $(BUILD_DIR)/trace-events-all \
 install-datadir install-localstatedir
 ifneq ($(TOOLS),)
 	$(call install-prog,$(subst qemu-ga,qemu-ga$(EXESUF),$(TOOLS)),$(DESTDIR)$(bindir))