[zlib] Ignore external ZLIB_DLL by dg0yt · Pull Request #26885 · microsoft/vcpkg

dg0yt · 2022-09-20T05:38:36Z

What does your PR fix?

Hardcodes the triplet linkage into the installed headers, so that usage no longer depends on external definition of ZLIB_DLL.
For example, GDAL always applies ZLIB_DLL. Noticed in log from [gdal] Update to v3.5.2 #26676 (comment):
```
LINK : warning LNK4286: symbol 'crc32' defined in 'zlib.lib(crc32.obj)' is imported by 'gdal.lib(cpl_minizip_unzip.cpp.obj)'
LINK : warning LNK4217: symbol 'deflateInit_' defined in 'zlib.lib(deflate.obj)' is imported by 'gdal.lib(cpl_vsil_gzip.cpp.obj)' in function 'CPLZLibDeflate'
```
Given the broad impact of port zlib changes, this PR should be considered for combination with other zlib changes:
[zlib] Fix CVE-2022-37434 #26792

Follow-up changes:
- Update minizip to the same source version as zlib.
- Revise libkml, triggered from zlib/minizip issues.
Which triplets are supported/not supported? Have you updated the CI baseline?

unchanged, no
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

Neumann-A · 2022-09-20T12:13:36Z

Why has this not been a problem before?

dg0yt · 2022-09-21T04:49:45Z

Why has this not been a problem before?

a) Packages tend to unconditionally set ZLIB_DLL.
Evidence: gdal as mentioned aboved; git grep ZLIB_DLL
b) There is only a warning in static builds, not an error.
c) Contributors and reviewers don't care for this warning. In particular when connected with other errors
Evidence: the mentioned gdal issue; https://github.com/microsoft/vcpkg/search?q=LNK4217&type=issues

dg0yt · 2022-09-21T04:50:55Z

libkml probably stumbles over a mix of headers from zlib and its vendored copy of minizip.

github-actions

You have modified or added at least one portfile where deprecated functions are used.

Details

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/libkml/portfile.cmake

You have modified or added at least one vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

ports/minizip/vcpkg.json

Valid values for the license field can be found in the documentation

github-actions

You have modified or added at least one vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

ports/minizip/vcpkg.json

Valid values for the license field can be found in the documentation

