On Mon, Oct 15, 2012 at 12:05:09PM +0200, Paolo Bonzini wrote:
> Il 12/10/2012 23:11, Michael Roth ha scritto:
> > +%.qidl.c: %.c $(SRC_PATH)/qidl.h $(addprefix $(SRC_PATH)/scripts/,lexer.py
> > qidl.py qidl_parser.py qapi.py qapi_visit.py)
>
> The rule here is wrong, because %.qidl.c is never produced by the
> commands. The output is in the qidl-generated subdirectory, and that
> cannot be expressed with pattern rules.
>
> I was worried that this causes many greps on every invocation of the
> Makefile---even though the way you hid them in makefile functions would
> hide them from the "make" output.
>
> However, this is not the case. What happens is quite complex.
> hw/foo.qidl.c (as opposed to hw/qidl-generated/foo.qidl.c) will never
> exist, but it is a dependency of hw/foo.o, so it is always rebuilt
> before it. But it is not rebuilt on every invocation even though it
> doesn't exist, because it only comes into play via implicit rules.
>
> I think this is quite clever and does exactly what you want. If you
> want to keep the qidl-generated directory, I cannot imagine any other
> way to do it. However, please rename %.qidl.c to something QIDL-PP-% so
> that it is clear that it is not a "real" file name.
At the time I was in battle to the death with make to have it let me use
qidl-generated/ directories to store the output, but I don't care too
much anymore :P I think it's worth having if it doesn't cause any problems
though.
There is another way to do this (still using a "dummy" target) that is a
bit less cryptic:
QIDL-PP-%: %.c qidl.h ...
<grep and create %.qidl.c>
%.o: QIDL-PP-%
<build normal or qidl CC>
But make detects that QIDL-PP-% is an intermediate target and removes
the *.qidl.c files after the build, which ends up forcing a re-build
each time. I played around with .INTERMEDIATE/.PRECIOUS directives to
have it keep the files but couldn't get it to cooperate, which is why I
ended up with the current approach.
>
> If we decide this is too clever (and it's not all of it, it took me a
> while to understand that Makefile functions are needed because
> quiet-command expands to a @-prefixed command...), we can drop the
> qidl-generated directory.
>
> > + $(call rm -f $(*D)/qidl-generated/$(*F).qidl.*)
Yes, and in fact this doesn't work. I hadn't noticed it because the old
files get clobbered by the code gen, so it only comes into play if you
take a QIDL'd file and then un-QIDL it (since the %.o rule sees the
.qidl.c file still there and tries to build it accordingly).
I'll go ahead and do a v5 with these changes in place.
>
> I think this $(call) is not what you want, you need just "@rm -f ..."
>
> Paolo
>
> > + $(if $(strip $(shell grep "QIDL_ENABLE()" $< 1>/dev/null && echo
> > "true")), \
> > + $(call quiet-command, \
> > + $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(CFLAGS) -E -c -DQIDL_GEN $<
> > | \
> > + $(PYTHON) $(SRC_PATH)/scripts/qidl.py $(QIDL_FLAGS) \
> > + --output-filepath=$(*D)/qidl-generated/$(*F).qidl.c || [ "$$?" -eq
> > 2 ], \
> > + "qidl PP $(*D)/$(*F).c"),)
> > +
> > +%.o: %.c %.qidl.c
> > + $(if $(strip $(shell test -f $(*D)/qidl-generated/$(*F).qidl.c && echo
> > "true")), \
> > + $(call quiet-command, \
> > + $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c \
> > + -DQIDL_ENABLED -include $< -o $@
> > $(*D)/qidl-generated/$(*F).qidl.c, \
> > + "qidl CC $@"), \
> > + $(call quiet-command, \
> > + $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c \
> > + -o $@ $<," CC $@"))
>
