Re: [Qemu-devel] [PATCH RFC 09/21] qapi: Don't absolutize include file name in error messages

[Marc-Andre Lureau]

On Fri, Feb 2, 2018 at 2:03 PM, Markus Armbruster <address@hidden> wrote:
> Error messages print absolute filenames of included files even gave a
> relative one on the command line:
>
>      PYTHONPATH=scripts python -B tests/qapi-schema/test-qapi.py 
> tests/qapi-schema/include-cycle.json
>     In file included from tests/qapi-schema/include-cycle.json:1:
>     In file included from 
> /work/armbru/qemu/tests/qapi-schema/include-cycle-b.json:1:
>     /work/armbru/qemu/tests/qapi-schema/include-cycle-c.json:1: Inclusion 
> loop for include-cycle.json
>
> Improve this to
>
>     In file included from tests/qapi-schema/include-cycle.json:1:
>     In file included from tests/qapi-schema/include-cycle-b.json:1:
>     tests/qapi-schema/include-cycle-c.json:1: Inclusion loop for 
> include-cycle.json
>
> Signed-off-by: Markus Armbruster <address@hidden>

most of the necessary refactoring/split is done in previous patch,

Reviewed-by: Marc-André Lureau <address@hidden>


> ---
>  scripts/qapi/common.py                | 12 ++++++------
>  tests/qapi-schema/include-no-file.err |  2 +-
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index be0fcc548a..6c6962a364 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -255,9 +255,8 @@ class QAPIDoc(object):
>  class QAPISchemaParser(object):
>
>      def __init__(self, fp, previously_included=[], incl_info=None):
> -        abs_fname = os.path.abspath(fp.name)
>          self.fname = fp.name
> -        previously_included.append(abs_fname)
> +        previously_included.append(os.path.abspath(fp.name))
>          self.incl_info = incl_info
>          self.src = fp.read()
>          if self.src == '' or self.src[-1] != '\n':
> @@ -288,7 +287,7 @@ class QAPISchemaParser(object):
>                  if not isinstance(include, str):
>                      raise QAPISemError(info,
>                                         "Value of 'include' must be a string")
> -                self._include(include, info, os.path.dirname(abs_fname),
> +                self._include(include, info, os.path.dirname(self.fname),
>                                previously_included)
>              elif "pragma" in expr:
>                  self.reject_expr_doc(cur_doc)
> @@ -321,7 +320,8 @@ class QAPISchemaParser(object):
>                  % doc.symbol)
>
>      def _include(self, include, info, base_dir, previously_included):
> -        incl_abs_fname = os.path.join(base_dir, include)
> +        incl_fname = os.path.join(base_dir, include)
> +        incl_abs_fname = os.path.abspath(incl_fname)
>          # catch inclusion cycle
>          inf = info
>          while inf:
> @@ -333,9 +333,9 @@ class QAPISchemaParser(object):
>          if incl_abs_fname in previously_included:
>              return
>          try:
> -            fobj = open(incl_abs_fname, 'r')
> +            fobj = open(incl_fname, 'r')
>          except IOError as e:
> -            raise QAPISemError(info, '%s: %s' % (e.strerror, include))
> +            raise QAPISemError(info, '%s: %s' % (e.strerror, incl_fname))
>          exprs_include = QAPISchemaParser(fobj, previously_included, info)
>          self.exprs.extend(exprs_include.exprs)
>          self.docs.extend(exprs_include.docs)
> diff --git a/tests/qapi-schema/include-no-file.err 
> b/tests/qapi-schema/include-no-file.err
> index d5b9b22d85..e42bcf4bc1 100644
> --- a/tests/qapi-schema/include-no-file.err
> +++ b/tests/qapi-schema/include-no-file.err
> @@ -1 +1 @@
> -tests/qapi-schema/include-no-file.json:1: No such file or directory: 
> include-no-file-sub.json
> +tests/qapi-schema/include-no-file.json:1: No such file or directory: 
> tests/qapi-schema/include-no-file-sub.json
> --
> 2.13.6
>