build: make libxml2 handling explicit and use in tests #21832
Conversation
|
@swift-ci please test |
|
CC: @gottesmm @Rostepher |
|
Build failed |
|
Build failed |
CMakeLists.txt
Outdated
| if(LLVM_ENABLE_LIBXML2) | ||
| set(LIBXML2_FOUND NO) | ||
| find_package(LibXml2) | ||
| if(LIBXML2_FOUND) |
There was a problem hiding this comment.
I took a quick look in clang/llvm to understand this a bit better.
This is in clang:
set (LIBXML2_FOUND 0)
find_package(LibXml2 2.5.3 QUIET)
if (LIBXML2_FOUND)
set(CLANG_HAVE_LIBXML 1)
endif()
Should we require a version like that as well? Also, maybe this bug is asking to hoist this into a macro in LLVM. (Just trying to understand).
There was a problem hiding this comment.
The version request only makes sense if there is a dependency on an API that was introduced after the initial release. I don't know if that is the case. I don't see any harm in requesting the minimum version to be the same as what is shipped on macOS just to ensure that we have the same base line of support. The problem is, I don't know what the version on macOS is :-p.
I don't really see there being a benefit to the 2 lines being in LLVM.
There was a problem hiding this comment.
Ok. I just wasn't sure what the comment was asking for (in terms of the 2 lines).
@jrose-apple do you know the version of libxml we should require here?
There was a problem hiding this comment.
Ping! I want to get this handled as I'm trying to handle the last of the failures on Windows.
There was a problem hiding this comment.
Oops, I must have missed this the first time around. We use libxml for doc comment processing, so @nkcsgexi would probably know what our dependencies are.
There was a problem hiding this comment.
@jrose-apple, yeah, which has some tests which should be conditional on whether we built with libxml2 or not.
|
@swift-ci please smoke test |
|
@swift-ci please smoke test |
|
@jrose-apple - any thoughts on whether LibXml2 should be required on Linux or make it optional? |
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
Again, that's a question for the SourceKit / source tooling folks rather than me. |
Address the TODO in the build and unify the libxml2 handling with LLVM/clang. Use the information now and propagate this into the lit configuration so that tests can be made aware of libxml2 status.
|
@swift-ci please smoke test |
|
I'm going to merge this since this is needed for the Windows test suite. This is no worse than the status quo, and we can add the versioning in a follow up. This effectively makes |
Address the TODO in the build and unify the libxml2 handling with
LLVM/clang. Use the information now and propagate this into the lit
configuration so that tests can be made aware of libxml2 status.
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves SR-NNNN.