[lodepng,lodepng-c, libjxl] Update, merge lodepng-c into lodepng by dg0yt · Pull Request #21846 · microsoft/vcpkg

dg0yt · 2021-12-04T07:23:19Z

What does your PR fix?

Updates port lodepng.
Merges lodepng-c into lodepng, resolving the header installation conflict, enabling CI for port lodepng.
Removes private API and examples.
Modernizes the portfile and adds (explicit) usage.

Revises port libjxl (but cannot resolve a strange build time issue for arm-uwp-rel).

Fixes accidental implicit lz4 dependency of gdal, [gdal] Feature lz4 must be explicitly declared #21648.
Which triplets are supported/not supported? Have you updated the CI baseline?

all, yes
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

dg0yt · 2021-12-04T08:12:38Z

Building lodepng in CI uncovers build errors in other ports which where hidden by cascade before.

dg0yt · 2021-12-04T09:15:46Z

The uwp issue is #9390, and a more complete solution is #16111. Hard to believe that it was reported two years ago.

The gdal issue is #21648. I don't want to add a new dependency/feature now. I want feature parity between windows and non-windows build, and I'm stuck with slow reviews of related PRs. So just ensuring lz4 is not activated by accident.

github-actions

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

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for libjxl but no changes to version or port version.
-- Version: 0.6.1
-- Old SHA: 4b89e4e9a92111a9733660afcecdfd59a6ee8d49
-- New SHA: ef9ced934f3664d62a5c546d7ba4a7ca439ce969
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

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

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

Diff

diff --git a/versions/l-/libjxl.json b/versions/l-/libjxl.json
index 9bb05e0..3324458 100644
--- a/versions/l-/libjxl.json
+++ b/versions/l-/libjxl.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "ef9ced934f3664d62a5c546d7ba4a7ca439ce969",
+      "git-tree": "4b89e4e9a92111a9733660afcecdfd59a6ee8d49",
       "version-semver": "0.6.1",
       "port-version": 0
     }

github-actions

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

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for libjxl but no changes to version or port version.
-- Version: 0.6.1
-- Old SHA: 4b89e4e9a92111a9733660afcecdfd59a6ee8d49
-- New SHA: 9d0693cbc5965a99511bf7d03ddbd4c341ba9613
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

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

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

Diff

diff --git a/versions/l-/libjxl.json b/versions/l-/libjxl.json
index b41d94a..3324458 100644
--- a/versions/l-/libjxl.json
+++ b/versions/l-/libjxl.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "9d0693cbc5965a99511bf7d03ddbd4c341ba9613",
+      "git-tree": "4b89e4e9a92111a9733660afcecdfd59a6ee8d49",
       "version-semver": "0.6.1",
       "port-version": 0
     }

github-actions

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

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for libjxl but no changes to version or port version.
-- Version: 0.6.1
-- Old SHA: 4b89e4e9a92111a9733660afcecdfd59a6ee8d49
-- New SHA: fd26e80abc484ec01d974c3dfd587161c4786dd0
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

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

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

Diff

diff --git a/versions/l-/libjxl.json b/versions/l-/libjxl.json
index ca522d0..3324458 100644
--- a/versions/l-/libjxl.json
+++ b/versions/l-/libjxl.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "fd26e80abc484ec01d974c3dfd587161c4786dd0",
+      "git-tree": "4b89e4e9a92111a9733660afcecdfd59a6ee8d49",
       "version-semver": "0.6.1",
       "port-version": 0
     }

github-actions

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

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for libjxl but no changes to version or port version.
-- Version: 0.6.1
-- Old SHA: 4b89e4e9a92111a9733660afcecdfd59a6ee8d49
-- New SHA: 1060ea6763a14a1b9c7b1fec1b14415a746ed805
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

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

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

Diff

diff --git a/versions/l-/libjxl.json b/versions/l-/libjxl.json
index fd776d9..3324458 100644
--- a/versions/l-/libjxl.json
+++ b/versions/l-/libjxl.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "1060ea6763a14a1b9c7b1fec1b14415a746ed805",
+      "git-tree": "4b89e4e9a92111a9733660afcecdfd59a6ee8d49",
       "version-semver": "0.6.1",
       "port-version": 0
     }