github-actions

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

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout d52632d9a0c4a5b57de285b1e03722a067dd80eb -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 6e8968a..8edb36d 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -3834,7 +3834,7 @@
     },
     "libkml": {
       "baseline": "1.3.0",
-      "port-version": 8
+      "port-version": 9
     },
     "liblas": {
       "baseline": "1.8.1",
diff --git a/versions/l-/libkml.json b/versions/l-/libkml.json
index 8b24a12..7b37667 100644
--- a/versions/l-/libkml.json
+++ b/versions/l-/libkml.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "b251016911a5eeae287e3a5bd67a30e3213b2b8e",
+      "version": "1.3.0",
+      "port-version": 9
+    },
     {
       "git-tree": "1ce9dd050baf9ae42da2bb0c6d0feae1241b6e01",
       "version-string": "1.3.0",

ports/zlib/portfile.cmake

dg0yt · 2022-09-23T17:19:29Z

I want to improve the libkml mingw patch.

github-actions

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

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 5f144173006dbeea7b9cb017775675606666a207 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 9f9b357..0c8358f 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -3838,7 +3838,7 @@
     },
     "libkml": {
       "baseline": "1.3.0",
-      "port-version": 8
+      "port-version": 9
     },
     "liblas": {
       "baseline": "1.8.1",
@@ -4769,8 +4769,8 @@
       "port-version": 1
     },
     "minizip": {
-      "baseline": "1.2.11",
-      "port-version": 11
+      "baseline": "1.2.12",
+      "port-version": 0
     },
     "minizip-ng": {
       "baseline": "3.0.5",
diff --git a/versions/l-/libkml.json b/versions/l-/libkml.json
index 8b24a12..5797ff9 100644
--- a/versions/l-/libkml.json
+++ b/versions/l-/libkml.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "9941e66029546372544fed13066e9a9756981dd3",
+      "version": "1.3.0",
+      "port-version": 9
+    },
     {
       "git-tree": "1ce9dd050baf9ae42da2bb0c6d0feae1241b6e01",
       "version-string": "1.3.0",
diff --git a/versions/m-/minizip.json b/versions/m-/minizip.json
index 9c1239b..c85a7b5 100644
--- a/versions/m-/minizip.json
+++ b/versions/m-/minizip.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "65a32bfac58033a215e271db94d5e022fce816e3",
+      "version-semver": "1.2.12",
+      "port-version": 0
+    },
     {
       "git-tree": "528703ed8d2b78aeaa55695765535efafa24540b",
       "version-semver": "1.2.11",

BillyONeal

I love this change. Are you saying you want me to cherry pick it into #27226 or vice versa?

dg0yt · 2022-10-19T03:39:43Z

Please do the cherry-pick.

BillyONeal · 2022-10-25T17:27:49Z

Thanks again!

dg0yt added 2 commits September 20, 2022 07:27

Don't use external ZLIB_DLL

5d52e4e

Update versions

5884a5c

github-actions bot previously approved these changes Sep 20, 2022

View reviewed changes

JonLiu1993 assigned FrankXie05 Sep 20, 2022

FrankXie05 added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Sep 20, 2022

dg0yt dismissed github-actions[bot]’s stale review via 27c605c September 21, 2022 06:41

github-actions bot previously approved these changes Sep 21, 2022

View reviewed changes

dg0yt dismissed github-actions[bot]’s stale review via fe8b598 September 22, 2022 04:26

dg0yt force-pushed the zlib-dll branch from 27c605c to fe8b598 Compare September 22, 2022 04:26

github-actions bot previously approved these changes Sep 22, 2022

View reviewed changes

dg0yt added 4 commits September 22, 2022 06:31

[minizip] Bump to zlib version

92ed840

Update versions

f39126d

[libkml] Fix mingw build

2a1076f

[libkml] Modernize

08f2128

dg0yt marked this pull request as draft September 22, 2022 05:38

dg0yt dismissed github-actions[bot]’s stale review via 7411813 September 22, 2022 05:38

dg0yt force-pushed the zlib-dll branch from fe8b598 to 7411813 Compare September 22, 2022 05:38

github-actions bot requested changes Sep 22, 2022

View reviewed changes

dg0yt added 3 commits September 22, 2022 08:33

[libkml] Fix minizip dependency

40eedb6

[libkml] No DLL

ea0afa5

Update versions

6f20678

dg0yt force-pushed the zlib-dll branch from 5f08712 to 6f20678 Compare September 22, 2022 06:36

github-actions bot previously approved these changes Sep 22, 2022

View reviewed changes

dg0yt marked this pull request as ready for review September 22, 2022 07:22

FrankXie05 reviewed Sep 23, 2022

View reviewed changes

ports/zlib/portfile.cmake Show resolved Hide resolved

FrankXie05 previously approved these changes Sep 23, 2022

View reviewed changes

FrankXie05 added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Sep 23, 2022

dg0yt marked this pull request as draft September 23, 2022 17:18

dg0yt added 2 commits September 23, 2022 20:31

[libkml] Update mingw patch

d247830

Update versions

fc3a058

dg0yt dismissed stale reviews from FrankXie05 and github-actions[bot] via fc3a058 September 23, 2022 18:32

github-actions bot previously approved these changes Sep 23, 2022

View reviewed changes

FrankXie05 removed the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Sep 26, 2022

Merge remote-tracking branch 'origin/master' into zlib-dll

1522a6a

dg0yt dismissed github-actions[bot]’s stale review via 3f4a571 October 1, 2022 05:18

dg0yt marked this pull request as ready for review October 1, 2022 05:18

github-actions bot requested changes Oct 1, 2022

View reviewed changes

Update versions

771e561

dg0yt force-pushed the zlib-dll branch from 3f4a571 to 771e561 Compare October 3, 2022 13:38

github-actions bot approved these changes Oct 3, 2022

View reviewed changes

dg0yt requested a review from FrankXie05 October 5, 2022 06:07

FrankXie05 approved these changes Oct 14, 2022

View reviewed changes

dg0yt mentioned this pull request Oct 14, 2022

[zlib, minizip] Update to 1.2.13, and embed the ZLIB_DLL setting. #27226

Merged

BillyONeal approved these changes Oct 19, 2022

View reviewed changes

BillyONeal closed this in #27226 Oct 25, 2022

dg0yt deleted the zlib-dll branch October 28, 2022 18:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[zlib] Ignore external ZLIB_DLL#26885

[zlib] Ignore external ZLIB_DLL#26885
dg0yt wants to merge 13 commits intomicrosoft:masterfrom
dg0yt:zlib-dll

dg0yt commented Sep 20, 2022 •

edited

Loading

Uh oh!

Neumann-A commented Sep 20, 2022

Uh oh!

dg0yt commented Sep 21, 2022

Uh oh!

dg0yt commented Sep 21, 2022

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot left a comment

Uh oh!

Uh oh!

dg0yt commented Sep 23, 2022

Uh oh!

github-actions bot left a comment

Uh oh!

BillyONeal left a comment

Uh oh!

dg0yt commented Oct 19, 2022

Uh oh!

BillyONeal commented Oct 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

dg0yt commented Sep 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does your PR fix?

Which triplets are supported/not supported? Have you updated the CI baseline?

Does your PR follow the maintainer guide?

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

Uh oh!

Neumann-A commented Sep 20, 2022

Uh oh!

dg0yt commented Sep 21, 2022

Uh oh!

dg0yt commented Sep 21, 2022

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dg0yt commented Sep 23, 2022

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

BillyONeal left a comment

Choose a reason for hiding this comment

Uh oh!

dg0yt commented Oct 19, 2022

Uh oh!

BillyONeal commented Oct 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dg0yt commented Sep 20, 2022 •

edited

Loading

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