Open TheRealCuran opened 5 years ago
Guy who wrote hatdcoded list and Mesa meson maintainer here.
I've thought about this a lot. I go back and forth on this issue a lot. On the one hand I agree with you, the hard coded list had a lit of drawbacks. On the other hand llvm-config is a giant pile of fail, many versions are broken in different ways and with each version I build the new llvm in a bunch of different configurations and test them before enabling the new version.
The best case solution is for llvm to drop llvm-config and ship pkg-config files so that we wouldn't need this madness.
Sorry for the short email, I'm on my phone.
On January 19, 2019 1:49:25 PM PST, TheRealCuran notifications@github.com wrote:
I just discovered that the names for
llvm-config
binary names is hardcoded in lines 214ff of mesonbuild/dependencies/dev.py. This is rather fragile and as of now breaks onllvm-config-9
. The name changed after the recent branch point of LLVM 8 and switching over to 9 for development.As a stop-gap measure please add
llvm-config-9
immediately to the list of binary names meson looks for.Though a more robust solution is needed in my opinion, because having default builds fail after every branch point of LLVM (or any other tool discovered in a similar way for that matter) until a new meson version is shipped, is nothing I'm looking forward to (yes, I know I can override the name of the
llvm-config
to use, but the default should not stop working unless there are some incompatible interface changes, eg. the binary is discontinued in favour of another mechanism). I would suggest to search for files starting withllvm-config
in$PATH
, order the result, maybe do some sanity checking, that it looks like an expected binary (ie.llvm-config(-(\d+(\.\d)?|devel))?
) and pick the newest by default. But any other solution that is stable is fine by me as well.This affects for example Mesa, which is now deprecating autotools and forcing transition to meson.
-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/mesonbuild/meson/issues/4802
I doubt that's going to happen soon. Therefore meson should be changed in a way that makes default behaviour ("use newest LLVM") not just break on branch points. The difference between 8 and 9 at this point is negligible, only a few hundred commits, meaning you don't really test anything meaningful AFAICS (sure, if you're talking only about released versions, there is going to be differences, but I'm talking about following upstream's SVN here).
And if you don't want to add this to meson: meson is already using CMake (NB: this really threw me off, because why not just use CMake in the first place then?) for discovery of libraries, why not for LLVM as well? At least as a fallback? <edit>I should probably clarify, that I was suggesting CMake here because LLVM generates CMake target definitions, see https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project. And since Meson is already using CMake this would be sort of free, right?</edit>
I haven't looked into the cmake stuff yet, it just landed a short while ago. The biggest concern I have with it (again haven't looked yet) is that cmake insists that putting stuff line -DNDEBUG
and -O3 -O1 -g -O2
in the cflags they pass you is a good thing thing to do.
You have two other options to make this work without altering meson, you can use a native or cross file to specify the llvm binary, or you can use debian's update-alternatives to make llvm-config-9 into llvm-config.
I haven't looked into the cmake stuff yet, it just landed a short while ago. The biggest concern I have with it (again haven't looked yet) is that cmake insists that putting stuff line
-DNDEBUG
and-O3 -O1 -g -O2
in the cflags they pass you is a good thing thing to do.
You wouldn't use CFLAGS from CMake AFAICS. You'd do something like in your CMakeLists.txt:
project(MesonDetectLLVM)
find_package(LLVM REQUIRED CONFIG)
message(STATUS "Found LLVM ${LLVM_PACKAGE_VERSION}")
message(STATUS "Using LLVMConfig.cmake in: ${LLVM_DIR}")
message(STATUS "include dirs: ${LLVM_INCLUDE_DIRS}")
message(STATUS "definitions: ${LLVM_DEFINITIONS}")
llvm_map_components_to_libnames(llvm_libs support core irreader)
message(STATUS "libs: ${llvm_libs}")
And get output like:
-- Found LLVM 9.0.0
-- Using LLVMConfig.cmake in: /usr/lib/llvm-9/cmake
-- include dirs: /usr/lib/llvm-9/include
-- definitions: -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
-- libs: LLVMSupport;LLVMCore;LLVMIRReader
If you don't use the definition variable, then nothing from there gets added.
For more available variables you could query, see https://github.com/llvm-mirror/llvm/blob/master/cmake/modules/LLVMConfig.cmake.in (looking at the generated files in /usr/lib/llvm-9/lib/cmake/llvm
I'm not seeing any CFLAGS or similar things added there, though I did only a cursory check).
And I just verified, that CMake is really always picking the latest LLVM version by default (<edit>to clarify: dlocate only shows the installed files here and above we saw, that LLVM 9 was picked from the three available options</edit>):
$ dlocate -S LLVMConfig.cmake
llvm-9-dev: /usr/lib/llvm-9/lib/cmake/llvm/LLVMConfig.cmake
llvm-7-dev: /usr/lib/llvm-7/lib/cmake/llvm/LLVMConfig.cmake
llvm-6.0-dev: /usr/lib/llvm-6.0/lib/cmake/llvm/LLVMConfig.cmake
You have two other options to make this work without altering meson, you can use a native or cross file to specify the llvm binary, or you can use debian's update-alternatives to make llvm-config-9 into llvm-config.
I know that I can work around this bug (see my initial report), but from my POV this is about default behaviour ("use newest LLVM") stopping to work when following LLVM's SVN. That being said, the alternatives system is not an option for package builds. The native file clutch is what I use at the moment to get working packages built. But I would really prefer not to have to carry that around if at all possible. Especially given the mission statement of meson (emphasises mine):
Meson is an open source build system meant to be both extremely fast, and, even more importantly, as user friendly as possible.
The main design point of Meson is that every moment a developer spends writing or debugging build definitions is a second wasted. So is every second spent waiting for the build system to actually start compiling code.
llvm-config80
is also absent from the list btw.
a native or cross file to specify the llvm binary
That works, but having to make a file feels a bit "heavyweight". In FreeBSD Ports, we'd much prefer to have an environment variable. E.g. that's how CMake projects usually do it:
-DLLVM_CONFIG_EXECUTABLE:PATH=${LOCALBASE}/bin/llvm-config${LLVM_VERSION}
We will not have an environment variable.
I'd be happy to to discuss solutions that aren't environment variables or painfully expensive (searching for llvm-config binaries in your path with regex is going to be an O^3 algorithm, which is pretty awful).
The only thing I've come up with that seems not terrible is the ability to read a file out of a user or system window location like /etc/meson or ~/.config/meson that overrides the list provided by meson itself. that would allow debian or FreeBSD to add new versions even if that change hasn't made it into meson itself yet.
We will not have an environment variable.
I'd be happy to to discuss solutions that aren't environment variables or painfully expensive (searching for llvm-config binaries in your path with regex is going to be an O^3 algorithm, which is pretty awful).
-Dc_args=-Og
to set the CFLAGS.The only thing I've come up with that seems not terrible is the ability to read a file out of a user or system window location like /etc/meson or ~/.config/meson that overrides the list provided by meson itself. that would allow debian or FreeBSD to add new versions even if that change hasn't made it into meson itself yet.
I don't think this will work. /etc/meson feels like a replication of the alternatives system, which is not going to work reliably if you have multiple LLVMs present in the build environment for whatever reason – especially if you want to do experimental builds against LLVM's SVN, which might not be allowed to set a new default value. ~/.config/meson is really not an option. That is forbidden by package build policy in Debian (for good reasons like reproducible builds):
Required targets must not attempt to write outside of the unpacked source package tree. There are two exceptions. Firstly, the binary targets may write the binary packages to the parent directory of the unpacked source package tree. Secondly, required targets may write to
/tmp
,/var/tmp
and to the directory specified by theTMPDIR
environment variable, but must not depend on the contents of any of these.This restriction is intended to prevent source package builds creating and depending on state outside of themselves, thus affecting multiple independent rebuilds. In particular, the required targets must not attempt to write into
HOME
.
Every configuration a package needs must be expressed through a dependency or something passed to the configuration utility at build time. And besides, I fail to see the improvement over the native files workaround here. Either file would have to be maintained by the package being built.
Therefore I would like to explore the "use CMake for LLVM discovery" option. CMake is a known robust system, is battle tested and used in many big projects. Plus, in the case of LLVM, it would give easy access to all the configuration data of a particular LLVM version in one go. Since meson already has support for using CMake, I'm not seeing an (additional) downside at the moment.
Oops, I was really tired when I wrote that. Yes, a command line flag, not an env variable.
Actually the native file is not that bad:
MESON_ARGS= --native-file "${WRKSRC}/llvm.ini"
pre-configure:
${PRINTF} "[binaries]\nllvm-config = '${LOCALBASE}/bin/llvm-config${MESA_LLVM_VER}'" \
> "${WRKSRC}/llvm.ini"
But it would be nice to skip the file creation step and directly pass the path as an argument.
if you're using bash you can just do something like <(echo "[binaries]\nllvm-config = '/foo/bar')"
(which is what gentoo does, IIRC).
Okay, I can talk about why we don't have a command line switch. That was actually my original proposal before I implemented the native file. The problem with the command line switch approach is a) we already had cross files and the ability to set llvm-config through the cross file, so adding a -D option would create a disconnect between how cross and native setups were done. If we went ahead and created the native file as well, we'd have two different interfaces to do the same thing, which we'd like to avoid if at all possible.
I can look into CMake, but I need to evaluate a lot of things there. 1) for mesa I need to be able to check the paths that llvm is installed into, does cmake give me that? 2) for mesa I need to be able to check whether rtti was used, does cmake give me that?
Essentially there's a lot of unkown's here, and it will involve some amount of time investment from me. It's on my list of things to do but I have other commitments and it's not at the top of my list. I'll open a proper issue and assign it.
I've opened #4861 about using cmake for LLVM
I can look into CMake, but I need to evaluate a lot of things there.
1. for mesa I need to be able to check the paths that llvm is installed into, does cmake give me that?
Yes, see my example above which already shows the include directory and the base LLVM directory as variables. LLVM generates a file called LLVMConfig.cmake
(again, see above), that contains everything about the build or at the very least can point you to the appropriate llvm-config
executable. There are for example additional variables like LLVM_INSTALL_PREFIX
, LLVM_LIBRARY_DIR
or LLVM_TOOLS_BINARY_DIR
.
2. for mesa I need to be able to check whether rtti was used, does cmake give me that?
Yes, in the variable called LLVM_ENABLE_RTTI
. Again LLVMConfig.cmake
contains – to the best of my knowledge – the entire configuration of the build.
Essentially there's a lot of unkown's here, and it will involve some amount of time investment from me. It's on my list of things to do but I have other commitments and it's not at the top of my list. I'll open a proper issue and assign it.
I've opened #4861 about using cmake for LLVM
Not sure why this ticket isn't enough, but if another ticket works better for you... ¯\_(ツ)_/ ¯
Because there's two fundamentally different questions. One is "is there a better way than a hardcoded list of llvm-config versions", and "can we use cmake". I'd prefer to keep this particular issue about the hardcoded list, and the other about using cmake.
I just discovered that the names for
llvm-config
binary names is hardcoded in lines 214ff of mesonbuild/dependencies/dev.py. This is rather fragile and as of now breaks onllvm-config-9
. The name changed after the recent branch point of LLVM 8 and switching over to 9 for development.As a stop-gap measure please add
llvm-config-9
immediately to the list of binary names meson looks for.Though a more robust solution is needed in my opinion, because having default builds fail after every branch point of LLVM (or any other tool discovered in a similar way for that matter) until a new meson version is shipped, is nothing I'm looking forward to (yes, I know I can override the name of the
llvm-config
to use, but the default should not stop working unless there are some incompatible interface changes, eg. the binary is discontinued in favour of another mechanism). I would suggest to search for files starting withllvm-config
in$PATH
, order the result, maybe do some sanity checking, that it looks like an expected binary (ie.llvm-config(-(\d+(\.\d)?|devel))?
) and pick the newest by default. But any other solution that is stable is fine by me as well.This affects for example Mesa, which is now deprecating autotools and forcing transition to meson.