Update analysis workflow

[remia]

This contains some update to the nightly build workflow and related areas:

• Update minimum versions of yaml-cpp to 0.7.0 and expat to 2.4.1
• Add analysis jobs running under macos-latest and windows-latest
• Allow for build of external projects on the latest available version with OCIO_INSTALL_EXT_PACKAGES_LATEST
• Fix external project build on Windows limited to debug only
• Prevent wheel workflow nightly to run on forks

This is still a draft but pushing it if there is any objection to the way it's headed.

[hodoulp]

LGTM. But it seems that there is a problem with NONE.

[hodoulp]

Refer to #1491 and #1466 for the third-party library updates.

[remia]

I wonder if we should update yaml-cpp and expat minimum version or just make sure we support newer version seamlessly. Also don't remember if any decision was made on how to handle dependency version update for new releases (major, minor or patch releases with probably different constraints).

Also noting that with this current PR, yaml-cpp 0.6.3 will still work (if we were to leave the FindExt.cmake to 0.6.3 or above) but unit test will fail because of the trailing whitespace differences.

[hodoulp]

We can safely update the third-party libraries because I did not plan to port this pull request in RB-2.1.
I think we agreed to not update a third-party library for a patch release.

The last update I did, I fixed the expected version because of mismatches between the versions (it was yaml). Supporting several versions could at some point become really troublesome to maintain.

[hodoulp]

We can safely update the third-party libraries because I did not plan to port this pull request in RB-2.1.

Less and less comfortable with the statement because the issues clearly highlight problems e.g. RB-2.1 and the latest yaml library at least!

[hodoulp]

As agreed by TSC, that's a go for a port in RB-2.1 when ready.

[hodoulp]

@remia Following the discussion at the last TSC meeting I propose to not use the latest version for private third-party libraries such as yaml-cpp, expat, etc because they are not part of the ASWF ecosystem and we agreed to only support a specific version but to enable it for imath.

[hodoulp]

@remia Did you plan to fix the NONE case in this pull request -- or -- to delay it to another pull request ?

[remia]

I'll add support for OCIO_INSTALL_EXT_PACKAGES=NONE then, we all agree it will produce configuration errors when required dependencies are missing I guess.

About the latest usage, currently it was only planned to be used for the nightly analysis build, are we saying we want to enable that for Imath? I'm not sure that would be something we want as the same library version could compile one day and the next fail due to some new Imath version introducing incompatible changes, or did I misinterpreted what was said?

[hodoulp]

I'll add support for OCIO_INSTALL_EXT_PACKAGES=NONE then, we all agree it will produce configuration errors when required dependencies are missing I guess.

Yes. That's the side-effect of using this option. But the default remains missing so, only some practitioners will use it.

[hodoulp]

About the latest usage, currently it was only planned to be used for the nightly analysis build, are we saying we want to enable that for Imath? I'm not sure that would be something we want as the same library version could compile one day and the next fail due to some new Imath version introducing incompatible changes, or did I misinterpreted what was said?

Yes. Only the nightly analysis CI build should use the latest version of the third-party libraries.

My point is related to the goal of the latest CI build. What are the libraries to validate?

As mentioned during the last TSC meeting and in previous comments from this pull request maintaining a range of versions for third-party libraries could be complex on long-term (I took the example of the yaml version conflicts I faced when updating that library). So, we agreed to stick to a specific version for the mandatory third-party libraries not part of the ASWF ecosystem (such as expat, yaml-cpp, etc.). BUT the latest CI build must take to the latest version for third-party libraries part of the ASWF ecosystem such as Imath, etc.

@michdolan @remia In conclusion I suggest that the latest CI build only validates OpenColorIO with the latest Imath, OpenImageIO and OpenShadingLanguage libraries.

[remia]

I will remove all the latest version override from the Find-X.cmake scripts except for Imath then. One thing I was finding useful with this latest everywhere way is that it would catch early issues which package maintainers face when third party library are updated using OCIO_INSTALL_EXT_PACKAGES=NONE and also when someone build OCIO with OCIO_INSTALL_EXT_PACKAGES=MISSING with third party dependencies above the requested version (which is permitted).

Also going to add OSL latest install for Linux builds.

[remia]

@hodoulp, I would recommend waiting for #1514 before merging this to make sure the Find-OSL script pick up the system wide install on the Linux analysis jobs. This is currently not the case, hence no OSL tests, but I believe this is fixed by the PR.

I'll also remove the temporary commit that enable the analysis build in PR when that's fixed.

[hodoulp]

Merge of OSL pull request is done, and this pull request is now updated.

[michdolan]

This will pre-install the latest Imath tag, which we are then doing again below (to a different location) with -DOCIO_INSTALL_EXT_PACKAGES=ALL and -DOCIO_INSTALL_EXT_PACKAGES_LATEST=ON. We will also be installing expat, lcms2, yaml-cpp, pystring, pybind11, and openfx twice with this current setup. Previously we used -DOCIO_INSTALL_EXT_PACKAGES=NONE for this workflow because these were all pre-installed with the scripts. If -DOCIO_INSTALL_EXT_PACKAGES_LATEST=ON is preferred, I would suggest only using the above install scripts for optional dependencies (and perhaps their dependencies, which may overlap with OCIO) which aren't installed by OCIO. Or, use -DOCIO_INSTALL_EXT_PACKAGES=MISSING to avoid duplicates where possible.

[hodoulp]

Perhaps the two defines approach is not the optimal one. To implement the right behavior, the code could support a fourth option dedicated to CI latest ?

[remia]

I could remove some of the dependencies install script but not much because OIIO need quite a lot of what OCIO already uses. I can remove all remaining OCIO_INSTALL_EXT_PACKAGES_LATEST stuff and only rely on install script, hoping that they will work on all 3 platforms (to that end we may want to move them to some non platform dependent sub folder as it seems it would only be copy and paste port). The advantage of OCIO_INSTALL_EXT_PACKAGES_LATEST was that it doesn't require any additional scripts to work but we already removed most of that and there might not be any use case out of this analysis workflow file.

[remia]

I'll try to have something ready this week to address this point.

[remia]

I pushed an update implementing the requested, change. As before analysis workflow temporarily enabled on PR to validate the changes.

[hodoulp]

Following comment from @michdolan, I'm now waiting for the new commit from @remia.

[hodoulp]

LGTM

[hodoulp]

@michdolan do you want to have a look before the merge?

[hodoulp]

When thinking about a patch version in a week, the pull request now needs to be merged to not delay too much the other pull requests.