Skip to content

[mapnik-polylabel] Replacement for mapbox-polylabel - #53409

Draft
SunBlack (SunBlack) wants to merge 2 commits into
microsoft:masterfrom
SunBlack:mapnik-polylabel
Draft

SunBlack (SunBlack) wants to merge 2 commits into
microsoft:masterfrom
SunBlack:mapnik-polylabel

Conversation

@SunBlack

@SunBlack SunBlack (SunBlack) commented Aug 13, 2026 •

Copy link
Copy Markdown
Contributor
  • 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, or no changes were necessary.
  • Any fixed CI baseline and CI feature baseline entries are removed from that file, or no entries needed to be changed.
  • All patch files in the port are applied and succeed.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Exactly one version is added in each modified versions file.

mapbox-polylabel removed the C++ code in mapbox/polylabel#124. As mapnik requires it, they created a fork of it (see mapnik/mapnik#4567), therefore replace the port.

We could also move mapnik-polylabel into the source code of mapnik, similar to mapnik-vector-tile

@SunBlack
SunBlack (SunBlack) force-pushed the mapnik-polylabel branch 2 times, most recently from dcdef73 to 2fd8143 Compare August 13, 2026 23:27
@BillyONeal

Copy link
Copy Markdown
Member

GPT 5.6 Sol reports:

The Mapnik 4.3.0 port does not control the new default-enabled combined input plugins. The port still passes obsolete feature options in portfile.cmake, while upstream now uses USE_PLUGIN_INPUT_GDAL_OGR, USE_PLUGIN_INPUT_POSTGIS_PGRASTER, and USE_PLUGIN_INPUT_TILES (dispatcher, defaults).

Consequently, default builds require undeclared dependencies and fail while locating GDAL across twelve tested Windows, Linux, and Android triplets. The PostGIS/PGRaster and tiles plugins similarly introduce undeclared PostgreSQL, SQLite, Boost, and platform-specific requirements (PostGIS/PGRaster, tiles).

[...]

Blocking finding: Mapnik's new plugin options are uncontrolled

The vcpkg port still maps the input-gdal, input-ogr, input-postgis, and input-pgraster features to the old separate options in ports/mapnik/portfile.cmake. In Mapnik 4.3.0, the plugin dispatcher no longer uses those options. Instead, it selects combined plugins through USE_PLUGIN_INPUT_GDAL_OGR and USE_PLUGIN_INPUT_POSTGIS_PGRASTER, and also selects the new tiles plugin through USE_PLUGIN_INPUT_TILES; see upstream plugins/input/CMakeLists.txt. All three are default-enabled in upstream CMakeLists.txt, but the port passes none of them.

I have not dug into the guts of the failures here because builds are broken. Drafting due to build failures, I am not making a concrete statement that all of the above need be addressed to merge.

@BillyONeal
Billy O'Neal (BillyONeal) marked this pull request as draft August 14, 2026 23:37
@SunBlack
SunBlack (SunBlack) force-pushed the mapnik-polylabel branch 16 times, most recently from 13e9f8c to 453970a Compare September 19, 2026 23:39
Comment thread ports/mapnik/vcpkg.json
Comment on lines 94 to +105
"input-gdal": {
"description": "GDAL input plugin",
"dependencies": [
"gdal"
]
},
"input-gdal-ogr": {
"description": "GDAL+OGR input plugin",
"dependencies": [
"gdal"
]
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suggest we keep this combined in input-gdal. I believe that wanting one without the other will be very unlikely. (It could still be achieved via triplet settings.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That was my first thought, too. But, as you've seen yourself, there's another OGR feature further down. So for now, I've decided to stay with the existing structure. I think we could look into merging some things there when the PR is ready, since the CI time for this port is really quite long. Right now, though, the PR is failing anyway because I still don't have any idea what the issue is with the Linux CI, and when I give the log to AI, the solution it suggests seems a bit strange to me (even though I haven't tested it yet).

Comment thread ports/mapnik/vcpkg.json
Comment on lines 115 to 120
"input-ogr": {
"description": "OGR input plugin",
"dependencies": [
"gdal"
]
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, another ogr?

@SunBlack
SunBlack (SunBlack) force-pushed the mapnik-polylabel branch 5 times, most recently from 45ab757 to 9a4f27b Compare September 23, 2026 22:46
@SunBlack
SunBlack (SunBlack) marked this pull request as ready for review September 24, 2026 08:06
@BillyONeal

Billy O'Neal (BillyONeal) commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

mapbox-polylabel removed the C++ code in mapbox/polylabel#124

Note that this isn't like "we removed the nice C++ wrappers around some C thing", this is a library where they had a C++ port of a Java thing in the same repo and they removed the C++ port. So there is no reason for C++ customers to expect support from the "original" implementation going forward, so we shouldn't fear the usual forms of conflict that come from forks.

(EDIT: To clarify, this is me confirming that I believe swapping in the fork is the correct thing to do)

@BillyONeal
Billy O'Neal (BillyONeal) marked this pull request as draft September 25, 2026 06:22
@BillyONeal

Copy link
Copy Markdown
Member

Drafting due to build failures, SunBlack#95 may fix.

Additionally, GPT 6 Sol reports:

  • input-gdal, input-ogr, input-postgis, and input-pgraster are still advertised in vcpkg.json and mapped to old options in portfile.cmake, but upstream's combined-plugin PR changed Mapnik 4.3.2's dispatcher to add only the combined GDAL+OGR and PostGIS+PGRaster plugins (upstream source). Selecting any of these four features installs its dependencies but no corresponding plugin, although the feature tests pass.
  • tiles.patch and mapnik-index.patch change general upstream build behavior, but no upstream issue or pull request is cited and repository searches found no submission. Submit the fixes upstream and cite them, or establish why they are vcpkg-specific, as required by the patching guidance.

@BillyONeal

Copy link
Copy Markdown
Member

(I didn't ask it to try to fix the features report because you and Kai Pastor (@dg0yt) were discussing above and as not a user of this myself it's not clear what the correct change is)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@BillyONeal

Copy link
Copy Markdown
Member

Looks like build is fixed but I'm still not sure what the right feature fix is.

This branch has not been deployed

No deployments
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.

4 participants