Re: [Qemu-devel] [PATCH v4 26/26] qidl: unit tests and build infrastructure

[Paolo Bonzini <pbonzini@redhat.com>]

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.

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.*)

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    $@"))