Support Ubuntu 24.04 and LLVM 14.0.5; automatically select LLVM version based on compiler version #309
Conversation
Python 3.12 doesn't have `imp`, and this file doesn't seem to actually use it.
| @@ -33,6 +33,7 @@ def main(args): | |||
| parser.add_argument('--binder', default='', help='Path to Binder tool. If none is given then download, build and install binder into main/source/build. Use "--binder-debug" to control which mode of binder (debug/release) is used.') | |||
| parser.add_argument("--binder-debug", action="store_true", help="Run binder tool in debug mode (only relevant if no '--binder' option was specified)") | |||
| parser.add_argument('--pybind11', default='', help='Path to pybind11 source tree') | |||
| parser.add_argument('--llvm-version', default=None, choices=['6.0.1', '13.0.0', '14.0.5'], help='Manually specify LLVM version to install') | |||
There was a problem hiding this comment.
Do you think we can avoid using choices so user can install arbitrary LLVM version? Note that i am not saying that we should do that, just raising the possibility since this approach will be most flexible.
There was a problem hiding this comment.
Probably, but the build system changes between major versions, so we'd need to have logic to parse the version and figure out what operations we think are going to be required to build that version.
There was a problem hiding this comment.
Fair point! - i guess let's merge this as-is then. Let re-read changes one more time and i will get back to you.
| # The LLVM tarballs may include a "cmake" directory that needs to be | ||
| # sibling to the main source directories in order for the build to | ||
| # work. That path can't depend on the LLVM version, so we track that ourselves. | ||
| cmake_path = os.path.join(prefix_root, "cmake") | ||
| cmake_version_path = os.path.join(cmake_path, "llvm_version.txt") |
There was a problem hiding this comment.
Do we really need this new "marker" (ie llvm_version.txt file)? We already have llvm_version encoded in our signature file, - is not that enough? (see https://github.com/RosettaCommons/binder/pull/309/files#diff-b70bfb027a8ef3ec4ab07a53a3b8ba83e63334ba3ce5ca6d9d773900ed452e5eL113). Please elaborate, Thanks,
There was a problem hiding this comment.
My understanding was that the signature was at the level of the build/compiled outputs. If the signature isn't current, then we check to see if we have the source we want. If not, we go get the source.
Because of the relative paths between the source tarballs that need to be right for the LLVM source to find its CMake macros (it looks at ../cmake), we can have copies of multiple versions of the unpacked actual source tarballs on disk at the same time, but only one version of the cmake macros. So we might have CMake macros at the path we want, but not the right CMake macros.
I didn't think that the version of LLVM that we have a finished build of would necessarily always be the same as the version of LLVM that we last downloaded the CMake macros for. (The user might abort a build of one version and then try and build a different version, or something.) So I made it tracked separately.
Also I wasn't actually able to cleanly abort and restart builds with different versions without manual deletion of some of the stuff that seemed like it was meant to be guarded by the signature, so I wasn't sure it really worked and was worried about building on it but not really confident in fixing it.
There was a problem hiding this comment.
Thank you for clarifying! The main idea behind signature approach was to make sure that aborting build and restarting should work, - and since it does not then there might be some bugs there that i will need to look up.
OK, let me see if i can add tests to CI that will use build.py in a few days and if that takes lineager not then we will merge this as is. Thanks,
|
@lyskov I mostly ran this locally on Ubuntu 24.04 on ARM. I also tried it on the host ARM Mac platform, but I'm not 100% sure I waited for an entire LLVM build there. |
There was a problem hiding this comment.
Looks good to me, - thank you for putting this together @adamnovak !
This PR updates Binder to work on Ubuntu 24.04, with its GCC 13.
It also implements the compiler-version-dependent LLVM version selection logic that the build scripts wanted, and adds an
--llvm-versioncommand line flag to them to override it.It includes LLVM 14.0.5, which will be automatically selected for GCC 13 or newer, since it's the oldest LLVM I could find that can build on GCC 13 and understand GCC 13's STL headers.
I had to change how the LLVM tarballs are extracted to not lose the
cmakedirectory that LLVM 14.0.5 needs next to its source directory to build. I added some version tracking for this directory, which will involve redownloading LLVM if it needs to change versions.