Re: [gentoo-portage-dev] [PATCH] Do not enable optimizations by default to work-around pycparser issue

[Michał Górny]

W dniu sob, 02.09.2017 o godzinie 12∶19 -0700, użytkownik Zac Medico
napisał:
> On 09/02/2017 10:46 AM, Michał Górny wrote:
> > dev-python/pycparser-2.18+ exposes a design flaw in dev-python/ply that
> > makes it unable to work with -OO code. Remove the optimizations from
> > Portage shebangs to prevent triggering the issue until we find a proper
> > solution for it.
> >
> > Bug: https://bugs.gentoo.org/628386
> > ---
> >  bin/clean_locks   | 2 +-
> >  bin/dispatch-conf | 2 +-
> >  bin/ebuild        | 2 +-
> >  bin/emaint        | 2 +-
> >  bin/env-update    | 2 +-
> >  bin/portageq      | 2 +-
> >  tabcheck.py       | 2 +-
> >  7 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/bin/clean_locks b/bin/clean_locks
> > index 13af06197..fb245972f 100755
> > --- a/bin/clean_locks
> > +++ b/bin/clean_locks
> > @@ -1,4 +1,4 @@
> > -#!/usr/bin/python -bO
> > +#!/usr/bin/python -b
>
> The diff shows -O, but the commit messages says -OO, so which one is it really?

Yes, it's a curious problem. -OO is the one breaking it but py<3.5 seems
to be happy to load -OO files with -O:

$ python2.7 -O -c 'import pycparser; print(pycparser.__file__)'
/usr/lib64/python2.7/site-packages/pycparser/c_parser.py:20: RuntimeWarning: parsing methods must have __doc__ for pycparser to work properly
  class CParser(PLYParser):
/usr/lib64/python2.7/site-packages/pycparser/__init__.pyo

$ python3.4 -O -c 'import pycparser; print(pycparser.__cached__)'
/usr/lib64/python3.4/site-packages/pycparser/c_parser.py:20: RuntimeWarning: parsing methods must have __doc__ for pycparser to work properly
  class CParser(PLYParser):
/usr/lib64/python3.4/site-packages/pycparser/__pycache__/__init__.cpython-34.pyo

This doesn't seem to be the case anymore for py3.5+:

$ python3.5 -O -c 'import pycparser; print(pycparser.__cached__)'
/usr/lib64/python3.5/site-packages/pycparser/__pycache__/__init__.cpython-35.opt-1.pyc

> Are we sure that it's worth it to load all of the __doc__ strings into
> memory, given that we have alternative implementations for everything
> that pycrypto provides?

I dare say that if -O can cause completely random total breakage, then
we shouldn't risk doing that for the sake of some minor memory savings.
It's not a case of 'support for pycryptodome' vs '1M memory saving'.
It's a case of 'loading random module can wreak total havoc in Portage'.

> For the record, I measure an increase of from 30248K to 31504K for -OO vs without when
> importing all portage modules, tested as follows:
>
> $ python3.6 -OO pym/portage/tests/runTests.py pym/portage/tests/lint/test_import_modules.py
> testImportModules (portage.tests.lint.test_import_modules.ImportModulesTestCase) ... 30248
>
> $ python3.6 -O pym/portage/tests/runTests.py pym/portage/tests/lint/test_import_modules.py
> testImportModules (portage.tests.lint.test_import_modules.ImportModulesTestCase) ... 31468
>
> $ python3.6  pym/portage/tests/runTests.py pym/portage/tests/lint/test_import_modules.py
> testImportModules (portage.tests.lint.test_import_modules.ImportModulesTestCase) ... 31504
>
> Using this patch:
>
> diff --git a/pym/portage/tests/lint/test_import_modules.py b/pym/portage/tests/lint/test_import_modules.py
> index fcdcb3b33..6350197eb 100644
> --- a/pym/portage/tests/lint/test_import_modules.py
> +++ b/pym/portage/tests/lint/test_import_modules.py
> @@ -2,6 +2,7 @@
>  # Distributed under the terms of the GNU General Public License v2
>
>  from itertools import chain
> +import resource
>
>  from portage.const import PORTAGE_PYM_PATH, PORTAGE_PYM_PACKAGES
>  from portage.tests import TestCase
> @@ -24,6 +25,7 @@ class ImportModulesTestCase(TestCase):
>                                 if mod not in expected_failures:
>                                         self.assertTrue(False, "failed to import '%s': %s" % (mod, e))
>                                 del e
> +               print(resource.getrusage(resource.RUSAGE_SELF).ru_maxrss)
>
>         def _iter_modules(self, base_dir):
>                 for parent, dirs, files in os.walk(base_dir):
>

-- 
Best regards,
Michał Górny