[Qemu-devel] [PATCH v5 00/18] qapi: add #if pre-processor conditions to generated code (part 3)

Markus Armbruster posted 18 patches 5 years, 2 months ago
Test asan passed
Test docker-clang@ubuntu failed
Test docker-mingw@fedora passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190214152251.2073-1-armbru@redhat.com
Maintainers: Cornelia Huck <cohuck@redhat.com>, David Hildenbrand <david@redhat.com>, Eric Blake <eblake@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, Gerd Hoffmann <kraxel@redhat.com>, Michael Roth <mdroth@linux.vnet.ibm.com>, Christian Borntraeger <borntraeger@de.ibm.com>, Peter Maydell <peter.maydell@linaro.org>, Eduardo Habkost <ehabkost@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Markus Armbruster <armbru@redhat.com>, Richard Henderson <rth@twiddle.net>, "Michael S. Tsirkin" <mst@redhat.com>
.gitignore                               |   1 +
Makefile                                 |   2 +-
Makefile.objs                            |  17 +-
Makefile.target                          |   1 +
docs/devel/qapi-code-gen.txt             |  82 +++-
hw/ppc/spapr_rtc.c                       |   2 +-
hw/s390x/s390-skeys.c                    |   2 +-
hw/timer/mc146818rtc.c                   |   4 +-
include/qapi/qmp/dispatch.h              |   1 -
include/sysemu/arch_init.h               |  11 -
monitor.c                                |  88 +---
qapi/Makefile.objs                       |  25 ++
qapi/misc.json                           | 485 +--------------------
qapi/qapi-schema.json                    |   1 +
qapi/qmp-registry.c                      |   8 -
qapi/target.json                         | 514 +++++++++++++++++++++++
qemu-deprecated.texi                     |   5 +
qmp.c                                    |  26 --
scripts/qapi/commands.py                 |   2 +-
scripts/qapi/common.py                   |  55 ++-
scripts/qapi/events.py                   |  38 +-
scripts/qapi/types.py                    |   4 +-
scripts/qapi/visit.py                    |   4 +-
stubs/Makefile.objs                      |   4 -
stubs/arch-query-cpu-def.c               |  11 -
stubs/arch-query-cpu-model-baseline.c    |  13 -
stubs/arch-query-cpu-model-comparison.c  |  13 -
stubs/arch-query-cpu-model-expansion.c   |  13 -
stubs/monitor.c                          |   2 +-
target/arm/helper.c                      |   3 +-
target/arm/monitor.c                     |   2 +-
target/i386/cpu.c                        |   7 +-
target/i386/sev_i386.h                   |   2 +-
target/ppc/translate_init.inc.c          |   3 +-
target/s390x/cpu_models.c                |   9 +-
tests/qapi-schema/comments.out           |   1 +
tests/qapi-schema/doc-bad-section.out    |   1 +
tests/qapi-schema/doc-good.out           |   1 +
tests/qapi-schema/empty.out              |   1 +
tests/qapi-schema/event-case.out         |   1 +
tests/qapi-schema/ident-with-escape.out  |   1 +
tests/qapi-schema/include-relpath.out    |   1 +
tests/qapi-schema/include-repetition.out |   1 +
tests/qapi-schema/include-simple.out     |   1 +
tests/qapi-schema/indented-expr.out      |   1 +
tests/qapi-schema/qapi-schema-test.out   |   1 +
tests/test-qmp-event.c                   |   1 +
tests/test-qobject-input-visitor.c       |   1 -
ui/vnc.c                                 |   3 +-
49 files changed, 735 insertions(+), 741 deletions(-)
create mode 100644 qapi/target.json
delete mode 100644 stubs/arch-query-cpu-def.c
delete mode 100644 stubs/arch-query-cpu-model-baseline.c
delete mode 100644 stubs/arch-query-cpu-model-comparison.c
delete mode 100644 stubs/arch-query-cpu-model-expansion.c
[Qemu-devel] [PATCH v5 00/18] qapi: add #if pre-processor conditions to generated code (part 3)
Posted by Markus Armbruster 5 years, 2 months ago
Marc-André posted a v1 that relies on a QAPI schema language extension
'top-unit' to permit splitting the generated code.  Here is his cover
letter:

    The thrid and last part (of "[PATCH v2 00/54] qapi: add #if
    pre-processor conditions to generated code") is about adding schema
    conditions based on the target.

    For now, the qapi code is compiled in common objects (common to all
    targets). This makes it impossible to add #if TARGET_ARM for example.

    The patch "RFC: qapi: learn to split the schema by 'top-unit'"
    proposes to split the schema by "top-unit", so that generated code can
    be built either in common objects or per-target. That patch is a bit
    rough, I would like to get some feedback about the approach before
    trying to improve it. The following patches demonstrate usage of
    target-based #if conditions, and getting rid of the
    qmp_unregister_command() hack.

We already have a way to split generated code: QAPI modules.  I took
the liberty to rework Marc-André's series to use a target module
instead.  Less code, no change to the QAPI language.

v5:
* PATCH 08,09: Computation of common-obj-y fixed [Marc-André]

v4:
* PATCH 03,04,16,17: Commit message improvements
* PATCH 07: New
* PATCH 08: Rebased onto PATCH 07, R-by dropped

v3:
* PATCH v2 01+02 have been merged
* PATCH 01-04: New
* PATCH 05: Rebase of PATCH v2 03
* PATCH 06-17: Trivially rebased, R-by and such retained

Marc-André Lureau (9):
  build-sys: move qmp-introspect per target
  qapi: make rtc-reset-reinjection and SEV depend on TARGET_I386
  qapi: make s390 commands depend on TARGET_S390X
  target.json: add a note about query-cpu* not being s390x-specific
  qapi: make query-gic-capabilities depend on TARGET_ARM
  qapi: make query-cpu-model-expansion depend on s390 or x86
  qapi: make query-cpu-definitions depend on specific targets
  qapi: remove qmp_unregister_command()
  qapi: move RTC_CHANGE to the target schema

Markus Armbruster (9):
  qapi: Belatedly document modular code generation
  qapi: Fix up documentation for recent commit a95291007b2
  qapi: Clean up modular built-in code generation a bit
  qapi: Prepare for system modules other than 'builtin'
  qapi: Generate QAPIEvent stuff into separate files
  build: Deal with all of QAPI's .o in qapi/Makefile.objs
  qapi: New module target.json
  Revert "qapi-events: add 'if' condition to implicit event enum"
  qmp: Deprecate query-events in favor of query-qmp-schema

 .gitignore                               |   1 +
 Makefile                                 |   2 +-
 Makefile.objs                            |  17 +-
 Makefile.target                          |   1 +
 docs/devel/qapi-code-gen.txt             |  82 +++-
 hw/ppc/spapr_rtc.c                       |   2 +-
 hw/s390x/s390-skeys.c                    |   2 +-
 hw/timer/mc146818rtc.c                   |   4 +-
 include/qapi/qmp/dispatch.h              |   1 -
 include/sysemu/arch_init.h               |  11 -
 monitor.c                                |  88 +---
 qapi/Makefile.objs                       |  25 ++
 qapi/misc.json                           | 485 +--------------------
 qapi/qapi-schema.json                    |   1 +
 qapi/qmp-registry.c                      |   8 -
 qapi/target.json                         | 514 +++++++++++++++++++++++
 qemu-deprecated.texi                     |   5 +
 qmp.c                                    |  26 --
 scripts/qapi/commands.py                 |   2 +-
 scripts/qapi/common.py                   |  55 ++-
 scripts/qapi/events.py                   |  38 +-
 scripts/qapi/types.py                    |   4 +-
 scripts/qapi/visit.py                    |   4 +-
 stubs/Makefile.objs                      |   4 -
 stubs/arch-query-cpu-def.c               |  11 -
 stubs/arch-query-cpu-model-baseline.c    |  13 -
 stubs/arch-query-cpu-model-comparison.c  |  13 -
 stubs/arch-query-cpu-model-expansion.c   |  13 -
 stubs/monitor.c                          |   2 +-
 target/arm/helper.c                      |   3 +-
 target/arm/monitor.c                     |   2 +-
 target/i386/cpu.c                        |   7 +-
 target/i386/sev_i386.h                   |   2 +-
 target/ppc/translate_init.inc.c          |   3 +-
 target/s390x/cpu_models.c                |   9 +-
 tests/qapi-schema/comments.out           |   1 +
 tests/qapi-schema/doc-bad-section.out    |   1 +
 tests/qapi-schema/doc-good.out           |   1 +
 tests/qapi-schema/empty.out              |   1 +
 tests/qapi-schema/event-case.out         |   1 +
 tests/qapi-schema/ident-with-escape.out  |   1 +
 tests/qapi-schema/include-relpath.out    |   1 +
 tests/qapi-schema/include-repetition.out |   1 +
 tests/qapi-schema/include-simple.out     |   1 +
 tests/qapi-schema/indented-expr.out      |   1 +
 tests/qapi-schema/qapi-schema-test.out   |   1 +
 tests/test-qmp-event.c                   |   1 +
 tests/test-qobject-input-visitor.c       |   1 -
 ui/vnc.c                                 |   3 +-
 49 files changed, 735 insertions(+), 741 deletions(-)
 create mode 100644 qapi/target.json
 delete mode 100644 stubs/arch-query-cpu-def.c
 delete mode 100644 stubs/arch-query-cpu-model-baseline.c
 delete mode 100644 stubs/arch-query-cpu-model-comparison.c
 delete mode 100644 stubs/arch-query-cpu-model-expansion.c

-- 
2.17.2


Re: [Qemu-devel] [PATCH v5 00/18] qapi: add #if pre-processor conditions to generated code (part 3)
Posted by Markus Armbruster 5 years, 2 months ago
Diff from v4:

diff --git a/Makefile.objs b/Makefile.objs
index 95150d7173..5fb022d7ad 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -79,8 +79,8 @@ common-obj-$(CONFIG_FDT) += device_tree.o
 ######################################################################
 # qapi
 
-common-obj-y += $(QAPI_COMMON_MODULES:%=qapi/qapi-commands-%.o)
 common-obj-y += qmp.o hmp.o
+common-obj-y += qapi/
 endif
 
 #######################################################################
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 654321de94..87e4df1660 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -18,8 +18,9 @@ util-obj-y += $(QAPI_COMMON_MODULES:%=qapi-visit-%.o)
 util-obj-y += qapi-emit-events.o
 util-obj-y += $(QAPI_COMMON_MODULES:%=qapi-events-%.o)
 
-common-obj-y += $(QAPI_COMMON_MODULES:%=qapi-commands-%.o)
+common-obj-y = $(QAPI_COMMON_MODULES:%=qapi-commands-%.o)
 
+obj-y = qapi-introspect.o
 obj-y += $(QAPI_TARGET_MODULES:%=qapi-types-%.o)
 obj-y += qapi-types.o
 obj-y += $(QAPI_TARGET_MODULES:%=qapi-visit-%.o)
@@ -28,4 +29,3 @@ obj-y += $(QAPI_TARGET_MODULES:%=qapi-events-%.o)
 obj-y += qapi-events.o
 obj-y += $(QAPI_TARGET_MODULES:%=qapi-commands-%.o)
 obj-y += qapi-commands.o
-obj-y += qapi-introspect.o

Re: [Qemu-devel] [PATCH v5 00/18] qapi: add #if pre-processor conditions to generated code (part 3)
Posted by Eric Blake 5 years, 2 months ago
On 2/14/19 9:24 AM, Markus Armbruster wrote:
> Diff from v4:
> 

> +++ b/qapi/Makefile.objs
> @@ -18,8 +18,9 @@ util-obj-y += $(QAPI_COMMON_MODULES:%=qapi-visit-%.o)
>  util-obj-y += qapi-emit-events.o
>  util-obj-y += $(QAPI_COMMON_MODULES:%=qapi-events-%.o)
>  
> -common-obj-y += $(QAPI_COMMON_MODULES:%=qapi-commands-%.o)
> +common-obj-y = $(QAPI_COMMON_MODULES:%=qapi-commands-%.o)

In other situations, use of = instead of += has resulted in inadvertent
omission of files. Should we just always use +=, rather than having to
remember to audit if the use of = is not overriding earlier lines?

>  
> +obj-y = qapi-introspect.o

and again

>  obj-y += $(QAPI_TARGET_MODULES:%=qapi-types-%.o)
>  obj-y += qapi-types.o
>  obj-y += $(QAPI_TARGET_MODULES:%=qapi-visit-%.o)
> @@ -28,4 +29,3 @@ obj-y += $(QAPI_TARGET_MODULES:%=qapi-events-%.o)
>  obj-y += qapi-events.o
>  obj-y += $(QAPI_TARGET_MODULES:%=qapi-commands-%.o)
>  obj-y += qapi-commands.o
> -obj-y += qapi-introspect.o
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] Proper use of unnest-vars (was: [PATCH v5 00/18] qapi: add #if pre-processor conditions to generated code (part 3))
Posted by Markus Armbruster 5 years, 2 months ago
Eric Blake <eblake@redhat.com> writes:

> On 2/14/19 9:24 AM, Markus Armbruster wrote:
>> Diff from v4:
>> 
>
>> +++ b/qapi/Makefile.objs
>> @@ -18,8 +18,9 @@ util-obj-y += $(QAPI_COMMON_MODULES:%=qapi-visit-%.o)
>>  util-obj-y += qapi-emit-events.o
>>  util-obj-y += $(QAPI_COMMON_MODULES:%=qapi-events-%.o)
>>  
>> -common-obj-y += $(QAPI_COMMON_MODULES:%=qapi-commands-%.o)
>> +common-obj-y = $(QAPI_COMMON_MODULES:%=qapi-commands-%.o)
>
> In other situations, use of = instead of += has resulted in inadvertent
> omission of files. Should we just always use +=, rather than having to
> remember to audit if the use of = is not overriding earlier lines?

There's quite some magic at work around here, which today I understand
better than last week (and probably worse than next week), but maybe not
well enough to get things quite right.  I'm cc'ing Paolo and Fam for
advice.

The Makefile.objs get included via the magical unnest-vars function.
Note that the file names in qapi/Makefile.objs are all relative to
qapi/, like:

    util-obj-y += qapi-emit-events.o

If we simply included this into the top level Makefile, this would be
wrong.  We'd have to say

    util-obj-y += qapi/qapi-emit-events.o

We don't, because unnest-vars takes care of adding the prefix.  How?
Let me show you with the help of this tracing patch:

diff --git a/Makefile.target b/Makefile.target
index d6ce549388..b2218011ef 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -167,7 +167,9 @@ GENERATED_FILES += hmp-commands.h hmp-commands-info.h
 
 endif # CONFIG_SOFTMMU
 
+$(info @target/ before $(obj-y))
 dummy := $(call unnest-vars,,obj-y)
+$(info @target/ after $(obj-y))
 all-obj-y := $(obj-y)
 
 target-obj-y :=
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 87e4df1660..1789811769 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -1,3 +1,4 @@
+$(info @qapi/ initial $(obj-y))
 util-obj-y = qapi-visit-core.o qapi-dealloc-visitor.o qobject-input-visitor.o
 util-obj-y += qobject-output-visitor.o qmp-registry.o qmp-dispatch.o
 util-obj-y += string-input-visitor.o string-output-visitor.o
@@ -29,3 +30,4 @@ obj-y += $(QAPI_TARGET_MODULES:%=qapi-events-%.o)
 obj-y += qapi-events.o
 obj-y += $(QAPI_TARGET_MODULES:%=qapi-commands-%.o)
 obj-y += qapi-commands.o
+$(info @qapi/ final $(obj-y))

Output for me with V=1:

    make: Entering directory '/work/armbru/qemu/bld-x86'
[Eliding stuff that's redundant for the points I'm trying to make...]
    make[1]: Entering directory '/work/armbru/qemu/bld-x86/x86_64-softmmu'
    @target/ before exec.o accel/ tcg/tcg.o tcg/tcg-op.o tcg/tcg-op-vec.o tcg/tcg-op-gvec.o tcg/tcg-common.o tcg/optimize.o fpu/softfloat.o target/i386/ disas.o gdbstub-xml.o arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o numa.o qtest.o hw/ qapi/ memory.o memory_mapping.o dump.o win_dump.o migration/ram.o hw/i386/

This is right before $(call unnest-vars,,obj-y).  Note $(obj-y) is
non-blank.

$(call unnest-vars,,obj-y) now includes dir/Makefile.objs for all dir/
in $(obj-y).  In particular, it includes qapi/Makefile.objs.  Make
output continues:

    @qapi/ initial 

Now $(obj-y) is blank!  That's because unnest-vars makes it blank, so it
can more easily add the prefix later.

    @qapi/ final qapi-introspect.o qapi-types-target.o qapi-types.o qapi-visit-target.o qapi-visit.o qapi-events-target.o qapi-events.o qapi-commands-target.o qapi-commands.o

Makefile.objs did its thing.  Now unnest-vars works its magic:

    @target/ after exec.o tcg/tcg.o tcg/tcg-op.o tcg/tcg-op-vec.o tcg/tcg-op-gvec.o tcg/tcg-common.o tcg/optimize.o fpu/softfloat.o disas.o gdbstub-xml.o arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o numa.o qtest.o memory.o memory_mapping.o dump.o win_dump.o migration/ram.o accel/accel.o accel/kvm/kvm-all.o accel/stubs/hax-stub.o accel/stubs/hvf-stub.o accel/stubs/whpx-stub.o accel/tcg/tcg-all.o accel/tcg/cputlb.o accel/tcg/tcg-runtime.o accel/tcg/tcg-runtime-gvec.o accel/tcg/cpu-exec.o accel/tcg/cpu-exec-common.o accel/tcg/translate-all.o accel/tcg/translator.o hw/9pfs/virtio-9p-device.o hw/block/virtio-blk.o hw/block/dataplane/virtio-blk.o hw/block/dataplane/xen-block.o hw/char/virtio-serial-bus.o hw/display/vga.o hw/display/virtio-gpu.o hw/display/virtio-gpu-3d.o hw/display/virtio-gpu-pci.o hw/display/virtio-vga.o hw/hyperv/hyperv.o hw/hyperv/hyperv_testdev.o hw/intc/apic.o hw/intc/apic_common.o hw/intc/ioapic.o hw/isa/lpc_ich9.o hw/misc/ivshmem.o hw/misc/pvpanic.o hw/net/virtio-net.o hw/net/vhost_net.o hw/rdma/rdma_utils.o hw/rdma/rdma_backend.o hw/rdma/rdma_rm.o hw/rdma/vmw/pvrdma_dev_ring.o hw/rdma/vmw/pvrdma_cmd.o hw/rdma/vmw/pvrdma_qp_ops.o hw/rdma/vmw/pvrdma_main.o hw/scsi/virtio-scsi.o hw/scsi/virtio-scsi-dataplane.o hw/scsi/vhost-scsi-common.o hw/scsi/vhost-scsi.o hw/timer/mc146818rtc.o hw/tpm/tpm_ppi.o hw/vfio/common.o hw/vfio/spapr.o hw/vfio/pci.o hw/vfio/pci-quirks.o hw/vfio/display.o hw/virtio/virtio.o hw/virtio/virtio-balloon.o hw/virtio/virtio-crypto.o hw/virtio/virtio-crypto-pci.o hw/virtio/vhost.o hw/virtio/vhost-backend.o hw/virtio/vhost-user.o hw/virtio/vhost-vsock.o hw/virtio/vhost-vsock-pci.o hw/virtio/vhost-scsi-pci.o hw/virtio/virtio-input-host-pci.o hw/virtio/virtio-input-pci.o hw/virtio/virtio-rng-pci.o hw/virtio/virtio-balloon-pci.o hw/virtio/virtio-9p-pci.o hw/virtio/virtio-scsi-pci.o hw/virtio/virtio-blk-pci.o hw/virtio/virtio-net-pci.o hw/virtio/virtio-serial-pci.o hw/xen/xen-host-pci-device.o hw/xen/xen_pt.o hw/xen/xen_pt_config_init.o hw/xen/xen_pt_graphics.o hw/xen/xen_pt_msi.o hw/xen/xen_pt_load_rom.o hw/i386/multiboot.o hw/i386/pc.o hw/i386/pc_piix.o hw/i386/pc_q35.o hw/i386/pc_sysfw.o hw/i386/x86-iommu.o hw/i386/intel_iommu.o hw/i386/x86-iommu.o hw/i386/amd_iommu.o hw/i386/vmport.o hw/i386/vmmouse.o hw/i386/kvmvapic.o hw/i386/acpi-build.o hw/i386/../xenpv/xen_machine_pv.o hw/i386/kvm/clock.o hw/i386/kvm/apic.o hw/i386/kvm/i8259.o hw/i386/kvm/ioapic.o hw/i386/kvm/i8254.o hw/i386/xen/xen_platform.o hw/i386/xen/xen_apic.o hw/i386/xen/xen_pvdevice.o hw/i386/xen/xen-hvm.o hw/i386/xen/xen-mapcache.o qapi/qapi-introspect.o qapi/qapi-types-target.o qapi/qapi-types.o qapi/qapi-visit-target.o qapi/qapi-visit.o qapi/qapi-events-target.o qapi/qapi-events.o qapi/qapi-commands-target.o qapi/qapi-commands.o target/i386/helper.o target/i386/cpu.o target/i386/gdbstub.o target/i386/xsave_helper.o target/i386/translate.o target/i386/bpt_helper.o target/i386/cc_helper.o target/i386/excp_helper.o target/i386/fpu_helper.o target/i386/int_helper.o target/i386/mem_helper.o target/i386/misc_helper.o target/i386/mpx_helper.o target/i386/seg_helper.o target/i386/smm_helper.o target/i386/svm_helper.o target/i386/machine.o target/i386/arch_memory_mapping.o target/i386/arch_dump.o target/i386/monitor.o target/i386/kvm.o target/i386/hyperv.o target/i386/sev.o

This is

* All the non-directory names: exec.o ... migration/ram.o

* Stuff from directories: accel/ hw/ hw/i386/ qapi/ target/i386/.  Note
  that directories are reordered.

You can see how unnest-vars replaces each directory in $(obj-y) by the
value of $(obj-y) set in that directory's Makefile.objs, prefixed with
the directory.

The key insight to address Eric's concern is unnest-vars empties out
$(obj-y) before it includes a Makefile.objs.  Therefore, my "obj-y =" in
qapi/Makefile.objs can't clobber anything.

Having convinced you of this proposition, I'm now going to convince you
of its negation ;-P

A few lines down in Makefile.target, we have

    target-obj-y :=
    block-obj-y :=
    common-obj-y :=
    chardev-obj-y :=
    slirp-obj-y :=
    include $(SRC_PATH)/Makefile.objs
    dummy := $(call unnest-vars,,target-obj-y)
    target-obj-y-save := $(target-obj-y)
    dummy := $(call unnest-vars,.., \
                   block-obj-y \
                   block-obj-m \
                   chardev-obj-y \
                   crypto-obj-y \
                   crypto-aes-obj-y \
                   qom-obj-y \
                   io-obj-y \
                   common-obj-y \
                   common-obj-m \
                   slirp-obj-y)
    target-obj-y := $(target-obj-y-save)
    all-obj-y += $(common-obj-y)
    all-obj-y += $(target-obj-y)
    all-obj-y += $(qom-obj-y)
    all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y) $(chardev-obj-y)
    all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y)
    all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y)
    all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y)
    all-obj-$(CONFIG_SOFTMMU) += $(slirp-obj-y)

