This is an archive of the discontinued LLVM Phabricator instance.

[Proof of concept] Add an opt-in way to statically link a hermetic libxml2
Needs ReviewPublic

Authored by thakis on Jul 13 2022, 9:44 AM.

Details

Reviewers
hans
Summary

When LLVM_ENABLE_LIBXML2 is set to HERMETIC, we now download libxml2
at cmake time, build it at build time, and statically link to it
in llvm-mt.

This reverts https://reviews.llvm.org/D129571 and uses
LLVM_ENABLE_LIBXML2=HERMETIC instead for the Windows installer.


This works for llvm-mt, but c-index-test and lldb also pick up the hermetic libxml2, and since the built libxml2 is so minimal, fail to link.

But even if that worked, I'm not sure if this is worth it. It looks kind of janky: The external project build step runs on every build; every target depending on libxml needs some extra code to force the install step to run, cmake is a bit temperamental about this, and so on. Maybe this isn't all that much easier than what D129571 did.

Or maybe there's a better way for doing this that I can't see.

Maybe this should be WindowsManifest-specific and WindowsManifest should optionally depend on the hermetic stuff, and HermeticLibXML2.cmake shouldn't try to pretend its find_package()-compatible. Then lldb and c-index-test wouldn't pick up the hermetic lib.

I've spent more time on this than I should've, so uploading as-is. Maybe someone has an opinion on this.

Diff Detail

Event Timeline

thakis created this revision.Jul 13 2022, 9:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 13 2022, 9:44 AM
thakis requested review of this revision.Jul 13 2022, 9:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 13 2022, 9:44 AM
hans added a comment.Jul 14 2022, 2:58 AM

It certainly simplifies build_llvm_release.bat a lot :-)

nit: Calling this hermetic seems counter-intuitive here, since it's actually pulling in stuff from way outside the build -- the opposite of hermetic really. Maybe the flag value should be "DOWNLOAD" or something like that?

But yeah, also not sure whether this is worth it. I kind of wish llvm-mt didn't have this dependency and could use something simpler, but I don't know whether that's realistic.