Guard against pkg_resources.resource_filename errors

[Christian Boos]

During a trac-svn-hook resync (trac-admin ... changeset added /var/svn/trac 10433):

[pid 1052 139942713288416] 2011-01-14 18:46:06,498 Trac[env] INFO: -------------------------------- environment startup [Trac 0.13dev-r10426] ------------------------
--------
[pid 1052 139942713288416] 2011-01-14 18:46:09,926 Trac[console] ERROR: Exception in trac-admin command:
Traceback (most recent call last):
...
  File "build/bdist.linux-x86_64/egg/trac/versioncontrol/admin.py", line 95, in _do_changeset_added
    rm.notify('changeset_added', reponame, revs)
  File "build/bdist.linux-x86_64/egg/trac/versioncontrol/api.py", line 620, in notify
    repositories = self.get_repositories_by_dir(dir)
...
  File "build/bdist.linux-x86_64/egg/tracext/hg/backend.py", line 210, in __init__
    locale_dir = pkg_resources.resource_filename(__name__, 'locale')
...
  File "/usr/local/virtualenv/0.13dev/lib/python2.5/site-packages/setuptools-0.6c9-py2.5.egg/pkg_resources.py", line 887, in extraction_error
    raise err
ExtractionError: Can't extract file(s) to egg cache


  [Errno 17] File exists: '/var/www/.python-eggs/TracMercurial-0.13.0.1dev_r10432-py2.5.egg-tmp/tracext/hg/locale'

So a duplicate of #9107 for the TracMercurial plugin. We can fix that particular occurrence, but I wonder if it wouldn't make sense to add a wrapper for the pkg_resources stuff (in trac.utils.dist?) and use that. That wrapper should be "process safe" not only thread-safe, so using a lock won't cut it.

Maybe trapping such errors and restarting the call once after a random amount of time between .3 to 1s would be enough.

[Christian Boos]

#8537 closed as duplicate:

• original report

    File "build\bdist.win32\egg\workfloweditor\workfloweditor_admin.py", line 21, in get_templates_dirs
      return [resource_filename(__name__, 'templates')]
  

• in comment:11:ticket:8537

    File "build/bdist.linux-x86_64/egg/trac/web/chrome.py", line 554, in get_templates_dirs
      pkg_resources.resource_filename('trac', 'templates'),
  

[Christian Boos]

Remy proposed another approach to fix this:

1. the wrapper should change the PYTHON_EGG_CACHE to a temporary directory
2. once the extraction is done, it should rename the extracted directory to the target below the original PYTHON_EGG_CACHE directory
   1. only if not already there?
   2. if anything changed?

That would probably fix the issue as well, without having to introduce a temporization.

For 1., we could pick a temporary folder below the original PYTHON_EGG_CACHE, that would probably guarantee that we have the correct permissions (provided PYTHON_EGG_CACHE has, as it should), and the same filesystem locality (for a quick rename).

For 2., the fastest choice would be a., but there would be cases where a legitimate update would fail: same toplevel name, e.g. when building an .egg from a svn checkout, with local modifications; we could state that this is not supported and that you'd better do python setup.py develop in that case anyway. 2.b. would probably be too slow.

[Remy Blank]

I would still like to mention that this really is an issue in pkg_resources, which should provide this atomicity itself. Maybe we can monkey-patch something in there (and report the issue so that it's fixed correctly at some point)?

[Remy Blank]

There seems to be at least some provision in pkg_resources for concurrent extraction (see ZipProvider._extract_resource(), but I would be surprised if it handled all situations correctly.

<rant>Why is the pkg_resources code so… weird? My eyes water every time I have to look at it.</rant>

[Jun Omae]

#11521 was closed as duplicate.