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

Jose Fonseca jfonseca at vmware.com
Fri Aug 17 15:32:30 UTC 2018


On 17/08/18 16:08, Jose Fonseca wrote:
> On 17/08/18 15:49, Mathieu Bridon wrote:
>> 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://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.python.org%2F2%2Flibrary%2Ffunctions.html%23open&data=02%7C01%7Cjfonseca%40vmware.com%7Ca7bc6963b38b491115e908d60450ac4b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636701141882257685&sdata=JDdi%2FrNCkD0UyBMKsNQwVF42a0eNp3r6137u%2FJ99YmA%3D&reserved=0 
>>> 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://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fbochecha%2Fmesa%2Ftree%2Fpython2-windows-scons&data=02%7C01%7Cjfonseca%40vmware.com%7Ca7bc6963b38b491115e908d60450ac4b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636701141882257685&sdata=wkWAs3fUgfbleKPvygfJdC4kroaaqxbFUZrjkb9G8R0%3D&reserved=0 
>>
>>
>> It should fix the problem.
>>
>>
> 
> Yes, it doesn indeed.  And in a much more elegant way.  Change is
> 
>    Reviewed-by: Jose Fonseca <jfonseca at vmware>
> 
> Thanks!
> 
> I didn't know about io module -- it seems quite handy for Python 2/3 
> migration.
> 
> Jose

Mathieu, unless there's any strong objection, it would be great to get 
this committed to unblock us.

Going forward I'll also try to fiddle Appveyor.yml so it checkouts stuff 
with CRLF line endings, and we can catch this sort of issue there too.

Jose


More information about the mesa-dev mailing list