Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[poppler,gdal] Major update to poppler; poppler feature for gdal #22720

Merged
merged 23 commits into from
Feb 18, 2022

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Jan 23, 2022

  • What does your PR fix?

    Updates poppler to 22.02.0.
    Switches to upstreams official gitlab repo. (github is a mirror and lags.)
    Revises dependencies and feature interface for poppler:

    • The Cairo backend can be requested via a feature now. (Used to be hard dependency on osx.)
    • Unstable API/ABI headers (used by packages like GDAL) can be requested via a feature now.
    • The default font configuration backend follows upstream. Windows defaults to win32 instead of fontconfig.
    • gperf is no longer used. (There are ready-to-use files. We can avoid patching and cross platform issues.)
      Adds a poppler feature to gdal. (Based on initial work by @m-kuhn in [gdal] Add poppler #22583.)
  • Which triplets are supported/not supported? Have you updated the CI baseline?

    all but uwp, yes (arm64-windows no longer blocked by gperf)

  • Does your PR follow the maintainer guide?

    yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    yes

Todo:

  • Fix exported pkgconfig files (static dependencies)
  • Export unofficial cmake target configuration
  • Verified exported poppler config and gdal wrapper via gdal[core,poppler,tools] CI runs.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/poppler/vcpkg.json b/ports/poppler/vcpkg.json
index 76dfbc4..3b94f4e 100644
--- a/ports/poppler/vcpkg.json
+++ b/ports/poppler/vcpkg.json
@@ -52,12 +52,6 @@
         }
       ]
     },
-    "fontconfig": {
-      "description": "Use fontconfig",
-      "dependencies": [
-        "fontconfig"
-      ]
-    },
     "font-configuration": {
       "description": "Defaut font configuration backend",
       "dependencies": [
@@ -71,6 +65,12 @@
         }
       ]
     },
+    "fontconfig": {
+      "description": "Use fontconfig",
+      "dependencies": [
+        "fontconfig"
+      ]
+    },
     "splash": {
       "description": "The splash backend is always enabled. This option is kept for compatibility.",
       "dependencies": [
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout df40d1c476dc02d71b113e4a63c3a32b00ebb5bd -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index b8df750..075ee8d 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -5401,8 +5401,8 @@
       "port-version": 4
     },
     "poppler": {
-      "baseline": "20.12.1",
-      "port-version": 6
+      "baseline": "22.0.1",
+      "port-version": 0
     },
     "popsift": {
       "baseline": "0.9",
diff --git a/versions/p-/poppler.json b/versions/p-/poppler.json
index 0b609a3..0965afe 100644
--- a/versions/p-/poppler.json
+++ b/versions/p-/poppler.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "4d910633c772683b5bfa9404228fceaba68fd939",
+      "version": "22.0.1",
+      "port-version": 0
+    },
     {
       "git-tree": "3f15f5c09cc977692e0c081d39e7e85f2229efe1",
       "version": "20.12.1",

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/poppler/vcpkg.json b/ports/poppler/vcpkg.json
index 76dfbc4..3b94f4e 100644
--- a/ports/poppler/vcpkg.json
+++ b/ports/poppler/vcpkg.json
@@ -52,12 +52,6 @@
         }
       ]
     },
-    "fontconfig": {
-      "description": "Use fontconfig",
-      "dependencies": [
-        "fontconfig"
-      ]
-    },
     "font-configuration": {
       "description": "Defaut font configuration backend",
       "dependencies": [
@@ -71,6 +65,12 @@
         }
       ]
     },
