[podofo] Change to github and update version to 0.10.0

[FrankXie05]

Fix #31578

Note: Changed to GitHub because upstream indicates that it has been transferred to github for development.
https://sourceforge.net/projects/podofo/

Development of PoDoFo has been moved to GitHub: https://github.com/podofo/podofo

However, the api has undergone major changes in version 0.10.0.
We need to perform compatibility tests for downstream.
https://github.com/podofo/podofo/wiki/PoDoFo-API-migration-guide/#098---0100
https://github.com/podofo/podofo#api-migration

• 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.

[FrankXie05]

@playgithub Has been passed the CI checking, could you please test the latest version api ?

[playgithub]

I'm just going to use podofo, not any experience now.
I'll give feedback when neccessary.
Thanks

[dg0yt]

We need to perform compatibility tests for downstream.

No. There is not a single port using it.

[FrankXie05]

It has been verified that the change of the api will carry over to the next version. But this port will not affect vcpkg.

[FrankXie05]

The usage(Including static and dynamic) has been tested successfully locally.

find_package(PoDoFo CONFIG REQUIRED)

target_link_libraries(main PRIVATE podofo_shared)
Or
target_link_libraries(main PRIVATE podofo_static podofo_private)

Waiting for feature fontconfig testing.

[dg0yt]

target_link_libraries(main PRIVATE podofo_static podofo_private)

Private is public, really?
Well, it is unofficial, anyways.

[FrankXie05]

@dg0yt

if(PODOFO_BUILD_SHARED)
    export(TARGETS podofo_shared FILE "${PROJECT_BINARY_DIR}/podofoConfig.cmake")
else()
    export(TARGETS podofo_static podofo_private FILE "${PROJECT_BINARY_DIR}/podofoConfig.cmake")
endif()

No, it's official. I just changed the location of the export.

[FrankXie05]

The feature fontconfig is tested successfully in the following triplet:

• x86-windows
• x64-windows
• x64-windows-stataic

[dg0yt]

No, it's official. I just changed the location of the export.

No, it is not official. The patch adds the export and the installation of the exported config. The keyword EXPORT isn't used upstream: https://github.com/podofo/podofo/blob/0.10.0/src/podofo/CMakeLists.txt.

[FrankXie05]

No, it is not official. The patch adds the export and the installation of the exported config. The keyword EXPORT isn't used upstream: https://github.com/podofo/podofo/blob/0.10.0/src/podofo/CMakeLists.txt.

No, upstream used the keyword EXPORT.
https://github.com/podofo/podofo/blob/0.10.0/CMakeLists.txt#:~:text=if(PODOFO_BUILD_SHARED),endif()

[dg0yt]

Oh, confusion, also on my side. But I thinked it was worth looking closer:

• Upstream CMakeLists does

  export(TARGETS ... FILE "${PROJECT_BINARY_DIR}/podofoConfig.cmake")

  This is specific to the build tree and not useful for vcpkg.
  But it does qualify as setting the standard for the official config name and namespace.
  (NB: podofoConfig.cmake is for case-sensitive package name matching on find_package(podofo).)

• The patch adds to src/CMakeLists.txt:

  install(TARGETS podofo_static podofo_private EXPORT PoDoFoConfig ...)
  install(EXPORT PoDoFoConfig DESTINATION share/podofo)

  These commands are for installed trees, i.e. needed for vcpkg.
  But it uses a different case-sensitive package name, matching find_package(PoDoFo). This is unofficial.

Now I see the following tasks:

• Rectify the config file name: the official package name is spelled podofo. Options:
  • Use podofo-config.cmake. This file name is for case-insensitive package name matching.
  • Use upstream's podofoConfig.cmake. However, to not break existing vcpkg users, you will need to provide a forwarding config under the old package name spelling.
• Add a usage file to get rid of vcpkg tools's limited heuristics.

podofo provides CMake targets:

    find_package(podofo CONFIG REQUIRED)
    target_link_libraries(main PRIVATE $<IF:$<TARGET_EXISTS:podofo_shared>,podofo_shared,podofo_static>)

[dg0yt]

My major concerns are resolved. Thank you for the improvements.

[FrankXie05]

Upstream doesn't depend on libxml2 features, so disabling it can reduce build time.
https://github.com/podofo/podofo/blob/33e9719ad7e2dd11631f3af6035986c781c18748/CMakeLists.txt#L196

[ceztko]

Upstream doesn't depend on libxml2 features, so disabling it can reduce build time.
https://github.com/podofo/podofo/blob/33e9719ad7e2dd11631f3af6035986c781c18748/CMakeLists.txt#L196

PoDoFo maintainer here: no, libxml2 is now a requirement. Also OpenSSL is added as a strict requirement. The bar has been raised on this principle: anything needed to access the structure/metadata of the document is now a requirement. Anything else needed to decode content can be optional. Based on this principle:

• libxml2: needed to access metadata that are unique to XMP (eg. PDF/A level), or all metadata entries if /Info is missing;
• OpenSSL: needed to decrypt structure of the document.

Instead libpng, libjpeg, libtiff are needed just to decode content, so they are left as optional.

PoDoFo wants to be as much package manager friendly as possible, so feel free to suggest (non invasive) changes in the build system. I appear to have inspected already the usage of PoDoFo in vcpkg and Conan, and I added few more CMake switches and clarified handling of static vs shared library compilation.

[dg0yt]

@ceztko Thanks for having an eye on this port.

Upstream doesn't depend on libxml2 features, so disabling it can reduce build time.

PoDoFo maintainer here: no, libxml2 is now a requirement. Also OpenSSL is added as a strict requirement. ...

Don't worry. The comment needs context to not be misunderstood.
libxml2[core] is a requirement.
What was requested and implemented for libxml2 is removing the hard dependency on the default features of port libxml2, i.e. features iconv, lzma, zlib. Users cannot opt out of building libxml2 feature lzma as long as some port depends on the default features of libxml2.

[ceztko]

The comment needs context to not be misunderstood.

Thank you for the clarification.

[ceztko]

if(PODOFO_BUILD_SHARED)
export(TARGETS podofo_shared FILE "${PROJECT_BINARY_DIR}/podofoConfig.cmake")
else()
export(TARGETS podofo_static podofo_private FILE "${PROJECT_BINARY_DIR}/podofoConfig.cmake")
endif()

@FrankXie05 This was present in PoDoFo 0.9.x as well, and I admit to have no idea what it does. Please advice if this can be migrated to more modern CMake usage.

[dg0yt]

This was present in PoDoFo 0.9.x as well

Cf. the first part of #31584 (review).
For the modern CMake usage, you need to export CMake config. Given the dependencies being linked as targets, this also means to provide the corresponding find_dependency calls which are added in the portfile here - normally only for static linkage, but LibXml2::LibXml2 also appears for dynamic linkage ATM.

Now that podofo no longer builds static and shared at the same time, you wouldn't need PODOFO_BUILD_STATIC but could follow the standard BUILD_SHARED_LIBS.
And in the end you could migrate to uniform target/libary name. But this would probably need to be complemented by some polyfill to facilitate migrations.

BTW this port update triggered vcpkg baseline regressions because the presence of other packages causes non-obvious behavioural changes - for the x64-windows triplet (the only one with dynamic linkage in vcpkg CI), the port unexpectedly installed pkg-config files... Follow-up: #31658

[ceztko]

For the modern CMake usage, you need to export CMake config.

Ok, I think I understood the matter and I am now considering merging your patch as is. I don't see much use of saving the config files in the binary dir, and PoDoFo CMakeLists.txt is well optimized to be included in other CMake projects.

the port unexpectedly installed pkg-config files... Follow-up: #31658

I'm not sure what you mean here but if I merge the patch above I think you are fine with that.

Now that podofo no longer builds static and shared at the same time

Previously I studied the matter and noticed that this was not working 100% as one would expect, so I consider this feature only "accidentally working". Now having both targets enabled would even more difficult because of private static targets so I just disabled it for the time being, or until a correct and maintainable solution to have both enabled is found.

you wouldn't need PODOFO_BUILD_STATIC but could follow the standard BUILD_SHARED_LIBS.

I thought about it, and I actually tried it and committed it during development. I ended not liking it, and semantically I just prefer PODOFO_BUILD_STATIC which is default false, and will make a side effect when set to true. There's no advantage for me in using BUILD_SHARED_LIBS other than being more known variable name. Also it does change the default of add_library which I don't use so it would be an unwanted side effect. Granted: BUILD_SHARED_LIBS may be nice for direct/indirect users of PoDoFo, but I think it's not a big deal because there's currently nothing in CMake that is trying to enforce that convention, and anything that can't be enforced is not a strict rule.

And in the end you could migrate to uniform target/libary name.

Please elaborate more what you need with this.

[dg0yt]

For the modern CMake usage, you need to export CMake config.

Ok, I think I understood the matter and I am now considering merging your patch as is. I don't see much use of saving the config files in the binary dir, and PoDoFo CMakeLists.txt is well optimized to be included in other CMake projects.

For the general case, it may take a little bit more effort than just the patch: the portfile injects find_dependency calls. This needs to be adjusted for upstreaming. You may want to limit it to static linkage.

the port unexpectedly installed pkg-config files... Follow-up: #31658

I'm not sure what you mean here but if I merge the patch above I think you are fine with that.

They should always be installed. External usage is independent of pkg-config availability at build time.
Changes are needed to support static linkage.

Now that podofo no longer builds static and shared at the same time

Previously I studied the matter and noticed that this was not working 100% as one would expect, so I consider this feature only "accidentally working". Now having both targets enabled would even more difficult because of private static targets so I just disabled it for the time being, or until a correct and maintainable solution to have both enabled is found.

vcpkg doesn't need/want both simultaneously.

you wouldn't need PODOFO_BUILD_STATIC but could follow the standard BUILD_SHARED_LIBS.

I thought about it, and I actually tried it and committed it during development. I ended not liking it, and semantically I just prefer PODOFO_BUILD_STATIC which is default false, and will make a side effect when set to true. There's no advantage for me in using BUILD_SHARED_LIBS other than being more known variable name. Also it does change the default of add_library which I don't use so it would be an unwanted side effect. Granted: BUILD_SHARED_LIBS may be nice for direct/indirect users of PoDoFo, but I think it's not a big deal because there's currently nothing in CMake that is trying to enforce that convention, and anything that can't be enforced is not a strict rule.

Well, it would be one variable less invented differently in each project, for a single problem.
If it is about the default, use

option(BUILD_SHARED_LIBS "Build a shared podofo library" ON)

(Alternatively, it would be possible to just derive the default of PODOFO_BUILD_STATIC from BUILD_SHARED_LIBS.)

And in the end you could migrate to uniform target/libary name.

Please elaborate more what you need with this.

I would prefer if I could uniformly use:

find_package(podofo REQUIRED)
target_link_libraries(main PRIVATE podofo::podofo)

This doesn't require to rename the targets - it can be implemented in the config using an interface target.

[ceztko]

For the general case, it may take a little bit more effort than just the patch [...] You may want to limit it to static linkage.

The problem here is I have no experience with usage of config files, I either include subprojects or find dependencies with find_package. Maybe you mean that the config files should better supply compile/linking flags, or specify variables that points to include directory locations? If you show me a (simple) project that does it right it would be easier for me to understand what to do here.

Well, it would be one variable less invented differently in each project, for a single problem.

You make it easy, but deriving PODOFO_BUILD_STATIC from BUILD_SHARED_LIBS may be unwanted, because it's easily inherited from external projects, and I want a predictable behavior without the need to set variables that has opposite meaning (which otherwise is a mess). Sorry, but as said: if it's not enforced by CMake I feel free to put my grain of salt here.

I would prefer if I could use uniformly use:
target_link_libraries(main PRIVATE podofo::podofo)

Recently we added support to refer to (most) dependencies with that target notation. Please show me a simple project that export targets this way from within the build project (not a Find<module>.cmake) and I will implement it.

[dg0yt]

Recently we added support to refer to (most) dependencies with that target notation. Please show me a simple project that export targets this way from within the build project (not a Find<module>.cmake) and I will implement it.

I'm not sure the aim became clear. My requests is that the user doesn't need to know if the podofo lib was build with static or shared linkage.
For "internal" builds, you should get away with add_library(podofo::podofo ALIAS <actual_target>). Examples:
CURL: https://github.com/curl/curl/blob/4efa0b5749bb7c366e1c3bda90650ef864d3978e/lib/CMakeLists.txt#L67-L70
PROJ: https://github.com/OSGeo/PROJ/blob/1600a3a84363e41cd10a6a9d711dc8b09c71da12/src/lib_proj.cmake#L365

[ceztko]

I'm not sure the aim became clear. My requests is that the user doesn't need to know if the podofo lib was build with static or shared linkage.

That's more clear. I think that's a reasonable request now that static/shared can't be build together (but consider there's a PODOFO_LIBRARIES variable with the right targets already). I will consider implementing changes discusses here for 0.10.1. I will ask for some feedback here when done.

[ceztko]

Hello, I merged your patch about installing config files during install in podofo/podofo@9a07376 . I'm not exporting them in the binary folder anymore, we'll see if someone complains about it (I personally don't use them), but the recommended approach is to just include the project file anyway for CMake users, and installing for everyone else. I also added the podofo::podofo alias as you recommended podofo/podofo@97447ad, and I released 0.10.1 . If you have further recommendations for future releases please tell me, I'm listening, as I want to maintain a policy of zero patches for package managers. If you have, please prefer scratching modifications in form of patches/small snippets, as I don't know all possible CMake usage patterns as you do, and it may be difficult for me to understand what to do by your descriptions.