Skip to content
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] Update to 2.13.5 #39478

Closed
wants to merge 65 commits into from
Closed

Conversation

jimwang118
Copy link
Contributor

@jimwang118 jimwang118 commented Jun 24, 2024

Fixes #39444
Update librsvg version to 2.40.21.
Update libxslt version to 1.1.41.

  • Changes comply with the maintainer guide.
  • SHA512s are updated for each updated download.
  • The "supports" clause reflects platforms that may be fixed by this new version.
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

All features passed with following triplet:

x64-windows

@jimwang118 jimwang118 added info:internal This PR or Issue was filed by the vcpkg team. category:port-update The issue is with a library, which is requesting update new revision labels Jun 24, 2024
@jimwang118 jimwang118 added the category:port-bug The issue is with a library, which is something the port should already support label Jun 25, 2024
@dg0yt
Copy link
Contributor

dg0yt commented Jun 25, 2024

Maybe you only update to 2.11.8 now.
The update to 2.13 could be used to trim patching.

ports/libxml2/fix_cmakelist.patch Outdated Show resolved Hide resolved
ports/libxml2/fix_cmakelist.patch Outdated Show resolved Hide resolved
ports/libxml2/fix_cmakelist.patch Outdated Show resolved Hide resolved
ports/libxml2/portfile.cmake Outdated Show resolved Hide resolved
ports/libxslt/vcpkg.json Outdated Show resolved Hide resolved
ports/libxslt/portfile.cmake Outdated Show resolved Hide resolved
@Osyotr
Copy link
Contributor

Osyotr commented Nov 29, 2024

@jimwang118
Copy link
Contributor Author

Please also include this patch https://gitlab.gnome.org/GNOME/libxslt/-/commit/7504032097712714aafe309d54f2ad57e3364bac

This patch is already in fix_cmakelist.patch.

@Osyotr
Copy link
Contributor

Osyotr commented Dec 2, 2024

This patch is already in fix_cmakelist.patch.

This is a patch for libxslt, not libxml2. They both had the same issue.

@jimwang118 jimwang118 marked this pull request as ready for review December 4, 2024 02:27
@jimwang118 jimwang118 marked this pull request as draft December 4, 2024 02:28
@jimwang118 jimwang118 marked this pull request as ready for review December 5, 2024 01:54
@jimwang118 jimwang118 mentioned this pull request Dec 5, 2024
7 tasks
Copy link
Contributor

@donny-dont donny-dont left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just making some suggestions based on my own experience doing #42528 to improve this.

Comment on lines +94 to +105
diff --git a/libxml2-config.cmake.cmake.in b/libxml2-config.cmake.cmake.in
index aead949..20eb836 100644
--- a/libxml2-config.cmake.cmake.in
+++ b/libxml2-config.cmake.cmake.in
@@ -157,7 +157,7 @@ if(NOT LIBXML2_SHARED)
endif()

if(WIN32)
- list(APPEND LIBXML2_LIBRARIES ws2_32)
+ list(APPEND LIBXML2_LIBRARIES ws2_32 bcrypt)
endif()
endif()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more robust version of this is on the 2.13 branch of the repository at GNOME/libxml2@fe1ee0f

Comment on lines +63 to +66
${CMAKE_CURRENT_BINARY_DIR}/libxml2-config-version.cmake
VERSION ${PROJECT_VERSION}
- COMPATIBILITY ExactVersion
+ COMPATIBILITY SameMajorVersion
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fixed in a commit to the 2.13 branch at GNOME/libxml2@b347a00

pkg_check_modules(ZLIB_PC IMPORTED_TARGET zlib)
if(ZLIB_PC_FOUND)
@@ -438,23 +438,9 @@ set_target_properties(
SOVERSION ${LIBXML_MAJOR_VERSION}
)

+set(XML_LIB_NAME xml2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more appropriate place to put the library name for pkgconfig is at https://github.com/GNOME/libxml2/blob/fe1ee0f25f43e33a9981fd6fe7b0483a8c8b5e8d/CMakeLists.txt#L661

set(XML_LIBS "-lxml2")

This is passed to the pkgconfig setup.

pkg_check_modules(ZLIB_PC IMPORTED_TARGET zlib)
if(ZLIB_PC_FOUND)
@@ -438,23 +438,9 @@ set_target_properties(
SOVERSION ${LIBXML_MAJOR_VERSION}
)

+set(XML_LIB_NAME xml2)
if(MSVC)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By moving the above this can turn into an if(0) which would then prevent the postfix being applied.

Comment on lines +7 to +13
target_link_libraries(LibXml2 PRIVATE ICU::data ICU::i18n ICU::uc)
if(WIN32)
- set(ICU_LDFLAGS "-licudt -licuin -licuuc")
+ set(ICU_LIBS "icu-i18n")
else()
- set(ICU_LDFLAGS "-licudata -licui18n -licuuc")
+ set(ICU_LIBS "icu-i18n")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is no longer needed. If you look further in the file at https://github.com/GNOME/libxml2/blob/fe1ee0f25f43e33a9981fd6fe7b0483a8c8b5e8d/CMakeLists.txt#L383-L388 you can see pkg config handling was added.

    pkg_check_modules(ICU_PC IMPORTED_TARGET icu-i18n)
    if(ICU_PC_FOUND)
        list(APPEND XML_PC_REQUIRES icu-i18n)
    else()
        list(APPEND XML_PC_LIBS "${ICU_LDFLAGS}")
    endif()

So it looks for the pkgconfig module and if it finds it it will end up in the Requires section of the generated .pc file.

The only gotcha is that you need to add this to the portfile so pkgconfig is detected. It is not detected on the x64-windows-static triplet which was a fun thing to figure out for me.

vcpkg_find_acquire_program(PKGCONFIG)
set(ENV{PKG_CONFIG} "${PKGCONFIG}")

Comment on lines +20 to +26
target_link_libraries(LibXml2 PRIVATE LibLZMA::LibLZMA)
- set(LibLZMA_LDFLAGS "-llzma")
+ set(LZMA_LIBS "liblzma")
list(APPEND XML_PRIVATE_LIBS "${LibLZMA_LDFLAGS}")
pkg_check_modules(LibLZMA_PC IMPORTED_TARGET liblzma)
if(LibLZMA_PC_FOUND)
@@ -401,7 +401,7 @@ endif()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed same as ☝️

Comment on lines +30 to +36
- set(ZLIB_LDFLAGS "-lz")
+ set(Z_LIBS "zlib")
list(APPEND XML_PRIVATE_LIBS "${ZLIB_LDFLAGS}")
pkg_check_modules(ZLIB_PC IMPORTED_TARGET zlib)
if(ZLIB_PC_FOUND)
@@ -438,23 +438,9 @@ set_target_properties(
SOVERSION ${LIBXML_MAJOR_VERSION}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed same as ☝️

@jimwang118
Copy link
Contributor Author

Duplicate of #42528.

@jimwang118 jimwang118 closed this Dec 6, 2024
@jimwang118 jimwang118 deleted the up-libxml2 branch December 6, 2024 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support category:port-update The issue is with a library, which is requesting update new revision info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libxml2] update to 2.13.1
7 participants