Conversation
|
I guess fontconfig needs an update to 2.17.1 to correctly use WrapDB v2? |
|
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 |
bgilbert
left a comment
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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' |
There was a problem hiding this comment.
Not your fault, but, ugh. Would be good to fix this upstream.
There was a problem hiding this comment.
I'll try for a future release of poppler.
e0198a8 to
e0b3656
Compare
1b3e7fc to
e27166b
Compare
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!