Skip to content
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

Hardcoded list of llvm-config binary names breaks on upstream LLVM development version changes (currently: llvm-config-9 is not discovered) #4802

Open
TheRealCuran opened this issue Jan 19, 2019 · 13 comments

Comments

@TheRealCuran
Copy link

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 on llvm-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 with llvm-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.

@dcbaker
Copy link
Member

dcbaker commented Jan 20, 2019 via email

@TheRealCuran
Copy link
Author

TheRealCuran commented Jan 20, 2019

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>

@TheRealCuran TheRealCuran changed the title Hardcored list of llvm-config binary names breaks on upstream LLVM development version changes (currently: llvm-config-9 is not discovered) Hardcoded list of llvm-config binary names breaks on upstream LLVM development version changes (currently: llvm-config-9 is not discovered) Jan 21, 2019
@dcbaker
Copy link
Member

dcbaker commented Jan 22, 2019

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.

@dcbaker
Copy link
Member

dcbaker commented Jan 22, 2019

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.

@TheRealCuran
Copy link
Author

TheRealCuran commented Jan 23, 2019

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.

@valpackett
Copy link
Contributor

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}

@dcbaker
Copy link
Member

dcbaker commented Jan 30, 2019

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.

@TheRealCuran
Copy link
Author

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).

  1. I think you might be misunderstanding @myfreeweb here. The example given is a normal flag/switch to the invoaction of cmake, much like what meson already has, eg. -Dc_args=-Og to set the CFLAGS.
  2. You haven't replied (in substance) to my proposal to use CMake to do the discovery (for which support is already implemented in general). What is your objection there?

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 the TMPDIR 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.

@valpackett
Copy link
Contributor

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.

@dcbaker
Copy link
Member

dcbaker commented Jan 31, 2019

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.

@dcbaker
Copy link
Member

dcbaker commented Jan 31, 2019

I've opened #4861 about using cmake for LLVM

@TheRealCuran
Copy link
Author

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... ¯\_(ツ)_/ ¯

@dcbaker
Copy link
Member

dcbaker commented Jan 31, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants