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

CI Workflows various fixes #1945

Merged
merged 4 commits into from
Mar 12, 2024

Conversation

remia
Copy link
Collaborator

@remia remia commented Feb 8, 2024

This PR attempt to fix various issues on the GHA workflows:

  • Dependency latest was failing due to a recent OSL update, I updated the code adding #ifdef to handle the changes, tagging @lgritz for visibility.
  • Platform latest failing on ubuntu-latest / Clang combo, proposing to disable those 2 jobs for the time being until the underlying issue gets fixed (ubuntu-latest runners have an incompatible combination of clang and libstdc++ actions/runner-images#8659). Also updated the C++ standard to 23 to stay "bleeding edge".
  • Wheel workflow Windows failure required an update of cibuildwheel version. Updating the upload artifact action to remove the deprecation warning also required a change in the artifact layout, now each wheel is upload as a separate artifact you can find and download on the workflow summary page.
  • Updating the Node JS based action to remove deprecation warnings yielded an issue when running in ASWF docker images (<= 2022 IIRC) due to incompatible GLibC version, see https://github.com/AcademySoftwareFoundation/OpenColorIO/actions/runs/7836526047/job/21384368269, might be something to raise on the CI working group.
  • The analysis workflow is still failing at the upload report step, discussing with @jfpanisset it seems like a security feature of Github where PR can't have access to secrets to avoid leakage, will have to see if the fix works after merging.
  • Update the CI on Linux to add VFX 2024 Reference Platform new containers (thanks @jfpanisset), there is currently an issue with OIIO's Ptex dependency which might need a fix in the OCIO Docker image. Revert this change, will wait for the next ASWF Docker release.
  • Fixed a likely issue of incorrect Pybind11 usage mentioned in python tests fail because failed GIL interaction #1939 which seem hard to reproduce but ran into it while updating Platform latest to C++23 (which required a Pybind11 version update) on the Windows jobs.

@remia remia marked this pull request as draft February 8, 2024 22:03
@remia remia force-pushed the fix-ci-workflows branch 4 times, most recently from 9790c5a to 071ee0e Compare February 12, 2024 20:20
@remia remia force-pushed the fix-ci-workflows branch 10 times, most recently from 6fbe6e5 to 3c6a5fe Compare February 25, 2024 11:31
@remia remia force-pushed the fix-ci-workflows branch 2 times, most recently from d4fe879 to ee1c548 Compare February 25, 2024 15:39
@remia remia force-pushed the fix-ci-workflows branch from 39e8212 to 5499733 Compare March 4, 2024 19:49
@remia remia changed the title [Draft] CI Workflows various fixes CI Workflows various fixes Mar 4, 2024
@remia remia marked this pull request as ready for review March 4, 2024 19:50
Copy link
Collaborator

@doug-walker doug-walker left a comment

Choose a reason for hiding this comment

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

LGTM!

@KelSolaar
Copy link
Contributor

My only comment is whether we should include the GIL release in this PR? https://github.com/AcademySoftwareFoundation/OpenColorIO/pull/1945/files#diff-6c87f76bfb1f06a48bb762f97632eee604f4d8cd2e852347c3b46209a056086a

Those seems to be beyond pure CI workflow changes.

If people are happy with it then fine.

@doug-walker
Copy link
Collaborator

@KelSolaar, given the large delays in getting PRs merged, we all sometimes include small miscellaneous improvements into a larger PR. It's not ideal, but it's a response to the current state of affairs where we don't have a sufficient number of reviewers.

@KelSolaar
Copy link
Contributor

@doug-walker : All good with me and I understand the frustration! It is one of the reason I have been hesitant to move colour to the ASWF and a more formal review process, it certainly hinders the development velocity.

@KelSolaar KelSolaar merged commit 9fe81cb into AcademySoftwareFoundation:main Mar 12, 2024
74 of 75 checks passed
@KelSolaar
Copy link
Contributor

KelSolaar commented Mar 12, 2024

Merging for good measure because the aforementioned note was the only comment I had! Thanks @remia.

@lgritz
Copy link
Collaborator

lgritz commented Mar 12, 2024

@doug-walker : All good with me and I understand the frustration! It is one of the reason I have been hesitant to move colour to the ASWF and a more formal review process, it certainly hinders the development velocity.

Such rules are made by every project TSC individually to pick the right set of procedures for their project and team. There is no ASWF-mandated policy on how reviews and merges work.

@lgritz
Copy link
Collaborator

lgritz commented Mar 12, 2024

I'm sorry, I see I was tagged here, but I didn't notice this PR until today. Apologies if you wanted me to look at something here and I failed to reply. (I do "watch" this project, but sometimes when I'm busy, I'm a little eager with the delete key if I don't see a PR title that I think is highly relevant to me.)

@remia
Copy link
Collaborator Author

remia commented Mar 12, 2024

@KelSolaar Agree the GIL changes could have been separated in a different PR, I included these here because it fixed the WIndows Platform Latest workflow when I switched them to C++23.

@lgritz No worry at all! I tagged you because I made some OSL related fix for the unit tests.

@remia remia deleted the fix-ci-workflows branch April 8, 2024 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants