[thorvg] update to 1.0.1

[laggykiller]

Summary

Changes to recipe: thorvg/1.0.1

Motivation

1.0 contains many improvements over 0.x.x, including some API breaking changes.

Details

This PR adds ability to build thorvg 1.0.1 while still allowing 0.x.x versions to build. (The only caveat is the need to change with_extras to with_lottie_exp)

Note that I previously opened a PR (#29481) for building thorvg 1.0.0 but I accidentally closed it by renaming branch name. I am reopening here.

---

• Read the contributing guidelines
• Checked that this PR is not a duplicate: list of PRs by recipe
• If this is a bug fix, please link related issue or provide bug details
• Tested locally with at least one configuration using a recent version of Conan

---

Add a 👍 reaction to pull requests you find important to help the team prioritize, thanks!

[uilianries]

Hello @laggykiller Thank you for your contribution!

As this is a new major version, and not only due to the ABI breakage but also as a matter of compatibility and maintenance, the version 0.x was considered unstable.

As a result of this new important release, I would suggest keeping only version 1.0.1 for this recipe: Remove previous versions from conandata.yml and config.yml, update the test package to expect only version 1.x, and simplify the all/conanfile.py to avoid doing checks based on the version.

https://github.com/thorvg/thorvg/releases/tag/v1.0.0

[laggykiller]

@uilianries I have dropped 0.x as your instruction

[uilianries]

Suggested change

	"with_lottie_exp": True,
	"with_openmp": False,
	"with_gl_variant": False,

Let's simplify it as much as possible: This recipe contains several options already, and as much as they look useful, the CI does not test their combination, which can result in errors for the buggy cases. Also, we don't know how useful they are in terms people wanting it.

Still, new options can be injected into a Meson build by using a custom Meson conf.ini file with your extra options, without changing the recipe with a new revision.

Reference:

• https://docs.conan.io/2/reference/tools/meson/mesontoolchain.html#project-options
• https://docs.conan.io/2/reference/tools/meson/meson.html#conf

[laggykiller]

Still, new options can be injected into a Meson build by using a custom Meson conf.ini file with your extra options, without changing the recipe with a new revision.

Do you mean that if the end user want to enable all these extra options, they have to add this into conanfile.py:

def generate(self):
    tc = MesonToolchain(self)
    tc.project_options["extra"] = "['opengl_es', 'lottie_exp', 'openmp']"
    tc.generate()

the CI does not test their combination, which can result in errors for the buggy cases.

But your suggestion would cause these flags not tested by CI at all, which is like burying head in sand and not to catch potential bugs in test.

Also I noted that actually OpenMP is actually enabled by default (https://github.com/thorvg/thorvg/blob/15aabb9b0eb9652345ac1f1f7e1fae8a5b5bcf90/meson_options.txt#L69). Let me update the PR.

With lottie_exp and openmp enabled by default in thorvg, I would say these options are quite useful and I think this change would make these options disabled by default and hard to re-enable by end-users.

[uilianries]

But your suggestion would cause these flags not tested by CI at all, which is like burying head in sand and not to catch potential bugs in test.

The CI idea is not to test flags, but to validate that a package is correctly done: a library can be linked, and other expected files are there, like headers and license files.

Covering if a flag or feature is working is not part of the package manager's duty; that's something expected from the upstream to have tested and validated it before. The test package should be as simple as possible, when you add a single new option, it opens a door for maintenance and increases the recipe complexity.

Also I noted that actually OpenMP is actually enabled by default (thorvg/thorvg@15aabb9/meson_options.txt#L69). Let me update the PR.

As OpenMP is enabled by default upstream, adding a new option for it in the recipe is redundant and adds unnecessary maintenance overhead. Users can still manage this through Conan's configuration features without modifying the recipe itself.

To move this PR forward, please revert the addition of these extra options, reducing the number of options. New options can be added later in case more people ask about it in a separate issue. Additionally, it's important to keep the PR scope focused. In this case, the primary objective is adding a new release.

I appreciate your effort in expanding the recipe feature, but adding new options now, only because having a possibility of using sounds too much. For instance, you will need to prove it locally and share a log for each combination, as the CI will not cover it, and for each new version, we will need to revisit carefully any possible change around those options. We have past experience with several recipes whose options were broken when trying to use a different value or in a different scenario.

[laggykiller]

I just noticed a problem: clang on macOS does not come with openmp support, hence compiling thorvg 1.x on macOS with default option of enabling openmp will fail with:

../src/src/renderer/sw_engine/tvgSwRenderer.cpp:30:14: fatal error: 'omp.h' file not found
   30 |     #include <omp.h>
      |              ^~~~~~~
1 error generated.

How should we handle the problem of openmp on macOS? We could:

1. Just leave openmp enabled in the recipe. macOS users would try to install thorvg and get the error as mentioned above, and they could either install openmp from brew or just disable openmp by setting with_openmp to False
2. Disable openmp by default in the recipe and let user who want openmp to set with_openmp to True (I personally think this is the best solution, but this would mean including with_openmp option in the recipe)
3. Add llvm-openmp dependencies automatically for macOS, like A case for improved OpenMP support on CCI #24577

It seems like there is a need to let user control if they want to compile with openmp no matter which option we choose.

I would also want to argue that in the past, we could decide if compile with lottie expressions by setting with_extra to lottie_expressions, the proposed change sounds like a regression

Also it seems like the decision of selecting which flag to include in the recipe is quite arbitrary, we could select what loaders, savers, bindings, tools to compile with, but why lottie_exp, openmp and gl_variant are considered not important and cause maintenance burden? Just because they are controlled under extra?

[laggykiller]

Noted test would not compile if thorvg compiled with OpenMP enabled, fixed by linking OpenMP in test_package CMakeLists.txt if available