From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60307) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WKpDj-0007HQ-4w for qemu-devel@nongnu.org; Tue, 04 Mar 2014 08:17:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WKpDe-0008QG-I8 for qemu-devel@nongnu.org; Tue, 04 Mar 2014 08:17:18 -0500 Received: from mx1.redhat.com ([209.132.183.28]:14064) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WKpDe-0008Q7-9w for qemu-devel@nongnu.org; Tue, 04 Mar 2014 08:17:14 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s24DHDZF019814 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 4 Mar 2014 08:17:13 -0500 Received: from blackfin.pond.sub.org (ovpn-116-60.ams2.redhat.com [10.36.116.60]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id s24DHBAi018531 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Tue, 4 Mar 2014 08:17:13 -0500 From: Markus Armbruster References: <20140228155309.28087.96600.stgit@fimbulvetr.bsc.es> <20140228155316.28087.30919.stgit@fimbulvetr.bsc.es> <87ob1qgvtj.fsf@blackfin.pond.sub.org> <87y50rxsss.fsf@fimbulvetr.bsc.es> <87vbvvwaog.fsf@blackfin.pond.sub.org> <87zjl7w74z.fsf@fimbulvetr.bsc.es> Date: Tue, 04 Mar 2014 14:17:11 +0100 In-Reply-To: <87zjl7w74z.fsf@fimbulvetr.bsc.es> (=?utf-8?Q?=22Llu=C3=ADs?= Vilanova"'s message of "Mon, 03 Mar 2014 17:59:08 +0100") Message-ID: <87txbeksrs.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 1/3] qapi: Use an explicit input file List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Llu=C3=ADs Vilanova writes: > Markus Armbruster writes: > [...] >>>>> self.fp =3D schema.fp >>>>> self.msg =3D msg >>>>> self.line =3D self.col =3D 1 >>>>> @@ -50,12 +52,17 @@ class QAPISchemaError(Exception): >>>>> self.col +=3D 1 >>>>>=20 >>>>> def __str__(self): >>>>> - return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col, >>>>> self.msg) >>>>> + name =3D os.path.relpath(self.fp.name, self.base) >>>>> + return "%s:%s:%s: %s" % (name, self.line, self.col, self.msg) >>>=20 >>>> Can you explain what this change does, and why it's wanted? >>>=20 >>> Paths are shown as relative so that the test outputs (stderr) can be ve= rified >>> with diff. Otherwise the actual message depends on the path from which = you're >>> running the tests. > >> Hmm. This is the applicable make rule: > >> $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH= )/%.json >> $(call quiet-command, PYTHONPATH=3D$(SRC_PATH)/scripts $(PYTHON) $(SRC_= PATH)/tests/qapi-schema/test-qapi.py "$^" >$*.test.out 2>$*.test.err; echo = $$? >$*.test.exit, " TEST $*.out") >> @diff -q $(SRC_PATH)/$*.out $*.test.out >> @diff -q $(SRC_PATH)/$*.err $*.test.err >> @diff -q $(SRC_PATH)/$*.exit $*.test.exit > >> Since $^ is in $(SRC_PATH), it's like $(SRC_PATH)/foo.json. If >> $(SRC_PATH)/foo.json has an error, the error messages duly points to >> $(SRC_PATH)/foo.json. > >> The "diff -q $(SRC_PATH)/$*.err $*.test.err" fails unless your SRC_PATH >> matches the one that's encoded in tests/qapi-schema/foo.err. Is that >> the problem you're trying to solve? > > Right. Paths are internally stored as absolute in "qapi.py" (to check for > include cycles), and the "base directory" is only used to show them as > relative. I can try to change the code to retain this functionality but w= ithout > special-casing the tests. Well, my fav method to check for include cycles is really simple: 1. Estimate how many levels of inclusion you're going to need. 2. Shift left a couple of times. 3. Error out when inclusion depth hits that limit. Obviously 100% reliable. File name comparisons tend to be unreliable or unobvious :) With this realpath business out of the way, file names in tests should all be of the form $(SRC_PATH)/tests/qapi-schema/*.json, shouldn't they? The $(SRC_PATH) prefix depends on where the user's build tree is, the rest is fixed. We could strip the prefix from error messages with a simple filter: perl -pe 's,\Q$(SRC_PATH)/tests/qapi-schema/\E,tests/qapi-schema/,'