Skip to content

Add wrap for poppler.#2668

Open
punytroll wants to merge 14 commits intomesonbuild:masterfrom
punytroll:poppler
Open

Add wrap for poppler.#2668
punytroll wants to merge 14 commits intomesonbuild:masterfrom
punytroll:poppler

Conversation

@punytroll
Copy link
Contributor

@punytroll punytroll commented Feb 23, 2026

This adds the library poppler in the most recent version 26.02.0 with its four wrappers (cpp, glib, qt5, qt6), tools, most of the tests and gir. There are some things missing, which are highlighted with @todo marks. Most of them, I don't yet know how to add.

However, the library is compilable and used in production code already.

I'm happy to apply fixes for any issues you can find!

@punytroll
Copy link
Contributor Author

punytroll commented Feb 24, 2026

I guess fontconfig needs an update to 2.17.1 to correctly use WrapDB v2?

@bgilbert
Copy link
Collaborator

2.17.1 isn't enough, actually. The wrap files have been fixed in fontconfig Git but there hasn't been a release with the fixes. For now, you could disable fatal_warnings in ci_config.json and leave a comment explaining why.

Copy link
Collaborator

@bgilbert bgilbert left a comment

Choose a reason for hiding this comment

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

Thanks for the submission. I have not thoroughly reviewed this, just skimmed it.

What's the goal of this wrap? If you're prototyping a Meson config that would eventually be merged upstream, this approach is generally fine, though if upstream will maintain parallel build systems you may not need to think about things like fuzzing support. If the goal is just to provide a wrap that can be invoked by projects that need poppler, I'd recommend cutting this down to improve maintainability. For example, parent projects presumably won't need the demo code or man pages, certainly won't need fuzzing, and won't need to run a comprehensive test suite if that's infeasible to add here. (Some tests should certainly still be built and run.)

Copy link
Collaborator

@bgilbert bgilbert Feb 24, 2026

Choose a reason for hiding this comment

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

Oof. We don't allow git:// wraps in WrapDB, and we presumably don't allow wrap files inside packagefiles though I'm not aware it's ever come up before. One of those problems could be solved by making a top-level poppler-test wrap, but since poppler doesn't tag its test repo, there's no Git tag to point to.

The options I see are: making a policy exception for the poppler test repo to allow a separate wrap that points to a specific Git commit, and omitting the tests. I think I'd lean toward the latter but I'm not sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm. I would really like to be able to run poppler's unit tests with my own unit tests in a release pipeline to make sure, that poppler built as expected on my platform. Especially, since the library is slightly more on the complex side.

But I can see how that collides with restrictions for wrapdb. I was kind of happy, that I could get it to work like this and I don't think it is particularly ugly as it doesn't spill out of this wrap.

I can see the argument for fixing the Git commit, though, so that is what I did.

If this needs more work, I'll do it, though.

Copy link
Contributor Author

@punytroll punytroll Feb 26, 2026

Choose a reason for hiding this comment

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

There is also an additional repository "poppler-data" which contains additional encoding files for use with poppler. The files have a different license, which is why there are separated. I was planning to add this in a future iteration of this wrap (gated by an option), but if subprojects inside of this project are not allowed for wrapdb, then it probably cannot happen.

)
have_cairo = cairo_dependency.found()
if have_cairo == true
cairo_feature = '#define POPPLER_HAS_CAIRO 1'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not your fault, but, ugh. Would be good to fix this upstream.

Copy link
Contributor Author

@punytroll punytroll Feb 26, 2026

Choose a reason for hiding this comment

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

I'll try for a future release of poppler.

@punytroll punytroll force-pushed the poppler branch 3 times, most recently from e0198a8 to e0b3656 Compare February 27, 2026 08:13
@punytroll punytroll force-pushed the poppler branch 2 times, most recently from 1b3e7fc to e27166b Compare February 27, 2026 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants