[Distutils] PATCH: setuptools-0.4a3 resource loading from egg problem

[Phillip J. Eby]

At 08:17 AM 6/14/2005 -0400, Ryan Tomayko wrote:
>Here's a test egg that illustrates the problem:
>
><http://lesscode.org/eggs/somepackage-1.0-py2.4.egg>
>
>Install that and then run the included "somescript.py". You should
>see the traceback above.

I don't; it works fine on my (Windows) PC, assuming all it does is spit out 
a filename.


>Attached is a patch against 0.4a3 that fixes this particular case but
>I'm not confident it won't cause problems elsewhere. I'll comment
>each hunk here with an explanation of what I think may be happening.
>
>@@ -573,7 +573,7 @@
>      def resource_listdir(self,resource_name):
>-        return self._listdir(self._fn(self.egg_info,resource_name))
>+        return self._listdir(self._fn(self.module_path, resource_name))
>      def metadata_listdir(self,name):
>          if self.egg_info:
>
>Here we see NullProvider.resource_listdir using [path]/EGG_INFO
>instead of [path]. This seems like a simple typo to me as there are
>resource_XXX and metadata_XXX methods, the first based on the module
>path and the second set based on the EGG_INFO dir.

Yeah, that's a typo.  :(


>@@ -703,7 +703,7 @@
>      def __init__(self, module):
>          DefaultProvider.__init__(self,module)
>          self.zipinfo = zipimport._zip_directory_cache [self.loader.archive]
>-        self.zip_pre = self.loader.archive+os.sep
>+        self.zip_pre = self.module_path + os.sep
>      def _short_name(self, path):
>          if path.startswith(self.zip_pre):
>
>This is where I was a bit shaky:
>
>     loader_archive = /path/to/the-egg.egg
>     module_path = /path/to/the-egg.egg/package
>
>Shouldn't resource loading be relative to the package directory and
>not the root of the egg?

zip_pre is used only to remove the front end of path strings in order to 
get them out of the zipimport module's zip directory cache.  It has to be 
loader.archive+os.sep, since that's how zipimport constructs its cache.  So 
this should not be changed.


>@@ -738,7 +738,7 @@
>      def _extract_resource(self, manager, resource_name):
>          if self.resource_isdir(resource_name):
>-            return self._extract_dir(resource_name)
>+            return self._extract_directory(manager, resource_name)
>          parts = resource_name.split('/')
>          zip_path = os.path.join(self.module_path, *parts)
>
>I believe this was a simple typo / refactoring-miss.

Yeah, I really need to get creative about how to unit test some of this 
stuff so I can get it back up to full test coverage.  :(

However, the refactorings I've just completed will make testing 
easier.  For example, I can now subclass PackageIndex and replace its 
open_url() method with a mock/stub for testing purposes.  Similarly, I can 
subclass easy_install and NullProvider to stub out methods for testing as well.

In the meantime, though, I'm definitely paying for my lack of upfront 
testing in several areas, as the entire system has grown from a few hundred 
to a few thousand lines of code.