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

builder.sdist: remove setuptools import #24

Closed
wants to merge 2 commits into from

Conversation

abn
Copy link
Member

@abn abn commented Apr 14, 2020

It appears we do not really need setuptools for now. The use of setuptools was preventing pip from installing poetry correctly for PEP-517 builds when --no-binary :all: was being used. Additionally, now we can install poetry-core from sdist as expected.

Resolves: python-poetry/poetry#454

@abn
Copy link
Member Author

abn commented Apr 14, 2020

@sdispater was there a reason why we needed setuptools? I can see you made this change a while ago.

python-poetry/poetry@f759876

@abn abn requested review from sdispater and removed request for a team April 15, 2020 21:08
@sdispater
Copy link
Member

@abn from what I recall, distutils is deprecated and the setup() of situtils does not support some keyword arguments that setuptools supports (like extras_require, for instance)

@abn
Copy link
Member Author

abn commented Apr 17, 2020

This might mean we need to specify setuptools as a dependency, since otherwise pep517 builds are failing unless the build-system.requires explicitly contains setuptools. At least until we can remove the need for the setup file. This might, however, mean we will end up re-implementing some logic from setuptools.

@abn
Copy link
Member Author

abn commented Apr 17, 2020

It looks like we might need to handle the issue some other way. The issue can only be reproduced if pip is called with --no-binary :all: for either poetry-core inteself or another package that uses it as the backend.

    Using cached https://files.pythonhosted.org/packages/b7/70/5c287bd1e7c8c86a318755514e02c6d431335de541c43eef65d0fb5af955/poetry-core-1.0.0a5.tar.gz
    Installing build dependencies: started
    Installing build dependencies: finished with status 'done'
    Getting requirements to build wheel: started
    Getting requirements to build wheel: finished with status 'done'
      Preparing wheel metadata: started
      Preparing wheel metadata: finished with status 'done'
  Skipping bdist_wheel for poetry-core, due to binaries being disabled for it.
  Installing collected packages: poetry-core
    Running setup.py install for poetry-core: started
      Running setup.py install for poetry-core: finished with status 'error'
      ERROR: Complete output from command /tmp/.venv/bin/python -u -c 'import setuptools, tokenize;__file__='"'"'/tmp/pip-install-uupqj3gr/poetry-core/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' install --record /tmp/pip-record-5zbpsbn6/install-record.txt --single-version-externally-managed --prefix /tmp/pip-build-env-2ulvu7lx/overlay --compile --install-headers /tmp/.venv/include/site/python3.7/poetry-core:
      ERROR: Traceback (most recent call last):
        File "<string>", line 1, in <module>
      ModuleNotFoundError: No module named 'setuptools'
      ----------------------------------------
  ERROR: Command "/tmp/.venv/bin/python -u -c 'import setuptools, tokenize;__file__='"'"'/tmp/pip-install-uupqj3gr/poetry-core/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' install --record /tmp/pip-record-5zbpsbn6/install-record.txt --single-version-externally-managed --prefix /tmp/pip-build-env-2ulvu7lx/overlay --compile --install-headers /tmp/.venv/include/site/python3.7/poetry-core" failed with error code 1 in /tmp/pip-install-uupqj3gr/poetry-core/

Related: https://discuss.python.org/t/pep-517-backend-bootstrapping/789/3

@abn abn marked this pull request as draft April 17, 2020 20:02
@abn
Copy link
Member Author

abn commented Apr 17, 2020

Closing this one for now, since the issue seems to be only present for an older version of pip in the environment.

@abn abn closed this Apr 17, 2020
@gweis
Copy link

gweis commented Apr 23, 2020

@abn from what I recall, distutils is deprecated and the setup() of situtils does not support some keyword arguments that setuptools supports (like extras_require, for instance)

@sdispater Sorry for the hijack.
I am curious to find out where distutils has been deprecated. https://docs.python.org/3/distutils/apiref.html# ... Seems to me it's still a big part of python standard lib. even setuptools is recommended. (stumbled upon this thread after hitting a wall pip installing this pkg ;( )

@abn
Copy link
Member Author

abn commented Apr 23, 2020

@gweis i do not think distutils is deprecated. Direct use is simply discouraged.

The setuptools project adds new capabilities to the setup function and other APIs, makes the API consistent across different Python versions, and is hence recommended over using distutils directly.

@abn abn deleted the remove-setuptools branch April 23, 2020 22:03
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.

ValueError/TypeError with path dependency
3 participants