Nemirtingas/clang-msvc-sdk

llvm-mt still broken on some Linux distributions

Closed this issue · 13 comments

mite-user commented

git cherry-picking e2b221f to my fork broke compilation environment on my Fedora docker image.
The error I'm getting

llvm-mt: error: no libxml2

It's caused by
https://github.com/llvm/llvm-project/blob/e0ccd190ae8b3a7e8d79258218703a6ecadbc883/llvm/lib/WindowsManifest/WindowsManifestMerger.cpp#L692
This code is compiled if "LLVM_ENABLE_LIBXML2" is false.

If llvm was built without libxml2, llvm-mt will be broken.

It was fixed in llvm-11 on Debian, i.e. the fix is specific to Debian.
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=990537
https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/commit/443eeecf7ba322fd6156a4f455aad27169382ccd

As long as at least some popular Linux distributions keep building llvm without libxml2, clang-cl-msvc.cmake should keep using dummy mt executable.

Nemirtingas commented

Hi,
Thanks for your issue, I'm very surprised that someone use this project 😄, do you have an idea to fix that behavior ? Like, is there a command that can be used to tell if LLVM is linked against libxml or not ?

I'm thinking of using a llvm-mt wrapper so it can just skip the manifest building if LLVM is missing libxml and call llvm-mt if its working fine.

mite-user commented

Relevant sources
https://github.com/llvm/llvm-project/blob/e0ccd190ae8b3a7e8d79258218703a6ecadbc883/llvm/tools/llvm-mt/llvm-mt.cpp
https://github.com/llvm/llvm-project/blob/e0ccd190ae8b3a7e8d79258218703a6ecadbc883/llvm/lib/WindowsManifest/WindowsManifestMerger.cpp
LLVM_ENABLE_LIBXML2 macro affects libLLVM library, not llvm-mt itself.
Looks like the macro doesn't modify anything else. So the only way to check for libxml2 is to directly or indirectly check libLLVM library.

Indirect way - create temporary file, call llvm-mt /manifest <tmp-file> and check the output.
If broken

llvm-mt: error: no libxml2

if not broken

llvm-mt: error: attempted to merge empty manifest

Direct way - check if libLLVM contains no libxml2 string.
Something like

#! /bin/bash
llvm_mt_exe="$(which llvm-mt)"
libLLVM_lib="$(ldd "${llvm_mt_exe}" | grep libLLVM | awk '{print $3}')"
if grep -q "no libxml2" "${libLLVM_lib}"; then
    echo "llvm-mt broken"
else
    echo "llvm-mt works"
fi
Nemirtingas commented

Ok, thanks for the explanation and your researchs. I'll try to push a fix asap.

Nemirtingas commented

I've pushed a branch that triies to handle this issue: llvm_mt_wrapper

Can you try it ?

mite-user commented

Tried to run the script in fedora:35 and ubuntu:22.04 docker images with and without llvm installed.

I gave a bad advice in the previous message. which tool is usually not installed on bare-bones docker images. So on fedora:35 I got

./mt: line 17: which: command not found
./mt: line 19: which: command not found

Also there are different versions of which. On ubuntu:22.04 it was silent, on fedora:35 there were annoying

which: no llvm-mt- in (/root/.local/bin:/root/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin)
which: no llvm-mt in (/root/.local/bin:/root/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin)

Other than that, the script appears to work as expected.

Then tried to git cherry-pick 2 last commits to the master branch of my fork.
It didn't break the build environment.

The script works, but need to find a way to remove which.

No hurry fixing the issue. I removed the cherry-picked commit from the master branch of my fork before I posted the issue. I did have a working environment even without the fix.

Nemirtingas commented

I'll just have to create my own which. Shouldn't be too hard, read the $
PATH var en check for the cmd.

Nemirtingas commented

I've updated the mt wrapper with a custom which implementation.

Nemirtingas commented

Does the latest version pleases you ?

mite-user commented

I was trying to say that type -P can replace which because that's a bash built-in. So there is no need to complicate the wrapper with custom functions.

I just noticed: there is no need to get path to executable or check if it exists. llvm-rc-wrapper assumes that llvm-rc-${LLVM_VER} is available. Checking for llvm-mt-${LLVM_VER} in mt wrapper is inconsistent with llvm-rc-wrapper.

Getting path to llvm-mt would be necessary to feed the path to ldd, to get the path to libLLVM. But that's not necessary if we are just checking the output of llvm-mt.

Other minor things:

  • $output should be echoed only in case of an error so the wrapper doesn't echo an empty line on success;
  • mt should be renamed to llvm-mt-wrapper to match llvm-rc-wrapper.

I think the final version should look like this. Feel free to copy.
https://github.com/mite-user/clang-msvc-sdk/tree/llvm_mt_wrapper

Nemirtingas commented

There, it is done x). Are we good ?

mite-user commented

Yes.

Nemirtingas commented

Nice, must test now :). Thanks for the help.