check-manifest icon indicating copy to clipboard operation
check-manifest copied to clipboard

checking via zest-releaser fails if setuptools is provided by a buildout

Open woutervh opened this issue 9 years ago • 9 comments

my use-case, I have a "tools"-buildout that installs zest.releaser, fabric, and check-manifest,... this buildout is bootstrapped from a clean python that deliberatetly has no setuptools installed, (so it can never conflict with a buildout-provided setuptools).

the check_manifest takes sys.executable as default python see https://github.com/mgedmin/check-manifest/blob/master/check_manifest.py#L572 while this is correct, the executable looses its sys.path,

A similar issue was fixed in zest.releaser, see https://github.com/zestsoftware/zest.releaser/issues/57 https://github.com/zestsoftware/zest.releaser/pull/58/files

as far as I see, the same solution ban be re-used.

woutervh avatar Nov 25 '14 18:11 woutervh

I don't quite understand the situation. Any chance for me to reproduce the issue?

mgedmin avatar Nov 25 '14 19:11 mgedmin

Steps to reproduce:

  1. cd /tmp && mkdir example && cd example
  2. virtualenv -p /usr/bin/python2.7 --no-setuptools /tmp/cleanenv
  3. wget http://downloads.buildout.org/2/bootstrap.py
  4. vi buildout.cfg:
[buildout]
parts = check-manifest

[check-manifest]
recipe = zc.recipe.egg
  1. /tmp/cleanenv/bin/python bootstrap.py
  2. bin/buildout
  3. bin/check-manifest ~/src/check-manifest

And here's the error:

building an sdist
['/tmp/cleanenv/bin/python', 'setup.py', 'sdist', '-d', '/tmp/check-manifest-nLHfR_-sdist'] failed (status 1):
Traceback (most recent call last):
  File "setup.py", line 3, in <module>
    from setuptools import setup
ImportError: No module named setuptools

mgedmin avatar Nov 30 '14 18:11 mgedmin

Actually the interesting zest.releaser commit is https://github.com/zestsoftware/zest.releaser/commit/48ab8433.

mgedmin avatar Nov 30 '14 18:11 mgedmin

So the thing is: check-manifest itself doesn't depend on setuptools. Therefore when you use it with zc.buildout, the bin/check-manifest script won't have setuptools added to sys.path. Therefore setting PYTHONPATH for the child process won't help.

It would likely help your use case, where check-manifest is invoked from zest.releaser, since zest.releaser depend on setuptools. I'd appreciate it if you could test this patch:

diff --git a/check_manifest.py b/check_manifest.py
index 75c251e..43773aa 100755
--- a/check_manifest.py
+++ b/check_manifest.py
@@ -127,9 +127,15 @@ def run(command, encoding=None):
     """
     if not encoding:
         encoding = locale.getpreferredencoding()
+    if command and command[0] == sys.executable:
+        # Workaround for zc.buildout, bootstrapped from a Python that lacks
+        # setuptools (see https://github.com/mgedmin/check-manifest/issues/35)
+        env = {'PYTHONPATH': os.pathsep.join(sys.path)}
+    else:
+        env = None
     try:
         pipe = subprocess.Popen(command, stdout=subprocess.PIPE,
-                                stderr=subprocess.STDOUT)
+                                stderr=subprocess.STDOUT, env=env)
     except OSError as e:
         raise Failure("could not run %s: %s" % (command, e))
     output = pipe.communicate()[0].decode(encoding)

In the end, I'm not sure what to do. The process of making source distributions is python setup.py sdist. If your setup.py needs setuptools, you should run it with a python that has setuptools. check-manifest lets you specify a python command to use with the -p command-line flag. In other words, I don't think there's need to add a workaround to the main check-manifest script. But the zest.releaser plugin, OTOH, has no way to supply a different Python command, and if this workaround fixes your use case, I'll commit it.

mgedmin avatar Nov 30 '14 19:11 mgedmin

@WouterVH any chance you can test this patch? I'd like to make a check-manifest release soon.

mgedmin avatar Dec 12 '14 06:12 mgedmin

I confirm this patch fixes the problem. Tested with python 2.7.10 and check_manifest 0.25

woutervh avatar Oct 30 '15 13:10 woutervh

Thank you!

mgedmin avatar Oct 30 '15 14:10 mgedmin

Hm. Should I also add a dependency on setuptools in my setup.py?

mgedmin avatar Oct 30 '15 16:10 mgedmin

I had to revert the zc.buildout support hack because it breaks other things -- see #56.

mgedmin avatar Nov 26 '15 06:11 mgedmin