$(call unnest-vars,,target-obj-y) is not interesting for us, as
$(target-obj-y) doesn't contain qapi/.  However, $(common-obj-y) does,
and the other $(call unnest-vars, ...) includes Makefile.objs again:

    @qapi/ initial exec.o tcg/tcg.o tcg/tcg-op.o tcg/tcg-op-vec.o tcg/tcg-op-gvec.o tcg/tcg-common.o tcg/optimize.o fpu/softfloat.o disas.o gdbstub-xml.o arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o numa.o qtest.o memory.o memory_mapping.o dump.o win_dump.o migration/ram.o accel/accel.o accel/kvm/kvm-all.o accel/stubs/hax-stub.o accel/stubs/hvf-stub.o accel/stubs/whpx-stub.o accel/tcg/tcg-all.o accel/tcg/cputlb.o accel/tcg/tcg-runtime.o accel/tcg/tcg-runtime-gvec.o accel/tcg/cpu-exec.o accel/tcg/cpu-exec-common.o accel/tcg/translate-all.o accel/tcg/translator.o hw/9pfs/virtio-9p-device.o hw/block/virtio-blk.o hw/block/dataplane/virtio-blk.o hw/block/dataplane/xen-block.o hw/char/virtio-serial-bus.o hw/display/vga.o hw/display/virtio-gpu.o hw/display/virtio-gpu-3d.o hw/display/virtio-gpu-pci.o hw/display/virtio-vga.o hw/hyperv/hyperv.o hw/hyperv/hyperv_testdev.o hw/intc/apic.o hw/intc/apic_common.o hw/intc/ioapic.o hw/isa/lpc_ich9.o hw/misc/ivshmem.o hw/misc/pvpanic.o hw/net/virtio-net.o hw/net/vhost_net.o hw/rdma/rdma_utils.o hw/rdma/rdma_backend.o hw/rdma/rdma_rm.o hw/rdma/vmw/pvrdma_dev_ring.o hw/rdma/vmw/pvrdma_cmd.o hw/rdma/vmw/pvrdma_qp_ops.o hw/rdma/vmw/pvrdma_main.o hw/scsi/virtio-scsi.o hw/scsi/virtio-scsi-dataplane.o hw/scsi/vhost-scsi-common.o hw/scsi/vhost-scsi.o hw/timer/mc146818rtc.o hw/tpm/tpm_ppi.o hw/vfio/common.o hw/vfio/spapr.o hw/vfio/pci.o hw/vfio/pci-quirks.o hw/vfio/display.o hw/virtio/virtio.o hw/virtio/virtio-balloon.o hw/virtio/virtio-crypto.o hw/virtio/virtio-crypto-pci.o hw/virtio/vhost.o hw/virtio/vhost-backend.o hw/virtio/vhost-user.o hw/virtio/vhost-vsock.o hw/virtio/vhost-vsock-pci.o hw/virtio/vhost-scsi-pci.o hw/virtio/virtio-input-host-pci.o hw/virtio/virtio-input-pci.o hw/virtio/virtio-rng-pci.o hw/virtio/virtio-balloon-pci.o hw/virtio/virtio-9p-pci.o hw/virtio/virtio-scsi-pci.o hw/virtio/virtio-blk-pci.o hw/virtio/virtio-net-pci.o hw/virtio/virtio-serial-pci.o hw/xen/xen-host-pci-device.o hw/xen/xen_pt.o hw/xen/xen_pt_config_init.o hw/xen/xen_pt_graphics.o hw/xen/xen_pt_msi.o hw/xen/xen_pt_load_rom.o hw/i386/multiboot.o hw/i386/pc.o hw/i386/pc_piix.o hw/i386/pc_q35.o hw/i386/pc_sysfw.o hw/i386/x86-iommu.o hw/i386/intel_iommu.o hw/i386/x86-iommu.o hw/i386/amd_iommu.o hw/i386/vmport.o hw/i386/vmmouse.o hw/i386/kvmvapic.o hw/i386/acpi-build.o hw/i386/../xenpv/xen_machine_pv.o hw/i386/kvm/clock.o hw/i386/kvm/apic.o hw/i386/kvm/i8259.o hw/i386/kvm/ioapic.o hw/i386/kvm/i8254.o hw/i386/xen/xen_platform.o hw/i386/xen/xen_apic.o hw/i386/xen/xen_pvdevice.o hw/i386/xen/xen-hvm.o hw/i386/xen/xen-mapcache.o qapi/qapi-introspect.o qapi/qapi-types-target.o qapi/qapi-types.o qapi/qapi-visit-target.o qapi/qapi-visit.o qapi/qapi-events-target.o qapi/qapi-events.o qapi/qapi-commands-target.o qapi/qapi-commands.o target/i386/helper.o target/i386/cpu.o target/i386/gdbstub.o target/i386/xsave_helper.o target/i386/translate.o target/i386/bpt_helper.o target/i386/cc_helper.o target/i386/excp_helper.o target/i386/fpu_helper.o target/i386/int_helper.o target/i386/mem_helper.o target/i386/misc_helper.o target/i386/mpx_helper.o target/i386/seg_helper.o target/i386/smm_helper.o target/i386/svm_helper.o target/i386/machine.o target/i386/arch_memory_mapping.o target/i386/arch_dump.o target/i386/monitor.o target/i386/kvm.o target/i386/hyperv.o target/i386/sev.o 9pfs/ acpi/ adc/ audio/ block/ bt/ char/ cpu/ display/ dma/ gpio/ hyperv/ i2c/ ide/ input/ intc/ ipack/ ipmi/ isa/ misc/ net/ rdma/ nvram/ pci/ pci-bridge/ pci-host/ pcmcia/ scsi/ sd/ ssi/ timer/ tpm/ usb/ vfio/ virtio/ watchdog/ xen/ mem/ smbios/ core/ 9pfs/ acpi/ adc/ audio/ block/ bt/ char/ cpu/ display/ dma/ gpio/ hyperv/ i2c/ ide/ input/ intc/ ipack/ ipmi/ isa/ misc/ net/ rdma/ nvram/ pci/ pci-bridge/ pci-host/ pcmcia/ scsi/ sd/ ssi/ timer/ tpm/ usb/ vfio/ virtio/ watchdog/ xen/ mem/ smbios/ core/ virtio-9p-device.o virtio-blk.o dataplane/ virtio-serial-bus.o vga.o virtio-gpu.o virtio-gpu-3d.o virtio-gpu-pci.o virtio-vga.o hyperv.o hyperv_testdev.o apic.o apic_common.o ioapic.o lpc_ich9.o ivshmem.o pvpanic.o virtio-net.o vhost_net.o rdma_utils.o rdma_backend.o rdma_rm.o vmw/pvrdma_dev_ring.o vmw/pvrdma_cmd.o vmw/pvrdma_qp_ops.o vmw/pvrdma_main.o virtio-scsi.o virtio-scsi-dataplane.o vhost-scsi-common.o vhost-scsi.o mc146818rtc.o tpm_ppi.o common.o spapr.o pci.o pci-quirks.o display.o virtio.o virtio-balloon.o virtio-crypto.o virtio-crypto-pci.o vhost.o vhost-backend.o vhost-user.o vhost-vsock.o vhost-vsock-pci.o vhost-scsi-pci.o virtio-input-host-pci.o virtio-input-pci.o virtio-rng-pci.o virtio-balloon-pci.o virtio-9p-pci.o virtio-scsi-pci.o virtio-blk-pci.o virtio-net-pci.o virtio-serial-pci.o xen-host-pci-device.o xen_pt.o xen_pt_config_init.o xen_pt_graphics.o xen_pt_msi.o xen_pt_load_rom.o

