-
-
Notifications
You must be signed in to change notification settings - Fork 32.7k
bpo-1294959 - better support for systems with /usr/lib64 #3698
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
didn't add a NEWS entry because I don't know how to do it ... is there a script that generates the file names for the NEWS.d structure? Or can I just drop a file called "bpo-1294959.txt" there? |
@matejcik You will have to use blurb for the NEWS entry: https://pypi.python.org/pypi/blurb |
In Fedora, we patch Python to support lib64 as follows: https://src.fedoraproject.org/rpms/python3/blob/master/f/00102-lib64.patch (only applied on 64bit builds) This is only a note, I haven't looked at this PR more closely just yet, I'll try to compare the patches as soon as I can, to see if this PR would enable us to drop both of them. |
Apparently, replacing those 2 patches with this one builds and tests fine. https://koji.fedoraproject.org/koji/taskinfo?taskID=22047740 |
news entry added |
What can we do to make this happen? |
I am a core dev now and I can commit to pushing this through. |
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.
General comment: I wonder if it would make sense to make use of ax_set_default_paths_system.m4 as well
# platsubdir must be defined before LIBPL definition | ||
AC_MSG_CHECKING(for custom platsubdir) | ||
AC_ARG_WITH(custom-platsubdir, | ||
[AS_HELP_STRING([--with-custom-platsubdir=<libdirname>], |
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.
instead of --with-custom...
just use --with
. Also this is more commonly called --with-libdir
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 whole point of the PR is that this is distinct from (how Python uses) libdir
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.
okay, I've finally had a chance to re-read the original bug and this code. I'm not the biggest fan of introducing Yet Another variable and would prefer to fix how libdir gets used, but that seems like a bigger and more invasive change. Lets at least remove the custom
portion of the name.
While I likely won't get to this for some time, it might be worth a more general conversation about what tunables we offer in configure, what they mean, and how they get used.
@@ -272,8 +273,19 @@ def test_user_similar(self): | |||
# before comparing | |||
global_path = global_path.replace(sys.base_prefix, sys.prefix) | |||
base = base.replace(sys.base_prefix, sys.prefix) | |||
|
|||
if platsubdir != "lib": |
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 don't like how we're special casing "lib" here. I'd prefer to always take the same code path and use some special token that gets replaced.
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.
this is the same thing the code did before though.
i suppose we could introduce puresubdir
as the other option and prefill it to lib
somewhere, is that what you're suggesting?
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.
hm, no, i misread which part of the code you're commenting. sorry.
@@ -0,0 +1,3 @@ | |||
On 64bit Linux systems that use ``$prefix/lib64``, this is now the default |
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.
This is not specific to Linux.
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.
Unix then?
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.
Unix-like?
@matejcik: I'm now interested to review your PR, but first I would like to see conflicts resolved. Would you like to fix conflicts? Or do you prefer that I create a new PR based on your PR? |
This introduces a new configure and sysconfig variable `platsubdir`, which is the name of subdirectory of $prefix that holds platform-specific libraries. On many Linux distributions, there is separation between /usr/lib and /usr/lib64, which was causing some problems due to the way Python manages its install paths. Now, platsubdir is used to select the appropriate lib directory.
news entry news entry
@vstinner thanks, I fixed the basic conflicts. Unfortunately, I see that the code (esp. in Modules/getpath.c) has drifted significantly, and this is no longer part of my day job. So I probably won't devote much time in getting this where it needs to go. Maybe @hroncok would be interested in taking over, or perhaps @mcepl ? |
Not me. I can help @vstinner get this done and tested in Fedora, but that's it, sorry. |
I can start working on it, should be having some more time soon. |
Opened #11755 in order to move this forward. |
Thank you to everyone for your work on this. I'm going to close this in favor of #11755 since that (at the time) was a rebase over master. If this one ends up being the better pull request to move the project forward, then this can be reopened. |
PR #11755 is an updated version of this PR. (EDIT: oops, I missed @stratakis's comment.) |
This introduces a new configure and sysconfig variable
platsubdir
, which is the name of subdirectory ofprefix
that holds platform-specific libraries. On many Linux distributions, there is separation between/usr/lib
and/usr/lib64
, as opposed toprefix
vsexec_prefix
, which is what Python is accustomed to.This should improve the situation, and
platsubdir
will be used to differentiate library subdirectories belowprefix
.Based on last comments in the bug report http://bugs.python.org/issue1294959 , I have renamed "platlibdir" to "platsubdir" -- IMHO it's more important to note that this is platform-specific than that it is a library directory. (otherwise we'd end up with
platlibsubdir
and that's horrible ;) )https://bugs.python.org/issue1294959