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

Investigate whether we can easily work with editable installs #152

Closed
stefanv opened this issue Jan 12, 2024 · 6 comments
Closed

Investigate whether we can easily work with editable installs #152

stefanv opened this issue Jan 12, 2024 · 6 comments

Comments

@stefanv
Copy link
Member

stefanv commented Jan 12, 2024

As per #150 (comment):

Since we now have a way to detect editable installs, we could potentially switch build and PYTHONPATH-setting to no-ops if the editable matches the current directory.

@lagru
Copy link
Member

lagru commented Jan 12, 2024

That would be very useful. 👍

@stefanv
Copy link
Member Author

stefanv commented Mar 21, 2024

xref #155

@oscarbenjamin
Copy link

I'm only recently trying spin out but I wonder if it really makes sense for spin to use editable installs. It seems to me that spin and editable installs both solve the same problems in different ways. For example it is nice that with an editable install you can just run pytest without needing to explicitly build/install and benefit from incremental rebuilds but spin test gives the same benefits in a better more controllable way.

It is certainly nice to be able to have editable installs and I am sure that I would still want to use them even if I am mostly using spin. That doesn't mean that I need spin to use my editable install rather than using its own build though.

Perhaps a different take on this is that it would be nice if spin was compatible with an editable install even if spin itself always uses its own build. Currently some spin commands don't work correctly if you also have an editable install e.g.:

$ PKG_CONFIG_PATH=$(pwd)/.local/lib/pkgconfig spin install
$ spin build
$ pytest --pyargs flint.test  # test using editable install
======================================== test session starts ========================================
platform linux -- Python 3.12.0, pytest-7.4.3, pluggy-1.3.0
rootdir: /home/oscar/current/active/python-flint
plugins: doctestplus-1.2.1, cov-4.1.0, hypothesis-6.88.4, xdist-3.4.0
collected 49 items                                                                                  

src/flint/test/test_all.py .................................................                  [100%]

======================================== 49 passed in 0.81s =========================================
$ LD_LIBRARY_PATH=$(pwd)/.local/lib spin test # test using spin's build
...
============================================== ERRORS ===============================================
______________________________ ERROR collecting flint/test/test_all.py ______________________________
import file mismatch:
imported module 'flint.test.test_all' has this __file__ attribute:
  /home/oscar/current/active/python-flint/src/flint/test/test_all.py
which is not the same as the test file we want to collect:
  /home/oscar/current/active/python-flint/build-install/usr/lib/python3.12/site-packages/flint/test/test_all.py
HINT: remove __pycache__ / .pyc files and/or use a unique basename for your test file modules
====================================== short test summary info ======================================
ERROR flint/test/test_all.py
$ pip uninstall python-flint -y
$ LD_LIBRARY_PATH=$(pwd)/.local/lib spin test # test spin without editable install
...
flint/test/test_all.py .................................................                      [100%]

======================================== 49 passed in 3.94s =========================================

Maybe what would be best is if commands like spin test could deliberately ignore the editable install and always use the spin build. Maybe even it would make most sense for commands like spin test to actually run the tests in an isolated venv rather than patching the build-install directory into spin's own Python environment with PYTHONPATH. One of the advantages of using a wrapper like spin test rather than invoking pytest directly is that it makes it possible to do things like automagically manage such an environment.

The only advantage as I see it of spin using the editable install is that the editable install and the spin commands benefit from shared incremental rebuilds. Maybe though there is a different way to get that advantage anyway like if spin build used the same build directory as the editable install (I don't know if that would cause lots of problems...).

@oscarbenjamin
Copy link

It would also be good to change this help text to more clearly clarify the difference:

$ spin --help
...
Build:
  build    🔧 Build package with Meson/ninja and install
  install  💽 Build and install package using pip.

The first of these does not "install" the package in the usual sense of the installing a python package. The second creates an editable install. The help text makes it seem as if these both do the same thing ("build and install package") but just using different tools.

@stefanv
Copy link
Member Author

stefanv commented May 6, 2024

@oscarbenjamin I think you are right that this documentation is confusing. I'll open an issue.

I hope we struck a good balance with the editable install functionality currently included. It allows you to do things like run a doc build etc. with the editable install, but while still supporting the traditional meson workflow (which many of us prefer).

@stefanv
Copy link
Member Author

stefanv commented May 6, 2024

With editable install functionality out there for testing, I'll close this for now, and we can open issues for specific behaviors.

In the scoping of spin, we decided not to deal with environments. This is because most of the developers of the tool liked being able to set up their environments, and to then build using no-isolation. Of course, nothing prevents a developer from using tox or nox to set things up differently! I think that sufficiently covers both cases, but we can reconsider if there are good reasons why spin should become environment-aware.

@stefanv stefanv closed this as completed May 6, 2024
stefanv added a commit to stefanv/spin that referenced this issue May 8, 2024
jarrodmillman pushed a commit that referenced this issue May 10, 2024
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