This time, $(obj-y) is very much not blank, and...

    @qapi/ final qapi-introspect.o qapi-types-target.o qapi-types.o qapi-visit-target.o qapi-visit.o qapi-events-target.o qapi-events.o qapi-commands-target.o qapi-commands.o
    [Trailing make output elided]

... qapi/Makefile.obj-y *does* clobber it.  Oww.

How come this works anyway?

Consider what would happen if qapi/Makefile.objs used += as suggested by
Eric.  We'd end up with

    obj-y = exec.o [...] xen_pt_load_rom.o qapi-introspect.o [...] qapi-commands.o

which is just as wrong: the file names from qapi/Makefile.objs lack
their qapi/ prefix.

It works because $(obj-y) is not used!

Perhaps unnest-vars could be more hygienic.  But that's not my immediate
concern.  All I want to know right now is whether I should refrain from
= and := in Makefile.objs.  Paolo, Fam?

>> +obj-y = qapi-introspect.o
>
> and again
>
>>  obj-y += $(QAPI_TARGET_MODULES:%=qapi-types-%.o)
>>  obj-y += qapi-types.o
>>  obj-y += $(QAPI_TARGET_MODULES:%=qapi-visit-%.o)
>> @@ -28,4 +29,3 @@ obj-y += $(QAPI_TARGET_MODULES:%=qapi-events-%.o)
>>  obj-y += qapi-events.o
>>  obj-y += $(QAPI_TARGET_MODULES:%=qapi-commands-%.o)
>>  obj-y += qapi-commands.o
>> -obj-y += qapi-introspect.o
>> 

