Update CI workflows#1770
Conversation
Signed-off-by: Rémi Achard <remiachard@gmail.com>
Signed-off-by: Rémi Achard <remiachard@gmail.com>
|
Chatting with @JeanChristopheMorinPerso, I disabled parallel build on OpenEXR ExternalProject, I'll try to trigger the build a couple of times but on the first try it seemed to have resolved the issue! |
Signed-off-by: Rémi Achard <remiachard@gmail.com>
Signed-off-by: Rémi Achard <remiachard@gmail.com>
doug-walker
left a comment
There was a problem hiding this comment.
The updated CI workflow looks great!
Few questions about the new workflows ...
| run: | | ||
| share/ci/scripts/multi/install_imath.sh latest $EXT_PATH | ||
| share/ci/scripts/multi/install_openexr.sh latest $EXT_PATH | ||
| share/ci/scripts/multi/install_oiio.sh latest $EXT_PATH |
There was a problem hiding this comment.
It seems we should have some tests with DOCIO_USE_OIIO_FOR_APPS=ON?
There was a problem hiding this comment.
Agreed I'll add some, unfortunately I think we will not be able to test until other issues are fixed in that workflow.
Actually I'll not include that change I think and leave that to the upcoming workflow update planned, do you agree @doug-walker?
| - name: Install indirect dependencies | ||
| run: | | ||
| share/ci/scripts/multi/install_pugixml.sh latest | ||
| - name: Install fixed ext package versions |
There was a problem hiding this comment.
I'm worried about these getting out of sync with the CMake script versions. Would it work to install only the Imath/EXR/OIIO/etc. manually and then use DOCIO_INSTALL_EXT_PACKAGES=MISSING for the rest?
There was a problem hiding this comment.
I think this is a valid point, though it seems to be a PR in itself, like you said I just copied over the workflow. I'm not confortable making this change without ability to test.
| run: | | ||
| EXT_PATH=/usr/local | ||
| echo "EXT_PATH=$EXT_PATH" >> $GITHUB_ENV | ||
| - name: Install indirect dependencies |
There was a problem hiding this comment.
Not related to your PR, I realize you just copied these over, but I'm curious what these are used for. Same for the other platforms.
There was a problem hiding this comment.
These are the dependencies of dependencies we are installing, like boost to name the obvious. In this workflow we are building more things than the CI, mostly OpenImageIO and OSL which have additional dependencies.
| shell: bash | ||
| - name: Install indirect dependencies | ||
| run: | | ||
| vcpkg install zlib:x64-windows |
There was a problem hiding this comment.
Seems like zlib gets installed twice?
There was a problem hiding this comment.
Yes, this seem to be introduced by #1725, this might be due to the specific version requirement for the last version of zlib which vcpkg and others didn't have at that time?
| # ------------------------------------------------------------------- | ||
| # VFX CY2022, C++17, docs, OpenFX | ||
| - build: 1 | ||
| build-docs: 'ON' |
There was a problem hiding this comment.
Is it worth having docs and openfx part of this matrix? It's not clear why Imath/EXR/OIIO/etc. would break those. I think you could leave them off for this workflow.
There was a problem hiding this comment.
Same remark as above, not updating this workflow in this PR.
|
For anyone else reviewing the new CI settings, I made a spreadsheet showing the current and new matrix: |
Signed-off-by: Rémi Achard <remiachard@gmail.com>
|
Thanks for the review @doug-walker, most of the points are about the old analysis workflow (new dependencies_latest) which I didn't touch and is currently failing in nightly for the past few months. From my understanding @cedrik-fuoco-adsk was about to work on these so I think it's best to push the comments for that upcoming work? I'm not really confortable updating things in there knowing the builds are going to fail early and I won't be able to test. |
doug-walker
left a comment
There was a problem hiding this comment.
Agree with all your points Remi. Let's go ahead and merge this and then follow up on the other items in a separate PR, after we get the workflows to start succeeding again.
* Update CI workflows Signed-off-by: Rémi Achard <remiachard@gmail.com> * Test building OpenEXR single threaded Signed-off-by: Rémi Achard <remiachard@gmail.com> * Add comment on disabling parallel build Signed-off-by: Rémi Achard <remiachard@gmail.com> * Update python_requires Signed-off-by: Rémi Achard <remiachard@gmail.com> * Remove comments around CI jobs Signed-off-by: Rémi Achard <remiachard@gmail.com> --------- Signed-off-by: Rémi Achard <remiachard@gmail.com> Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
As discussed during TSC meetings, here are some update and new CI workflows.
This PR contains the following changes:
Note some jobs changed their names in the CI workflow, updating the required list is again going to be needed, hopefully this will happen less in the future as I removed some variability from these name like compiler versions which are not always fixed even in ci-20xx docker images (Clang). I also had a recurring issue where the GCC based linux platform latest build was killed by GitHub Action during compilation of OpenEXR with no diagnostic messages, apparently this can happen when some hardware resources limits are reached. This was on my fork but may be different here, this could require further investigations, usually manual relaunch of the failed jobs fix the issue.