[Mesa-dev] [PATCH] python: Help Python 2 print the line

[Mathieu Bridon]

On Fri, 2018-08-17 at 15:30 +0100, Jose Fonseca wrote:
> On 17/08/18 15:26, Mathieu Bridon wrote:
> > On Fri, 2018-08-17 at 15:08 +0100, Jose Fonseca wrote:
> > > On 17/08/18 15:06, Jose Fonseca wrote:
> > > > On 17/08/18 14:52, Jose Fonseca wrote:
> > > > > On 17/08/18 14:30, Jose Fonseca wrote:
> > > > > > On 17/08/18 14:22, Mathieu Bridon wrote:
> > > > > > > ---
> > > > > > > Jose, can you test whether this patch fixes your build
> > > > > > > issues?
> > > > > > > 
> > > > > > > I don't have access to a Windows machine, and I can't
> > > > > > > reproduce your
> > > > > > > problem on Linux.
> > > > > > > 
> > > > > > >    src/util/xmlpool/gen_xmlpool.py | 5 +++++
> > > > > > >    1 file changed, 5 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/src/util/xmlpool/gen_xmlpool.py
> > > > > > > b/src/util/xmlpool/gen_xmlpool.py
> > > > > > > index 327709c7f8d..12177dc50f5 100644
> > > > > > > --- a/src/util/xmlpool/gen_xmlpool.py
> > > > > > > +++ b/src/util/xmlpool/gen_xmlpool.py
> > > > > > > @@ -218,6 +218,11 @@ for line in template:
> > > > > > >            assert len(descMatches) == 0
> > > > > > >            descMatches = [matchDESC_BEGIN]
> > > > > > >        else:
> > > > > > > +        # In Python 2, stdout expects encoded byte
> > > > > > > strings,
> > > > > > > or else
> > > > > > > it will
> > > > > > > +        # encode them with the ascii 'codec'
> > > > > > > +        if sys.version_info.major == 2:
> > > > > > > +            line = line.encode('utf-8')
> > > > > > > +
> > > > > > >            print(line, end='')
> > > > > > >    template.close()
> > > > > > > 
> > > > > > 
> > > > > > It fixes the UnicodeEncodeError.  I need to do more testing
> > > > > > to
> > > > > > see if
> > > > > > it fixes all errors.
> > > > > > 
> > > > > > 
> > > > > > I think we should fix the print(end ..) statemet.   In
> > > > > > fact, it
> > > > > > might
> > > > > > be easier to have an helper function (e.g., write()
> > > > > > )  which
> > > > > > does the
> > > > > > sys.version check , utf8 encoding, and print, and use it
> > > > > > everywhere
> > > > > > print is used now.
> > > > > > 
> > > > > > 
> > > > > > Jose
> > > > > 
> > > > > 
> > > > > Unfortunately I still get build failures with your change:
> > > > > 
> > > > >     Compiling src\gallium\auxiliary\pipe-loader\pipe_loader.c 
> > > > > ...
> > > > > pipe_loader.c
> > > > > c:\hudson\workspace\mesa-msvc\src\gallium\auxiliary\pipe-
> > > > > loader\driinfo_gallium.h(2):
> > > > > error C2146: syntax error: missing ';' before identifier
> > > > > 'gettext'
> > > > > c:\hudson\workspace\mesa-msvc\src\gallium\auxiliary\pipe-
> > > > > loader\driinfo_gallium.h(2):
> > > > > error C2143: syntax error: missing ')' before 'string'
> > > > > c:\hudson\workspace\mesa-msvc\src\gallium\auxiliary\pipe-
> > > > > loader\driinfo_gallium.h(2):
> > > > > error C2143: syntax error: missing '{' before 'string'
> > > > > c:\hudson\workspace\mesa-msvc\src\gallium\auxiliary\pipe-
> > > > > loader\driinfo_gallium.h(2):
> > > > > error C2059: syntax error: 'string'
> > > > > c:\hudson\workspace\mesa-msvc\src\gallium\auxiliary\pipe-
> > > > > loader\driinfo_gallium.h(2):
> > > > > error C2059: syntax error: ')'
> > > > > 
> > > > > whereas if I just revert your
> > > > > bd27203f4d808763ac24ac94eb677cacf3e7cb99
> > > > > change these errors go away.
> > > > > 
> > > > > I compared the generated options.h, before/after your change,
> > > > > and
> > > > > it
> > > > > seems the problem is that we're now emitting `gettext(...)`
> > > > > function
> > > > > calls which don't exist on Windows:
> > > > > 
> > > > > --- options.h.old       2018-08-17 14:48:09.000000000 +0100
> > > > > +++ options.h.new       2018-08-17 14:41:36.000000000 +0100
> > > > > @@ -57,101 +57,101 @@
> > > > >     */
> > > > >    #define DRI_CONF_SECTION_DEBUG \
> > > > >    DRI_CONF_SECTION_BEGIN \
> > > > > -       DRI_CONF_DESC(en,"Debugging")
> > > > > +       DRI_CONF_DESC(en,gettext("Debugging"))
> > > > > 
> > > > >    #define DRI_CONF_NO_RAST(def) \
> > > > >    DRI_CONF_OPT_BEGIN_B(no_rast, def) \
> > > > > 
> > > > > [...]
> > > > > 
> > > > > 
> > > > > Jose
> > > > 
> > > > 
> > > > Ok, I have a strong hunch to what's the problem, with your
> > > > change
> > > > you're
> > > > reading the input as bytes
> > > > 
> > > >      template = open (template_header_path, "rb")  <=========
> > > > 
> > > > which means that on Windows the lines will have "\r\n" which
> > > > then
> > > > causes
> > > > all regular expressions to fail (as $ won't match)
> > > > 
> > > > And this finally explains why this afeccts some systems but not
> > > > others:
> > > > it depends on whether git clones t_options.h with CRLF or
> > > > LF!!!!
> > 
> > The thing is, the old code would do:
> > 
> >    template = open (template_header_path, "r")
> >    for line in template:
> >        …
> > 
> > In my testing, with Python 2 the lines **already** end with "\r\n",
> > so
> > I don't see how this could have worked before. :-/
> 
> That's because you're testing on Linux.  On Linux "b" has no effect. 
> But on Windows the default is "t" and "b" makes it binary.  That's
> why 
> this worked before.
> 
> 
> See https://docs.python.org/2/library/functions.html#open which says:
> 
>       Append 'b' to the mode to open the file in binary mode, on
> systems 
> that differentiate between binary and text files; on systems that
> don’t 
> have this distinction, adding the 'b' has no effect.

Can you try this branch?

https://gitlab.freedesktop.org/bochecha/mesa/tree/python2-windows-scons

It should fix the problem.


-- 
Mathieu