-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Install meson.exe as the entrypoint on Windows #4004
Conversation
042e78d
to
ddf5855
Compare
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.
Looks good to me, and actually allows me to have usable meson after install on Windows, up until now I was always explicitly typing python3 C:/path/to/meson.py
, that was a bit cumbersome :)
mesonbuild/mesonmain.py
Outdated
if 'MESON_COMMAND_TESTS' in os.environ: | ||
mlog.log('meson_command is {!r}'.format(mesonlib.meson_command)) | ||
print('meson_command is {!r}'.format(mesonlib.meson_command)) |
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.
hrm, why would you switch to a print? If I understand correctly, mlog.log will jump through some extra hoops to avoid locale errors etc
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.
Hmm, good point. I thought it would be simpler because it's only used in tests, but then the tests might fail on certain locales.
How strange. |
Thanks to Rafael Rivera for the suggestion Fixes #1877
c7bd268
to
2ee2802
Compare
This gives us a consistent experience and a simpler setup across all operating systems. Setuptools is available everywhere these days.
So I nuked distutils and now we require setuptools. Do we need to support distutils for some platforms or is this ok? I know for a fact that macOS and Windows Python installers ship with setuptools, and on MSYS2 you can install it with pacman. That leaves Linux, and Fedora ships with setuptools by default. |
There are so many things depending on setuptools for install that I don't think this is going to be a problem. |
Do not ever touch dead distutils :) nothing in this world works without setuptools |
I have no idea why we try to support both distutils and setuptools. The commit message which added setuptools doesn't say:
and the initial |
Setuptools is not in the Python standard library, only distutils is so our requirement for not depending on anything outside of the standard library comes into play here. Also, setuptools produced broken files with |
That requirement is for running meson, though. For instance, we depend on hotdoc for our documentation.
We have tests for this now, although we don't check the complete files list in it yet. |
Afair the generated entry point scripts depend on pkg_resources |
run_meson_command_tests.py
Outdated
@@ -19,6 +19,7 @@ | |||
import unittest | |||
import subprocess | |||
import zipapp | |||
from glob import glob |
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.
[Flake8]
[F401] 'glob.glob' imported but unused
b225f25
to
f6836f2
Compare
run_meson_command_tests.py
Outdated
self.assertTrue(bindir.is_dir()) | ||
self.assertTrue(pylibdir.is_dir()) | ||
from setup import packages |
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.
[Flake8]
[E271] multiple spaces after keyword
Ensure that the installed files list matches what we expect, to avoid surprises at release time.
f6836f2
to
c0413f5
Compare
I've added tests that verify the files list. Looks fine. Can you run it on Debian to test? |
That module is shipped with setuptools, right? |
I tested this branch with Debian Sid's setuptools and the result was working just fine. Whatever the issue was it seems to be gone now. |
I think we should merge this so we have good Windows support out-of-the-box. |
When will this patch be released? I don't see it in the latest 0.47.2 as currently available from PyPI. |
This will be in the 0.48 release. You can find a full list of changes that went into 0.47.2 at https://github.com/mesonbuild/meson/milestone/29?closed=1. |
However, you can use it with git master by doing |
thanks! |
@jpakkane I guess this is where https://bugs.debian.org/909440 came from? |
Repeating what I said on IRC, pkg-resources is a part of setuptools, and we require setuptools now. No idea why Debian splits that out to its own package and adds it as a dependency to setuptools, but simply depending on setuptools in Debian should be enough. |
@nirbheek The comment in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=909440#10 says that meson is not supposed to depend on anything outside of python3. If python(3)-setuptools is required, then such a dependency would need to be added to the package. |
See above, we discussed this requirement before committing this. I commented here to state that @jpakkane has forgotten and commented mistakenly on the Debian bug. |
Ah ok. Regarding your remark about "no idea why Debian splits that out to its own package", I only found:
Looking at python3-pkg-resources, it weighs 535K, |
Ah, so setuptools is supposed to be a build-time dependency and pkg-resources a runtime dependency. We don't need the runtime dependency in Meson though. |
I think it's the other way around: meson needs only pkg-resources as a runtime dependency, or am I completely confused? |
Meson doesn't really need the pkg-resources runtime dependency on Linux since we just need to call We could quite easily remove the dependency by using |
Well, there is
in /usr/bin/meson, so I guess meson does need pkg-resources at runtime |
Yes, and that dependency comes from our usage of setuptools |
@nirbheek this breaks for me in msys2 https://github.com/Alexpux/MINGW-packages/issues/4659 @lazka any ideas? |
Thanks to Rafael Rivera for the suggestion
Fixes #1877