[openexr3] Add new port by simogasp · Pull Request #25238 · microsoft/vcpkg

simogasp · 2022-06-14T20:27:07Z

Following the discussion in #20957 with @JackBoosY @chausner this PR introduces the openexr port for its version 3 as a separate port openexr3.
OpenEXR 3 introduced many changes in the way it is built, from adopting modern CMake to splitting out a library, Imath, for which a proper port already exists.
Some ports still rely on 2.x versions of openexr and they cannot be upgraded without introducing a lot of patchworks or changing the source code. On the other hand, other ports would upgrade to version 3.x and they are blocked

The rationale behind this PR is that like, e.g. opencv, is to separate the two incompatible versions and manage the two separately until version 2.x will be phased out at some point or all the ports will have upgraded their source code to version 3.

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

all triplets seem to work, the baseline has been updated

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

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

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/openexr3/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/openexr3/vcpkg.json

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

ports/openexr3/portfile.cmake

dg0yt · 2022-06-15T10:30:49Z

ports/openexr3/portfile.cmake

This usually needs to go to OPTIONS_DEBUG because it is unused in release builds.

Setting CMAKE_DEBUG_POSTFIX is disallowed by maintainer guidelines unless upstream sets it themselves.

Well, upstream sets CMAKE_DEBUG_POSTFIX=_d anyways. IIRC there was an issue why it make sense to keep it explicit (like use before set), but with the new version, this might be gone.

Yes I kept that from the other port, I don't know if it is needed or not.
What I noticed was that the libs are exported with the version appended, i.e. like libOpenEXR-3_1, so I was imagining that there was some problem with maybe parsing the the "d" in that case and "_d" worked better. But it's just speculation.

I'm the one that added the _d originally in OpenEXR's own cmake files ~ I couldn't find a consensus on Windows, and when it came down to d or _d, I followed the boost convention, sort of, by separating qualifiers with an underscore, rather than simply appending qualifiers.

dg0yt · 2022-06-15T10:34:11Z

ports/openexr3/portfile.cmake

I guess that both port openexr and port openexr3 would like to provide config for package OpenEXR. But only one can install the config to share/openexr. Given that CMake can deal with different versions for config, this could be solved but needs extra work.

Oh yes sorry I haven't thought of that.
Maybe it will be better to export it in lib/cmake/${port}/ but I guess that CMake won't be able to recognize openexr3 as a valid name to find the config file...

meshula · 2022-06-15T17:05:22Z

This new approach makes sense to me, it'll unblock things nicely. Hopefully there's some way that projects depending on OpenEXR 3 (both within and outside of the vcpkg ecosystem) can discover openexr3 without needing to patch their own cmakefiles. If there's something that can be done on the OpenEXR side of the equation to make things easier, please feel free to suggest it.

dg0yt · 2022-06-15T17:27:26Z

The CMake config problem might be resolved.
But the more serious problem is /include/OpenEXR, used by both ports. This will break vcpkg CI as soon as there are consumers for each openexr port.

simogasp · 2022-06-15T19:25:52Z

I underestimated the complexity of having both versions available at the same time.
I don't think it is possible to have both versions available at the same time, so if one is installed the other should be not.
This is how e.g. opencv works for the different versions opencv2, opencv3 and opencv4.
At this point the simplest solution would be to add in both portfiles something like this

if (EXISTS "${CURRENT_INSTALLED_DIR}/share/openexr")
  message(FATAL_ERROR "OpenEXR 2 is installed, please uninstall and try again:\n    vcpkg remove openexr")
endif()

and, mutatis mutandis, the same in openexr 2.

There are not so many ports that depend on openexr (although a big one, opencv), so hopefully they will soon manage to upgrade to openexr 3.

What do you guys think?

meshula · 2022-06-16T04:10:24Z

I vote for just going with the fatal error.

In the wild, openexr 2 and 3 do coexist, but never in the same root ~ people often push one of them down, e.g. /my/local/include/openexr2/openexr, and add /my/local/include/openexr2 as needed as an additional system dir. I'm not proposing any action here, just making a note. The fatal error on installing both seems reasonable.

dg0yt · 2022-06-16T04:47:53Z

The fatal error on installing both seems reasonable.

This is used only as an execption in vcpkg. It excludes one version from CI, and every other port depending on the excluded version. And it may confuse users who pull both versions via transitive dependencies.

/my/local/include/openexr2

This could be a workaround if openexr is upgraded and openexr2 is added as a separate port to satisfy a few remaining consumers, in particular if they use cmake config.

Cheney-W · 2022-06-16T06:32:00Z

I think it would be better to add judgment to both portfile.cmake.
After that we also need to add all triplets of openexr3 as skip to ci.baseline.txt.