+    "fontconfig": {
+      "description": "Use fontconfig",
+      "dependencies": [
+        "fontconfig"
+      ]
+    },
     "splash": {
       "description": "The splash backend is always enabled. This option is kept for compatibility.",
       "dependencies": [
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout df40d1c476dc02d71b113e4a63c3a32b00ebb5bd -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index b8df750..075ee8d 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -5401,8 +5401,8 @@
       "port-version": 4
     },
     "poppler": {
-      "baseline": "20.12.1",
-      "port-version": 6
+      "baseline": "22.0.1",
+      "port-version": 0
     },
     "popsift": {
       "baseline": "0.9",
diff --git a/versions/p-/poppler.json b/versions/p-/poppler.json
index 0b609a3..91e9e83 100644
--- a/versions/p-/poppler.json
+++ b/versions/p-/poppler.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "9b4dfc97042ad1f48473ecefaaa77a27d95f85c7",
+      "version": "22.0.1",
+      "port-version": 0
+    },
     {
       "git-tree": "3f15f5c09cc977692e0c081d39e7e85f2229efe1",
       "version": "20.12.1",

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/poppler/vcpkg.json b/ports/poppler/vcpkg.json
index 76dfbc4..3b94f4e 100644
--- a/ports/poppler/vcpkg.json
+++ b/ports/poppler/vcpkg.json
@@ -52,12 +52,6 @@
         }
       ]
     },
-    "fontconfig": {
-      "description": "Use fontconfig",
-      "dependencies": [
-        "fontconfig"
-      ]
-    },
     "font-configuration": {
       "description": "Defaut font configuration backend",
       "dependencies": [
@@ -71,6 +65,12 @@
         }
       ]
     },
+    "fontconfig": {
+      "description": "Use fontconfig",
+      "dependencies": [
+        "fontconfig"
+      ]
+    },
     "splash": {
       "description": "The splash backend is always enabled. This option is kept for compatibility.",
       "dependencies": [
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout df40d1c476dc02d71b113e4a63c3a32b00ebb5bd -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index b8df750..075ee8d 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -5401,8 +5401,8 @@
       "port-version": 4
     },
     "poppler": {
-      "baseline": "20.12.1",
-      "port-version": 6
+      "baseline": "22.0.1",
+      "port-version": 0
     },
     "popsift": {
       "baseline": "0.9",
diff --git a/versions/p-/poppler.json b/versions/p-/poppler.json
index 0b609a3..dba2740 100644
--- a/versions/p-/poppler.json
+++ b/versions/p-/poppler.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "8c3cdf8e728363b73dd28b1ee8f59196aa4bdb77",
+      "version": "22.0.1",
+      "port-version": 0
+    },
     {
       "git-tree": "3f15f5c09cc977692e0c081d39e7e85f2229efe1",
       "version": "20.12.1",

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/poppler/vcpkg.json b/ports/poppler/vcpkg.json
index d5ee3d1..8a853b1 100644
--- a/ports/poppler/vcpkg.json
+++ b/ports/poppler/vcpkg.json
@@ -45,12 +45,6 @@
         }
       ]
     },
-    "fontconfig": {
-      "description": "Use fontconfig",
-      "dependencies": [
-        "fontconfig"
-      ]
-    },
     "font-configuration": {
       "description": "Defaut font configuration backend",
       "dependencies": [
@@ -64,6 +58,12 @@
         }
       ]
     },
