[poppler] Fix poppler/paraview name clash #35472
Conversation
dg0yt
changed the title
LilyWangLL
added
the
category:port-bug
| +namespace poppler_private { | ||
| class POPPLER_PRIVATE_EXPORT Parser |
There was a problem hiding this comment.
This is going to affect exported names. Is that OK? It would be better if upstream gets to see that before we merge it...
There was a problem hiding this comment.
It affects the ABI, and it is source-incompatible with regard to forward declarations. But these headers are private includes. We only install them because GDAL needs them to implement proper memory management (across DLL boundaries, IIRC).
|
This LGTM subject to upstream seeing the patch |
For upstream, the change should be extended to similar classes (as mentioned). It will take time. And it changes the ABI. |
They still need to see it. If they look at this and go 'heck no', then it would be wrong for vcpkg to expose that behavior, since that is claiming they wrote something they didn't write. |
|
Upstream contacted:
|
|
This is what I did: https://gitlab.freedesktop.org/poppler/poppler/-/issues/1449 |
# Conflicts: # ports/poppler/vcpkg.json # versions/baseline.json # versions/p-/poppler.json
dg0yt
changed the title
LilyWangLL
added
the
depends:upstream-changes
|
Since poppler's upstream maintainers seem ... uninterested in resolving this situation, would it make sense to try to patch paraview to stop stomping on the name instead? |
|
I know little about paraview. This is the PR I trust. The patch is small, and it should be maintainable even if the test port finds other collisions in the future. |
I agree, I think this is the right change! But I'm uncomfortable speaking for upstream when they seem to have explicitly said no to it. :( |
|
IMO we shouldn't take upstream's response as meaning "no". It is not the first time that communication with this project is ... difficult. Let us accept their priorities, even if it means holding back direct contributions. |
BillyONeal
added
the
requires:vcpkg-team-review
|
I agree. Tagging vcpkg-team-review for a 2nd opinion from the other maintainers on just taking this anyway given upstream's inconsistent statements. |
LilyWangLL
removed
depends:upstream-changes
Cherry-picked from #35437:
Add a test port which covers non-default features of
gdal.Fix duplicate symbol definition in poppler and paraview found by the test-port. (#35437 (comment). paraview using vtk using gdal using poppler.)
Disallow cascaded failure of gdal.