dg0yt · 2022-06-16T06:47:25Z

After that we also need to add all triplets of openexr3 as skip to ci.baseline.txt.

NO. We certainly don't want to skip the new version.

Cheney-W · 2022-06-16T07:39:12Z

Do you mean skip openexr or make both ports work at the same time?

dg0yt · 2022-06-16T07:45:19Z

Do you mean skip openexr or make both ports work at the same time?

Skip openexr, the old port. Which has nearly the same effect as simply upgrading openexr, and add the consumers which can't be upgraded to the fail list. Unless they can opt out of the openexr dependency.

Cheney-W · 2022-06-16T08:00:40Z

@simogasp Could you please modify this PR with following suggestion:

Add judgment in both portfile.cmake of openexr and openexr3
Add all triplets of openexr as skip in ci.baseline.txt

simogasp · 2022-06-16T09:19:49Z

@Cheney-W I'll do that. Just to be sure thought, shall I do something like this in the ci.baseline.txt

openexr:arm64-windows      = skip
openexr:arm-uwp            = skip
openexr:x64-linux          = skip
openexr:x64-osx            = skip
openexr:x64-uwp            = skip
openexr:x64-windows        = skip
openexr:x64-windows-static = skip
openexr:x64-windows-static-md=skip
openexr:x86-windows        = skip

and keep the current fail cases of openexr even for openexr3

openexr3:arm64-windows=fail
openexr3:arm-uwp=fail
openexr3:x64-uwp=fail

Is it so? Thanks!

Cheney-W · 2022-06-16T10:13:48Z

Yes

* add openexr3 * made openexr 2 and 3 mutually exclusive * only openexr in the ci.baseline

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/openexr/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/openexr/vcpkg.json

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

simogasp · 2022-06-16T10:49:37Z

the problem is that all the ports depending on openexr (most notably opencv) will now fail on CI.
Shall I revert back and let openexr build on CI and skip openexr3 (no one depends on this atm)?

PS
is there a way with vcpkg command line to list all the packages that depend on a given package? With vcpkg search not all the results pop up, e.g. openimageio depends on openexr but it is not shown, it seems it only checks package names and the optional features (as in opencv3[openexr])

meshula · 2022-06-16T23:49:35Z

@simogasp The reason no one depends on openexr3 is because the projects that need to move to openexr3 are blocked due to the unavailability of openexr3 on vcpkg.

Everyone depending on vcpkg supporting openexr3 and no longer being stuck on openexr 2 thanks you for your efforts and patience on this. Please don't skip openexr3!

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/openexr/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/openexr/vcpkg.json

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

Cheney-W · 2022-06-17T07:25:50Z

@simogasp Thanks for your PR!
If you want this PR to be merged, there are currently two ways:

Add all triplet of openexr3 as skip in the ci.baseline.txt, this can avoid the problem that all ports currently depened on openexr cannot pass the CI test, because if the CI test is not all passed, the PR will not be merged.
Change all the dependencies of ports that depend on openexr to openexr3, and solve any compilation problems that may arise above. If you need to modify upstream code, then you need to get upstream's approval.

simogasp · 2022-06-17T07:32:31Z

@simogasp Thanks for your PR! If you want this PR to be merged, there are currently two ways:

Add all triplet of openexr3 as skip in the ci.baseline.txt, this can avoid the problem that all ports currently depened on openexr cannot pass the CI test, because if the CI test is not all passed, the PR will not be merged.

Change all the dependencies of ports that depend on openexr to openexr3, and solve any compilation problems that may arise above. If you need to modify upstream code, then you need to get upstream's approval.

I started 2) with just opencv4 because it's the big one. Still waiting for CI to finish but there is an early error on microsoft.vcpkg.pr (x64_windows) with this message

REGRESSION: vcpkg-ci-opencv:x64-windows cascaded, but it is required to pass. (C:\a\1\s\scripts\azure-pipelines/../ci.baseline.txt).

Any clue?

dg0yt · 2022-06-17T07:37:20Z

the problem is that all the ports depending on openexr (most notably opencv) will now fail on CI. Shall I revert back and let openexr build on CI and skip openexr3 (no one depends on this atm)?

That is what I meant when I pointed to the effects of skip : some ports will loose CI testing until upgraded.
Note that vcpkg-ci-opencv fails because CI is instructed to that this port must pass now, not just skipped because dependecies are missing. (This is new, and it is a good thing. CC @cenit)

PS is there a way with vcpkg command line to list all the packages that depend on a given package? With vcpkg search not all the results pop up, e.g. openimageio depends on openexr but it is not shown, it seems it only checks package names and the optional features (as in opencv3[openexr])

