cript0nauta / pynixify

Nix expression generator for Python projects
GNU General Public License v3.0
66 stars 11 forks source link

Add support to detect setup.cfg #31

Closed llazzaro closed 4 years ago

llazzaro commented 4 years ago

Some python packages uses empty setup() and all dependencies are saved on setup.cfg file.

This commit updates the setuptools v44.0.0 patch for saving deps on .txt files

cript0nauta commented 4 years ago

To facilitate review, here is the diff between setuptools with the original patch applied and setuptools with this new patch applied:

diff --git a/setuptools/__init__.py b/setuptools/__init__.py
index 400952d9..3dbb72bb 100644
--- a/setuptools/__init__.py
+++ b/setuptools/__init__.py
@@ -135,11 +135,6 @@ def _install_setup_requires(attrs):
     ))
     # Honor setup.cfg's options.
     dist.parse_config_files(ignore_option_errors=True)
-    if dist.setup_requires:
-        dist.fetch_build_eggs(dist.setup_requires)
-
-
-def setup( **attrs):
     if 'PYPI2NIXKPGS' in os.environ:
         from pathlib import Path
         try:
@@ -147,9 +142,11 @@ def setup( **attrs):
         except KeyError:
             print("out environment variable not defined")
             sys.exit(1)
+        setup_requires = dist.setup_requires
+        install_requires = dist.install_requires
         targets = [
-            ('setup_requires.txt', attrs.get('setup_requires', [])),
-            ('install_requires.txt', attrs.get('install_requires', [])),
+            ('setup_requires.txt', attrs.get('setup_requires', setup_requires)),
+            ('install_requires.txt', attrs.get('install_requires', install_requires)),
             ('tests_requires.txt', attrs.get('tests_require', [])),
         ]
         for (filename, requirements) in targets:
@@ -170,10 +167,15 @@ def setup( **attrs):
             meta[meta_attr] = attrs.get(meta_attr)
         with (out / 'meta.json').open('w') as fp:
             json.dump(meta, fp)
-
     else:
-        # Make sure we have any requirements needed to interpret 'attrs'.
-        _install_setup_requires(attrs)
+        if dist.setup_requires:
+            dist.fetch_build_eggs(dist.setup_requires)
+
+
+def setup( **attrs):
+    # Make sure we have any requirements needed to interpret 'attrs'.
+    _install_setup_requires(attrs)
+    if 'PYPI2NIXKPGS' not in os.environ:
         return distutils.core.setup(**attrs)

 setup.__doc__ = distutils.core.setup.__doc__
cript0nauta commented 4 years ago

Thanks for contributing! Patching the internal _install_setup_requires function seems a bit risky. But this whole setuptools patching phase is risky by itself, with or without this change. So the change looks good to me.

I'll be testing this change in order to confirm it doesn't produce any regressions, and that the patch works with all supported setuptools version. In case this succeeds, the change is ready to be merged.