github-actions

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

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for libjxl but no changes to version or port version.
-- Version: 0.6.1
-- Old SHA: 4b89e4e9a92111a9733660afcecdfd59a6ee8d49
-- New SHA: 462a6d613121a35cd0c2925baaa4519bca910616
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

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

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

Diff

diff --git a/versions/l-/libjxl.json b/versions/l-/libjxl.json
index abdbb0f..3324458 100644
--- a/versions/l-/libjxl.json
+++ b/versions/l-/libjxl.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "462a6d613121a35cd0c2925baaa4519bca910616",
+      "git-tree": "4b89e4e9a92111a9733660afcecdfd59a6ee8d49",
       "version-semver": "0.6.1",
       "port-version": 0
     }

github-actions

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

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for libjxl but no changes to version or port version.
-- Version: 0.6.1
-- Old SHA: 4b89e4e9a92111a9733660afcecdfd59a6ee8d49
-- New SHA: a784ab72a98843f1a2a42945eff68ce23736a765
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

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

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

Diff

diff --git a/versions/l-/libjxl.json b/versions/l-/libjxl.json
index 31b8513..3324458 100644
--- a/versions/l-/libjxl.json
+++ b/versions/l-/libjxl.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "a784ab72a98843f1a2a42945eff68ce23736a765",
+      "git-tree": "4b89e4e9a92111a9733660afcecdfd59a6ee8d49",
       "version-semver": "0.6.1",
       "port-version": 0
     }

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 63e935d967a3410e26bf4a708efa39d8384d2bbb -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index fb56559..6ecd0a6 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -2366,7 +2366,7 @@
     },
     "gdal": {
       "baseline": "3.4.0",
-      "port-version": 2
+      "port-version": 3
     },
     "gdcm": {
       "baseline": "3.0.7",
diff --git a/versions/g-/gdal.json b/versions/g-/gdal.json
index bbabc8f..b8b4c84 100644
--- a/versions/g-/gdal.json
+++ b/versions/g-/gdal.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "164700a47428c2ded10ee9dc8e28fb189fca0f94",
+      "version-semver": "3.4.0",
+      "port-version": 3
+    },
     {
       "git-tree": "f0f7e3cd7a86246d31f892678b2e06d01c455c5b",
       "version-semver": "3.4.0",

dg0yt · 2021-12-06T08:17:14Z

In all build attempts, arm-uwp-rel consistently took 2 hours, without clear reason. If this happens again now, I will add libjxl:arm-uwp = skip to the ci baseline.
A significant spile in build time is also visible in x64-uwp-rel with regard to x64-uwp-dbg.
I even diffed non-parallel CI build logs, with common dbg/rel difference replaced and sorting, cf. attachment. The resgen difference seem strange, but I don't know enough about uwp:

dbg only:

_AddVCLibs140UniversalCrtXxxxxReference
No "APPX" attributes indicating app package locations were found in the SDK manifest. If an app package is required at runtime the project may not run.
No FrameworkIdentity attributes were found in the SDK manifest, treating this SDK as a non-framework SDK.
Reading SDK manifest file "C:\Program Files (x86)\Microsoft SDKs\Windows Kits\10\ExtensionSDKs\Microsoft.UniversalCRT.Xxxxx\10.0.19041.0\SDKManifest.xml".
resgen.exe /useSourcePath /compile "C:\Program Files (x86)\Microsoft SDKs\Windows Kits\10\ExtensionSDKs\Microsoft.UniversalCRT.Xxxxx\10.0.19041.0\\redist\Xxxxx\arm\ucrtbased.dll"
Targeted configuration and architecture "Xxxxx|arm"

rel only:

resgen.exe /useSourcePath /compile D:\buildtrees\libjxl\arm-uwp-XXX\Xxxxx\jxl_dec.dll

dbg-rel.diff.txt. Xxxxx replace Debug/Release.

JackBoosY · 2021-12-06T09:48:12Z

ports/lodepng/usage

@@ -0,0 +1,9 @@
+The package lodepng provides CMake targets:


The usage is not installed?

Thanks for the reminder. I see the benefit. However:

For vpkg install lodepng, it makes no difference. The usage file is also displayed when installing a cached binary artifact which does not contain the installed file. This is worrying, due to a possible version mismatch.

Given that vcpkg install doesn't indicate the errror, it is no surprise that there are more ports which don't seem install the usage file:

$ for I in ports/*/usage; do grep -q '/usage' ${I%/usage}/portfile.cmake || echo $I; done ports/boost/usage ports/bullet3/usage ports/colmap/usage ports/dcmtk/usage ports/farmhash/usage ports/ffmpeg/usage ports/googleapis/usage ports/gtest/usage ports/imgui-sfml/usage ports/libgeotiff/usage ports/nmap/usage ports/opengl/usage ports/pthreads/usage ports/unixodbc/usage ports/vulkan/usage ports/xerces-c/usage

For vpkg install lodepng, it makes no difference. The usage file is also displayed when installing a cached binary artifact which does not contain the installed file. This is worrying, due to a possible version mismatch.

vcpkg should print the cmake usage automaticly. If not, we should add usage such like this and install it. Otherwise it will not be printed.

Given that vcpkg install doesn't indicate the errror, it is no surprise that there are more ports which don't seem install the usage file:

I think they are left over from history.

For vpkg install lodepng, it makes no difference. The usage file is also displayed when installing a cached binary artifact which does not contain the installed file. This is worrying, due to a possible version mismatch.

vcpkg should print the cmake usage automaticly. If not, we should add usage such like this and install it. Otherwise it will not be printed.

My point is: vcpkg should print the installed file, not the source file. Then there can't be a version mismatch, and the maintainer would notice the issue befroe committing.

See https://github.com/microsoft/vcpkg-tool/blob/56228f80dcf757494a8dc45b206fe224a23c2129/include/vcpkg/installedpaths.h#L25:

Path usage_file(const PackageSpec& p) const { return share_dir(p) / "usage"; }

vcpkg should print the installed file, not the source file

That's correct, see https://github.com/microsoft/vcpkg-tool/blob/56228f80dcf757494a8dc45b206fe224a23c2129/src/vcpkg/install.cpp#L613.
vcpkg will extract the target in add_library according to the generated target.cmake content, and combine to generate the prompt information of find_package/target_link_libraries.

# Conflicts: # scripts/ci.baseline.txt

…e version database.

BillyONeal

I'm far from an expert for most of these ports but the changes herein seem reasonable to me. I also pushed a merge of #21840 into this and fixed the version database to properly use version-date.

I did some investigation as to why the usage file is printed or not and came up empty 🤷

BillyONeal · 2021-12-07T05:51:09Z

Thanks for the merger!

dg0yt added 9 commits December 4, 2021 07:23

Modernize lodepng portfile

ee387b1

Update to 8c6a9e30

58e3e65

Install LICENSE as copyright

6da574f

Don't install examples

e539f5f

Remove unused parameter

42d8857

Merge lodepng-c into lodepng

94371ae

Add usage

d1405ac

Omit lodepng-util: not official API

462de14

Update versions

47266a4

github-actions bot approved these changes Dec 4, 2021

View reviewed changes

dg0yt mentioned this pull request Dec 4, 2021

Revert baseline changes from #20449 #21840

Merged

github-actions bot approved these changes Dec 4, 2021

View reviewed changes

dg0yt force-pushed the lodepng-update branch from 3d3cce7 to 5274327 Compare December 4, 2021 09:38

github-actions bot requested changes Dec 4, 2021

View reviewed changes

dg0yt mentioned this pull request Dec 4, 2021

[vcpkg-cmake] Fix windows toolchain chainloading for uwp #21857

Merged

dg0yt changed the title ~~[lodepng,lodepng-c] Update, merge~~ [lodepng,lodepng-c, libjxl] Update, merge lodepng-c into lodepng Dec 4, 2021

github-actions bot requested changes Dec 4, 2021

View reviewed changes

dg0yt force-pushed the lodepng-update branch from 6af522a to aa61c16 Compare December 4, 2021 16:12

github-actions bot requested changes Dec 4, 2021

View reviewed changes

[gdal] Don't use lz4: not a dependency

4ad5c4e

dg0yt force-pushed the lodepng-update branch from 9756648 to 9c63d2e Compare December 4, 2021 18:23

github-actions bot requested changes Dec 4, 2021

View reviewed changes

dg0yt marked this pull request as draft December 4, 2021 20:42

dg0yt force-pushed the lodepng-update branch 2 times, most recently from e651975 to 99723ac Compare December 4, 2021 23:18

github-actions bot requested changes Dec 4, 2021

View reviewed changes

dg0yt force-pushed the lodepng-update branch from 205eef8 to 36cc06c Compare December 5, 2021 08:56

dg0yt added 6 commits December 6, 2021 08:36

[libjxl] Fix link flags

e04662b

Revise install cleanup, handle .dylib

b6f7a38

Add extra flags for uwp

97fa26d

Workaround uwp toolchain quirks

812e42c

More dependency control, jxl_extras for tools only

c91b2e1

Pass the version to the build

1d12e76

dg0yt force-pushed the lodepng-update branch from 40503c4 to 6bd8f05 Compare December 6, 2021 08:00

dg0yt marked this pull request as ready for review December 6, 2021 08:00

github-actions bot requested changes Dec 6, 2021

View reviewed changes

dg0yt added 2 commits December 6, 2021 09:45

Update versions

a6a7c92

Skip CI for libjxl:arm-uwp

77ab710

dg0yt force-pushed the lodepng-update branch from 6bd8f05 to 77ab710 Compare December 6, 2021 08:51

github-actions bot approved these changes Dec 6, 2021

View reviewed changes

JackBoosY suggested changes Dec 6, 2021

View reviewed changes

JackBoosY added the requires:author-response label Dec 6, 2021

dg0yt added 2 commits December 6, 2021 21:51

Install usage file

a9664e7

Update versions

bd5f7ed

github-actions bot approved these changes Dec 6, 2021

View reviewed changes

BillyONeal added 2 commits December 6, 2021 20:54

Merge remote-tracking branch 'origin/master' into HEAD

3acbc05

# Conflicts: # scripts/ci.baseline.txt

Use version-date for lodepng, mark lodepng-c as deprecated, and updat…

ad5a968

…e version database.

github-actions bot approved these changes Dec 7, 2021

View reviewed changes

BillyONeal approved these changes Dec 7, 2021

View reviewed changes

BillyONeal requested a review from JackBoosY December 7, 2021 05:08

JackBoosY approved these changes Dec 7, 2021

View reviewed changes

JackBoosY added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed requires:author-response labels Dec 7, 2021

BillyONeal merged commit df8276a into microsoft:master Dec 7, 2021

dg0yt deleted the lodepng-update branch December 8, 2021 07:07

Conversation

dg0yt commented Dec 4, 2021 • 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!

dg0yt commented Dec 4, 2021

Uh oh!

dg0yt commented Dec 4, 2021

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!

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!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

dg0yt commented Dec 6, 2021

Uh oh!

JackBoosY Dec 6, 2021

Choose a reason for hiding this comment

Uh oh!

dg0yt Dec 6, 2021

Choose a reason for hiding this comment

Uh oh!

JackBoosY Dec 7, 2021

Choose a reason for hiding this comment

Uh oh!

dg0yt Dec 7, 2021

Choose a reason for hiding this comment

Uh oh!

JackBoosY Dec 7, 2021

Choose a reason for hiding this comment

Uh oh!

BillyONeal left a comment

Choose a reason for hiding this comment

Uh oh!

BillyONeal commented Dec 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dg0yt commented Dec 4, 2021 •

edited

Loading

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