This is an archive of the discontinued LLVM Phabricator instance.

Build Windows releases with libxml enabled, to unbreak llvm-mt
ClosedPublic

Authored by hans on Jul 12 2022, 9:07 AM.

Details

Summary

Recent CMake versions have started to prefer llvm-mt when using clang-cl, which doesn't work at all when llvm-mt is built without libxml which has been the case so far.

See https://github.com/llvm/llvm-project/issues/55817

Diff Detail

Event Timeline

hans created this revision.Jul 12 2022, 9:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 12 2022, 9:07 AM
hans requested review of this revision.Jul 12 2022, 9:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 12 2022, 9:07 AM
thakis accepted this revision.Jul 12 2022, 9:24 AM

Oh, cool!

I always imagined we'd (optionally, default-off, but then probably default-on on windows) ExternalProject_Add libxml from somewhere (similar to https://reviews.llvm.org/D122082#inline-1186216) – then this would be less bolted on. But doing this here first is better than what we currently have, so lg :)

This revision is now accepted and ready to land.Jul 12 2022, 9:24 AM

(See also https://llvm.org/PR38966 for some past discussions on this.)

thakis added inline comments.Jul 12 2022, 9:29 AM
llvm/utils/release/build_llvm_release.bat
155

(why do we do this twice?)

155

(…oh, different bitness, nvm.)

Oh, cool!

I always imagined we'd (optionally, default-off, but then probably default-on on windows) ExternalProject_Add libxml from somewhere

(…or with https://cmake.org/cmake/help/latest/module/FetchContent.html)

penzn added a subscriber: penzn.Jul 12 2022, 10:11 AM

This change LGTM, can we also add some description of this process to developer docs? Anyone who would building LLVM on Windows is going to be hit by llvm-mt errors in current Visual Studio versions.

This revision was landed with ongoing or failed builds.Jul 12 2022, 11:14 AM
This revision was automatically updated to reflect the committed changes.
hans added a comment.Jul 12 2022, 11:17 AM

This change LGTM, can we also add some description of this process to developer docs? Anyone who would building LLVM on Windows is going to be hit by llvm-mt errors in current Visual Studio versions.

Yes, I'll try to follow up with that. It only hits users trying to use a self-built clang-cl to build something else that's configured with cmake though (such as when bootstrapping clang-cl), and I think for those cases a work-around such as -DCMAKE_MT=“C:/Program Files (x86)/Windows Kits/10/bin/10.0.19041.0/x64/mt.exe” is probably better than this giant kludge.

@hans Looking at your changes as part of https://reviews.llvm.org/D129559, the stage1 configuration fails with the error:

-- Looking for xmlReadMemory
-- Looking for xmlReadMemory - not found
CMake Error at cmake/config-ix.cmake:156 (message):
  Failed to configure libxml2

I think the issue is with the -DLIBXML2_INCLUDE_DIR. Looking at the CMake documentation:

LIBXML2_INCLUDE_DIR the directory containing LibXml2 headers
LIBXML2_INCLUDE_DIRS list of the include directories needed to use LibXml2

list(APPEND CMAKE_REQUIRED_INCLUDES ${LIBXML2_INCLUDE_DIRS})
list(APPEND CMAKE_REQUIRED_LIBRARIES ${LIBXML2_LIBRARIES})
...
check_symbol_exists(xmlReadMemory libxml/xmlreader.h HAVE_LIBXML2)
...
if(LLVM_ENABLE_LIBXML2 STREQUAL FORCE_ON AND NOT HAVE_LIBXML2)
  message(FATAL_ERROR "Failed to configure libxml2")
endif()

The CMake script is looking in ${LIBXML2_INCLUDE_DIRS}
Changing from -DLIBXML2_INCLUDE_DIR=.... to -DLIBXML2_INCLUDE_DIRS=... solved the issue and managed to build both 32/64 binaries.

hans added a comment.Jul 14 2022, 6:41 AM

This change LGTM, can we also add some description of this process to developer docs? Anyone who would building LLVM on Windows is going to be hit by llvm-mt errors in current Visual Studio versions.

Sent https://reviews.llvm.org/D129770 for the docs update.

@hans Looking at your changes as part of https://reviews.llvm.org/D129559, the stage1 configuration fails with the error:

-- Looking for xmlReadMemory
-- Looking for xmlReadMemory - not found
CMake Error at cmake/config-ix.cmake:156 (message):
  Failed to configure libxml2

I think the issue is with the -DLIBXML2_INCLUDE_DIR. Looking at the CMake documentation:

LIBXML2_INCLUDE_DIR the directory containing LibXml2 headers
LIBXML2_INCLUDE_DIRS list of the include directories needed to use LibXml2

list(APPEND CMAKE_REQUIRED_INCLUDES ${LIBXML2_INCLUDE_DIRS})
list(APPEND CMAKE_REQUIRED_LIBRARIES ${LIBXML2_LIBRARIES})
...
check_symbol_exists(xmlReadMemory libxml/xmlreader.h HAVE_LIBXML2)
...
if(LLVM_ENABLE_LIBXML2 STREQUAL FORCE_ON AND NOT HAVE_LIBXML2)
  message(FATAL_ERROR "Failed to configure libxml2")
endif()

The CMake script is looking in ${LIBXML2_INCLUDE_DIRS}
Changing from -DLIBXML2_INCLUDE_DIR=.... to -DLIBXML2_INCLUDE_DIRS=... solved the issue and managed to build both 32/64 binaries.

It worked on my machine :-/ I suppose CMake may have changed (I used 3.20). Let me check if changing to LIBXML2_INCLUDE_DIRS works for me also...

hans added a comment.Jul 14 2022, 10:23 AM

Let me check if changing to LIBXML2_INCLUDE_DIRS works for me also...

It does. Updated in 77350198d344f7c07ef221e967613c9e65f88aca