Edgewall Software
Modify

Opened 8 years ago

Closed 2 months ago

#10418 closed defect (fixed)

Simplify imports in trac/web/__init__.py

Reported by: anatoly techtonik <techtonik@…> Owned by:
Priority: normal Milestone: 1.3.2
Component: web frontend/mod_python Version: 0.12.2
Severity: normal Keywords: patch
Cc: osimons Branch:
Release Notes:
API Changes:

Description

pkg_resources.get_distribution() is unable to find modules already available in sys.path

>>> import sys, os, pkg_resources
>>> pkg_resources.get_distribution('genshi')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 298, in get_distribution
    if isinstance(dist,Requirement): dist = get_provider(dist)
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 177, in get_provider
    return working_set.find(moduleOrReq) or require(str(moduleOrReq))[0]
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 654, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 552, in resolve
    raise DistributionNotFound(req)
pkg_resources.DistributionNotFound: genshi
>>> import genshi
>>> genshi.__version__
'0.6.1'
>>> 

This problem makes it impossible to execute Trac admin command with custom sys.path (for example with IDE button), and requires system or virtualenv installation for all packages, which makes development inconvenient. Is it really necessary?

Traceback (most recent call last):
  File "bootstrap.py", line 49, in <module>
    import trac.admin.console
  File "/home/user07/trac/0.12dev/trac/admin/console.py", line 27, in <module>
    from trac.env import Environment
  File "/home/user07/trac/0.12dev/trac/env.py", line 35, in <module>
    from trac.versioncontrol import RepositoryManager
  File "/home/user07/trac/0.12dev/trac/versioncontrol/__init__.py", line 1, in <module>
    from trac.versioncontrol.api import *
  File "/home/user07/trac/0.12dev/trac/versioncontrol/api.py", line 27, in <module>
    from trac.web.api import IRequestFilter
  File "/home/user07/trac/0.12dev/trac/web/__init__.py", line 14, in <module>
    if not os.path.isdir(get_distribution('genshi').location):
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 298, in get_distribution
    if isinstance(dist,Requirement): dist = get_provider(dist)
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 177, in get_provider
    return working_set.find(moduleOrReq) or require(str(moduleOrReq))[0]
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 654, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 552, in resolve
    raise DistributionNotFound(req)
pkg_resources.DistributionNotFound: genshi

Attachments (0)

Change History (21)

comment:1 by anatoly techtonik <techtonik@…>, 8 years ago

I hope this TODO can be removed now with Genshi 6.1?

  • trac/web/__init__.py

     
    33import mimetypes
    44mimetypes.init()
    55
    6 # With mod_python we'll have to delay importing trac.web.api until
    7 # modpython_frontend.handler() has been called since the
    8 # PYTHON_EGG_CACHE variable is set from there
    9 #
    10 # TODO: Remove this once the Genshi zip_safe issue has been resolved.
    116
    12 import os
    13 from pkg_resources import get_distribution
    14 if not os.path.isdir(get_distribution('genshi').location):
    15     try:
    16         import mod_python.apache
    17         import sys
    18         if 'trac.web.modpython_frontend' in sys.modules:
    19             from trac.web.api import *
    20     except ImportError:
    21         from trac.web.api import *
    22 else:
    23     from trac.web.api import *
     7from trac.web.api import *

comment:2 by anatoly techtonik <techtonik@…>, 8 years ago

Keywords: patch added

comment:3 by Remy Blank, 8 years ago

If you get a DistributionNotFound exception, then something is missing from your Genshi install, probably the egg-info. If you run Genshi from a working copy, you need to run:

python setup.py develop

in Genshi's source directory to put in place the relevant hooks.

The simplest is to create a virtualenv for your test Trac installation, and to install the relevant packages in development mode in there.

