Page MenuHomePhabricator

build: avoid hardcoding the libxml2 library name
ClosedPublic

Authored by compnerd on Oct 24 2019, 3:08 PM.

Details

Reviewers
xiaobai
beanz
Summary

FindLibXml2 will set the LIBXML2_LIBRARIES variable to the libraries that we must link against. This will be an empty string if libxml2 is not found. Avoid hardcoding the library name as xml2 in the configuration. Simplify the usage in the WindowsManifest library.

Diff Detail

Repository
rL LLVM

Event Timeline

compnerd created this revision.Oct 24 2019, 3:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 24 2019, 3:08 PM
Herald added a subscriber: mgorny. · View Herald Transcript
beanz accepted this revision.Oct 25 2019, 2:59 PM

LGTM

This revision is now accepted and ready to land.Oct 25 2019, 2:59 PM
xiaobai accepted this revision.Oct 25 2019, 3:08 PM
srj added a subscriber: srj.Dec 1 2019, 5:37 PM

This has injected a bug into llvm-config --system-libs (at least, on OSX and Linux) for me; previously, the output would be something like

-lz -lrt -ldl -lpthread -lm -lxml2

but now, it's

-lz -lrt -ldl -lpthread -lm -l/usr/lib/x86_64-linux-gnu/libxml2.so

which is not a legal set of args to the linker. (This is breaking Halide's downstream use of LLVM, as it relies on llvm-config --system-libs.)

srj added a comment.Dec 2 2019, 11:35 AM

I haven't found any reasonable workarounds for this breakage; CMake is apparently splitting -l/usr/lib/x86_64-linux-gnu/libxml2.so into -l/usr/lib/x86_64 and -linux-gnu/libxml2.so, neither of which are valid dependencies, so our builds are totally broken. If a fix isn't forthcoming, can we please roll this change back at your earliest convenience?

There isn't a workaround for this, its just a bug in our CMake. I've fixed the issue with our CMake logic in GIT 372ad32734ecb455f9fb4d0601229ca2dfc78b66.