Re: [Freeipa-devel] [PATCH 536] ipapython: remove default_encoding_utf8
On 15.1.2016 13:43, Tomas Babej wrote: On 01/12/2016 10:24 AM, Jan Cholasta wrote: On 6.1.2016 12:33, Christian Heimes wrote: On 2016-01-05 11:30, Tomas Babej wrote: On 01/05/2016 08:54 AM, Jan Cholasta wrote: Hi, the attached patch replaces the default_encoding_utf8 binary module with 2 lines of equivalent Python code. Honza This looks fine to me, however, I wonder, why this approach was ever taken? The sys.setdefaultencoding is available in all versions of Python ever supported by FreeIPA. Is it possible we're missing something here? Or was this option simply overlooked? sys.setdefaultencoding() is not available unless you use a hack and reload the sys module. The function is hidden for a very good reason. It can and will break internal assumption as well as libraries in bad, hard to detect ways. For example it wreaks havoc on hashing for dicts and sets. The blog posting https://anonbadger.wordpress.com/2015/06/16/why-sys-setdefaultencoding-will-break-code/ explains the problem in much greater detail. Tomáši, does this answer your question? Not really, I was more curious as to why the current, more complex solution using the C extension was ever preferred over pure python version. Updated patch attached. Patch works fine, ACK. Pushed to master: 7e56b4bbd79d9d42af23babc7496dd15d85d28ea ... and to ipa-4-3: 1cf005679d9c27e57ee3507e574f11b6a20f4450 -- Jan Cholasta From 7f9a2f52c9661ff2e310aab11ec00effcb7af434 Mon Sep 17 00:00:00 2001 From: Jan CholastaDate: Tue, 5 Jan 2016 08:49:04 +0100 Subject: [PATCH] ipapython: remove default_encoding_utf8 Replace the "import default_encoding_utf8" in ipalib/cli.py with equivalent Python code. https://fedorahosted.org/freeipa/ticket/5596 --- .gitignore | 1 - freeipa.spec.in| 2 - ipalib/cli.py | 13 ++--- ipapython/Makefile | 2 +- ipapython/py_default_encoding/Makefile | 25 -- .../py_default_encoding/default_encoding_utf8.c| 57 -- ipapython/py_default_encoding/setup.py | 45 - ipatests/pytest.ini| 1 - 8 files changed, 4 insertions(+), 142 deletions(-) delete mode 100644 ipapython/py_default_encoding/Makefile delete mode 100644 ipapython/py_default_encoding/default_encoding_utf8.c delete mode 100644 ipapython/py_default_encoding/setup.py diff --git a/.gitignore b/.gitignore index 06b017d..9375590 100644 --- a/.gitignore +++ b/.gitignore @@ -73,7 +73,6 @@ freeipa2-dev-doc /ipapython/setup.py /ipapython/version.py !/ipapython/Makefile -!/ipapython/py_default_encoding/Makefile !/ipapython/ipap11helper/Makefile /ipaplatform/__init__.py diff --git a/freeipa.spec.in b/freeipa.spec.in index 7d9e750..961d8c3 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -1311,13 +1311,11 @@ fi %{python_sitelib}/ipalib/* %dir %{python_sitelib}/ipaplatform %{python_sitelib}/ipaplatform/* -%attr(0644,root,root) %{python_sitearch}/default_encoding_utf8.so %attr(0644,root,root) %{python_sitearch}/_ipap11helper.so %{python_sitelib}/ipapython-*.egg-info %{python_sitelib}/ipalib-*.egg-info %{python_sitelib}/freeipa-*.egg-info %{python_sitelib}/ipaplatform-*.egg-info -%{python_sitearch}/python_default_encoding-*.egg-info %{python_sitearch}/_ipap11helper-*.egg-info diff --git a/ipalib/cli.py b/ipalib/cli.py index 567b599..3700572 100644 --- a/ipalib/cli.py +++ b/ipalib/cli.py @@ -39,16 +39,9 @@ from six.moves import input if six.PY3: unicode = str -try: -#pylint: disable=F0401 -import default_encoding_utf8 -except ImportError: -# This is a chicken-and-egg problem. The api can't be imported unless -# this is already installed and since it is installed with IPA therein -# lies the problem. Skip it for now so ipalib can be imported in-tree -# even in cases that IPA isn't installed on the dev machine. -# Also, under Python 3, default_encoding_utf8 is not built at all. -pass +if six.PY2: +reload(sys) +sys.setdefaultencoding('utf-8') # pylint: disable=no-member from ipalib import frontend from ipalib import backend diff --git a/ipapython/Makefile b/ipapython/Makefile index 833f3cd..201c589 100644 --- a/ipapython/Makefile +++ b/ipapython/Makefile @@ -1,7 +1,7 @@ PYTHON ?= /usr/bin/python2 PYTHONLIBDIR ?= $(shell $(PYTHON) -c "from distutils.sysconfig import *; print(get_python_lib())") -SUBDIRS = py_default_encoding ipap11helper +SUBDIRS = ipap11helper all: @for subdir in $(SUBDIRS); do \ diff --git a/ipapython/py_default_encoding/Makefile b/ipapython/py_default_encoding/Makefile deleted file mode 100644 index a73f429..000 --- a/ipapython/py_default_encoding/Makefile +++ /dev/null @@ -1,25 +0,0 @@ -PYTHON ?= /usr/bin/python2 -PYTHONLIBDIR ?= $(shell $(PYTHON) -c "from distutils.sysconfig import *;
Re: [Freeipa-devel] [PATCH 536] ipapython: remove default_encoding_utf8
On 01/12/2016 10:24 AM, Jan Cholasta wrote: > On 6.1.2016 12:33, Christian Heimes wrote: >> On 2016-01-05 11:30, Tomas Babej wrote: >>> >>> >>> On 01/05/2016 08:54 AM, Jan Cholasta wrote: Hi, the attached patch replaces the default_encoding_utf8 binary module with 2 lines of equivalent Python code. Honza >>> >>> This looks fine to me, however, I wonder, why this approach was ever >>> taken? The sys.setdefaultencoding is available in all versions of Python >>> ever supported by FreeIPA. >>> >>> Is it possible we're missing something here? Or was this option simply >>> overlooked? >> >> sys.setdefaultencoding() is not available unless you use a hack and >> reload the sys module. The function is hidden for a very good reason. It >> can and will break internal assumption as well as libraries in bad, hard >> to detect ways. For example it wreaks havoc on hashing for dicts and >> sets. >> >> The blog posting >> https://anonbadger.wordpress.com/2015/06/16/why-sys-setdefaultencoding-will-break-code/ >> >> explains the problem in much greater detail. > > Tomáši, does this answer your question? > Not really, I was more curious as to why the current, more complex solution using the C extension was ever preferred over pure python version. > Updated patch attached. Patch works fine, ACK. Pushed to master: 7e56b4bbd79d9d42af23babc7496dd15d85d28ea Tomas -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 536] ipapython: remove default_encoding_utf8
On 6.1.2016 12:33, Christian Heimes wrote: On 2016-01-05 11:30, Tomas Babej wrote: On 01/05/2016 08:54 AM, Jan Cholasta wrote: Hi, the attached patch replaces the default_encoding_utf8 binary module with 2 lines of equivalent Python code. Honza This looks fine to me, however, I wonder, why this approach was ever taken? The sys.setdefaultencoding is available in all versions of Python ever supported by FreeIPA. Is it possible we're missing something here? Or was this option simply overlooked? sys.setdefaultencoding() is not available unless you use a hack and reload the sys module. The function is hidden for a very good reason. It can and will break internal assumption as well as libraries in bad, hard to detect ways. For example it wreaks havoc on hashing for dicts and sets. The blog posting https://anonbadger.wordpress.com/2015/06/16/why-sys-setdefaultencoding-will-break-code/ explains the problem in much greater detail. Tomáši, does this answer your question? Updated patch attached. -- Jan Cholasta From ce4202372b5b6bede727bae10101b306d9d3428b Mon Sep 17 00:00:00 2001 From: Jan CholastaDate: Tue, 5 Jan 2016 08:49:04 +0100 Subject: [PATCH] ipapython: remove default_encoding_utf8 Replace the "import default_encoding_utf8" in ipalib/cli.py with equivalent Python code. https://fedorahosted.org/freeipa/ticket/5596 --- .gitignore | 1 - freeipa.spec.in| 2 - ipalib/cli.py | 13 ++--- ipapython/Makefile | 2 +- ipapython/py_default_encoding/Makefile | 25 -- .../py_default_encoding/default_encoding_utf8.c| 57 -- ipapython/py_default_encoding/setup.py | 45 - ipatests/pytest.ini| 1 - 8 files changed, 4 insertions(+), 142 deletions(-) delete mode 100644 ipapython/py_default_encoding/Makefile delete mode 100644 ipapython/py_default_encoding/default_encoding_utf8.c delete mode 100644 ipapython/py_default_encoding/setup.py diff --git a/.gitignore b/.gitignore index 06b017d..9375590 100644 --- a/.gitignore +++ b/.gitignore @@ -73,7 +73,6 @@ freeipa2-dev-doc /ipapython/setup.py /ipapython/version.py !/ipapython/Makefile -!/ipapython/py_default_encoding/Makefile !/ipapython/ipap11helper/Makefile /ipaplatform/__init__.py diff --git a/freeipa.spec.in b/freeipa.spec.in index de3ae2f..a963b0f 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -1309,13 +1309,11 @@ fi %{python_sitelib}/ipalib/* %dir %{python_sitelib}/ipaplatform %{python_sitelib}/ipaplatform/* -%attr(0644,root,root) %{python_sitearch}/default_encoding_utf8.so %attr(0644,root,root) %{python_sitearch}/_ipap11helper.so %{python_sitelib}/ipapython-*.egg-info %{python_sitelib}/ipalib-*.egg-info %{python_sitelib}/freeipa-*.egg-info %{python_sitelib}/ipaplatform-*.egg-info -%{python_sitearch}/python_default_encoding-*.egg-info %{python_sitearch}/_ipap11helper-*.egg-info diff --git a/ipalib/cli.py b/ipalib/cli.py index 3b1b5a3..a956485 100644 --- a/ipalib/cli.py +++ b/ipalib/cli.py @@ -39,16 +39,9 @@ from six.moves import input if six.PY3: unicode = str -try: -#pylint: disable=F0401 -import default_encoding_utf8 # pylint: disable=unused-import -except ImportError: -# This is a chicken-and-egg problem. The api can't be imported unless -# this is already installed and since it is installed with IPA therein -# lies the problem. Skip it for now so ipalib can be imported in-tree -# even in cases that IPA isn't installed on the dev machine. -# Also, under Python 3, default_encoding_utf8 is not built at all. -pass +if six.PY2: +reload(sys) +sys.setdefaultencoding('utf-8') # pylint: disable=no-member from ipalib import frontend from ipalib import backend diff --git a/ipapython/Makefile b/ipapython/Makefile index 833f3cd..201c589 100644 --- a/ipapython/Makefile +++ b/ipapython/Makefile @@ -1,7 +1,7 @@ PYTHON ?= /usr/bin/python2 PYTHONLIBDIR ?= $(shell $(PYTHON) -c "from distutils.sysconfig import *; print(get_python_lib())") -SUBDIRS = py_default_encoding ipap11helper +SUBDIRS = ipap11helper all: @for subdir in $(SUBDIRS); do \ diff --git a/ipapython/py_default_encoding/Makefile b/ipapython/py_default_encoding/Makefile deleted file mode 100644 index a73f429..000 --- a/ipapython/py_default_encoding/Makefile +++ /dev/null @@ -1,25 +0,0 @@ -PYTHON ?= /usr/bin/python2 -PYTHONLIBDIR ?= $(shell $(PYTHON) -c "from distutils.sysconfig import *; print(get_python_lib())") -PYTHONVERSION ?= $(shell $(PYTHON) -c "import sys; print(sys.version_info[0])") - -all: - if [ "$(PYTHONVERSION)" = "2" ]; then \ - python2 setup.py build; \ - fi - -install: - # Skip this module under Python 3 - if [ "$(PYTHONVERSION)" = "2" ]; then \ - if [ "$(DESTDIR)" = "" ]; then \ - python2 setup.py install; \ -
Re: [Freeipa-devel] [PATCH 536] ipapython: remove default_encoding_utf8
On 2016-01-05 11:30, Tomas Babej wrote: > > > On 01/05/2016 08:54 AM, Jan Cholasta wrote: >> Hi, >> >> the attached patch replaces the default_encoding_utf8 binary module with >> 2 lines of equivalent Python code. >> >> Honza >> >> >> > > This looks fine to me, however, I wonder, why this approach was ever > taken? The sys.setdefaultencoding is available in all versions of Python > ever supported by FreeIPA. > > Is it possible we're missing something here? Or was this option simply > overlooked? sys.setdefaultencoding() is not available unless you use a hack and reload the sys module. The function is hidden for a very good reason. It can and will break internal assumption as well as libraries in bad, hard to detect ways. For example it wreaks havoc on hashing for dicts and sets. The blog posting https://anonbadger.wordpress.com/2015/06/16/why-sys-setdefaultencoding-will-break-code/ explains the problem in much greater detail. signature.asc Description: OpenPGP digital signature -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 536] ipapython: remove default_encoding_utf8
On 01/05/2016 08:54 AM, Jan Cholasta wrote: > Hi, > > the attached patch replaces the default_encoding_utf8 binary module with > 2 lines of equivalent Python code. > > Honza > > > This looks fine to me, however, I wonder, why this approach was ever taken? The sys.setdefaultencoding is available in all versions of Python ever supported by FreeIPA. Is it possible we're missing something here? Or was this option simply overlooked? Ccing Rob. Tomas -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 536] ipapython: remove default_encoding_utf8
On 5.1.2016 11:54, Alexander Bokovoy wrote: On Tue, 05 Jan 2016, Tomas Babej wrote: On 01/05/2016 08:54 AM, Jan Cholasta wrote: Hi, the attached patch replaces the default_encoding_utf8 binary module with 2 lines of equivalent Python code. Honza This looks fine to me, however, I wonder, why this approach was ever taken? The sys.setdefaultencoding is available in all versions of Python ever supported by FreeIPA. Is it possible we're missing something here? Or was this option simply overlooked? There is more history to it and it is mostly ugly: https://bugzilla.redhat.com/show_bug.cgi?id=243541 What is actually ugly is badly written code which assumes a specific encoding anywhere instead of using an encoding appropriate in the given context. Rather than working around it using hacks such as changing the default encoding, the preferrable solution should be to fix the badly written code itself (which is not always easy, as is the case with IPA). -- Jan Cholasta -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 536] ipapython: remove default_encoding_utf8
On Tue, 05 Jan 2016, Jan Cholasta wrote: On 5.1.2016 11:54, Alexander Bokovoy wrote: On Tue, 05 Jan 2016, Tomas Babej wrote: On 01/05/2016 08:54 AM, Jan Cholasta wrote: Hi, the attached patch replaces the default_encoding_utf8 binary module with 2 lines of equivalent Python code. Honza This looks fine to me, however, I wonder, why this approach was ever taken? The sys.setdefaultencoding is available in all versions of Python ever supported by FreeIPA. Is it possible we're missing something here? Or was this option simply overlooked? There is more history to it and it is mostly ugly: https://bugzilla.redhat.com/show_bug.cgi?id=243541 What is actually ugly is badly written code which assumes a specific encoding anywhere instead of using an encoding appropriate in the given context. Rather than working around it using hacks such as changing the default encoding, the preferrable solution should be to fix the badly written code itself (which is not always easy, as is the case with IPA). I do agree with you in general but this case is sufficiently different enough to warrant what we have in place. -- / Alexander Bokovoy -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 536] ipapython: remove default_encoding_utf8
On Tue, 05 Jan 2016, Tomas Babej wrote: On 01/05/2016 08:54 AM, Jan Cholasta wrote: Hi, the attached patch replaces the default_encoding_utf8 binary module with 2 lines of equivalent Python code. Honza This looks fine to me, however, I wonder, why this approach was ever taken? The sys.setdefaultencoding is available in all versions of Python ever supported by FreeIPA. Is it possible we're missing something here? Or was this option simply overlooked? There is more history to it and it is mostly ugly: https://bugzilla.redhat.com/show_bug.cgi?id=243541 -- / Alexander Bokovoy -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [PATCH 536] ipapython: remove default_encoding_utf8
Hi, the attached patch replaces the default_encoding_utf8 binary module with 2 lines of equivalent Python code. Honza -- Jan Cholasta From c69fece2fb2bcc68c7efec841178ea4cb4cfd48d Mon Sep 17 00:00:00 2001 From: Jan CholastaDate: Tue, 5 Jan 2016 08:49:04 +0100 Subject: [PATCH] ipapython: remove default_encoding_utf8 Replace the "import default_encoding_utf8" in ipalib/cli.py with equivalent Python code. --- .gitignore | 1 - freeipa.spec.in| 2 - ipalib/cli.py | 13 ++--- ipapython/Makefile | 2 +- ipapython/py_default_encoding/Makefile | 25 -- .../py_default_encoding/default_encoding_utf8.c| 57 -- ipapython/py_default_encoding/setup.py | 45 - ipatests/pytest.ini| 1 - 8 files changed, 4 insertions(+), 142 deletions(-) delete mode 100644 ipapython/py_default_encoding/Makefile delete mode 100644 ipapython/py_default_encoding/default_encoding_utf8.c delete mode 100644 ipapython/py_default_encoding/setup.py diff --git a/.gitignore b/.gitignore index 06b017d..9375590 100644 --- a/.gitignore +++ b/.gitignore @@ -73,7 +73,6 @@ freeipa2-dev-doc /ipapython/setup.py /ipapython/version.py !/ipapython/Makefile -!/ipapython/py_default_encoding/Makefile !/ipapython/ipap11helper/Makefile /ipaplatform/__init__.py diff --git a/freeipa.spec.in b/freeipa.spec.in index d4e23bc..ea4b3b8 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -1309,13 +1309,11 @@ fi %{python_sitelib}/ipalib/* %dir %{python_sitelib}/ipaplatform %{python_sitelib}/ipaplatform/* -%attr(0644,root,root) %{python_sitearch}/default_encoding_utf8.so %attr(0644,root,root) %{python_sitearch}/_ipap11helper.so %{python_sitelib}/ipapython-*.egg-info %{python_sitelib}/ipalib-*.egg-info %{python_sitelib}/freeipa-*.egg-info %{python_sitelib}/ipaplatform-*.egg-info -%{python_sitearch}/python_default_encoding-*.egg-info %{python_sitearch}/_ipap11helper-*.egg-info diff --git a/ipalib/cli.py b/ipalib/cli.py index 3b1b5a3..a956485 100644 --- a/ipalib/cli.py +++ b/ipalib/cli.py @@ -39,16 +39,9 @@ from six.moves import input if six.PY3: unicode = str -try: -#pylint: disable=F0401 -import default_encoding_utf8 # pylint: disable=unused-import -except ImportError: -# This is a chicken-and-egg problem. The api can't be imported unless -# this is already installed and since it is installed with IPA therein -# lies the problem. Skip it for now so ipalib can be imported in-tree -# even in cases that IPA isn't installed on the dev machine. -# Also, under Python 3, default_encoding_utf8 is not built at all. -pass +if six.PY2: +reload(sys) +sys.setdefaultencoding('utf-8') # pylint: disable=no-member from ipalib import frontend from ipalib import backend diff --git a/ipapython/Makefile b/ipapython/Makefile index 833f3cd..201c589 100644 --- a/ipapython/Makefile +++ b/ipapython/Makefile @@ -1,7 +1,7 @@ PYTHON ?= /usr/bin/python2 PYTHONLIBDIR ?= $(shell $(PYTHON) -c "from distutils.sysconfig import *; print(get_python_lib())") -SUBDIRS = py_default_encoding ipap11helper +SUBDIRS = ipap11helper all: @for subdir in $(SUBDIRS); do \ diff --git a/ipapython/py_default_encoding/Makefile b/ipapython/py_default_encoding/Makefile deleted file mode 100644 index a73f429..000 --- a/ipapython/py_default_encoding/Makefile +++ /dev/null @@ -1,25 +0,0 @@ -PYTHON ?= /usr/bin/python2 -PYTHONLIBDIR ?= $(shell $(PYTHON) -c "from distutils.sysconfig import *; print(get_python_lib())") -PYTHONVERSION ?= $(shell $(PYTHON) -c "import sys; print(sys.version_info[0])") - -all: - if [ "$(PYTHONVERSION)" = "2" ]; then \ - python2 setup.py build; \ - fi - -install: - # Skip this module under Python 3 - if [ "$(PYTHONVERSION)" = "2" ]; then \ - if [ "$(DESTDIR)" = "" ]; then \ - python2 setup.py install; \ - else \ - python2 setup.py install --root $(DESTDIR); \ - fi; \ - fi - -clean: - rm -rf build - -distclean: clean - -maintainer-clean: distclean diff --git a/ipapython/py_default_encoding/default_encoding_utf8.c b/ipapython/py_default_encoding/default_encoding_utf8.c deleted file mode 100644 index 07adf28..000 --- a/ipapython/py_default_encoding/default_encoding_utf8.c +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Authors: - * John Dennis - * - * Copyright (C) 2009 Red Hat - * see file 'COPYING' for use and warranty information - * - * This program is free software you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied