Page MenuHomeFreeBSD

textproc/libxml2: Update to 2.9.13 and migrate to CMake
ClosedPublic

Authored by diizzy on Feb 22 2022, 10:59 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 7, 5:29 PM
Unknown Object (File)
Feb 19 2024, 3:27 AM
Unknown Object (File)
Feb 19 2024, 3:27 AM
Unknown Object (File)
Feb 19 2024, 3:27 AM
Unknown Object (File)
Feb 19 2024, 3:27 AM
Unknown Object (File)
Feb 19 2024, 3:27 AM
Unknown Object (File)
Feb 19 2024, 3:27 AM
Unknown Object (File)
Feb 19 2024, 3:26 AM

Details

Summary

Update to 2.9.13
Convert to CMake
Depend on ICU and (lib)readline to follow other distros

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

diizzy created this revision.

Poudriere doesn't like the python port for some reason, no idea why as it works in ports.

[00:00:04] Warning: (textproc/py-libxml2@py38): Error: Invalid FLAVOR 'py38' for textproc/py-libxml2

The python port doesn't egg-info now, not sure if that really matters (this is also the case with other distros packaging though)...

tcberner added inline comments.
textproc/libxml2/Makefile
24
CMAKE_OFF+=${LIBXML2_SLAVE:DLIBXML2_WITH_PYTHON}

should make it work without the if.

36

^multiple dependencies on the same port.

41

^again redundant dependencies

