[ocio] new port added

[theblackunknown]

Hi there,

I am interested to have OpenColorIO as part of vcpkg packages, and it seems I am not the only one (cf. #4175 & #5310 )

Here is a first step I got to pass, please let me know if this is good enough to be integrated or what needs to be changed

[msftclas]

All CLA requirements met.

[MVoz]

warn as error = off ?

[theblackunknown]

warn as error = off ?

It is true that we don't mind them as we are building a package

[theblackunknown]

@grdowns can I ask you to review this PR please ?

[cbezault]

@theblackunknown could you go over what the purpose of each of the patch files are. They are of a non-trivial size and I want to make sure I understand what is going on.

[theblackunknown]

Sure @cbezault

ports/ocio/0001-lcms-dependency-search.patch : allow to find lcms dependency built from vcpkg
This previously failed as it try to find lcms through pkgconfig and directly fallback to use embedded lcms if failed (cf. https://github.com/imageworks/OpenColorIO/blob/v1.1.1/src/apps/ociobakelut/CMakeLists.txt)

What my patch allow is :

1. Try finding lcms using PkgConfig (as to keep existing behavior)
2. If failed, use standard CMake functions to find lcms package (namely using find_package_handle_standard_args)
3. If failed, use embedded lcms (previous fallback behavior)

ports/ocio/0002-msvc-cpluscplus.patch : this allow to build correctly build ocioconvert, ociodisplay & ociolutimage as their source code rely on __cplusplus preprocessor value here, here and here

Unfortunately MVSC compiler requires an additional flag for it to have the right value, which is added by this patch

ports/ocio/0003-osx-self-assign-field.patch fix an programming error which is caught by AppleClang on vcpkg CI

ports/ocio/0004-yaml-dependency-search.patch : as 0001 allow to use yaml-cpp built from vcpkg rather than the embedded one

as 0001 we rely on CMake find_package here to find yaml-cpp package which is a more widely available and standard solution.
This additional requires more tweaks into the CMakeLists :

• /CMakeLists.txt:71 : even though it says so, I haven't seen any boost headers dependencies in vcpkg current yaml-cpp package, so I narrowed down the test for it
• /src/core/CMakeLists.txt:84 : the CMakeLists.txt is badly written and do not handle correctly requiring a static build with external yaml & lcms
• /src/core/OCIOYaml.cpp:104 : silent msvc compiler warning about inconsistent noexcept declaration between an a user runtime_error subclass and compiler libc++ definition

ports/ocio/0005-tinyxml-dependency-search.patch : as done in 0004 for yaml, the CMakeLists.txt is badly written and do not handle correctly requiring a static build with external tinyxml

ports/yaml-cpp/0003-cxx-std-features.patch yaml-cpp requires some C++11 feature but this requirement was not reported anywhere (for example it add compiler flag to set the expected C++ standard to use when linking this library)

Does it cover everything you wanted to know ?
Let me know if there is a better way to proceed for those !

[cbezault]

Everything looks good to me! Is there an upstream issue for patch 0003?

[theblackunknown]

I haven't opened any issues yet in upstream as I wanted to resolve the PR discussion first.
Once the PR is accepted I'll open issue in both OpenColorIO and yamp-cpp if this is fine for you ?

[cbezault]

In general we prefer having an issue/PR opened upstream for PRs with patches that fix bugs before we accept them so that people with more expertise have eyes on them.

But we're pretty lax about it so I'll probably still merge.

[theblackunknown]

Another question, more a matter of taste : should we call this port ocio or opencolorio ?
To me ocio is a more familiar term, but opencolorio would match the repo name (in lowercase) and be consistent with openimageio port name

What do you think ?

[cbezault]

In general we want port names to be the name by which the library's community refer to it as. This has some caveats, where we don't want to conflict with other known names out in the world.

In this case opencolorio doesn't seem like a bad name, but ocio doesn't seem bad either. I guess to err on the side of caution I'll say to go with opencolorio.

[cbezault]

Also ignore that windows failure, looks like one of the CI machines was forced to reboot.

[theblackunknown]

Thanks for your review work @cbezault !