-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
libxml2: fix mingw on Linux + double quotes + libxml2 does not create cmake targets #2714
Conversation
All green in build 1 (
|
Could you also bump |
recipes/libxml2/all/conanfile.py
Outdated
if self.settings.os == "Linux" or self.settings.os == "Macos": | ||
self.cpp_info.system_libs.append('m') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really required for Macos?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know of another recipe that adds m
for Macos.
I can't check it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's good catch, it shouldn't be needed on macOS - everything reside in libSystem.dylib
All green in build 2 (
|
@SpaceIm @danimtb @prince-chrismc |
ext = ".exe" if self.settings.os == "Windows" else "" | ||
autotools.make(["xmllint" + ext, "xmlcatalog" + ext, "xml2-config"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be a function since it's duplicated here
recipes/libxml2/all/conanfile.py
Outdated
if not self.options.shared: | ||
os.unlink(os.path.join(self.package_folder, "bin", "libxml2.dll")) | ||
os.unlink(os.path.join(self.package_folder, 'lib', 'libxml2_a_dll.lib')) | ||
os.unlink(os.path.join(self.package_folder, 'lib', 'libxml2_a.lib' if self.options.shared else 'libxml2.lib')) | ||
os.unlink(os.path.join(self.package_folder, "lib", "libxml2_a_dll.lib")) | ||
os.unlink(os.path.join(self.package_folder, "lib", "libxml2_a.lib" if self.options.shared else "libxml2.lib")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bit is a little confusing
shared --> remove dll
remove _a_
dll
shared --> remove _a_
lib
static--> remove lib ???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's weird, but it works. After this cleanup, only relevant static or import lib remains.
There's only one component which looks like the default target. Was the motivation for the CMake components the extra "FIXME"s? |
On my MinGW@windows, static works, but not shared:
Maybe msys2/MSYS2-packages#171 or msys2/MSYS2-packages#1917 could help? Would it work again by reverting MinGW modifications in #2149 (call |
recipes/libxml2/all/conanfile.py
Outdated
|
||
self.cpp_info.components["xml2lib"].names["pkg_config"] = "libxml-2.0" | ||
self.cpp_info.filenames["cmake_find_package"] = "LibXml2" | ||
self.cpp_info.filenames["cmake_find_package_multi"] = "LibXml2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So why not libxml2
(for cmake_find_package_multi
only)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'll remove the cmake_find_package
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why removing cmake_find_package? I'm a little bit lost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just checked a output and the configuration creates libxml2-config.cmake
.
So, as you said, it should set the cmake_find_package_multi
name to libxml2
.
So I think cmake_find_package
should not be set.
Or should it be set to a hidden name? Like _libxml2
.
Or should it be set to the same name as cmake_find_package_multi
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LibXml2
for cmake_find_package
is fine: https://cmake.org/cmake/help/latest/module/FindLibXml2.html
But all of this (module and config) doesn't require components (except if you anticipate "executable components")
So to resume:
- cmake_find_package:
- filename: LibXml2
- importedtarget: LibXml2::LibXml2
- cmake_find_package_multi:
- filename: libxml2
- importedtarget: whatever you want (let's say LibXml2::LibXml2)
Does it not work on mingw@windows for you?
I don't think you added |
An unexpected error happened and has been reported. Help is on its way! 🏇 |
Some configurations of 'libxml2/2.9.9' failed in build 6 (
|
Sorry I don't understand. With my MinGW, libxml2 shared doesn't work with autotools, this why I've restored makefile.mingw build. |
…orrect library for utils
@SpaceIm While trying out #3314 on mingw@Windows, I needed to apply these patches to libxml2 (libxml2 is a dependency of xmlsec) Can you please review? |
Some configurations of 'libxml2/2.9.9' failed in build 8 (
|
I detected other pull requests that are modifying libxml2/all recipe: This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This pull request has been automatically closed because it has not had recent activity. Thank you for your contributions. |
Specify library name and version: libxml2/all
Fixes #2708
@SpaceIm
conan-center hook activated.