Re: [Qemu-devel] Proper use of unnest-vars (was: [PATCH v5 00/18] qapi: add #if pre-processor conditions to generated code (part 3))
Posted by Paolo Bonzini 5 years, 2 months ago
On 15/02/19 08:53, Markus Armbruster wrote:
> This time, $(obj-y) is very much not blank, and...
> 
>     @qapi/ final qapi-introspect.o qapi-types-target.o qapi-types.o qapi-visit-target.o qapi-visit.o qapi-events-target.o qapi-events.o qapi-commands-target.o qapi-commands.o
>     [Trailing make output elided]
> 
> ... qapi/Makefile.obj-y *does* clobber it.  Oww.
> 
> How come this works anyway?

It works because at this point obj-y is not used anymore, it is assigned
to all-obj-y a couple lines before:

	all-obj-y := $(obj-y)

As an aside, target-obj-y seems unnecessary to me.

> Perhaps unnest-vars could be more hygienic.

Macro hygiene and Make in the same sentence? (well, not sentence but
still...).

> But that's not my immediate
> concern.  All I want to know right now is whether I should refrain from
> = and := in Makefile.objs.  Paolo, Fam?

No, there is no need for that.

Really the answer is that we are kind of pushing Makefiles to the limit
here.  We do get good expressiveness, but at the cost of hiding things
behind black magic.  In the end I think it's a net benefit, but the cost
does exist.

Paolo

Re: [Qemu-devel] Proper use of unnest-vars
Posted by Markus Armbruster 5 years, 2 months ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 15/02/19 08:53, Markus Armbruster wrote:
>> This time, $(obj-y) is very much not blank, and...
>> 
>>     @qapi/ final qapi-introspect.o qapi-types-target.o qapi-types.o qapi-visit-target.o qapi-visit.o qapi-events-target.o qapi-events.o qapi-commands-target.o qapi-commands.o
>>     [Trailing make output elided]
>> 
>> ... qapi/Makefile.obj-y *does* clobber it.  Oww.
>> 
>> How come this works anyway?
>
> It works because at this point obj-y is not used anymore, it is assigned
> to all-obj-y a couple lines before:
>
> 	all-obj-y := $(obj-y)

Confirms my findings.

> As an aside, target-obj-y seems unnecessary to me.

I have no idea :)

