-
-
Notifications
You must be signed in to change notification settings - Fork 652
Meson: automatically install into venv if activated #39954
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
Conversation
Documentation preview for this PR (built with commit 3f166c3; changes) is ready! 🎉 |
Personally I just use editable install and pip, but the change looks safe enough. (according to the documentation the default is To check: Is the documentation change still correct i.e. are the listed meson commands what pip uses under the hood after the change? |
Co-authored-by: user202729 <25191436+user202729@users.noreply.github.com>
By the way, what's this
pending status for this CI job I see across a number of PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I'll wait under docbuild finishes, to flip this to positive (something I can finally do myself)
After #39641 , test-new no longer exists. Not sure why GitHub still shows the check in the UI. |
It's a "required check". Someone of the @sagemath/core admins need to change the branch protection rules to no longer include the "test-new" workflow, but instead all the meson workflows. Thanks in advance! Thanks for the review! Flipping to positive review since the docbuild finished without error. |
|
||
.. CODE-BLOCK:: shell-session | ||
|
||
$ PYTHONPATH=/desired/install/path ./sage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you sure this actually works, in the previous version the PYTHONPATH
is set to …/site-packages
(I haven't tested either)
(there's also the alternative of removing the note entirely… does anyone need it?)
sagemathgh-39954: Meson: automatically install into venv if activated <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> Makes the meson experience a bit smoother by no longer requiring that a manual prefix is specified (it still can be specified if wanted). So `meson install` will just install into the current venv, the same as `pip install .`. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] The title is concise and informative. - [ ] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39954 Reported by: Tobias Diez Reviewer(s): Dima Pasechnik, user202729
Makes the meson experience a bit smoother by no longer requiring that a manual prefix is specified (it still can be specified if wanted). So
meson install
will just install into the current venv, the same aspip install .
.📝 Checklist
⌛ Dependencies