About the TODO related to mod_python, I'm OK to remove it, but it needs to be tested with mod_python. Unfortunately, I don't have a working instance of the module anymore, so I rely on your (or someone else's) testing.

in reply to:  3 comment:4 by anatoly techtonik <techtonik@…>, 8 years ago

Replying to rblank:

About the TODO related to mod_python, I'm OK to remove it, but it needs to be tested with mod_python. Unfortunately, I don't have a working instance of the module anymore, so I rely on your (or someone else's) testing.

Unfortunately, my setup is NGINX+FastCGI.

comment:5 by Remy Blank, 8 years ago

Component: generalweb frontend/mod_python
Milestone: unscheduled

Too bad. Oh well, then it's going to remain a TODO until either someone can test with mod_python, or mod_python disappears.

comment:6 by Remy Blank, 8 years ago

Summary: DistributionNotFound with Genshi available in sys.pathSimplify imports in trac/web/__init__.py

comment:7 by osimons, 8 years ago

Cc: osimons added

The code shouldn't be needed anymore, and the patch works in that respect. I never install anything as zipped eggs, so can't really verify it directly.

The Genshi issue should be fixed, so that Genshi will not by default install zipped egg IF it contains a compiled extension. However, since the workaround was made have also added Babel, but don't think that requires extraction - at least not before we hit trac.web.modpython_frontend. So currently I don't think the code is needed.

Remove it in trunk?

in reply to:  7 comment:8 by Remy Blank, 8 years ago

Milestone: unscheduled0.13

Replying to osimons:

Remove it in trunk?

All right. Want to do it?

comment:9 by osimons, 8 years ago

Owner: set to osimons

Did some digging. The changeset that fixed the behavior was genshi:changeset:928 and committed quite some time before 0.6 release. As even Trac 0.12 requires Genshi≥0.6 it should be OK to remove it there too.

It is ugly code, and if we can do without then that is a good thing. I'd rather see it gone from stable too… Objections?

in reply to:  9 comment:10 by Christian Boos, 8 years ago

Replying to osimons:

Did some digging. The changeset that fixed the behavior was genshi:changeset:928

… which says: "zip_safe to False if the c speedup module is successfully built"

And later, with [G1089] the C speedup module is no longer built by default, which means that usually Genshi is built as zip-safe. So as long as we want to support mod_python 3.3.1 (which still should work fine(?)) and Genshi 0.6, I think this code is needed.

comment:11 by osimons, 8 years ago

No, I don't think that is correct Christian. Python and mod_python has no problem reading Genshi compiled byte code from an egg - Python knows how to import from zipped archive without extraction.

The problem was when the Genshi byte code referenced a C extension that (default or not) existed inside the egg. That would trigger an extraction in order to load it as Python tries to do the right thing. And, that would of course happen BEFORE Trac ever reached its frontend code setting a proper extraction path.

To my knowledge Genshi has no resources except the C extension that require extraction, so zipped egg is fine since genshi:changeset:928 as we then can be sure it won't contain the extension. Remember that we waited quite some time for 0.6 release after that changeset. With 0.6 release requirement and [9523] all this went away.

in reply to:  11 comment:12 by Christian Boos, 8 years ago

Replying to osimons:

… Python and mod_python has no problem reading Genshi compiled byte code from an egg

Oh right, that's must be the meaning of zip-safe :P) Please proceed as if I said nothing…

comment:13 by osimons, 8 years ago

A little experiment to detect what actually loads:

>>> import sys
>>> before = set(sys.modules.keys())
>>> import trac.web
>>> after = set(sys.modules.keys())
>>> print(sorted(after.difference(before)))
['BaseHTTPServer',
 'Cookie',
 'HTMLParser',
 'SocketServer',
 '_ast',
 '_scproxy',
 'array',
 'babel', # + 23 further Babel modules
 'base64',
 'cgi',
 'decimal',
 'genshi', # + 47 further Genshi modules
 'htmlentitydefs',
 'markupbase',
 'mimetools',
 'mimetypes',
 'mod_python', # + 9 further mod_python modules, scheduled for removal by change
 'numbers',
 'pytz', # + 12 further pytz modules
 'rfc822',
 'ssl',
 'trac', # + 54 further trac modules
 'unicodedata',
 'urllib',
 'urlparse']

What about Babel? Anything else to worry about? Also, this was my development virutalenv OSX with Python 2.6.6 - are there perhaps significant platform differences?

comment:14 by Christian Boos, 8 years ago

Well, Babel is and was zip_safe=False from the start. I would say at this point of the discussion it's OK to do this move on trunk and if there's any problem with that change it can only be discovered with a bit more exposure (and perhaps only after the release… but we'll know where to look).

comment:15 by osimons, 8 years ago

The other simple option we have is to move the frontend handler to tracopt.modpython so that it can load without side effects. We could then just leave a small check in the old module that writes new handler location the apache error log on each module load, but otherwise just imports and forwards to the proper handler.

Embedding frontend scripts so deep into the code base has not been without issues, and we've now removed all others (wsgi/fcgi/cgi) by way of the deploy command. Perhaps it is time to move mod_python handler too?

It would also better correspond with the current status of modpython and our support for it (even though it still works well).

comment:16 by Remy Blank, 7 years ago

Milestone: 1.01.0-triage

Preparing for 1.0.

comment:17 by Ryan J Ollos, 4 years ago

Owner: osimons removed

comment:18 by Ryan J Ollos, 4 years ago

Milestone: next-stable-1.0.xnext-dev-1.1.x

comment:19 by Ryan J Ollos, 4 years ago

Milestone: next-dev-1.1.xnext-dev-1.3.x

Narrowing focus for milestone:1.2. Please move ticket to milestone:1.2 if you intend to fix it.

comment:20 by Ryan J Ollos, 3 months ago

Milestone: next-dev-1.3.xnext-dev-1.5.x

Milestone renamed

in reply to:  1 comment:21 by Ryan J Ollos, 2 months ago

Milestone: next-dev-1.5.x1.3.2
Resolution: fixed
Status: newclosed

Replying to anatoly techtonik <techtonik@…>:

I hope this TODO can be removed now with Genshi 6.1?

The code was removed in r15422 (#12639).

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The ticket will remain with no owner.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from (none) to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.