Skip to content

[llvm] Disable optional dependencies to libxml2 and zlib#11180

Closed
yurybura wants to merge 1 commit intomicrosoft:masterfrom
yurybura:llvm10-build-fix
Closed

[llvm] Disable optional dependencies to libxml2 and zlib#11180
yurybura wants to merge 1 commit intomicrosoft:masterfrom
yurybura:llvm10-build-fix

Conversation

@yurybura
Copy link
Copy Markdown
Contributor

@yurybura yurybura commented May 5, 2020 •
edited
Loading

This PR fixes LLVM CMake error on Linux with installed Libxml2:

/usr/bin/ar: creating t.a
CMake Error at lib/WindowsManifest/CMakeLists.txt:11 (get_filename_component):
  get_filename_component unknown component optimized

-- Version: 0.0.0
-- Performing Test HAVE_GNU_POSIX_REGEX
-- Performing Test HAVE_GNU_POSIX_REGEX
-- Performing Test HAVE_GNU_POSIX_REGEX -- failed to compile
-- Performing Test HAVE_POSIX_REGEX
-- Performing Test HAVE_POSIX_REGEX
-- Performing Test HAVE_POSIX_REGEX -- success
-- Performing Test HAVE_STEADY_CLOCK
-- Performing Test HAVE_STEADY_CLOCK
-- Performing Test HAVE_STEADY_CLOCK -- success

Steps to reproduce:

vcpkg install libxml2 llvm

Related to #11174, #10295

@Neumann-A
Copy link
Copy Markdown
Contributor

Neumann-A commented May 5, 2020

The correct fix would be to fix lib/WindowsManifest/CMakeLists.txt

Just patch the logic there away and leave only

  set_property(TARGET LLVMWindowsManifest PROPERTY
    LLVM_SYSTEM_LIBS xml2)

basically get_filename_component(xml2_library ${LIBXML2_LIBRARIES} NAME) is always wrong..... no matter how you try to use it.

@yurybura
Copy link
Copy Markdown
Contributor Author

yurybura commented May 5, 2020 •
edited
Loading

@Neumann-A You are right. I decided to disable Libxml2 because I want to have the same dependencies list on all machines. Otherwise, I should add an explicit dependency in the CONTROL file. Please make a separate issue if you really need this dependency. Thank you.

@yurybura yurybura mentioned this pull request May 5, 2020
Merged
@strega-nil
Copy link
Copy Markdown
Contributor

strega-nil commented May 5, 2020

This looks good to me; we can add the features back later after we fix CI.

@strega-nil strega-nil self-requested a review May 5, 2020 19:01
@LilyWangL LilyWangL self-assigned this May 6, 2020
@BillyONeal
Copy link
Copy Markdown
Member

BillyONeal commented May 6, 2020

prbuild says this change didn't fix it ?

@BillyONeal
Copy link
Copy Markdown
Member

BillyONeal commented May 6, 2020

#11190 <-- I'm baselining this for now so that other PRs can make progress.

@yurybura
Copy link
Copy Markdown
Contributor Author

yurybura commented May 6, 2020 •
edited
Loading

@BillyONeal This PR fixes LLVM CMake error in the PR #11174 which should fix LLVM link error)
After merge this PR please re-run CI in the #11174
Thank you!

@BillyONeal
Copy link
Copy Markdown
Member

BillyONeal commented May 6, 2020

@yurybura OK I cherry picked this commit into that one so we can see if it passes.

@strega-nil
Copy link
Copy Markdown
Contributor

strega-nil commented May 6, 2020

I think the best thing would be to merge #11174 including this commit, but not this PR. Therefore, I'm considering this PR closed for now.

@strega-nil strega-nil closed this May 6, 2020
@BillyONeal BillyONeal mentioned this pull request May 6, 2020
Merged
strega-nil pushed a commit that referenced this pull request May 15, 2020
@yurybura
[llvm] add more backend options, fix issues (#11268)
99a832a 
* [llvm] add more backend options, fix #11158

* [llvm] disable optional dependencies to libxml2 and zlib (merge PR #11180)

* [llvm] update version

* [llvm] fix copy-paste typo

* [llvm] disable dl library on Windows, fixes #11309
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@strega-nil strega-nil strega-nil approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@LilyWangL LilyWangL

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@yurybura @Neumann-A @strega-nil @BillyONeal @LilyWangL