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

[Jose Fonseca]

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.


Jose