textproc/libxml2/files/patch-CMakeLists.txt
8 ↗(On Diff #103090)

why?

textproc/py-libxml2/Makefile
14–16

it's a bit asymmetric that you have the _ON here but the _OFF in the main-Makefile.

textproc/py-libxml2/Makefile
12

This should go unless we need egg files and if so patches are needed

textproc/py-libxml2/Makefile
32

hard-coded /usr/local -- python.mk should provide you with the proper path as a single variable for the correct verison.

textproc/libxml2/Makefile
24

Unfortunately not as the slave port (or variable?) gets processed earlier

root@freebsd-dev:/usr/ports/textproc/py-libxml2 # make -V CMAKE_ARGS
... -DLIBXML2_WITH_PYTHON:BOOL=ON -DLIBXML2_WITH_PYTHON:BOOL=OFF ...
36

I've seen both preferences when it comes to LIB_DEPENDS but I can trim that

41

Same as above

textproc/libxml2/files/patch-CMakeLists.txt
8 ↗(On Diff #103090)

This seems more consistent with other projects at least looking on my textbox and is also the same behaviour of the current version of port using autotools

textproc/libxml2/files/patch-CMakeLists.txt
8 ↗(On Diff #103090)

looks unnecessary to me -- does anything break without this patch?

Rework python support option (cred tcberner)
Fix plist issue for documentation
Use correct install path variable for python slave port

Guard PORTDOCS variable for slave ports
Drop patch for removing version infromation for install path of cmake files (not needed)
Reference:
https://cmake.org/cmake/help/latest/command/find_package.html - Config Mode Search Procedure

Poudriere fails when the python slave port is pulled in as a dependency

[00:00:04] Warning: (textproc/py-libxml2@py38): Error: Invalid FLAVOR 'py38' for textproc/py-libxml2

Hi koobs!

If you have time, could you have a look at the python port which Poudriere isn't happy about?

Best regards,
Daniel

Additional note:
mandree@ tested all variants of Python successfully using Poudriere

doesn't work when built as a port with Python 3.8 and 3.10 installed, something is being mixed up in the build. The intention is to build a Py38 based port, but the result is that Py310 seeps in.

$ make -C /usr/ports/textproc/py-libxml2/
...
===>  Staging for py38-libxml2-2.9.13
===>   py38-libxml2-2.9.13 depends on file: /usr/local/bin/python3.8 - found
===>   Generating temporary packing list
/bin/mkdir -p /usr/ports/textproc/py-libxml2/work-py38/stage/usr/local/lib/python3.8/site-packages
install  -m 555 /usr/ports/textproc/py-libxml2/work-py38/libxml2-2.9.13/python/drv_libxml2.py /usr/ports/textproc/py-libxml2/work-py38/stage/usr/local/lib/python3.8/site-packages
install  -m 555 /usr/ports/textproc/py-libxml2/work-py38/.build/libxml2.py /usr/ports/textproc/py-libxml2/work-py38/stage/usr/local/lib/python3.8/site-packages
install  -s -m 0644 /usr/ports/textproc/py-libxml2/work-py38/.build/libxml2mod.so /usr/ports/textproc/py-libxml2/work-py38/stage/usr/local/lib/python3.8/site-packages
install -l rs /usr/ports/textproc/py-libxml2/work-py38/stage/usr/local/lib/python3.8/site-packages/libxml2mod.so /usr/ports/textproc/py-libxml2/work-py38/stage/usr/local/lib/python3.8/site-packages/libxml2mod.so.2.9.13
====> Compressing man pages (compress-man)
====> Running Q/A tests (stage-qa)
Error: /usr/local/lib/python3.8/site-packages/libxml2mod.so is linked to /usr/local/lib/libpython3.10.so.1.0 from lang/python310 but it is not declared as a dependency
This revision now requires changes to proceed.Feb 22 2022, 10:56 PM

Help CMake determine correct (target) version of Python if multiple versions are installed

mandree requested changes to this revision.EditedFeb 24 2022, 11:12 PM

Much better for py-libxml2.

Findings:

  • "make -C /usr/ports/textproc/libxml2 test" does not seem to work yet, however, although it does set TEST_TARGET=test, the cmake rig does not seem to provide a "test" target - renaming TEST_TEST_TARGET=test makes the TEST_TARGET conditional on the TEST option, so "make test" gets ignore if the TEST option is disabled, fixing the bug.
===>  Testing for libxml2-2.9.13
ninja: error: unknown target 'test', did you mean 'help'?
*** Error code 1
  • it would be good to see tests execute in parallel. For textproc/py-libxml2 that seems to work (I used CTEST_PARALLEL_LEVEL=16 for 8 cores * 2 threads). ctest also supports a -j argument, and there is _MAKE_JOBS that you can leverage (it includes -j and the MAKE_JOBS_NUMBER).
This revision now requires changes to proceed.Feb 24 2022, 11:12 PM

You need to enable the "TEST" menu option, I don't think we need safeguard it though as if you unset TEST_TARGET it'll exit without running any tests which I would regard as a false positive.

Daniel, I think the port is broken the way it is. Either it enables the test configuration unconditionally (and without a TEST option), or else you need to make it conditional as suggested. It is common for ports that do not have support for tests to just silently pass a "make test" in that case (for instance, if TEST_TARGET is empty).

Move DOCS to plist file in master port
Move TEST_TARGET to TEST menu option

Include py-libxml2 changes

Thanks for all the hard work!

Looks good to me, passes bulk -tC, passes bare build with all Python versions installed and make test (and with CTEST_PARALLEL_LEVEL=16 for me)

  • Suggestion #1: run tests in parallel on multicore machines. Add to textproc/libxml2/Makefile this line: MAKE_ENV+= CTEST_PARALLEL_LEVEL=${MAKE_JOBS_NUMBER}
  • Suggestion #2: if it were my port, I would leave PORTDOCS=... and NOT add a gazillion lines to pkg-plist that need to be maintained on future updates.
This revision is now accepted and ready to land.Feb 26 2022, 9:03 AM
textproc/py-libxml2/Makefile
14–16

I think it makes more sense to keep port related configuration (or at least as much as possible) within the port rather than spreading it around and I'm not sure how to "fix" this without reverting to if statements which I would like to avoid if possible but that's just my thoughts on it.

Rename DEBUG option to DEBUG_MOD as upstream refers to as "the debugging module"
Enable it by default as it's required by itstool to build gtk-doc

This revision now requires review to proceed.Feb 27 2022, 8:07 AM

To avoid unnecessary "foot shooting" always enable the debugging module

Hmm... so the generated pc file is a bit wonky :-/

prefix=${pcfiledir}/../..
exec_prefix=${prefix}
libdir=${prefix}/lib
includedir=${prefix}/include
modules=1

Name: libXML
Version: 2.9.13
Description: libXML library version2.
Requires:
Libs: -L${libdir} -lxml2
Libs.private: -licudata -licui18n -licuuc -pthread -lz -llzma  -lm
Cflags: -I${includedir}/libxml2

Prefix is for sure wrong, however should Cflags also include -I${includedir} (I see some ports doing that)?
I'm asking because audio/libmusicbrainz5 throws

/usr/local/include/libxml2/libxml/encoding.h:31:10: fatal error: 'unicode/ucnv.h' file not found

Output is now:

prefix=/usr/local
exec_prefix=${prefix}
libdir=${prefix}/lib
includedir=${prefix}/include
modules=1

Name: libXML
Version: 2.9.13
Description: libXML library version2.
Requires:
Libs: -L${libdir} -lxml2
Libs.private: -licudata -licui18n -licuuc -pthread -lz -llzma  -lm
Cflags: -I${includedir}/libxml2 -I${includedir}
This revision was not accepted when it landed; it landed in state Needs Review.Mar 8 2022, 1:34 PM
This revision was automatically updated to reflect the committed changes.

Adjust prefix path in xml2-config
Add -L/usr/lib to libs in pkgconfig file (.pc) and xml2-config
Install m4 file

Forgot to remove extra USES= line during testing

Adjust cflags in pkgconfig file and xml2-config

textproc/libxml2/files/patch-CMakeLists.txt
12 ↗(On Diff #103685)

^that should not be necessary

Remove temporary USES variable again....

This revision was not accepted when it landed; it landed in state Needs Review.Mar 25 2022, 5:15 PM
This revision was automatically updated to reflect the committed changes.
def added inline comments.
textproc/libxml2/pkg-plist
52

@diizzy The current textproc/libxml2 package doesn't include the static library. Was it removed by mistake?