Re: [Qemu-devel] [PATCH] build: Use separate makefile for "trace/"

[Lluís Vilanova]

Paolo Bonzini writes:
[...]
>>>> -
>>>> -$(trace-obj-y): $(GENERATED_HEADERS)
>>>> +trace-obj-y += trace/
>> 
>>> Please just put it into oslib-obj-y.
>> 
>> You mean line "trace-obj-y += trace/"?

> Yeah, make it

> oslib-obj-y += trace/

> and get rid of trace-obj-y.

Mmm, but according to Makefile.objs:

  oslib-obj-y is code depending on the OS (win32 vs posix)


[...]
>> "trace-obj-y" is required by some binaries other than QEMU itself, as they 
>> use
>> routines that require the tracing infrastructure (e.g., qemu-img, which only
>> includes "qemu-timer-common.o" as a dependency through "tools-obj-y").

> Yes, but they all use oslib-obj-y too.  qemu-timer-common.o is part of
> oslib-obj-y.

Removed. Still, see above.


>> I'm not sure how the subdir magic treats paths, but mapping all paths in 
>> final
>> vars into their respective absolute path should simplify things.

> Difficult to do in make. :(

Among many other things, AFAIR the linux kernel build system uses absolute
paths, but I think it also uses make to get into each subdirectory (as opposed
to QEMU), which I simplifies coding such a build system.


>> In any case, compiling with backends none, stderr, and stdout turns up no
>> linking problems.

> And simple too?  (I suppose stdout is a typo for simple).

Yes, sorry.


>>>> +ifeq ($(TRACE_BACKEND),dtrace)
>>>> +TRACE_H_EXTRA_DEPS=trace/generated-dtrace.h
>>>> +endif
>>>> +trace.h: trace.h-timestamp $(TRACE_H_EXTRA_DEPS)
>>>> +trace.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
>>>> +  $(call quiet-command,$(TRACETOOL) \
>>>> +          --format=h \
>>>> +          --backend=$(TRACE_BACKEND) \
>>>> +          < $< > $@,"  GEN   trace.h")
>>>> +  @cmp -s $@ trace.h || cp $@ trace.h
>>>> +
>>>> +
>>>> +trace/generated.c: trace/generated.c-timestamp
>> 
>>> Please use $(obj)/generated.c, and similarly for everything else.
>> 
>> Tried it, but "obj" is set to ".", although looking at "rules.mak" that 
>> should
>> not be the case...
>> 
>> Inserting "$(error obj=$(obj))" in "trace/Makefile.objs" shows "./trace", but
>> echoing "obj" in any of the rules shows ".".

> In the rules it is, because the rules expand variables later when the
> command runs.  But in the targets it should be evaluated correctly,
> because the targets expand variables earlier when they are read.  As
> long as you use $@ and $< and $^ in the rules, you're safe.  Anyway,
> this can be done later.

Ok, done. But this behaviour easily leads to errors. It might be safer to switch
into a per-directory call to $(MAKE), although this would for sure be a
completely different patch.


>> BTW, can I extend the GENERATED_HEADERS variable contents right from
>> "trace/Makefile.objs" even though it's not explicitly included from the
>> top-level makefile?

> Hmm, risky. :)  Need to look at the actual patch, let's do one thing at
> a time.

For now I'll leave it just as it is.


>> I'm sure this has already been previously discussed to the point of 
>> extenuation,
>> but what are the reasons for not using autotools?

> Autoconf -> no point, but someone needs to do the work.

> Automake -> the build system is just too different.

> Libtool -> using it already. :)


Ok, so it's not something against the suite per-se, but about porting work.

The thing I like about automake is that it provides a clear set of vars to
manage the per-dir builds, thanks to using a per-dir $(MAKE); but I'm not sure
how the per-target build would be managed (except by having a separate
configure+make for each of them).

This could also be provided by having the QEMU build infrastructure use $(MAKE)
to enter into each directory, and having it produce an ar file (or a set of 
them)
with a "standard" name as a result (using libtool).

But I'm not sure how well would that work when building in Windows (is libtool
available there?).


Thanks,
  Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth