Skip to content

Conversation

matejcik
Copy link
Contributor

@matejcik matejcik commented Sep 22, 2017

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, as opposed to prefix vs exec_prefix, which is what Python is accustomed to.

This should improve the situation, and platsubdir will be used to differentiate library subdirectories below prefix.

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

@matejcik
Copy link
Contributor Author

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?

@stratakis
Copy link
Contributor

@matejcik You will have to use blurb for the NEWS entry: https://pypi.python.org/pypi/blurb

@hroncok
Copy link
Contributor

hroncok commented Sep 23, 2017

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)
https://src.fedoraproject.org/rpms/python3/blob/master/f/00205-make-libpl-respect-lib64.patch (always applied)

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.

@hroncok
Copy link
Contributor

hroncok commented Sep 24, 2017

Apparently, replacing those 2 patches with this one builds and tests fine. https://koji.fedoraproject.org/koji/taskinfo?taskID=22047740

@matejcik
Copy link
Contributor Author

news entry added

@hroncok
Copy link
Contributor

hroncok commented Nov 28, 2017

What can we do to make this happen?

@encukou
Copy link
Member

encukou commented May 15, 2018

I am a core dev now and I can commit to pushing this through.
Please rebase and re-check on Fedora; I can then do the sanity check on Ubuntu that Matthias requested on the issue.
If this ever again becomes stuck on the CPython side, ping me directly.

Copy link
Contributor

@grimreaper grimreaper left a 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>],
Copy link
Contributor

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

Copy link
Contributor Author

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

Copy link
Contributor

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":
Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor Author

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
Copy link
Contributor

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unix then?

Copy link
Member

Choose a reason for hiding this comment

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

Unix-like?

@vstinner
Copy link
Member

@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?

matejcik added 2 commits January 31, 2019 16:06
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
@matejcik
Copy link
Contributor Author

@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 ?

@hroncok
Copy link
Contributor

hroncok commented Jan 31, 2019

Not me. I can help @vstinner get this done and tested in Fedora, but that's it, sorry.

@mcepl
Copy link
Contributor

mcepl commented Jan 31, 2019

@matejcik @hroncok I hopefully will get to this later, but currently I struggle with fixing many packages failing on 3.7.*.

@stratakis
Copy link
Contributor

I can start working on it, should be having some more time soon.

@stratakis
Copy link
Contributor

Opened #11755 in order to move this forward.

@brettcannon brettcannon added the type-feature A feature request or enhancement label Mar 29, 2019
@csabella
Copy link
Contributor

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.

@vstinner
Copy link
Member

vstinner commented Feb 12, 2020

PR #11755 is an updated version of this PR. (EDIT: oops, I missed @stratakis's comment.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.