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

python module: produce the correct install path on every OS #9156

Merged
merged 1 commit into from
Aug 27, 2021

Conversation

eli-schwartz
Copy link
Member

@eli-schwartz eli-schwartz commented Aug 23, 2021

The sysconfig paths are, by default, correct for every OS -- they are supposed to follow the scheme that python knows about per default.

For some reason, this overrode the scheme to posix_prefix, which is the default for posix OSes like linux and macOS, but wrong on Windows. Simply deleting this entirely makes everything that used to work, still work, and a couple new things start working.

@MathieuDuponchelle I'm seeking insight into why this originally got implemented the way it did. Do you happen to remember?

Fixes #3995

The sysconfig paths are, by default, correct for every OS -- they are
supposed to follow the scheme that python knows about per default.

For some reason, this overrode the scheme to posix_prefix, which is the
default for posix OSes like linux and macOS, but wrong on Windows.
Simply deleting this entirely makes everything that used to work, still
work, and a couple new things start working.
@codecov
Copy link

codecov bot commented Aug 23, 2021

Codecov Report

Merging #9156 (688f176) into master (0063eb2) will decrease coverage by 3.17%.
The diff coverage is n/a.

❗ Current head 688f176 differs from pull request most recent head f87ada2. Consider uploading reports for the commit f87ada2 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9156      +/-   ##
==========================================
- Coverage   66.69%   63.52%   -3.18%     
==========================================
  Files         378      189     -189     
  Lines       84510    42192   -42318     
  Branches    17491     8733    -8758     
==========================================
- Hits        56368    26802   -29566     
+ Misses      23349    13038   -10311     
+ Partials     4793     2352    -2441     
Impacted Files Coverage Δ
mesonbuild/compilers/mixins/clang.py
mesonbuild/dependencies/hdf5.py
mesonbuild/backend/backends.py
mesonbuild/interpreter/compiler.py
mesonbuild/interpreter/__init__.py
mesonbuild/cmake/generator.py
mesonbuild/interpreter/interpreterobjects.py
mesonbuild/compilers/mixins/gnu.py
mesonbuild/modules/unstable_simd.py
mesonbuild/dependencies/cmake.py
... and 179 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0063eb2...f87ada2. Read the comment docs.

@xclaesse
Copy link
Member

I'm wondering if we should go one step further on Windows. By default sys.path has C:\Python38\lib\site-packages but with this PR meson is going to install into <prefix>\lib\site-packages which by default is C:\lib\site-packages. Note sure what should be the best heuristic here, if meson prefix is the root of the same drive where python is installed, then we use the platlib without overriding 'base': ''?

@xclaesse
Copy link
Member

otoh, this is always going to be at best a guess. Maybe what we really need is adding an option, we discussed recently we should allow modules to add their own options. Currently every project that install python modules pretty much have to add their own option which makes it inconsistent.

@MathieuDuponchelle
Copy link
Contributor

@eli-schwartz I remember writing this specific line, but I really don't remember the reason, should have added an inline comment probably :(

@eli-schwartz
Copy link
Member Author

Forgot that this isn't merged yet... Note this has been discussed previously in #3995 which this would fix.

@eli-schwartz eli-schwartz added the modules:python Issues specific to the python module label Aug 27, 2021
@xclaesse xclaesse merged commit 6170f11 into mesonbuild:master Aug 27, 2021
@eli-schwartz eli-schwartz deleted the python-install-scheme branch August 27, 2021 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modules:python Issues specific to the python module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

install_sources installs python sources into wrong directory on windows
3 participants