Page MenuHomePhabricator

Support RHEL 7 and similar systems that use architecture-specific Python lib dirs.
ClosedPublic

Authored by tfiala on Oct 10 2015, 7:52 PM.

Details

Reviewers
emaste
dawn
labath
Summary

This change fixes 'lldb -P' (retrieve lldb-module python path) to work correctly on cmake-based builds on POSIX systems.

Previously, this would assume the python module installation path was in the lldb lib directory. This is not accurate for RHEL, which uses an architecture-specific directory name (i.e. lib64 for x86_64 versions).

Now, with this change, the python identified during build is called with a script at configure time and builds with a #define for the relative directory path used for this system's particular python style. On RHEL, this turns into lib64/python{maj}.{min}/..., while it will be lib/python{maj}.{min}/... on other systems. This removes the lldb code from guessing it in this case.

The old behavior is preserved in the event that the configure-time python script fails to determine the relative python module lib dir.

This fixes:
https://llvm.org/bugs/show_bug.cgi?id=25134

Diff Detail

Event Timeline

tfiala updated this revision to Diff 37040.Oct 10 2015, 7:52 PM
tfiala retitled this revision from to Support RHEL 7 and similar systems that use architecture-specific Python lib dirs..
tfiala updated this object.
tfiala added reviewers: labath, dawn.
tfiala added a subscriber: lldb-commits.
labath requested changes to this revision.Oct 12 2015, 6:32 AM
labath edited edge metadata.

Could you submit the change with full context next time? It makes review much easier.

