From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60911) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WUi6l-0000HB-7I for qemu-devel@nongnu.org; Mon, 31 Mar 2014 15:43:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WUi6d-0003o0-9t for qemu-devel@nongnu.org; Mon, 31 Mar 2014 15:42:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11093) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WUi6d-0003np-0h for qemu-devel@nongnu.org; Mon, 31 Mar 2014 15:42:51 -0400 Message-ID: <5339C534.4000806@redhat.com> Date: Mon, 31 Mar 2014 13:42:44 -0600 From: Eric Blake MIME-Version: 1.0 References: <20140331191631.21034.25433.stgit@fimbulvetr.bsc.es> <20140331191642.21034.11615.stgit@fimbulvetr.bsc.es> In-Reply-To: <20140331191642.21034.11615.stgit@fimbulvetr.bsc.es> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="fDOTXlu48ruwkSOVLA60vHXdWU68Mce6N" Subject: Re: [Qemu-devel] [PATCH v6 2/4] qapi: Use an explicit input file List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?TGx1w61zIFZpbGFub3Zh?= , qemu-devel@nongnu.org Cc: =?UTF-8?B?QmVub8OudCBDYW5ldA==?= , Markus Armbruster , Luiz Capitulino This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --fDOTXlu48ruwkSOVLA60vHXdWU68Mce6N Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 03/31/2014 01:16 PM, Llu=C3=ADs Vilanova wrote: > Use an explicit input file on the command-line instead of reading from = standard input >=20 > Signed-off-by: Llu=C3=ADs Vilanova > --- > +++ b/Makefile > @@ -238,33 +238,33 @@ qapi-py =3D $(SRC_PATH)/scripts/qapi.py $(SRC_PAT= H)/scripts/ordereddict.py > qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.= h :\ > $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(q= api-py) > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ > - $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \ > + $(gen-out-type) -i "$<" -o qga/qapi-generated -p "qga-", \ I still wonder if: -o qga/qapi-generated -p "qga-" -i "$<" is easier to read than injecting -i up front. But it's not something that I will block the patch for. > @@ -368,7 +368,8 @@ check-tests/test-qapi.py: tests/test-qapi.py > =20 > .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y)) > $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PA= TH)/%.json > - $(call quiet-command, PYTHONPATH=3D$(SRC_PATH)/scripts $(PYTHON) $(SR= C_PATH)/tests/qapi-schema/test-qapi.py <$^ >$*.test.out 2>$*.test.err; ec= ho $$? >$*.test.exit, " TEST $*.out") > + $(call quiet-command, PYTHONPATH=3D$(SRC_PATH)/scripts $(PYTHON) $(SR= C_PATH)/tests/qapi-schema/test-qapi.py "$^" >$*.test.out 2>$*.test.err; e= cho $$? >$*.test.exit, " TEST $*.out") Why is this line still long? Shouldn't it have been split in 1/4? And why is this form using an argument of the input file, instead of adding a -i option like all the others? > + @sed -i -e "s|$(SRC_PATH)/||g" $*.test.err 'sed -i' is a GNU-ism, and not portable to all sed. It's not the first use of 'sed -i' in the code base, but most of those uses so far are for situations that are Linux-specific where GNU sed can be assumed. > @diff -q $(SRC_PATH)/$*.out $*.test.out > @diff -q $(SRC_PATH)/$*.err $*.test.err You could instead have done: @sed "s|$(SRC_PATH)/||g" $*.test.err \ | diff -q $(SRC_PATH_/$*.err - to avoid the portability question. > +++ b/tests/qapi-schema/test-qapi.py > @@ -12,10 +12,11 @@ > =20 > from qapi import * > from pprint import pprint > +import os > import sys > =20 > try: > - exprs =3D parse_schema(sys.stdin) > + exprs =3D parse_schema(sys.argv[1]) Here's where I found it inconsistent with the rest of the patch. It seems it would be better to either use -i everywhere, or use sys.argv[1] everywhere. > +++ b/tests/qapi-schema/trailing-comma-list.err > @@ -1 +1 @@ > -:2:36: Expected "{", "[" or string > +tests/qapi-schema/trailing-comma-list.json:2:36: Expected "{", "[" or = string These are some long error messages; is it also worth trimming the leading "tests/qapi-schema/" in the sed script where you massage the data before doing the diff on expected errors? It's too late for this to go into 2.0, but I think we're getting close to having a solution worth using at the start of the 2.1 devel cycle. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --fDOTXlu48ruwkSOVLA60vHXdWU68Mce6N Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTOcU0AAoJEKeha0olJ0NqAJYIAI68EZsGEwjWT5+VhkmFicXu hzxDpxWVUa35gFlniqsAmFJWrlsNEKnTasEbYL2e46o+yNrWUvbzFflUVJl3r/W3 sJXTczv/YAHKacsE26TDe0cg7+0l2YhLIgHJStCsOBPB+bpkcaSwIVgmbJ/fxFaJ vHyZrPohjOK7Pvod6vnUTO6kTBziJPOKlaezkhbhQ7Ec+hhQFFpcNFUREbiuNGXw h8fjmjRNTVfJ+tY8yVXeTEtuTCTgZbkX7cgUADrByW5XE/42hicrol76GTYNOc5j xId45SCM1YNc+f9b5H6qWgOJBM3wN83CybMMUnoBFni7sIuCTpNLDMdFY0L/vD4= =Rd/V -----END PGP SIGNATURE----- --fDOTXlu48ruwkSOVLA60vHXdWU68Mce6N--