All the python script in scripts/qapi is used to generate qapi code. Use
wildcard to simplify it.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
Makefile | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/Makefile b/Makefile
index 155f066a20..38b74782d9 100644
--- a/Makefile
+++ b/Makefile
@@ -541,13 +541,7 @@ qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated
qemu-keymap$(EXESUF): LIBS += $(XKBCOMMON_LIBS)
qemu-keymap$(EXESUF): QEMU_CFLAGS += $(XKBCOMMON_CFLAGS)
-qapi-py = $(SRC_PATH)/scripts/qapi/commands.py \
-$(SRC_PATH)/scripts/qapi/events.py \
-$(SRC_PATH)/scripts/qapi/introspect.py \
-$(SRC_PATH)/scripts/qapi/types.py \
-$(SRC_PATH)/scripts/qapi/visit.py \
-$(SRC_PATH)/scripts/qapi/common.py \
-$(SRC_PATH)/scripts/qapi/doc.py \
+qapi-py = $(wildcard $(SRC_PATH)/scripts/qapi/*.py) \
$(SRC_PATH)/scripts/qapi-gen.py
qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h \
--
2.19.1
On 5/21/19 3:12 AM, Wei Yang wrote: > All the python script in scripts/qapi is used to generate qapi code. Use > wildcard to simplify it. > > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > --- > Makefile | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) I'm not a fan of $(wildcard). It makes your tarball creation non-deterministic - if there is a leftover file from development that is no longer part of the build, wildcard will still pick it up. Explicit lists are better. I'm inclined to NACK this, but Markus has final say since he maintains the qapi generator. > > diff --git a/Makefile b/Makefile > index 155f066a20..38b74782d9 100644 > --- a/Makefile > +++ b/Makefile > @@ -541,13 +541,7 @@ qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated > qemu-keymap$(EXESUF): LIBS += $(XKBCOMMON_LIBS) > qemu-keymap$(EXESUF): QEMU_CFLAGS += $(XKBCOMMON_CFLAGS) > > -qapi-py = $(SRC_PATH)/scripts/qapi/commands.py \ > -$(SRC_PATH)/scripts/qapi/events.py \ > -$(SRC_PATH)/scripts/qapi/introspect.py \ > -$(SRC_PATH)/scripts/qapi/types.py \ > -$(SRC_PATH)/scripts/qapi/visit.py \ > -$(SRC_PATH)/scripts/qapi/common.py \ > -$(SRC_PATH)/scripts/qapi/doc.py \ > +qapi-py = $(wildcard $(SRC_PATH)/scripts/qapi/*.py) \ > $(SRC_PATH)/scripts/qapi-gen.py > > qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h \ > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Eric Blake <eblake@redhat.com> writes: > On 5/21/19 3:12 AM, Wei Yang wrote: >> All the python script in scripts/qapi is used to generate qapi code. Use >> wildcard to simplify it. >> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >> --- >> Makefile | 8 +------- >> 1 file changed, 1 insertion(+), 7 deletions(-) > > I'm not a fan of $(wildcard). It makes your tarball creation > non-deterministic - if there is a leftover file from development that is > no longer part of the build, wildcard will still pick it up. Explicit Actually, in this case can "only" adds spurious prerequisites. > lists are better. I'm inclined to NACK this, but Markus has final say > since he maintains the qapi generator. I consider use of $(wildcard) for the purpose of collecting sources a lazy mistake.
On Tue, May 21, 2019 at 05:28:27PM +0200, Markus Armbruster wrote: >Eric Blake <eblake@redhat.com> writes: > >> On 5/21/19 3:12 AM, Wei Yang wrote: >>> All the python script in scripts/qapi is used to generate qapi code. Use >>> wildcard to simplify it. >>> >>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >>> --- >>> Makefile | 8 +------- >>> 1 file changed, 1 insertion(+), 7 deletions(-) >> >> I'm not a fan of $(wildcard). It makes your tarball creation >> non-deterministic - if there is a leftover file from development that is >> no longer part of the build, wildcard will still pick it up. Explicit > >Actually, in this case can "only" adds spurious prerequisites. Hmm... which spurious prerequisites will be added? I may miss something here. > >> lists are better. I'm inclined to NACK this, but Markus has final say >> since he maintains the qapi generator. > >I consider use of $(wildcard) for the purpose of collecting sources a >lazy mistake. -- Wei Yang Help you, Help me
On 5/21/19 7:53 PM, Wei Yang wrote: > On Tue, May 21, 2019 at 05:28:27PM +0200, Markus Armbruster wrote: >> Eric Blake <eblake@redhat.com> writes: >> >>> On 5/21/19 3:12 AM, Wei Yang wrote: >>>> All the python script in scripts/qapi is used to generate qapi code. Use >>>> wildcard to simplify it. >>>> >>>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >>>> --- >>>> Makefile | 8 +------- >>>> 1 file changed, 1 insertion(+), 7 deletions(-) >>> >>> I'm not a fan of $(wildcard). It makes your tarball creation >>> non-deterministic - if there is a leftover file from development that is >>> no longer part of the build, wildcard will still pick it up. Explicit >> >> Actually, in this case can "only" adds spurious prerequisites. > > Hmm... which spurious prerequisites will be added? I may miss something here. If I touch the file scripts/qapi/foo.py, then that becomes a prerequisite, and will rebuild my files even though it doesn't need to. And if the definition of $(qapi-py) is ever applied to determining which files to include in a tarball, then my tarball will include foo.py, even though it is not essential to the tarball, while the next person, who does not have foo.py lying around in their directory, produces a different tarball. Non-deterministic results are bad, hence our rule of thumb to avoid $(wildcard) when listing files in a Makefile. > >> >>> lists are better. I'm inclined to NACK this, but Markus has final say >>> since he maintains the qapi generator. >> >> I consider use of $(wildcard) for the purpose of collecting sources a >> lazy mistake. Which is another polite way of saying that this patch should not be applied. If you WANT to factor things for less typing, you could use something like: var=commands.py events.py introspect.py types.py visit.py common.py doc.py $(addprefix $(SRC_PATH)/scripts/qapi/,$(var)) for some suitably-named var. Or even factor out the .py suffix as well. That doesn't use the problematic $(wildcard), but still lets you get away with easier maintenance of adding a new file to the explicit list should we add a file in the future. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Tue, May 21, 2019 at 08:36:42PM -0500, Eric Blake wrote: >On 5/21/19 7:53 PM, Wei Yang wrote: >> On Tue, May 21, 2019 at 05:28:27PM +0200, Markus Armbruster wrote: >>> Eric Blake <eblake@redhat.com> writes: >>> >>>> On 5/21/19 3:12 AM, Wei Yang wrote: >>>>> All the python script in scripts/qapi is used to generate qapi code. Use >>>>> wildcard to simplify it. >>>>> >>>>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >>>>> --- >>>>> Makefile | 8 +------- >>>>> 1 file changed, 1 insertion(+), 7 deletions(-) >>>> >>>> I'm not a fan of $(wildcard). It makes your tarball creation >>>> non-deterministic - if there is a leftover file from development that is >>>> no longer part of the build, wildcard will still pick it up. Explicit >>> >>> Actually, in this case can "only" adds spurious prerequisites. >> >> Hmm... which spurious prerequisites will be added? I may miss something here. > >If I touch the file scripts/qapi/foo.py, then that becomes a >prerequisite, and will rebuild my files even though it doesn't need to. >And if the definition of $(qapi-py) is ever applied to determining which >files to include in a tarball, then my tarball will include foo.py, even >though it is not essential to the tarball, while the next person, who >does not have foo.py lying around in their directory, produces a >different tarball. Non-deterministic results are bad, hence our rule of >thumb to avoid $(wildcard) when listing files in a Makefile. > Ok, so $(wildcard) should be avoided in Makefile. Will not try to do this next time. >> >>> >>>> lists are better. I'm inclined to NACK this, but Markus has final say >>>> since he maintains the qapi generator. >>> >>> I consider use of $(wildcard) for the purpose of collecting sources a >>> lazy mistake. > >Which is another polite way of saying that this patch should not be applied. > >If you WANT to factor things for less typing, you could use something like: > >var=commands.py events.py introspect.py types.py visit.py common.py doc.py >$(addprefix $(SRC_PATH)/scripts/qapi/,$(var)) > >for some suitably-named var. Or even factor out the .py suffix as well. >That doesn't use the problematic $(wildcard), but still lets you get >away with easier maintenance of adding a new file to the explicit list >should we add a file in the future. > >-- >Eric Blake, Principal Software Engineer >Red Hat, Inc. +1-919-301-3226 >Virtualization: qemu.org | libvirt.org > -- Wei Yang Help you, Help me
On Tue, May 21, 2019 at 07:00:12AM -0500, Eric Blake wrote: >On 5/21/19 3:12 AM, Wei Yang wrote: >> All the python script in scripts/qapi is used to generate qapi code. Use >> wildcard to simplify it. >> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >> --- >> Makefile | 8 +------- >> 1 file changed, 1 insertion(+), 7 deletions(-) > >I'm not a fan of $(wildcard). It makes your tarball creation >non-deterministic - if there is a leftover file from development that is >no longer part of the build, wildcard will still pick it up. Explicit >lists are better. I'm inclined to NACK this, but Markus has final say >since he maintains the qapi generator. > Thanks for comment. Yes, for tarball, the order is a problem. But those python scripts are not used in tarballs. For leftover files, you mean those files not used and should be deleted? -- Wei Yang Help you, Help me
© 2016 - 2026 Red Hat, Inc.