I don't understand what are you trying to achieve completely, but it feels to me you are trying to solve the wrong side of the problem. If we are on a system which uses lib64 as the directory for it's libraries, then lldb libraries should go into the lib64 folder as well (by compiling with -DLLVM_LIBDIR_SUFFIX=64). If the directory still comes out wrong (or the libraries don't end up in lib64) then we should fix that. Could you try if -DLLVM_LIBDIR_SUFFIX=64 solves your problems first?

This revision now requires changes to proceed.Oct 12 2015, 6:32 AM

Could you submit the change with full context next time? It makes review much easier.

Tell me the diff line you want. I'm doing the same type of diffs I have submitted for years. I'll adjust my process.

I don't understand what are you trying to achieve completely, but it feels to me you are trying to solve the wrong side of the problem. If we are on a system which uses lib64 as the directory for it's libraries, then lldb libraries should go into the lib64 folder as well (by compiling with -DLLVM_LIBDIR_SUFFIX=64). If the directory still comes out wrong (or the libraries don't end up in lib64) then we should fix that. Could you try if -DLLVM_LIBDIR_SUFFIX=64 solves your problems first?

Python's choice of desired lib dir is orthogonal to whatever else we want to do on a system. The most precise way to handle that is to ask python to tell us where it wants its lib/module dirs placed. If we go to a system where python (for whatever reason, including somebody who builds a custom python that does something else entirely) wants to put it in some entirely different lib configuration, this will handle it. Going to a fixed directory to satisfy python's module directory, which itself can change based on build configuration, doesn't seem like a good idea.

This change asks python "where do you put your libs" on POSIX systems (Xcode build excluded). And then lldb learns and responds to it. I much prefer that to saying the lib directory for all of lldb must match whatever the python happens to be doing.

That's my take.

Also - there are some places in the code where the lib dir (as 'lib') is assumed, and those would need to change as well. Including taking note of where it was passed in cmake, which will look something similar to what I did here, but now with the added burden that two things must line up (the python module lib dir) with the lldb chosen lib dir.

So this reason and the previous is why I do not see it being the right approach to just say 'switch the overall lib dir' to address this.

Could you submit the change with full context next time? It makes review much easier.

Tell me the diff line you want. I'm doing the same type of diffs I have submitted for years. I'll adjust my process.

It depends on your process I guess. If you are generating the diff from svn, the command is svn diff --diff-cmd "diff -x -U9999". If you are using arcanist to upload your diffs (which I recommend, btw :) ), it will do that for you.

I don't understand what are you trying to achieve completely, but it feels to me you are trying to solve the wrong side of the problem. If we are on a system which uses lib64 as the directory for it's libraries, then lldb libraries should go into the lib64 folder as well (by compiling with -DLLVM_LIBDIR_SUFFIX=64). If the directory still comes out wrong (or the libraries don't end up in lib64) then we should fix that. Could you try if -DLLVM_LIBDIR_SUFFIX=64 solves your problems first?

Python's choice of desired lib dir is orthogonal to whatever else we want to do on a system. The most precise way to handle that is to ask python to tell us where it wants its lib/module dirs placed. If we go to a system where python (for whatever reason, including somebody who builds a custom python that does something else entirely) wants to put it in some entirely different lib configuration, this will handle it. Going to a fixed directory to satisfy python's module directory, which itself can change based on build configuration, doesn't seem like a good idea.

This change asks python "where do you put your libs" on POSIX systems (Xcode build excluded). And then lldb learns and responds to it. I much prefer that to saying the lib directory for all of lldb must match whatever the python happens to be doing.

That's my take.

Ok. Is this some hypothetical scenario, or have you actually encountered a system where python uses e.g. lib64 and the rest of the system the opposite?

I understand your desires, but your approach e.g. makes cross-compiling impossible (or, at least assumes that target python uses the same directory as the build one (if the build machine even has python in the first place)). I'd like to avoid doing this if it is possible to work around it. Have you checked whether it is possible to extract this information from cmake, without running any code (perhaps parsing PYTHON_LIBRARIES or something).

Tell me the diff line you want. I'm doing the same type of diffs I have submitted for years. I'll adjust my process.

Are you wanting something like this (assuming done with git diff)?
git diff --no-prefix -U{SOME-REALLY-BIG-NUMBER-GREATER-THAN-NUMBER-OF-LINES-IN-THE-FILE}

Tell me the diff line you want. I'm doing the same type of diffs I have submitted for years. I'll adjust my process.

Are you wanting something like this (assuming done with git diff)?
git diff --no-prefix -U{SOME-REALLY-BIG-NUMBER-GREATER-THAN-NUMBER-OF-LINES-IN-THE-FILE}

Yup. That's it.

Could you submit the change with full context next time? It makes review much easier.

Tell me the diff line you want. I'm doing the same type of diffs I have submitted for years. I'll adjust my process.

It depends on your process I guess. If you are generating the diff from svn, the command is svn diff --diff-cmd "diff -x -U9999". If you are using arcanist to upload your diffs (which I recommend, btw :) ), it will do that for you.

I'll check out archaist, thanks.

I don't understand what are you trying to achieve completely, but it feels to me you are trying to solve the wrong side of the problem. If we are on a system which uses lib64 as the directory for it's libraries, then lldb libraries should go into the lib64 folder as well (by compiling with -DLLVM_LIBDIR_SUFFIX=64). If the directory still comes out wrong (or the libraries don't end up in lib64) then we should fix that. Could you try if -DLLVM_LIBDIR_SUFFIX=64 solves your problems first?

Python's choice of desired lib dir is orthogonal to whatever else we want to do on a system. The most precise way to handle that is to ask python to tell us where it wants its lib/module dirs placed. If we go to a system where python (for whatever reason, including somebody who builds a custom python that does something else entirely) wants to put it in some entirely different lib configuration, this will handle it. Going to a fixed directory to satisfy python's module directory, which itself can change based on build configuration, doesn't seem like a good idea.

This change asks python "where do you put your libs" on POSIX systems (Xcode build excluded). And then lldb learns and responds to it. I much prefer that to saying the lib directory for all of lldb must match whatever the python happens to be doing.

That's my take.

Ok. Is this some hypothetical scenario, or have you actually encountered a system where python uses e.g. lib64 and the rest of the system the opposite?

RHEL 7 is already doing this - most of lib is 64-bit. All of lib64 is 64-bit. They already use both (~600 MB in lib, ~1GB in lib64 on a stock system). I have also built custom pythons, and by default python will use lib. So this is a bad user experience for an lldb developer to have to know that they would have to set their python and their cmake build of lldb to the same lib dir.

I understand your desires, but your approach e.g. makes cross-compiling impossible (or, at least assumes that target python uses the same directory as the build one (if the build machine even has python in the first place)). I'd like to avoid doing this if it is possible to work around it.

Why would that be the case? If the target python for the cross compiler is the one specified in the lldb build (which it would have to be, wouldn't it?), that is the one that will be queried. i.e. the one getting embedded in the lldb build is the one being asked. I'd hope that can answer the question correctly. Also, the approach used here to find the directory is the same one we use to construct the lldb python module in the first place. So this can't make us any further broken for cross compilation than we already are, and if the cross-compiled python can answer the question correctly, we're all set.

Have you checked whether it is possible to extract this information from cmake, without running any code (perhaps parsing PYTHON_LIBRARIES or something).

No, I haven't looked at this angle. I'm asking python the same question we ask it already during the construction of the lldb module packaging. I'd rather use the same approach that we already use to locate the directory than fabricate another one, precisely because they'll be consistent.

Here's what I'd rather do:

  • Keep with this approach since it uses both the same approach we already use (distutils python module) to gather the right location, and the python lib dir is allowed to vary independently of the lib dir used by the rest of the build. This allows us to get it right with no input from the developer regardless of which python they're using, stock or custom.
  • If it is proven we are doing something wrong for cross compiling, and somebody wants to make that right, we then change over from using the disutils approach both in generating the partial relative directory for answering 'lldb -P' and in the finalization of python class swig generation step.

I do think it is a worthwhile cleanup to do at some point to get rid of any direct use of 'lib' in the source to use what cmake provides (through the lib dir macros you mentioned previously). But I'd see that as a cleanup activity orthogonal to this change.

Thoughts?

If it is proven we are doing something wrong for cross compiling, and somebody wants to make that right, we then change over from using the disutils approach both in generating the partial relative directory for answering 'lldb -P' and in the finalization of python class swig generation step.

The one piece I would totally be agreeable to changing and adding here for the sake of my statement above is to make the packaging of the lldb module call the exact same code rather than use the same underlying method. Right now they both use distutils, but through two different call sites. It would maybe make it easier to change this in the future if changing one place covered both of them.

Tell me the diff line you want. I'm doing the same type of diffs I have submitted for years. I'll adjust my process.

Are you wanting something like this (assuming done with git diff)?
git diff --no-prefix -U{SOME-REALLY-BIG-NUMBER-GREATER-THAN-NUMBER-OF-LINES-IN-THE-FILE}

Yup. That's it.

Okay, I can do that. I'll also look at alchemist, haven't touched that before.

labath accepted this revision.Oct 12 2015, 9:08 AM
labath edited edge metadata.

RHEL 7 is already doing this - most of lib is 64-bit. All of lib64 is 64-bit. They already use both (~600 MB in lib, ~1GB in lib64 on a stock system). I have also built custom pythons, and by default python will use lib. So this is a bad user experience for an lldb developer to have to know that they would have to set their python and their cmake build of lldb to the same lib dir.

Ok, thanks for the explanation. I guess we'll have to find some way around it then.

I understand your desires, but your approach e.g. makes cross-compiling impossible (or, at least assumes that target python uses the same directory as the build one (if the build machine even has python in the first place)). I'd like to avoid doing this if it is possible to work around it.

Why would that be the case?

You might be cross-compiling from windows-x86 to linux-arm. It's unlikely you will be able to run the target python to ask it something. Unless the distutils thing somehow accesses that info through some magic I am not aware of, in which case I apologise. But ok, I am not that worried about setups like this at the moment.

If the target python for the cross compiler is the one specified in the lldb build (which it would have to be, wouldn't it?), that is the one that will be queried. i.e. the one getting embedded in the lldb build is the one being asked. I'd hope that can answer the question correctly. Also, the approach used here to find the directory is the same one we use to construct the lldb python module in the first place. So this can't make us any further broken for cross compilation than we already are, and if the cross-compiled python can answer the question correctly, we're all set.

Yeah, that is probably broken as well, but that's out of scope for this change.

Have you checked whether it is possible to extract this information from cmake, without running any code (perhaps parsing PYTHON_LIBRARIES or something).

No, I haven't looked at this angle. I'm asking python the same question we ask it already during the construction of the lldb module packaging. I'd rather use the same approach that we already use to locate the directory than fabricate another one, precisely because they'll be consistent.

Here's what I'd rather do:

  • Keep with this approach since it uses both the same approach we already use (distutils python module) to gather the right location, and the python lib dir is allowed to vary independently of the lib dir used by the rest of the build. This allows us to get it right with no input from the developer regardless of which python they're using, stock or custom.
  • If it is proven we are doing something wrong for cross compiling, and somebody wants to make that right, we then change over from using the disutils approach both in generating the partial relative directory for answering 'lldb -P' and in the finalization of python class swig generation step.

    I do think it is a worthwhile cleanup to do at some point to get rid of any direct use of 'lib' in the source to use what cmake provides (through the lib dir macros you mentioned previously). But I'd see that as a cleanup activity orthogonal to this change.

    Thoughts?

Ok, let's keep this as is. At some point, someone will have to go in and fix this though.

This revision is now accepted and ready to land.Oct 12 2015, 9:08 AM

RHEL 7 is already doing this - most of lib is 64-bit. All of lib64 is 64-bit. They already use both (~600 MB in lib, ~1GB in lib64 on a stock system). I have also built custom pythons, and by default python will use lib. So this is a bad user experience for an lldb developer to have to know that they would have to set their python and their cmake build of lldb to the same lib dir.

Ok, thanks for the explanation. I guess we'll have to find some way around it then.

I understand your desires, but your approach e.g. makes cross-compiling impossible (or, at least assumes that target python uses the same directory as the build one (if the build machine even has python in the first place)). I'd like to avoid doing this if it is possible to work around it.

Why would that be the case?

You might be cross-compiling from windows-x86 to linux-arm. It's unlikely you will be able to run the target python to ask it something. Unless the distutils thing somehow accesses that info through some magic I am not aware of, in which case I apologise. But ok, I am not that worried about setups like this at the moment.

Okay, I'm totally up for helping if we hit this issue.

If the target python for the cross compiler is the one specified in the lldb build (which it would have to be, wouldn't it?), that is the one that will be queried. i.e. the one getting embedded in the lldb build is the one being asked. I'd hope that can answer the question correctly. Also, the approach used here to find the directory is the same one we use to construct the lldb python module in the first place. So this can't make us any further broken for cross compilation than we already are, and if the cross-compiled python can answer the question correctly, we're all set.

Yeah, that is probably broken as well, but that's out of scope for this change.

Have you checked whether it is possible to extract this information from cmake, without running any code (perhaps parsing PYTHON_LIBRARIES or something).

No, I haven't looked at this angle. I'm asking python the same question we ask it already during the construction of the lldb module packaging. I'd rather use the same approach that we already use to locate the directory than fabricate another one, precisely because they'll be consistent.

Here's what I'd rather do:

  • Keep with this approach since it uses both the same approach we already use (distutils python module) to gather the right location, and the python lib dir is allowed to vary independently of the lib dir used by the rest of the build. This allows us to get it right with no input from the developer regardless of which python they're using, stock or custom.
  • If it is proven we are doing something wrong for cross compiling, and somebody wants to make that right, we then change over from using the disutils approach both in generating the partial relative directory for answering 'lldb -P' and in the finalization of python class swig generation step.

    I do think it is a worthwhile cleanup to do at some point to get rid of any direct use of 'lib' in the source to use what cmake provides (through the lib dir macros you mentioned previously). But I'd see that as a cleanup activity orthogonal to this change.

    Thoughts?

Ok, let's keep this as is. At some point, someone will have to go in and fix this though.

That sounds totally reasonable.

Thanks, Pavel!

I'll wait for a while to hear if either @emaste or @dawn chime in.

I'll wait for a while to hear if either @emaste or @dawn chime in.

I'm going to get this in. If any of the rest of you find issues, we'll get them taken care of at that point. Thanks!

tfiala closed this revision.Oct 12 2015, 1:14 PM

Committed here:

svn commit
Adding         scripts/get_relative_lib_dir.py
Sending        source/Host/CMakeLists.txt
Sending        source/Host/posix/HostInfoPosix.cpp
Sending        www/build.html
Transmitting file data ....
Committed revision 250093.