vcpkg
vcpkg copied to clipboard
[openexr3] Add new port
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/
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.
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.
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?
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.
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.
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.
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.
Do you mean skip openexr or make both ports work at the same time?
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.
@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
@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!
Yes
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]
)
@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!
@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 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?
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 inopencv3[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.
- 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.
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]
.
what a rabbit hole I put myself in! :laughing:
It seems impossible to update only a bunch of ports to openexr 3 without skipping all the others in the CI.
If we have to update all the ports that depend on openexr we are back to @chausner 's work in #20957 and having openexr3
does not make sense anymore, it's better to update the existing one,
This is the original list chausner made to which I added my attempts (OK/NOK notes)
- [X] devil
- [x] directxtex --> OK need to fix headers in patch + need a fix in CMakeLists.txt
- [X] freeimage
- [ ] magnum-plugins
- [X] ogre
- [ ] opencv
- [ ] opencv2
- [ ] opencv3
- [X] opencv4 --> OK
- [X] openimageio --> OK
- [X] openvdb --> OK
- [ ] 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
@simogasp Could we convert this PR into draft first?
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
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
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
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.
I would like to see this moved forward, integration the patches to consuming ports from https://github.com/microsoft/vcpkg/pull/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?
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.
Ping @simogasp for response. Is work still being done for this PR?
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.
New direct update PR: #26862.