[Mesa-dev] [PATCH] r600/egd_tables.py: added support for python 3
Dylan Baker
dylan at pnwbakers.com
Tue Feb 27 17:43:01 UTC 2018
Quoting Emil Velikov (2018-02-27 03:56:30)
> On 23 February 2018 at 15:03, Stefan Dirsch <sndirsch at suse.de> wrote:
> > Patch by "Tomas Chvatal" <tchvatal at suse.com> with modifications
> > by "Michal Srb" <msrb at suse.com> to not break python 2.
> > https://bugzilla.suse.com/show_bug.cgi?id=1082303
> >
> > Signed-off-by: Stefan Dirsch <sndirsch at suse.de>
> > ---
> > src/gallium/drivers/r600/egd_tables.py | 52 +++++++++++++++++-----------------
> > 1 file changed, 26 insertions(+), 26 deletions(-)
> >
> > diff --git a/src/gallium/drivers/r600/egd_tables.py b/src/gallium/drivers/r600/egd_tables.py
> > index d7b78c7fb1..b3b8b50292 100644
> > --- a/src/gallium/drivers/r600/egd_tables.py
> > +++ b/src/gallium/drivers/r600/egd_tables.py
> > @@ -60,7 +60,7 @@ class StringTable:
> > """
> > fragments = [
> > '"%s\\0" /* %s */' % (
> > - te[0].encode('string_escape'),
> > + te[0].encode('unicode_escape'),
> > ', '.join(str(idx) for idx in te[2])
This is relying on bad behavior, in python2 this is going to be bytes, but in
python3 it will be a unicode string, and you can't encode a unicode, only decode
it. There are two choices, either change the parse file to open in bytes mode,
or change it to open in text mode (use io.open for that) and convert the file to
assume unicode strings instead of bytestrings.
> > )
> > for te in self.table
> > @@ -217,10 +217,10 @@ def write_tables(regs, packets):
> > strings = StringTable()
> > strings_offsets = IntTable("int")
> >
> > - print '/* This file is autogenerated by egd_tables.py from evergreend.h. Do not edit directly. */'
> > - print
> > - print CopyRight.strip()
> > - print '''
> > + print('/* This file is autogenerated by egd_tables.py from evergreend.h. Do not edit directly. */')
This is not sufficient, in python 2.x this is now a print statement with
grouping parens, but in python3 this is a print function. They have different
behavior. To address this we should add `from __future__ import print_function`
as the first import of the file, so that the semantics are closer (more recent
versions of python 3 have additional optional parameters for print(), but that's
a different problem.)
> > + print('')
> > + print(CopyRight.strip())
> > + print('''
> > #ifndef EG_TABLES_H
> > #define EG_TABLES_H
> >
> > @@ -242,20 +242,20 @@ struct eg_packet3 {
> > unsigned name_offset;
> > unsigned op;
> > };
> > -'''
> > +''')
> >
> > - print 'static const struct eg_packet3 packet3_table[] = {'
> > + print('static const struct eg_packet3 packet3_table[] = {')
> > for pkt in packets:
> > - print '\t{%s, %s},' % (strings.add(pkt[5:]), pkt)
> > - print '};'
> > - print
> > + print('\t{%s, %s},' % (strings.add(pkt[5:]), pkt))
> > + print('};')
> > + print('')
> >
> > - print 'static const struct eg_field egd_fields_table[] = {'
> > + print('static const struct eg_field egd_fields_table[] = {')
> >
> > fields_idx = 0
> > for reg in regs:
> > if len(reg.fields) and reg.own_fields:
> > - print '\t/* %s */' % (fields_idx)
> > + print('\t/* %s */' % (fields_idx))
> >
> > reg.fields_idx = fields_idx
> >
> > @@ -266,34 +266,34 @@ struct eg_packet3 {
> > while value[1] >= len(values_offsets):
> > values_offsets.append(-1)
> > values_offsets[value[1]] = strings.add(strip_prefix(value[0]))
> > - print '\t{%s, %s(~0u), %s, %s},' % (
> > + print('\t{%s, %s(~0u), %s, %s},' % (
> > strings.add(field.name), field.s_name,
> > - len(values_offsets), strings_offsets.add(values_offsets))
> > + len(values_offsets), strings_offsets.add(values_offsets)))
> > else:
> > - print '\t{%s, %s(~0u)},' % (strings.add(field.name), field.s_name)
> > + print('\t{%s, %s(~0u)},' % (strings.add(field.name), field.s_name))
> > fields_idx += 1
> >
> > - print '};'
> > - print
> > + print('};')
> > + print('')
> >
> > - print 'static const struct eg_reg egd_reg_table[] = {'
> > + print('static const struct eg_reg egd_reg_table[] = {')
> > for reg in regs:
> > if len(reg.fields):
> > - print '\t{%s, %s, %s, %s},' % (strings.add(reg.name), reg.r_name,
> > - len(reg.fields), reg.fields_idx if reg.own_fields else reg.fields_owner.fields_idx)
> > + print('\t{%s, %s, %s, %s},' % (strings.add(reg.name), reg.r_name,
> > + len(reg.fields), reg.fields_idx if reg.own_fields else reg.fields_owner.fields_idx))
> > else:
> > - print '\t{%s, %s},' % (strings.add(reg.name), reg.r_name)
> > - print '};'
> > - print
> > + print('\t{%s, %s},' % (strings.add(reg.name), reg.r_name))
> > + print('};')
> > + print('')
> >
> > strings.emit(sys.stdout, "egd_strings")
> >
> > - print
> > + print('')
> >
> > strings_offsets.emit(sys.stdout, "egd_strings_offsets")
> >
> > - print
> > - print '#endif'
> > + print('')
> > + print('#endif')
> >
> >
> > def main():
> > --
> Dylan, being our python expert can you please take a look at this patch?
> It looks great on my end, modulo some minor tweaks in the commit message. Say:
>
> "make the script python 2+3 compatible"
>
> Thanks
> Emil
I reviewed the last patch, and I'll give the same review here. Writing code that
is correct in both python 2 and python 3 is *really* hard without a helper
library like six. I'd really prefer to either add six as a dependency, or start
dropping support for python 2 and moving purely to python 3 anywhere possible (I
know that scons for example requires python 2.x, so we can't completely drop
autotools while we have scons). If we're going to go to a hybrid model then we
also need to drop support for python 2.6, there was a lot of python 3.x
compatibility code added in 2.7 that makes this much more possible. That said,
I've left some inline review comments and would be okay with landing this if
those are addressed, and the following comment.
There's also the problem that none of the classes descend from object, which
means their behavior will be different in python 2 and python 3. In python 2
they'll be old style classes, but in python 3 they'll be new style. Making the
inherit from object will make the new style in both (there is no old style in
python 3).
Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180227/89779f5b/attachment-0001.sig>
More information about the mesa-dev
mailing list