+    "fontconfig": {
+      "description": "Use fontconfig",
+      "dependencies": [
+        "fontconfig"
+      ]
+    },
     "splash": {
       "description": "The splash backend is always enabled. This option is kept for compatibility."
     },
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout df40d1c476dc02d71b113e4a63c3a32b00ebb5bd -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index b8df750..075ee8d 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -5401,8 +5401,8 @@
       "port-version": 4
     },
     "poppler": {
-      "baseline": "20.12.1",
-      "port-version": 6
+      "baseline": "22.0.1",
+      "port-version": 0
     },
     "popsift": {
       "baseline": "0.9",
diff --git a/versions/p-/poppler.json b/versions/p-/poppler.json
index 0b609a3..94a913a 100644
--- a/versions/p-/poppler.json
+++ b/versions/p-/poppler.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "1389a9fc794fce9273559e8a5e720ab2fc7387c0",
+      "version": "22.0.1",
+      "port-version": 0
+    },
     {
       "git-tree": "3f15f5c09cc977692e0c081d39e7e85f2229efe1",
       "version": "20.12.1",

@dg0yt dg0yt marked this pull request as draft January 23, 2022 21:54
@LilyWangLL LilyWangLL added the category:port-update The issue is with a library, which is requesting update new revision label Jan 24, 2022
@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 24, 2022

UWP:

5>D:\buildtrees\poppler\src\er-22.01.0-24b8c57e16.clean\goo\gfile.cc(363,21): error C3861: 'CreateFileA': identifier not found [D:\buildtrees\poppler\x64-uwp-dbg\poppler.vcxproj]
5>D:\buildtrees\poppler\src\er-22.01.0-24b8c57e16.clean\goo\gfile.cc(370,21): error C3861: 'CreateFileW': identifier not found [D:\buildtrees\poppler\x64-uwp-dbg\poppler.vcxproj]

=> unsupported

@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 31, 2022

This version of poppler is currently not usable with gdal. gdal uses the poppler C++ API, and poppler uses C++ 17 in the API (std::optional).
The C++17 requirement in poppler came app. three months ago. The newer version include security updates, so I wouldn't really want to go back. Not sure if this can be resolved in another way.

@m-kuhn
Copy link
Contributor

m-kuhn commented Jan 31, 2022

Is there a ticket about this open on gdal side?

@dg0yt dg0yt force-pushed the poppler branch 4 times, most recently from bf5d6ad to 728ea0c Compare February 1, 2022 07:58
@dg0yt dg0yt force-pushed the poppler branch 3 times, most recently from 5b18956 to 4bbf2c0 Compare February 2, 2022 07:38
@dg0yt
Copy link
Contributor Author

dg0yt commented Feb 11, 2022

I update license field for gdal and unofficial namespace for poppler exports. No other changes planned.

ports/gdal/vcpkg.json Show resolved Hide resolved
@JackBoosY JackBoosY added requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function and removed requires:author-response labels Feb 14, 2022
@LilyWangLL
Copy link
Contributor

LilyWangLL commented Feb 14, 2022

@dg0yt poppler[fontconfig]:x64-windows build failed with the following error:

H:\Lily\22720\vcpkg\buildtrees\poppler\src\er-22.02.0-e605c044ce.clean\poppler\GlobalParams.cc(751): error C3861: 'strndup': identifier not found
H:\Lily\22720\vcpkg\buildtrees\poppler\src\er-22.02.0-e605c044ce.clean\poppler\GlobalParams.cc(719): warning C4996: 'strdup': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _strdup. See online help for details.

Other features test pass on x64-windows.

@dg0yt
Copy link
Contributor Author

dg0yt commented Feb 14, 2022

Okay, poppler[fontconfig] is now unsupported on windows (except mingw).

@dg0yt
Copy link
Contributor Author

dg0yt commented Feb 16, 2022

AFAICS the author gave all required response.

@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function labels Feb 17, 2022
+ $<INSTALL_INTERFACE:include/poppler/fofi>
+ $<INSTALL_INTERFACE:include/poppler/goo>
+)
+set_target_properties(poppler PROPERTIES EXPORT_NAME poppler-private)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this rename? (considering we're already in the namespace unofficial::poppler::)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The name "poppler" is fine when used inside the Poppler project. However, upstream is really putting much emphasis on that this shall not be used by other projects and that such usage is entirely unsupported. However, some projects need it (e.g. GDAL). I want to carry upstreams point of view. Even more in the light of tools like `vcpkg? blindly reporting all targets as if they were public API.

I also want to add proper CMake config export upstream, and I expect better acceptance for this name.

ports/poppler/portfile.cmake Show resolved Hide resolved
@vicroms vicroms merged commit d4422c3 into microsoft:master Feb 18, 2022
@dg0yt dg0yt deleted the poppler branch February 18, 2022 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants