* [Qemu-devel] [PATCH v4 0/2] Fix compilation with python-3 if en_US.UTF-8 is unavailable @ 2018-06-18 17:59 Markus Armbruster 2018-06-18 17:59 ` [Qemu-devel] [PATCH v4 1/2] qapi: Open files with encoding='utf-8' Markus Armbruster 2018-06-18 17:59 ` [Qemu-devel] [PATCH v4 2/2] Revert commit d4e5ec877ca Markus Armbruster 0 siblings, 2 replies; 14+ messages in thread From: Markus Armbruster @ 2018-06-18 17:59 UTC (permalink / raw) To: qemu-devel; +Cc: tamiko, arfrever.fta v4: - Rewrite PATCH 1. v3: - reverse order of patches - rename second patch to "Revert commit ..." - drop "UTF-8" argument in decode()/encode(); bot functions default to utf-8 already. v2: - ensure compatibility with python2 by only calling encode()/decode() if script is run with a python3 interpreter Markus Armbruster (1): qapi: Open files with encoding='utf-8' Matthias Maier (1): Revert commit d4e5ec877ca Makefile | 6 ++---- scripts/qapi/common.py | 17 ++++++++++++++--- tests/Makefile.include | 6 +++--- 3 files changed, 19 insertions(+), 10 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH v4 1/2] qapi: Open files with encoding='utf-8' 2018-06-18 17:59 [Qemu-devel] [PATCH v4 0/2] Fix compilation with python-3 if en_US.UTF-8 is unavailable Markus Armbruster @ 2018-06-18 17:59 ` Markus Armbruster 2018-06-18 21:35 ` Eduardo Habkost 2018-06-19 11:02 ` Eric Blake 2018-06-18 17:59 ` [Qemu-devel] [PATCH v4 2/2] Revert commit d4e5ec877ca Markus Armbruster 1 sibling, 2 replies; 14+ messages in thread From: Markus Armbruster @ 2018-06-18 17:59 UTC (permalink / raw) To: qemu-devel; +Cc: tamiko, arfrever.fta Python 2 happily reads UTF-8 files in text mode, but Python 3 requires either UTF-8 locale or an explicit encoding passed to open(). Commit d4e5ec877ca fixed this by setting the en_US.UTF-8 locale. Falls apart when the locale isn't be available. Matthias Maier and Arfrever Frehtes Taifersar Arahesis proposed to use binary mode instead, with manual conversion from bytes to str. Works, but opening with an explicit encoding is simpler, so do that. Since Python 2's open() doesn't support the encoding parameter, we need to suppress it with a version check. Reported-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com> Reported-by: Matthias Maier <tamiko@43-1.org> Signed-off-by: Markus Armbruster <armbru@redhat.com> --- scripts/qapi/common.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index 2462fc0291..832f11438a 100644 --- a/scripts/qapi/common.py +++ b/scripts/qapi/common.py @@ -16,6 +16,7 @@ import errno import os import re import string +import sys from collections import OrderedDict builtin_types = { @@ -340,7 +341,10 @@ class QAPISchemaParser(object): return None try: - fobj = open(incl_fname, 'r') + if sys.version_info[0] >= 3: + fobj = open(incl_fname, 'r', encoding='utf-8') + else: + fobj = open(incl_fname, 'r') except IOError as e: raise QAPISemError(info, '%s: %s' % (e.strerror, incl_fname)) return QAPISchemaParser(fobj, previously_included, info) @@ -1492,7 +1496,11 @@ class QAPISchemaEvent(QAPISchemaEntity): class QAPISchema(object): def __init__(self, fname): self._fname = fname - parser = QAPISchemaParser(open(fname, 'r')) + if sys.version_info[0] >= 3: + f = open(fname, 'r', encoding='utf-8') + else: + f = open(fname, 'r') + parser = QAPISchemaParser(f) exprs = check_exprs(parser.exprs) self.docs = parser.docs self._entity_list = [] @@ -2006,7 +2014,10 @@ class QAPIGen(object): if e.errno != errno.EEXIST: raise fd = os.open(pathname, os.O_RDWR | os.O_CREAT, 0o666) - f = os.fdopen(fd, 'r+') + if sys.version_info[0] >= 3: + f = open(fd, 'r+', encoding='utf-8') + else: + f = os.fdopen(fd, 'r+') text = (self._top(fname) + self._preamble + self._body + self._bottom(fname)) oldtext = f.read(len(text) + 1) -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] qapi: Open files with encoding='utf-8' 2018-06-18 17:59 ` [Qemu-devel] [PATCH v4 1/2] qapi: Open files with encoding='utf-8' Markus Armbruster @ 2018-06-18 21:35 ` Eduardo Habkost 2018-06-19 6:28 ` Markus Armbruster 2018-06-19 11:02 ` Eric Blake 1 sibling, 1 reply; 14+ messages in thread From: Eduardo Habkost @ 2018-06-18 21:35 UTC (permalink / raw) To: Markus Armbruster; +Cc: qemu-devel, tamiko, arfrever.fta On Mon, Jun 18, 2018 at 07:59:57PM +0200, Markus Armbruster wrote: > Python 2 happily reads UTF-8 files in text mode, but Python 3 requires > either UTF-8 locale or an explicit encoding passed to open(). Commit > d4e5ec877ca fixed this by setting the en_US.UTF-8 locale. Falls apart > when the locale isn't be available. > > Matthias Maier and Arfrever Frehtes Taifersar Arahesis proposed to use > binary mode instead, with manual conversion from bytes to str. Works, > but opening with an explicit encoding is simpler, so do that. > > Since Python 2's open() doesn't support the encoding parameter, we > need to suppress it with a version check. > > Reported-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com> > Reported-by: Matthias Maier <tamiko@43-1.org> > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > scripts/qapi/common.py | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py > index 2462fc0291..832f11438a 100644 > --- a/scripts/qapi/common.py > +++ b/scripts/qapi/common.py > @@ -16,6 +16,7 @@ import errno > import os > import re > import string > +import sys > from collections import OrderedDict > > builtin_types = { > @@ -340,7 +341,10 @@ class QAPISchemaParser(object): > return None > > try: > - fobj = open(incl_fname, 'r') > + if sys.version_info[0] >= 3: > + fobj = open(incl_fname, 'r', encoding='utf-8') > + else: > + fobj = open(incl_fname, 'r') I dislike the Python version check, but getting rid of it would require rewriting the QAPI modules to not use the Python 2 str type (that has different semantics from Python 3 str type). The python-future package would help us write code for a single file/string API instead of two different APIs, but it's not a QEMU build dependency (yet?), so this patch is good enough for now. Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Acked-by: Eduardo Habkost <ehabkost@redhat.com> > except IOError as e: > raise QAPISemError(info, '%s: %s' % (e.strerror, incl_fname)) > return QAPISchemaParser(fobj, previously_included, info) > @@ -1492,7 +1496,11 @@ class QAPISchemaEvent(QAPISchemaEntity): > class QAPISchema(object): > def __init__(self, fname): > self._fname = fname > - parser = QAPISchemaParser(open(fname, 'r')) > + if sys.version_info[0] >= 3: > + f = open(fname, 'r', encoding='utf-8') > + else: > + f = open(fname, 'r') > + parser = QAPISchemaParser(f) > exprs = check_exprs(parser.exprs) > self.docs = parser.docs > self._entity_list = [] > @@ -2006,7 +2014,10 @@ class QAPIGen(object): > if e.errno != errno.EEXIST: > raise > fd = os.open(pathname, os.O_RDWR | os.O_CREAT, 0o666) > - f = os.fdopen(fd, 'r+') > + if sys.version_info[0] >= 3: > + f = open(fd, 'r+', encoding='utf-8') > + else: > + f = os.fdopen(fd, 'r+') > text = (self._top(fname) + self._preamble + self._body > + self._bottom(fname)) > oldtext = f.read(len(text) + 1) > -- > 2.17.1 > > -- Eduardo ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] qapi: Open files with encoding='utf-8' 2018-06-18 21:35 ` Eduardo Habkost @ 2018-06-19 6:28 ` Markus Armbruster 2018-06-19 10:50 ` Eduardo Habkost 0 siblings, 1 reply; 14+ messages in thread From: Markus Armbruster @ 2018-06-19 6:28 UTC (permalink / raw) To: Eduardo Habkost; +Cc: Markus Armbruster, tamiko, qemu-devel, arfrever.fta Eduardo Habkost <ehabkost@redhat.com> writes: > On Mon, Jun 18, 2018 at 07:59:57PM +0200, Markus Armbruster wrote: >> Python 2 happily reads UTF-8 files in text mode, but Python 3 requires >> either UTF-8 locale or an explicit encoding passed to open(). Commit >> d4e5ec877ca fixed this by setting the en_US.UTF-8 locale. Falls apart >> when the locale isn't be available. >> >> Matthias Maier and Arfrever Frehtes Taifersar Arahesis proposed to use >> binary mode instead, with manual conversion from bytes to str. Works, >> but opening with an explicit encoding is simpler, so do that. >> >> Since Python 2's open() doesn't support the encoding parameter, we >> need to suppress it with a version check. >> >> Reported-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com> >> Reported-by: Matthias Maier <tamiko@43-1.org> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> scripts/qapi/common.py | 17 ++++++++++++++--- >> 1 file changed, 14 insertions(+), 3 deletions(-) >> >> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py >> index 2462fc0291..832f11438a 100644 >> --- a/scripts/qapi/common.py >> +++ b/scripts/qapi/common.py >> @@ -16,6 +16,7 @@ import errno >> import os >> import re >> import string >> +import sys >> from collections import OrderedDict >> >> builtin_types = { >> @@ -340,7 +341,10 @@ class QAPISchemaParser(object): >> return None >> >> try: >> - fobj = open(incl_fname, 'r') >> + if sys.version_info[0] >= 3: >> + fobj = open(incl_fname, 'r', encoding='utf-8') >> + else: >> + fobj = open(incl_fname, 'r') > > I dislike the Python version check, but getting rid of it would > require rewriting the QAPI modules to not use the Python 2 str > type (that has different semantics from Python 3 str type). The version check is ugly, but it has a property I rather like: when we drop support for Python 2, the conditional becomes True, and partial evaluation results in the Python 3 code we actually want. > The python-future package would help us write code for a single > file/string API instead of two different APIs, but it's not a > QEMU build dependency (yet?), so this patch is good enough for > now. Please do not invest more than absolutely necessary in Python 2 support. All such investment will turn into technical debt in less than two years. If you must invest, pick a solution that will result in less technical debt. We can accept local ugliness for that. In my personal opinion, dumb ideas like supporting Python 2 this close to its EOL ought to look ugly. > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > Acked-by: Eduardo Habkost <ehabkost@redhat.com> Uh, what does "Acked-by" add over "Reviewed-by"? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] qapi: Open files with encoding='utf-8' 2018-06-19 6:28 ` Markus Armbruster @ 2018-06-19 10:50 ` Eduardo Habkost 2018-06-19 11:52 ` Markus Armbruster 2018-06-19 12:05 ` Daniel P. Berrangé 0 siblings, 2 replies; 14+ messages in thread From: Eduardo Habkost @ 2018-06-19 10:50 UTC (permalink / raw) To: Markus Armbruster; +Cc: tamiko, qemu-devel, arfrever.fta On Tue, Jun 19, 2018 at 08:28:08AM +0200, Markus Armbruster wrote: > Eduardo Habkost <ehabkost@redhat.com> writes: > > > On Mon, Jun 18, 2018 at 07:59:57PM +0200, Markus Armbruster wrote: > >> Python 2 happily reads UTF-8 files in text mode, but Python 3 requires > >> either UTF-8 locale or an explicit encoding passed to open(). Commit > >> d4e5ec877ca fixed this by setting the en_US.UTF-8 locale. Falls apart > >> when the locale isn't be available. > >> > >> Matthias Maier and Arfrever Frehtes Taifersar Arahesis proposed to use > >> binary mode instead, with manual conversion from bytes to str. Works, > >> but opening with an explicit encoding is simpler, so do that. > >> > >> Since Python 2's open() doesn't support the encoding parameter, we > >> need to suppress it with a version check. > >> > >> Reported-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com> > >> Reported-by: Matthias Maier <tamiko@43-1.org> > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > >> --- > >> scripts/qapi/common.py | 17 ++++++++++++++--- > >> 1 file changed, 14 insertions(+), 3 deletions(-) > >> > >> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py > >> index 2462fc0291..832f11438a 100644 > >> --- a/scripts/qapi/common.py > >> +++ b/scripts/qapi/common.py > >> @@ -16,6 +16,7 @@ import errno > >> import os > >> import re > >> import string > >> +import sys > >> from collections import OrderedDict > >> > >> builtin_types = { > >> @@ -340,7 +341,10 @@ class QAPISchemaParser(object): > >> return None > >> > >> try: > >> - fobj = open(incl_fname, 'r') > >> + if sys.version_info[0] >= 3: > >> + fobj = open(incl_fname, 'r', encoding='utf-8') > >> + else: > >> + fobj = open(incl_fname, 'r') > > > > I dislike the Python version check, but getting rid of it would > > require rewriting the QAPI modules to not use the Python 2 str > > type (that has different semantics from Python 3 str type). > > The version check is ugly, but it has a property I rather like: when we > drop support for Python 2, the conditional becomes True, and partial > evaluation results in the Python 3 code we actually want. > > > The python-future package would help us write code for a single > > file/string API instead of two different APIs, but it's not a > > QEMU build dependency (yet?), so this patch is good enough for > > now. > > Please do not invest more than absolutely necessary in Python 2 support. > All such investment will turn into technical debt in less than two > years. If you must invest, pick a solution that will result in less > technical debt. We can accept local ugliness for that. > > In my personal opinion, dumb ideas like supporting Python 2 this close > to its EOL ought to look ugly. That's the whole point: python-future allows us to not worry about Python 2 support in the code anymore because it exposes the Python 3 string API (and others) even if we're running Python 2. After we stop supporting Python 2, we can simply delete the "from __future__ import .*" and "from builtins import .*" lines. Anyway, I will send a RFC series demonstrating that, and then we can discuss if it's worth it. My main worry is not the extra imports in Python code, but the introduction of a new build dependency only for a few (one?) releases. > > > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > > Acked-by: Eduardo Habkost <ehabkost@redhat.com> > > Uh, what does "Acked-by" add over "Reviewed-by"? It was supposed to indicate that I agree it can be merged through other maintainers. But it looks like this is not part of the original definition of "Acked-by"? -- Eduardo ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] qapi: Open files with encoding='utf-8' 2018-06-19 10:50 ` Eduardo Habkost @ 2018-06-19 11:52 ` Markus Armbruster 2018-06-19 12:05 ` Daniel P. Berrangé 1 sibling, 0 replies; 14+ messages in thread From: Markus Armbruster @ 2018-06-19 11:52 UTC (permalink / raw) To: Eduardo Habkost; +Cc: Markus Armbruster, tamiko, qemu-devel, arfrever.fta Eduardo Habkost <ehabkost@redhat.com> writes: > On Tue, Jun 19, 2018 at 08:28:08AM +0200, Markus Armbruster wrote: >> Eduardo Habkost <ehabkost@redhat.com> writes: >> >> > On Mon, Jun 18, 2018 at 07:59:57PM +0200, Markus Armbruster wrote: >> >> Python 2 happily reads UTF-8 files in text mode, but Python 3 requires >> >> either UTF-8 locale or an explicit encoding passed to open(). Commit >> >> d4e5ec877ca fixed this by setting the en_US.UTF-8 locale. Falls apart >> >> when the locale isn't be available. >> >> >> >> Matthias Maier and Arfrever Frehtes Taifersar Arahesis proposed to use >> >> binary mode instead, with manual conversion from bytes to str. Works, >> >> but opening with an explicit encoding is simpler, so do that. >> >> >> >> Since Python 2's open() doesn't support the encoding parameter, we >> >> need to suppress it with a version check. >> >> >> >> Reported-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com> >> >> Reported-by: Matthias Maier <tamiko@43-1.org> >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> >> --- >> >> scripts/qapi/common.py | 17 ++++++++++++++--- >> >> 1 file changed, 14 insertions(+), 3 deletions(-) >> >> >> >> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py >> >> index 2462fc0291..832f11438a 100644 >> >> --- a/scripts/qapi/common.py >> >> +++ b/scripts/qapi/common.py >> >> @@ -16,6 +16,7 @@ import errno >> >> import os >> >> import re >> >> import string >> >> +import sys >> >> from collections import OrderedDict >> >> >> >> builtin_types = { >> >> @@ -340,7 +341,10 @@ class QAPISchemaParser(object): >> >> return None >> >> >> >> try: >> >> - fobj = open(incl_fname, 'r') >> >> + if sys.version_info[0] >= 3: >> >> + fobj = open(incl_fname, 'r', encoding='utf-8') >> >> + else: >> >> + fobj = open(incl_fname, 'r') >> > >> > I dislike the Python version check, but getting rid of it would >> > require rewriting the QAPI modules to not use the Python 2 str >> > type (that has different semantics from Python 3 str type). >> >> The version check is ugly, but it has a property I rather like: when we >> drop support for Python 2, the conditional becomes True, and partial >> evaluation results in the Python 3 code we actually want. >> >> > The python-future package would help us write code for a single >> > file/string API instead of two different APIs, but it's not a >> > QEMU build dependency (yet?), so this patch is good enough for >> > now. >> >> Please do not invest more than absolutely necessary in Python 2 support. >> All such investment will turn into technical debt in less than two >> years. If you must invest, pick a solution that will result in less >> technical debt. We can accept local ugliness for that. >> >> In my personal opinion, dumb ideas like supporting Python 2 this close >> to its EOL ought to look ugly. > > That's the whole point: python-future allows us to not worry > about Python 2 support in the code anymore because it exposes the > Python 3 string API (and others) even if we're running Python 2. > > After we stop supporting Python 2, we can simply delete the "from > __future__ import .*" and "from builtins import .*" lines. You're right, __future__ is one of the least annoying ways to keep Python 2 working. But is the improvement over my stupid, ugly solution worth your while? You decide. > Anyway, I will send a RFC series demonstrating that, and then we > can discuss if it's worth it. My main worry is not the extra > imports in Python code, but the introduction of a new build > dependency only for a few (one?) releases. The sane extra dependency to add would be Python 3. Not worth arguing again; time's on my side ;) >> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> >> > Acked-by: Eduardo Habkost <ehabkost@redhat.com> >> >> Uh, what does "Acked-by" add over "Reviewed-by"? > > It was supposed to indicate that I agree it can be merged through > other maintainers. But it looks like this is not part of the > original definition of "Acked-by"? I'll drop the Acked-by then. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] qapi: Open files with encoding='utf-8' 2018-06-19 10:50 ` Eduardo Habkost 2018-06-19 11:52 ` Markus Armbruster @ 2018-06-19 12:05 ` Daniel P. Berrangé 2018-06-19 13:06 ` Eduardo Habkost 1 sibling, 1 reply; 14+ messages in thread From: Daniel P. Berrangé @ 2018-06-19 12:05 UTC (permalink / raw) To: Eduardo Habkost; +Cc: Markus Armbruster, tamiko, qemu-devel, arfrever.fta On Tue, Jun 19, 2018 at 07:50:32AM -0300, Eduardo Habkost wrote: > On Tue, Jun 19, 2018 at 08:28:08AM +0200, Markus Armbruster wrote: > > Eduardo Habkost <ehabkost@redhat.com> writes: > > > > > On Mon, Jun 18, 2018 at 07:59:57PM +0200, Markus Armbruster wrote: > > >> Python 2 happily reads UTF-8 files in text mode, but Python 3 requires > > >> either UTF-8 locale or an explicit encoding passed to open(). Commit > > >> d4e5ec877ca fixed this by setting the en_US.UTF-8 locale. Falls apart > > >> when the locale isn't be available. > > >> > > >> Matthias Maier and Arfrever Frehtes Taifersar Arahesis proposed to use > > >> binary mode instead, with manual conversion from bytes to str. Works, > > >> but opening with an explicit encoding is simpler, so do that. > > >> > > >> Since Python 2's open() doesn't support the encoding parameter, we > > >> need to suppress it with a version check. > > >> > > >> Reported-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com> > > >> Reported-by: Matthias Maier <tamiko@43-1.org> > > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > > >> --- > > >> scripts/qapi/common.py | 17 ++++++++++++++--- > > >> 1 file changed, 14 insertions(+), 3 deletions(-) > > >> > > >> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py > > >> index 2462fc0291..832f11438a 100644 > > >> --- a/scripts/qapi/common.py > > >> +++ b/scripts/qapi/common.py > > >> @@ -16,6 +16,7 @@ import errno > > >> import os > > >> import re > > >> import string > > >> +import sys > > >> from collections import OrderedDict > > >> > > >> builtin_types = { > > >> @@ -340,7 +341,10 @@ class QAPISchemaParser(object): > > >> return None > > >> > > >> try: > > >> - fobj = open(incl_fname, 'r') > > >> + if sys.version_info[0] >= 3: > > >> + fobj = open(incl_fname, 'r', encoding='utf-8') > > >> + else: > > >> + fobj = open(incl_fname, 'r') > > > > > > I dislike the Python version check, but getting rid of it would > > > require rewriting the QAPI modules to not use the Python 2 str > > > type (that has different semantics from Python 3 str type). > > > > The version check is ugly, but it has a property I rather like: when we > > drop support for Python 2, the conditional becomes True, and partial > > evaluation results in the Python 3 code we actually want. > > > > > The python-future package would help us write code for a single > > > file/string API instead of two different APIs, but it's not a > > > QEMU build dependency (yet?), so this patch is good enough for > > > now. > > > > Please do not invest more than absolutely necessary in Python 2 support. > > All such investment will turn into technical debt in less than two > > years. If you must invest, pick a solution that will result in less > > technical debt. We can accept local ugliness for that. > > > > In my personal opinion, dumb ideas like supporting Python 2 this close > > to its EOL ought to look ugly. > > That's the whole point: python-future allows us to not worry > about Python 2 support in the code anymore because it exposes the > Python 3 string API (and others) even if we're running Python 2. > > After we stop supporting Python 2, we can simply delete the "from > __future__ import .*" and "from builtins import .*" lines. > > Anyway, I will send a RFC series demonstrating that, and then we > can discuss if it's worth it. My main worry is not the extra > imports in Python code, but the introduction of a new build > dependency only for a few (one?) releases. Using __future__ doesn't add an build dependancy AFAIK. __future__ is bundled with core python library rather than being an addon module. > > > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > > > Acked-by: Eduardo Habkost <ehabkost@redhat.com> > > > > Uh, what does "Acked-by" add over "Reviewed-by"? > > It was supposed to indicate that I agree it can be merged through > other maintainers. But it looks like this is not part of the > original definition of "Acked-by"? Yeah that's what I always thought it meant too. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] qapi: Open files with encoding='utf-8' 2018-06-19 12:05 ` Daniel P. Berrangé @ 2018-06-19 13:06 ` Eduardo Habkost 0 siblings, 0 replies; 14+ messages in thread From: Eduardo Habkost @ 2018-06-19 13:06 UTC (permalink / raw) To: Daniel P. Berrangé Cc: Markus Armbruster, tamiko, qemu-devel, arfrever.fta On Tue, Jun 19, 2018 at 01:05:10PM +0100, Daniel P. Berrangé wrote: [...] > > > > The python-future package would help us write code for a single > > > > file/string API instead of two different APIs, but it's not a > > > > QEMU build dependency (yet?), so this patch is good enough for > > > > now. > > > > > > Please do not invest more than absolutely necessary in Python 2 support. > > > All such investment will turn into technical debt in less than two > > > years. If you must invest, pick a solution that will result in less > > > technical debt. We can accept local ugliness for that. > > > > > > In my personal opinion, dumb ideas like supporting Python 2 this close > > > to its EOL ought to look ugly. > > > > That's the whole point: python-future allows us to not worry > > about Python 2 support in the code anymore because it exposes the > > Python 3 string API (and others) even if we're running Python 2. > > > > After we stop supporting Python 2, we can simply delete the "from > > __future__ import .*" and "from builtins import .*" lines. > > > > Anyway, I will send a RFC series demonstrating that, and then we > > can discuss if it's worth it. My main worry is not the extra > > imports in Python code, but the introduction of a new build > > dependency only for a few (one?) releases. > > Using __future__ doesn't add an build dependancy AFAIK. __future__ is > bundled with core python library rather than being an addon module. The extra dependency is because of the "builtins" module, provided by python-future in Python 2.7. -- Eduardo ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] qapi: Open files with encoding='utf-8' 2018-06-18 17:59 ` [Qemu-devel] [PATCH v4 1/2] qapi: Open files with encoding='utf-8' Markus Armbruster 2018-06-18 21:35 ` Eduardo Habkost @ 2018-06-19 11:02 ` Eric Blake 1 sibling, 0 replies; 14+ messages in thread From: Eric Blake @ 2018-06-19 11:02 UTC (permalink / raw) To: Markus Armbruster, qemu-devel; +Cc: tamiko, arfrever.fta On 06/18/2018 12:59 PM, Markus Armbruster wrote: > Python 2 happily reads UTF-8 files in text mode, but Python 3 requires > either UTF-8 locale or an explicit encoding passed to open(). Commit > d4e5ec877ca fixed this by setting the en_US.UTF-8 locale. Falls apart > when the locale isn't be available. > > Matthias Maier and Arfrever Frehtes Taifersar Arahesis proposed to use > binary mode instead, with manual conversion from bytes to str. Works, > but opening with an explicit encoding is simpler, so do that. > > Since Python 2's open() doesn't support the encoding parameter, we > need to suppress it with a version check. > > Reported-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com> > Reported-by: Matthias Maier <tamiko@43-1.org> > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > scripts/qapi/common.py | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH v4 2/2] Revert commit d4e5ec877ca 2018-06-18 17:59 [Qemu-devel] [PATCH v4 0/2] Fix compilation with python-3 if en_US.UTF-8 is unavailable Markus Armbruster 2018-06-18 17:59 ` [Qemu-devel] [PATCH v4 1/2] qapi: Open files with encoding='utf-8' Markus Armbruster @ 2018-06-18 17:59 ` Markus Armbruster 2018-06-18 21:35 ` Eduardo Habkost ` (2 more replies) 1 sibling, 3 replies; 14+ messages in thread From: Markus Armbruster @ 2018-06-18 17:59 UTC (permalink / raw) To: qemu-devel; +Cc: tamiko, arfrever.fta From: Matthias Maier <tamiko@43-1.org> This commit removes the PYTHON_UTF8 workaround. The problem with setting LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8 is that the en_US.UTF-8 locale might not be available. In this case setting above locales results in build errors even though another UTF-8 locale was originally set [1]. The only stable way of fixing the encoding problem is by explicitly annotating encoding/decoding in the python script. [1] https://bugs.gentoo.org/657766 Signed-off-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com> Signed-off-by: Matthias Maier <tamiko@43-1.org> Message-Id: <20180615220205.6929-3-tamiko@43-1.org> --- Makefile | 6 ++---- tests/Makefile.include | 6 +++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index e46f2b625a..7ed9cc4a21 100644 --- a/Makefile +++ b/Makefile @@ -20,8 +20,6 @@ ifneq ($(wildcard config-host.mak),) all: include config-host.mak -PYTHON_UTF8 = LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8 $(PYTHON) - git-submodule-update: .PHONY: git-submodule-update @@ -576,7 +574,7 @@ qga/qapi-generated/qga-qapi-commands.h qga/qapi-generated/qga-qapi-commands.c \ qga/qapi-generated/qga-qapi-doc.texi: \ qga/qapi-generated/qapi-gen-timestamp ; qga/qapi-generated/qapi-gen-timestamp: $(SRC_PATH)/qga/qapi-schema.json $(qapi-py) - $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-gen.py \ + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-gen.py \ -o qga/qapi-generated -p "qga-" $<, \ "GEN","$(@:%-timestamp=%)") @>$@ @@ -676,7 +674,7 @@ qapi/qapi-introspect.h qapi/qapi-introspect.c \ qapi/qapi-doc.texi: \ qapi-gen-timestamp ; qapi-gen-timestamp: $(qapi-modules) $(qapi-py) - $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-gen.py \ + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-gen.py \ -o "qapi" -b $<, \ "GEN","$(@:%-timestamp=%)") @>$@ diff --git a/tests/Makefile.include b/tests/Makefile.include index ca91da26cb..88f1bc1242 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -678,13 +678,13 @@ tests/test-qapi-events.c tests/test-qapi-events.h \ tests/test-qapi-introspect.c tests/test-qapi-introspect.h: \ tests/test-qapi-gen-timestamp ; tests/test-qapi-gen-timestamp: $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(qapi-py) - $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-gen.py \ + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-gen.py \ -o tests -p "test-" $<, \ "GEN","$(@:%-timestamp=%)") @>$@ tests/qapi-schema/doc-good.test.texi: $(SRC_PATH)/tests/qapi-schema/doc-good.json $(qapi-py) - $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-gen.py \ + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-gen.py \ -o tests/qapi-schema -p "doc-good-" $<, \ "GEN","$@") @mv tests/qapi-schema/doc-good-qapi-doc.texi $@ @@ -942,7 +942,7 @@ check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y)) $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \ - $(PYTHON_UTF8) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \ + $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \ $^ >$*.test.out 2>$*.test.err; \ echo $$? >$*.test.exit, \ "TEST","$*.out") -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] Revert commit d4e5ec877ca 2018-06-18 17:59 ` [Qemu-devel] [PATCH v4 2/2] Revert commit d4e5ec877ca Markus Armbruster @ 2018-06-18 21:35 ` Eduardo Habkost 2018-06-19 11:03 ` Eric Blake 2018-06-19 11:59 ` Markus Armbruster 2 siblings, 0 replies; 14+ messages in thread From: Eduardo Habkost @ 2018-06-18 21:35 UTC (permalink / raw) To: Markus Armbruster; +Cc: qemu-devel, tamiko, arfrever.fta On Mon, Jun 18, 2018 at 07:59:58PM +0200, Markus Armbruster wrote: > From: Matthias Maier <tamiko@43-1.org> > > This commit removes the PYTHON_UTF8 workaround. The problem with setting > > LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8 > > is that the en_US.UTF-8 locale might not be available. In this case > setting above locales results in build errors even though another UTF-8 > locale was originally set [1]. The only stable way of fixing the > encoding problem is by explicitly annotating encoding/decoding in the > python script. > > [1] https://bugs.gentoo.org/657766 > > Signed-off-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com> > Signed-off-by: Matthias Maier <tamiko@43-1.org> > Message-Id: <20180615220205.6929-3-tamiko@43-1.org> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Acked-by: Eduardo Habkost <ehabkost@redhat.com> -- Eduardo ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] Revert commit d4e5ec877ca 2018-06-18 17:59 ` [Qemu-devel] [PATCH v4 2/2] Revert commit d4e5ec877ca Markus Armbruster 2018-06-18 21:35 ` Eduardo Habkost @ 2018-06-19 11:03 ` Eric Blake 2018-06-19 11:46 ` Markus Armbruster 2018-06-19 11:59 ` Markus Armbruster 2 siblings, 1 reply; 14+ messages in thread From: Eric Blake @ 2018-06-19 11:03 UTC (permalink / raw) To: Markus Armbruster, qemu-devel; +Cc: tamiko, arfrever.fta On 06/18/2018 12:59 PM, Markus Armbruster wrote: > From: Matthias Maier <tamiko@43-1.org> > > This commit removes the PYTHON_UTF8 workaround. The problem with setting > > LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8 > > is that the en_US.UTF-8 locale might not be available. In this case > setting above locales results in build errors even though another UTF-8 > locale was originally set [1]. The only stable way of fixing the > encoding problem is by explicitly annotating encoding/decoding in the > python script. > > [1] https://bugs.gentoo.org/657766 > > Signed-off-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com> > Signed-off-by: Matthias Maier <tamiko@43-1.org> > Message-Id: <20180615220205.6929-3-tamiko@43-1.org> > --- Markus, does this need S-o-b from you? (Then again, if it goes in through your tree, that's a moot point, as you'd add it as maintainer) Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] Revert commit d4e5ec877ca 2018-06-19 11:03 ` Eric Blake @ 2018-06-19 11:46 ` Markus Armbruster 0 siblings, 0 replies; 14+ messages in thread From: Markus Armbruster @ 2018-06-19 11:46 UTC (permalink / raw) To: Eric Blake; +Cc: Markus Armbruster, qemu-devel, tamiko, arfrever.fta Eric Blake <eblake@redhat.com> writes: > On 06/18/2018 12:59 PM, Markus Armbruster wrote: >> From: Matthias Maier <tamiko@43-1.org> >> >> This commit removes the PYTHON_UTF8 workaround. The problem with setting >> >> LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8 >> >> is that the en_US.UTF-8 locale might not be available. In this case >> setting above locales results in build errors even though another UTF-8 >> locale was originally set [1]. The only stable way of fixing the >> encoding problem is by explicitly annotating encoding/decoding in the >> python script. >> >> [1] https://bugs.gentoo.org/657766 >> >> Signed-off-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com> >> Signed-off-by: Matthias Maier <tamiko@43-1.org> >> Message-Id: <20180615220205.6929-3-tamiko@43-1.org> >> --- > > Markus, does this need S-o-b from you? (Then again, if it goes in > through your tree, that's a moot point, as you'd add it as maintainer) I intend to take it through my tree. > Reviewed-by: Eric Blake <eblake@redhat.com> Thanks! ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] Revert commit d4e5ec877ca 2018-06-18 17:59 ` [Qemu-devel] [PATCH v4 2/2] Revert commit d4e5ec877ca Markus Armbruster 2018-06-18 21:35 ` Eduardo Habkost 2018-06-19 11:03 ` Eric Blake @ 2018-06-19 11:59 ` Markus Armbruster 2 siblings, 0 replies; 14+ messages in thread From: Markus Armbruster @ 2018-06-19 11:59 UTC (permalink / raw) To: Markus Armbruster; +Cc: qemu-devel, tamiko, arfrever.fta Markus Armbruster <armbru@redhat.com> writes: > From: Matthias Maier <tamiko@43-1.org> > > This commit removes the PYTHON_UTF8 workaround. The problem with setting > > LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8 > > is that the en_US.UTF-8 locale might not be available. In this case > setting above locales results in build errors even though another UTF-8 > locale was originally set [1]. The only stable way of fixing the > encoding problem is by explicitly annotating encoding/decoding in the > python script. I'd like to tweak the last sentence to The only stable way of fixing the encoding problem is by specifying the encoding in Python, like the previous commit does. if there are no objections. > > [1] https://bugs.gentoo.org/657766 > > Signed-off-by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com> > Signed-off-by: Matthias Maier <tamiko@43-1.org> > Message-Id: <20180615220205.6929-3-tamiko@43-1.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-06-19 13:06 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-18 17:59 [Qemu-devel] [PATCH v4 0/2] Fix compilation with python-3 if en_US.UTF-8 is unavailable Markus Armbruster 2018-06-18 17:59 ` [Qemu-devel] [PATCH v4 1/2] qapi: Open files with encoding='utf-8' Markus Armbruster 2018-06-18 21:35 ` Eduardo Habkost 2018-06-19 6:28 ` Markus Armbruster 2018-06-19 10:50 ` Eduardo Habkost 2018-06-19 11:52 ` Markus Armbruster 2018-06-19 12:05 ` Daniel P. Berrangé 2018-06-19 13:06 ` Eduardo Habkost 2018-06-19 11:02 ` Eric Blake 2018-06-18 17:59 ` [Qemu-devel] [PATCH v4 2/2] Revert commit d4e5ec877ca Markus Armbruster 2018-06-18 21:35 ` Eduardo Habkost 2018-06-19 11:03 ` Eric Blake 2018-06-19 11:46 ` Markus Armbruster 2018-06-19 11:59 ` Markus Armbruster
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.