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 :)
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/,'