I'm not aware of such a command. But there is:

./vcpkg depend-info --overlay-ports scripts/test vcpkg-ci-opencv:x64-windows

Studying the dependencies, openexr could be taken out of this special CI port by removing the features openexr and ovis from the dependencies of vcpkg-ci-opencv. At least until these dependency chains are updated or resolved in another way.

But this is just the tip of the iceberg.

$ git grep -l '"openexr"' ports/*/vcpkg.json
ports/devil/vcpkg.json
ports/directxtex/vcpkg.json
ports/freeimage/vcpkg.json
ports/ilmbase/vcpkg.json
ports/magnum-plugins/vcpkg.json
ports/opencv/vcpkg.json
ports/opencv2/vcpkg.json
ports/opencv3/vcpkg.json
ports/opencv4/vcpkg.json
ports/openexr/vcpkg.json
ports/openimageio/vcpkg.json
ports/openvdb/vcpkg.json
ports/osg/vcpkg.json
ports/pangolin/vcpkg.json

These are 13 ports which directly depend on openexr, at least via opt-in features. So CI is potentially disabled for these ports if not upgraded. And you can continue exploring the dependencies.

$ git grep -l '"freeimage"' ports/*/vcpkg.json
ports/colmap/vcpkg.json
ports/forge/vcpkg.json
ports/freeimage/vcpkg.json
ports/ignition-common1/vcpkg.json
ports/ignition-common3/vcpkg.json
ports/ogre-next/vcpkg.json
ports/ogre/vcpkg.json
ports/opencascade/vcpkg.json

...

This is not meant to disccourage your effort. I would rather see it as an encouragement to gain more contributors.

dg0yt · 2022-06-17T08:55:09Z

Add all triplet of openexr3 as skip in the ci.baseline.txt, this can avoid the problem that all ports currently depened on openexr cannot pass the CI test, because if the CI test is not all passed, the PR will not be merged.

This would mean to keep the testing of legacy configurations (which are already known to work now) while blocking tests of PRs which migrate openexr consumers to openexr3 (which really need testing). This doesn't make sense to me.

dg0yt · 2022-06-17T09:03:29Z

REGRESSION: vcpkg-ci-opencv:x64-windows cascaded, but it is required to pass. (C:\a\1\s\scripts\azure-pipelines/../ci.baseline.txt).

Any clue?

Extending my previous comment: You updated opencv4, but there is a transitive dependency via ogre from opencv4[ovis].

simogasp · 2022-06-18T20:20:09Z

Cheney-W · 2022-06-22T08:11:21Z

@simogasp Could we convert this PR into draft first?

cenit · 2022-06-22T09:03:20Z

design question: maybe it's better to move current openexr to openexr2, make openexr a metaport that just depends on openexr3 and that's it? so that an overlay just has to replace the dependency of openexr to openexr2

simogasp · 2022-06-22T20:14:38Z

design question: maybe it's better to move current openexr to openexr2, make openexr a metaport that just depends on openexr3 and that's it? so that an overlay just has to replace the dependency of openexr to openexr2

In a different way that's the original spirit of this PR. The problem is, even with your proposal, people would like to have openexr 3 in the CI, which is a problem because other ports require openexr 2 to build in CI and we cannot have both versions built at the same time

cenit · 2022-06-25T10:00:47Z

if they cannot made be compatible to co-exist, then yes, every port should be made compatible with openexr3 before the upgrade. Same situation for ffmpeg5

fabiencastan · 2022-07-04T22:31:08Z

Alternately, vcpkg should remove the ports that don't consistently update their dependencies. When they update, they ought to be re-added.
Due to other less significant projects that are not routinely maintained, OpenEXR version cannot remain stagnant for years in vcpkg; otherwise, vcpkg's relevance will decline.

dg0yt · 2022-07-07T04:16:14Z