$ git-grep target-obj-y
Makefile.objs:target-obj-y += trace/
Makefile.target:target-obj-y :=
Makefile.target:dummy := $(call unnest-vars,,target-obj-y)
Makefile.target:target-obj-y-save := $(target-obj-y)
Makefile.target:target-obj-y := $(target-obj-y-save)
Makefile.target:all-obj-y += $(target-obj-y)
trace/Makefile.objs:target-obj-y += generated-helpers.o
trace/Makefile.objs:target-obj-y += control-target.o

>> Perhaps unnest-vars could be more hygienic.
>
> Macro hygiene and Make in the same sentence? (well, not sentence but
> still...).

You're right.  My "perhaps could" should be read like "Perhaps we could
make pigs fly, if we apply enough force, say with a trebuchet".

>> But that's not my immediate
>> concern.  All I want to know right now is whether I should refrain from
>> = and := in Makefile.objs.  Paolo, Fam?
>
> No, there is no need for that.

Thanks!

> Really the answer is that we are kind of pushing Makefiles to the limit
> here.  We do get good expressiveness, but at the cost of hiding things
> behind black magic.  In the end I think it's a net benefit, but the cost
> does exist.

Concur.

It's actually not bad most of the time.  Once in a great while, I need
to do something more fancy, and then I'm prone to waste a few hours on
(re-)learning how to work with the magic.