Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
BLD Add Meson support #28040
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
BLD Add Meson support #28040
Changes from all commits
819fdea
5991c28
ebc8417
aa79db7
85b9609
9c4918c
1041a03
c9a951e
b8d172f
6f71330
efb86eb
25af17e
7ac46d2
04b1da7
c69dc5a
2657f15
4ce60b3
f6c6b3e
f85ff96
5c1cecf
091e337
c414a79
5dd266d
f715560
ba20258
9f5ccd0
9f1dcf9
bb0bf24
fffde7c
a2ec96b
d63f123
2f0572a
b848aa2
bd16b40
72c0667
b4181ad
01feca3
caa8835
c280ec4
edd91af
0cd6bf2
1bd626d
ed5f415
95ac135
c6d4aee
317884e
8556034
08baec4
b1dcd08
4cd9b57
5c34498
7d1beef
5ad4da7
2f730eb
8bef673
232fca4
b9008ce
256dcf5
fd8dd5a
f659cbe
2d63ae6
bff0012
bd8734f
8cf804d
9e11497
90aa8bc
9887507
9c3bc25
90c51f9
5119d1b
39bd6db
7005c44
7eceb5c
1d72f3f
015ea5b
252fb93
6f00d82
289ed43
1121cbd
1137360
7382c1f
0869a8d
2c2463c
5ca5a6a
ae23cdd
b91bfca
4d66a68
dd8a0d2
932e2e4
14fd52c
2693724
22c505d
b6bd9d1
1e0c921
fd2c2fd
3405310
f70f0da
3f8e979
1c9c277
aa30cdb
80137fa
ef148f2
8f30885
182f1c5
2754b6d
3c7db36
d9d9142
765e2c6
908472f
56791a9
ad27f92
c2bbd12
54ce5f6
ffc688a
6043f36
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
clean-meson
does not clean the build artifacts obtained viadev-meson
.Do we want to introduce this semantic or not?
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.
Good point, I would keep it like this for now. My main goal with
make clean-meson
is to be able to switch back to work with setuptools easily and possibly back again with Meson. Keeping Meson build artifacts is useful for the latter.If you want a full clean,
make clean
will remove thebuild
folder which contain Meson build artifacts (unless you make the effort to pass a different build directory with a complicated command likepip install -e . -Csetup-args=...
)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.
Meson, unlike setuptools, is very safe here. It does a very good job at marking artifacts as stale and rebuilding them when needed. You don't generally need to manually clean it, because you can just re-run
ninja
and keep the good artifacts and regenerate the outdated artifacts.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.
so this does an editable install. How does one installs in non-editable mode?
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.
I would argue that for now editable command is the way to go. As I mentioned a few times, the idea is to have maintainers include Meson in their day-to-day work and improve/tweak things as we go.
Do you have a specific reason in mind, why you would want to use non-editable installs? One way is to edit
pyproject.toml
setbuild-backend
and use thepip
command without--editable
.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.
The idea is to see how the workflow works for us, that includes:
make
I'm quite uncomfortable with having a python file calling pip. Also, I would like to separate build from install. We should have the equivalent of
pip install -e . --no-build-isolation
as well asmake in
, which do very different things. The former including the latter as a step. Also, amake clean
shouldn't uninstall the package.Exactly because the idea is to check our day-to-day work, I'm not comfortable with the current solution.
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.
Maybe I was not explicit enough about this and/or it may have been lost in the many comments above. This is the way I see things unfolding (happy to tweak things according to feed-back):
make dev-meson
and report issues so that we can improve the situation if needed. During this transition period, non-maintainers are not expected to try out Meson. Maintainers that want to keep using setuptools are completely free to do so (i.e. usemake in
,pip install -e . --no-use-pep517 --no-build-isolation
orpython setup.py develop
)build-backend = "mesonpy"
inpyproject.toml
. This removes the need frombuild_tools/build-meson-editable-install.py
(I guess this is what you mean by by "a python file calling pip"). This file main goal is to conveniently build with Meson during the transition period.setuptools
if you want (viamake in
orpython setup.py develop
).setup.py
and Meson is our only way to build scikit-learnHopefully, this alleviates some of your concerns 🤞!
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.
Did we take this file from another repository?
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.
I see this is also in NumPy, we could have a header mentioning which version it was taken and the location in case we just want to make some update time to time.
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.
It is indeed taken from a repo (maybe scipy?) and adapted slightly. For example our template files ends with
.tp
and other projects seem to use.in
. IIRC I also had some encoding issues when I tried testing on Windows withtempita.Template.from_filename
.I would say this is unlikely to change much, this is mostly doing creating a
tempita.Template
from a file and calling.substitute
on it.