I would like to see this moved forward, integration the patches to consuming ports from #20957.
(I want to update openimageio/opencolorio, #23918.)

if they cannot made be compatible to co-exist, then yes, every port should be made compatible with openexr3 before the upgrade.

I don't think this a viable option in general. It is close to feature development via patches.

This is the original list chausner made to which I added my attempts (OK/NOK notes)

magnum-plugins

Looking into the portfile, it turns out that openexr is only needed for two features, and these features are only built in HEAD mode. So this is not used in vcpkg CI.

opencv

This is actually forwarding to opencv4 which is said to be OK. The dependency is behind an optional feature.

opencv2

opencv3

Not built in CI. The dependency is behind an optional feature.

osg --> [optional] NOK openexr is optional (if the plugins are built) but in the CMakeLists file it searches for it in any case, so it cannot be disabled with the current code. This is clearly a bug in the upstream cmake code

pangolin --> [optional] NOK, it's only optional but it requires some work on the source code to update

If optional, it is not blocking vcpkg CI.

So what is actually lost be moving forward, given that there are manifest mode and overlay ports for users that need an older configuration?

dg0yt · 2022-07-18T05:20:30Z

I see that requires:author-response was removed. @simogasp are you willing to continue this PR? The last CI log isn't accessible so it would be good to merge the current master into your branch.

Cheney-W · 2022-08-05T07:12:26Z

Ping @simogasp for response. Is work still being done for this PR?

simogasp · 2022-08-05T08:49:16Z

Ping @simogasp for response. Is work still being done for this PR?

Not at the moment. I'm on vacation and without a proper PC. I'll try to have another go in 20 days or so.

dg0yt · 2022-09-19T06:11:39Z

New direct update PR: #26862.

simogasp · 2022-09-26T08:52:14Z

replaced by #26862.

github-actions bot approved these changes Jun 14, 2022

View reviewed changes

simogasp force-pushed the ports/openexr3 branch from d5a0e64 to 9eac451 Compare June 14, 2022 20:44

github-actions bot approved these changes Jun 14, 2022

View reviewed changes

JonLiu1993 assigned Cheney-W Jun 15, 2022

Cheney-W added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Jun 15, 2022

Cheney-W reviewed Jun 15, 2022

View reviewed changes

ports/openexr3/portfile.cmake Outdated Show resolved Hide resolved

Cheney-W added the requires:author-response label Jun 15, 2022

Cheney-W changed the title ~~openexr3~~ [openexr3] Add new port Jun 15, 2022

simogasp force-pushed the ports/openexr3 branch from 9eac451 to 82e091b Compare June 15, 2022 09:06

github-actions bot approved these changes Jun 15, 2022

View reviewed changes

dg0yt suggested changes Jun 15, 2022

View reviewed changes

simogasp added 2 commits June 16, 2022 12:24

[openexr3] working on win linux and silicon

8c1ace6

* add openexr3 * made openexr 2 and 3 mutually exclusive * only openexr in the ci.baseline

[openexr3] add baseline

a89aca3

simogasp force-pushed the ports/openexr3 branch from 82e091b to a89aca3 Compare June 16, 2022 10:27

github-actions bot approved these changes Jun 16, 2022

View reviewed changes

[opencv4] switch to openexr3

1164e0e

[opencv4] update baseline

be39cf9

github-actions bot approved these changes Jun 17, 2022

View reviewed changes

simogasp marked this pull request as draft June 22, 2022 09:44

Cheney-W removed the requires:author-response label Jul 15, 2022

This was referenced Sep 16, 2022

[openexr] Side-by-side installation of version 2 and 3 #26788

Closed

[openexr] Update from 2 to 3.1.5, patch all consuming ports #26862

Merged

simogasp closed this Sep 26, 2022

simogasp deleted the ports/openexr3 branch September 26, 2022 08:52

Conversation

simogasp commented Jun 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dg0yt Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

Neumann-A Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

dg0yt Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

simogasp Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

meshula Jun 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dg0yt Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

simogasp Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

meshula commented Jun 15, 2022

Uh oh!

dg0yt commented Jun 15, 2022

Uh oh!

simogasp commented Jun 15, 2022

Uh oh!

meshula commented Jun 16, 2022

Uh oh!

dg0yt commented Jun 16, 2022

Uh oh!

Cheney-W commented Jun 16, 2022

Uh oh!

dg0yt commented Jun 16, 2022

Uh oh!

Cheney-W commented Jun 16, 2022

Uh oh!

dg0yt commented Jun 16, 2022

Uh oh!

Cheney-W commented Jun 16, 2022

Uh oh!

simogasp commented Jun 16, 2022

Uh oh!

Cheney-W commented Jun 16, 2022

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

simogasp commented Jun 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

meshula commented Jun 16, 2022

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Cheney-W commented Jun 17, 2022

Uh oh!

simogasp commented Jun 17, 2022

Uh oh!

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

Uh oh!

Uh oh!

dg0yt commented Jun 17, 2022

Uh oh!

dg0yt commented Jun 17, 2022

Uh oh!

simogasp commented Jun 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

simogasp commented Jun 14, 2022 •

edited

Loading

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

meshula Jun 15, 2022 •

edited

Loading

simogasp commented Jun 16, 2022 •

edited

Loading

dg0yt commented Jun 17, 2022 •

edited

Loading

simogasp commented Jun 18, 2022 •

edited

Loading

simogasp commented Jun 22, 2022 •

edited

Loading