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

cmake invocation shouldn't install dependencies #363

Closed
jagerman opened this issue Aug 26, 2016 · 2 comments
Closed

cmake invocation shouldn't install dependencies #363

jagerman opened this issue Aug 26, 2016 · 2 comments

Comments

@jagerman
Copy link
Member

(@dean0x7d)

One of the changes in PR #321 (18319d5) invokes pip during cmake to install pytest if it isn't found, but this seems conceptually wrong: a cmake invocation really shouldn't be modifying anything outside the project dir.

I propose that this should be changed to produce a fatal error if pytest isn't found (perhaps with a message to invoke pip install pytest or pip install --user pytest), or at the very least protected behind a disabled-by-default PYBIND11_INSTALL_DEPS option. Otherwise we're installing something outside of the pybind11 directory without asking just because someone decides to configure the pybind11 test code build.

(As a side note, even with the option approach, I think the debian builds should go back to install python$PY-pytest instead of python$PY-pip python$PY-setuptools—part of the point of those builds was to install all deps via native system packages rather than via pip; it also ensures that everything works as expected without pip/setuptools available.)

@aldanor
Copy link
Member

aldanor commented Aug 26, 2016

Agreed, I was surprised by this as well. I'd probably prefer it to just fail if py.test executable is not found (or if ${PYTHON} -m pytest --version fails).

@dean0x7d
Copy link
Member

a cmake invocation really shouldn't be modifying anything outside the project dir.

True. In trying to make it more convenient, it may have become too intrusive. I guess failing with a nice error message should be the way to go then. And yeah, at that point the docker build on Travis should switch to the system pytest package.

Slightly off-topic: pip and setuptools are bundled with Python but Linux distributions specifically remove them from their Python packages. Seeing a pure-Python package like pytest as a system package is baffling. System packages used to be a necessary evil for difficult to install stuff like numpy and scipy, but having the entire PyPI catalogue recreated with every distribution's package manager seems counterproductive.

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

No branches or pull requests

3 participants