Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setupy: mixed up test and setup runtime deps #73

Merged
merged 1 commit into from
Sep 20, 2020

Conversation

blshkv
Copy link
Contributor

@blshkv blshkv commented Sep 14, 2020

No description provided.

@jfhbrook jfhbrook merged commit 9a8d55a into jfhbrook:master Sep 20, 2020
@blshkv
Copy link
Contributor Author

blshkv commented Sep 21, 2020

so you fixed it and broke 5 min later in 71fbfa9.
Can you review it again? pytest-runner is NOT required during run-time and should be in "test" section.
Thanks

@blshkv
Copy link
Contributor Author

blshkv commented Sep 21, 2020

@blshkv
Copy link
Contributor Author

blshkv commented Sep 21, 2020

FYI, Using setup_requires is discouraged in favor of PEP-518

install_requires should be probably used instead.

@jfhbrook
Copy link
Owner

I put it in the setup_requires section because what I saw was the install barfing because the test command wasn't working. The setup.py needs to actually run regardless of whether the tests themselves are executed.

I'm not going to pull a patch. I'm sorry, I know that all the cool kids hate github pull requests but they're what I know and they're what my current tooling works with. If you want this changed, send a pull request, include documentation of why you moved each thing, and a command I can run to prove to myself that it is OK.

Happy to have someone else use 518, but I'm unlikely to do that work myself unless and until it breaks.

@jfhbrook
Copy link
Owner

Your patch also moves twisted, which is not required to run the setup. It's only a dependency for tests. If you're using it with twisted, you're expected to install it yourself. There's probably a way to mark it as an optional dependency but I haven't messed with it.

@blshkv
Copy link
Contributor Author

blshkv commented Sep 21, 2020

I'm too lazy and fine with my patch. Without it, I'm getting the following:

>>> Compiling source in /var/tmp/portage/dev-python/pyee-8.0.1/work/pyee-8.0.1 ...
 * python3_7: running distutils-r1_run_phase distutils-r1_python_compile
python3.7 setup.py build -j 4
WARNING: The pip package is not available, falling back to EasyInstall for handling setup_requires/test_requires; this is deprecated and will be removed in a future version.
Traceback (most recent call last):
  File "/usr/lib/python3.7/site-packages/setuptools/installer.py", line 62, in fetch_build_egg
    pkg_resources.get_distribution('pip')
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 481, in get_distribution
    dist = get_provider(dist)
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 357, in get_provider
    return working_set.find(moduleOrReq) or require(str(moduleOrReq))[0]
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 900, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 786, in resolve
    raise DistributionNotFound(req, requirers)
pkg_resources.DistributionNotFound: The 'pip' distribution was not found and is required by the application

@jfhbrook
Copy link
Owner

My guess is that setuptools requires pip, and that installing twisted incidentally installs pip.

@jfhbrook
Copy link
Owner

The patch you probably want to make is adding setuptools to setup_requires.

@blshkv
Copy link
Contributor Author

blshkv commented Sep 21, 2020

the thing is, I'm not using pip. At gentoo, we have our own packet manager.
I also have twisted (https://pypi.org/project/Twisted/) installed which is probably used by this project (but you should know it better).

So far, it looks like that pytest-runner pulls pip. So the patch fixes it.

@blshkv
Copy link
Contributor Author

blshkv commented Sep 21, 2020

btw, I dont see how vcversioner used in the project. Not in the release at least.

@jfhbrook
Copy link
Owner

vcversioner is used when creating bundles, it's the thing that's familiar with the version.

You don't use pip to manage your packages, but pip is part of the tooling that the setup.py uses. You can see that your build is using easy_install instead of pip, which is, trust me, not what you want. I haven't worked with gentoo, but if it's anything like arch MAKEPKGs you use pip while creating the package and then that gets installed by the gentoo analog to pacman.

@blshkv
Copy link
Contributor Author

blshkv commented Sep 21, 2020

as I said, we don't use pip and neither setup.py does (or should not at least). From the log above, you can see that our ebuild runs "python setup.py build" and "python setup.py install".

@jfhbrook
Copy link
Owner

It's extremely unlikely that portage intentionally uses easy_install. You probably never run "pip install" on your gentoo system - I certainly wouldn't recommend it - but the standard toolchain for managing python dependencies is pip, regardless of whether you're resolving dependencies or not. It certainly looks like setuptools requires pip, and setuptools is the implementation of setup() I'm using. Either way, you are definitely installing pip on accident with one of these other dep changes - that's what's happening, regardless of whether gentoo does or doesn't use pip.

@blshkv
Copy link
Contributor Author

blshkv commented Sep 21, 2020

I have done some testing. It's vcversioner which cases the main issue:

  File "/usr/lib/python3.7/site-packages/setuptools/command/easy_install.py", line 1159, in run_setup
    raise DistutilsError("Setup script exited with %s" % (v.args[0],))
distutils.errors.DistutilsError: Setup script exited with error: SandboxViolation: open('/var/tmp/portage/dev-python/pyee-8.0.1/work/pyee-8.0.1-python3_7/lib/vcversioner.py', 'wb') {}

@blshkv blshkv mentioned this pull request Dec 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants