Markus Armbruster writes: > Lluís Vilanova <vilan...@ac.upc.edu> writes: >> Markus Armbruster writes: >> [...] >>>>>> self.fp = schema.fp >>>>>> self.msg = msg >>>>>> self.line = self.col = 1 >>>>>> @@ -50,12 +52,17 @@ class QAPISchemaError(Exception): >>>>>> self.col += 1 >>>>>> >>>>>> def __str__(self): >>>>>> - return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col, >>>>>> self.msg) >>>>>> + name = os.path.relpath(self.fp.name, self.base) >>>>>> + return "%s:%s:%s: %s" % (name, self.line, self.col, self.msg) >>>> >>>>> Can you explain what this change does, and why it's wanted? >>>> >>>> Paths are shown as relative so that the test outputs (stderr) can be >>>> verified >>>> 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=$(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 >> without >> 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 :) Yes, I just wanted to error-out in a more helpful way. I really hate to have some arbitrary limit and an unhelpful (or unnecessarily long) message. > 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/,' Right, post-processing is the other option, silly me. Lluis -- "And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer." -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
