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

Brian Paul brianp at vmware.com
Fri Aug 17 15:37:53 UTC 2018


On 08/17/2018 09:32 AM, Jose Fonseca wrote:
> 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.

Done.

-Bria

> 
> 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