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

[imath] Avoid conflicts between openexr and imath #26513

Closed
wants to merge 2 commits into from

Conversation

fabiencastan
Copy link
Contributor

Describe the pull request

Imath is included in openexr and the install of imath or openexr will overide the other one.
So, currently the system is broken and we cannot install both openexr and alembic, as it will end up in 2 differents versions mixed up.

  • What does your PR fix?

Use openexr which contains the latest version of imath.

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

Does not change anything.

Yes

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

Not yet, I would like to get feedback.

Fabien Castan added 2 commits August 25, 2022 10:59
Openexr-2 already includes imath, so we cannot mix them.
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!

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: checked-in files for opencolorio have changed but the version was not updated
version: 2.1.2
old SHA: d728bb2681e89ffbe9c6e1ec2811d5d194d9fd09
new SHA: 87a5292148dd3f18a44a6c7bdbc7ba45d52c3d8f
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***
error: checked-in files for alembic have changed but the version was not updated
version: 1.8.3#1
old SHA: 16bef528ef38e7426a333739878188d9ca909bc5
new SHA: 955d7dbf5f765fb910fbcc92b2f47f588d4517a3
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

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

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

  • ports/alembic/vcpkg.json

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

@dg0yt
Copy link
Contributor

dg0yt commented Aug 25, 2022

I think this PR is invalid. (The problem is valid, but the solution will be different.)

Use openexr which contains the latest version of imath.

Actually port imath is the latest version. Port openexr in vcpkg is an older version, which includes an older version of imath.

You can install both ports at the same time, but you can't use them at the same time.
For this reason, you currently cannot install openimageio[opencolorio] with the latest versions of these ports, because opencolorio was updated to use the new imath while openimageio still uses the outdated openexr.
However, in manifest mode, you could request an old, compatible version of opencolorio.
There is a version check added as a patch to openimageio:

+if(USE_OPENCOLORIO AND TARGET Imath::Imath AND OIIO_USING_IMATH STREQUAL "2")
+ message(FATAL_ERROR
+ "OpenColorIO and OpenEXR use incompatible versions of Imath. "
+ "You cannot use openimageio[opencolorio] for this configuration.")
+endif()

The ultimate solution is to update openexr to version 3. There have been two attempts already, but they are somewhat stuck due to depending ports not being ready for the update. And vcpgk makes it hard to have an openexr3 (or openexr2) in addition to openexr. But you know that, as you commented on these PRs.

@LilyWangLL LilyWangLL added category:port-bug The issue is with a library, which is something the port should already support requires:author-response labels Aug 26, 2022
@xchen31u
Copy link

Hi, please accelerate this if possible; is breaking OpenColorIO


file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/include")

file(INSTALL "${SOURCE_PATH}/LICENSE.md" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}" RENAME copyright)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't just remove this port, add set(VCPKG_POLICY_EMPTY_PACKAGE enabled) here, and add openexr to imath's dependency in vcpkg.json.
You can refer the change of PR #26655.

Copy link
Contributor

Choose a reason for hiding this comment

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

This part is invalid anyways. Imath is separate from openexr 3.
This whole PR is obsolete with #26862.

@LilyWangLL LilyWangLL changed the title Avoid conflicts between openexr and imath [imath] Avoid conflicts between openexr and imath Sep 19, 2022
@LilyWangLL
Copy link
Contributor

I think this PR is invalid. (The problem is valid, but the solution will be different.)

Use openexr which contains the latest version of imath.

Actually port imath is the latest version. Port openexr in vcpkg is an older version, which includes an older version of imath.

You can install both ports at the same time, but you can't use them at the same time. For this reason, you currently cannot install openimageio[opencolorio] with the latest versions of these ports, because opencolorio was updated to use the new imath while openimageio still uses the outdated openexr. However, in manifest mode, you could request an old, compatible version of opencolorio. There is a version check added as a patch to openimageio:

+if(USE_OPENCOLORIO AND TARGET Imath::Imath AND OIIO_USING_IMATH STREQUAL "2")
+ message(FATAL_ERROR
+ "OpenColorIO and OpenEXR use incompatible versions of Imath. "
+ "You cannot use openimageio[opencolorio] for this configuration.")
+endif()

The ultimate solution is to update openexr to version 3. There have been two attempts already, but they are somewhat stuck due to depending ports not being ready for the update. And vcpgk makes it hard to have an openexr3 (or openexr2) in addition to openexr. But you know that, as you commented on these PRs.

Ping @fabiencastan for response.

@LilyWangLL
Copy link
Contributor

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

@dg0yt
Copy link
Contributor

dg0yt commented Oct 8, 2022

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

I hope not. Cf. #26513 (comment).

@fabiencastan
Copy link
Contributor Author

No, everything is working fine with the great work to move to openexr-3!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants