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

grpython use posix prefix scheme #5739

Merged
merged 1 commit into from
Apr 26, 2022

Conversation

maitbot
Copy link
Contributor

@maitbot maitbot commented Apr 2, 2022

Recently in Debian's pthon3.10 the default scheme for sysconfig
changed from 'posix_prefix' to 'posix_local'. GrPython.cmake
expects the 'posix_prefix' behavior. So explicity use it.

Signed-off-by: A. Maitland Bottoms bottoms@debian.org


name: grpython use posix prefix scheme
about: GrPython.cmake
title: ''
labels: ''
assignees: ''


Pull Request Details

Description

Install path changed to /usr/local with python3.10 - even though the desire
is to set the install directories using gnuradio's cmake paths.

Related Issue

Debian bug:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1008854

Which blocks/areas does this affect?

Not only gnuradio itself - but every OOT project that inherits GrPython.cmake.

Testing Done

Debian builds as part of the python3.10-default transition.

Checklist

Recently in Debian's pthon3.10 the default scheme for sysconfig
changed from 'posix_prefix' to 'posix_local'. GrPython.cmake
expects the 'posix_prefix' behavior. So explicity use it.

Signed-off-by: A. Maitland Bottoms <bottoms@debian.org>
@willcode willcode added CMake Backport-3.10 Should be backported to 3.10 labels Apr 2, 2022
Copy link
Member

@willcode willcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since prefix_posix is the default scheme
https://docs.python.org/3/library/sysconfig.html#installation-paths
and we are not currently specifying a scheme, this seems safe.
@ryanvolz can you verify that this is ok for Windows, since there are separate schemes listed for nt.

@willcode willcode requested a review from ryanvolz April 2, 2022 23:09
@ryanvolz
Copy link
Contributor

ryanvolz commented Apr 3, 2022

Hmm, clearly there are no issues with the conda builds because the recipe sets GR_PYTHON_PATH (and that's still true for the conda-forge packages), so I don't expect to personally see any problems because of this change. But I do wonder about other theoretical Windows users since this does change the default scheme from "nt" to "posix_prefix" in that case. (It looks like there will be a new scheme option with Python 3.11 that would do what @mait wants but also keep the nt scheme on Windows, "venv", but obviously it would be a while until we could use that.) Maybe a ternary expression like "posix_prefix" if os.name != "nt" else "nt" would result in minimal change for users?

@drmpeg
Copy link
Member

drmpeg commented Apr 22, 2022

Since Ubuntu 22.04 is using Python 3.10, this patch is pretty much necessary now. Otherwise, the PYTHONPATH (<install_prefix>/local/lib/python3.10/dist-packages) is dramatically different than before.

Copy link
Member

@drmpeg drmpeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on Ubuntu 22.04.

@mormj
Copy link
Contributor

mormj commented Apr 26, 2022

Let's go ahead and merge this on main but hold off on backport, given the Windows concerns.

@mormj mormj merged commit 0d60e80 into gnuradio:main Apr 26, 2022
@willcode willcode added the Hold Backport Hold off on backport label Apr 29, 2022
@willcode willcode removed the Hold Backport Hold off on backport label May 19, 2022
@willcode willcode added ported-to-3.10 Has been ported to 3.10 and removed Backport-3.10 Should be backported to 3.10 labels May 19, 2022
@maitbot maitbot deleted the mait/grpython-use-posix-prefix-scheme branch October 9, 2023 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake ported-to-3.10 Has been ported to 3.10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants