AcademySoftwareFoundation / openexr

The OpenEXR project provides the specification and reference implementation of the EXR file format, the professional-grade image storage format of the motion picture industry.
http://www.openexr.com/
BSD 3-Clause "New" or "Revised" License
1.63k stars 614 forks source link

VFX Platform 2021 CI #1143

Open kdt3rd opened 3 years ago

kdt3rd commented 3 years ago

It would appear the 2021 CI builds are configuring with c++14 and the comment says gcc 6.3. VFX Platform 2021 states gcc 9.3.1 and c++17, we should normalize this. Also, do we need to keep all the variants from VFX Platform 2020 or drop it down to 1 or 2 as VFX Platform 2019 is?

meshula commented 3 years ago

What are you thinking normalizing C++ version might involve?

kdt3rd commented 3 years ago

oh, just need to edit the CI build script and specify 17 for the c++ version in the matrix specs, and think about reducing the size of the build matrix (or not, separate concerns). In the middle of some other stuff, just put this in so I didn't forget...

meshula commented 3 years ago

I suppose the CI should be running C++ versions according to the vfxplatform recommendation of supporting this year's platform, and the three prior years? That would suggest building against only c++14 and c++17. And I guess to future proof ourselves, it should also build against c++20, since latest MSVC, clang, gcc all support it already...

lgritz commented 3 years ago

I think we're crazy not to test against C++11 (unless we're going to say we no longer support it) as well as C++20 (because at the very least, why not fix anything broken early?).

lgritz commented 3 years ago

Testing VFX Platform configurations (in particular, on CentOS images) is necessary, but nowhere near sufficient.

meshula commented 3 years ago

Fair enough. I think we are setting C++11 explicitly in Imath at least. That argues for having C++11 in the mix. If we're still supporting pthreads and whatnot for pre-c++11, it's likely time to remove that permanently, unless we want to have c++03 in the CI as well...

kdt3rd commented 3 years ago

well, I was thinking about suggesting to change the build matrix to have the following convention:

I think we can continue to skip the various versions of ubuntu, and just run on ubuntu-latest for linux (the current setting). If Microsoft does actually deploy ARM chips to Azure (there are rumors), this means it'd probably become available in github CI, which would grow the above

lgritz commented 3 years ago

That sounds reasonable to me.

I think it's also useful to have (either using the aswf containers or just plain ubuntu) a couple tests of newer compilers. VP's gcc9 is already old news, the cool kids use gcc11 (it's reasonable to combine that with the C++20 test).

meshula commented 3 years ago

what was the end of the sentence "(it would appear we are unable to target"?

I think it all sounds good to me.

xlietz commented 3 years ago

I am planning on updating the CI to include the following builds:

VFX 2022

VFX 2021

VFX 2020

VFX 2019

Questions: 1) For the experimental C++20 build, none of the docker images currently have GCC 11 and I don't think I can assume that the nodes will. Should I ask Aloys to create an image with the GCC 11 toolset? 2) Do we need a legacy C++14 test on the VFX 2021 image? I omitted it since it is covered in VFX 2020/2019. 3) Imath repo - same list of tests? Does Imath need a Threads=OFF test or is that only relevant in OpenEXR? 4) Any omissions? Other comments?

lgritz commented 3 years ago
  1. In my projects, to get gcc11/C++20 I just use the base Ubuntu distro, not the ASWF CentOS docker image (Ubundu also, then, doubles as yet another test case, to be sure we aren't making software or a build system that only works for a typical VFX CentOS setup).

Relevant links: https://github.com/OpenImageIO/oiio/blob/9d2a52341d9cff5a5397dffef2de6c456c674f08/.github/workflows/ci.yml#L376 https://github.com/OpenImageIO/oiio/blob/9d2a52341d9cff5a5397dffef2de6c456c674f08/src/build-scripts/gh-installdeps.bash#L77

On the other hand, if we made a "bleeding edge" ASWF CentOS image with gcc11, I would probably use it for some tests.

  1. I think we should definitely still be testing C++14 even for an otherwise-2021 build.

  2. There's no threading in Imath.

kdt3rd commented 3 years ago

Imath has no threading, there does not need to be that variant, but should have python / no python (which openexr does not need).

That looks like a good list to me. For clarity, it might be good to tag the experimental / legacy things separately, even if you use the vfx platform docker to build them (the VFX platform year defines the c++ version to use)

Not sure how full a matrix we need for the MacOS and Windows builds, you could probably continue to have just 4? notice that they are now officially denoted w/ what SDK versions to use on vfxplatform.com: VFX 2022 (c++17), Shared, Release VFX 2021 (c++17), Shared, Release VFX 2021 (c++17), Static, Release VFX 2021 (c++17), Shared, Debug <-- I think this is safe now that we split out all the tests to run individually, right?

lgritz commented 3 years ago

I'm not sure that he CAN easily create a CentOS image with gcc11 -- I don't think CentOS has a devtoolset with that yet, do they?

kdt3rd commented 3 years ago

I'm not sure that he CAN easily create a CentOS image with gcc11 -- I don't think CentOS has a devtoolset with that yet, do they?

probably best to use a fedora variant or something? (or ubuntu would be nice to have a non-RedHat)

xlietz commented 3 years ago

Kimball, thanks for mentioning MacOS and Windows builds - I will update those to the 4 builds you recommended.

I can check with Aloys regarding whether a CentOS image with gcc11 is possible. It looks like CentOS-8 has a gcc11 toolset: https://centos.pkgs.org/8-stream/centos-appstream-x86_64/gcc-toolset-11-build-11.0-0.el8.x86_64.rpm.html The current ASWF docker base images are all CentOS-7 so it depends on whether it is possible to build CentOS-8 images.

If he can make a CentOS-8 gcc11 image, would we want 2 experimental builds - one on CentOS, one on ubuntu (prefer this to fedora since I can leverage Larry's oiio CI setup) ? So we still have one on a non-RedHat distro as well?

xlietz commented 2 years ago

For the overdue Imath 2022 CI update, I am planning on updating the matrix as follows:

Linux

VFX 2022

VFX 2021

VFX 2020

VFX 2019

MacOS

C++17, Release Shared C++17, Debug Shared C++17, Release Static C++14, Release Shared C++11, Release Shared

Windows

C++17, Release Shared C++17, Debug Shared (if it works, it's currently disabled) C++17, Release Static C++14, Release Shared C++11, Release Shared

Questions:

  1. In VFX 2022 builds, repeat all builds python on/off?
  2. Keep C++11 builds on Mac/Windows?
  3. Keep VFX 2019 builds (also C++11)?
  4. Keep both gcc/Clang for VFX 2020-2021?
  5. Any others to remove or add?
xlietz commented 2 years ago

I edited the previous comment to fix a few mistakes and add better formatting.

meshula commented 2 years ago

In VFX 2022 builds, repeat all builds python on/off?

It makes sense to build on/off

Keep C++11 builds on Mac/Windows?

yes, see next

Keep VFX 2019 builds (also C++11)?

The vfxplatform survey tells us a significant number of studios are transitioning off VFX 2019 some time this year, and that it is safe to drop VFX 2019 at the end of the year.

So,,,, keep

Keep both gcc/Clang for VFX 2020-2021?

seems important

Any others to remove or add?

VS2022 is gaining steam. We added it to the OTIO matrix without a problem. Same with Python 3.10. Neither is part of the platform, but in practical terms, many users are using them already, in particular, there's been a lot of attention from Blender devs on these two.

meshula commented 2 years ago

re: vs2022 and py 3.10 ~ these wouldn't need full matrixing, they could be a vfx2023 or vfx202x variant.

xlietz commented 2 years ago

What is OTIO using for CI? I couldn't find a github actions workflow but maybe I wasn't looking in the right place. I am not sure how to get VS 2022 for the windows builds as I thought I was at the mercy of whatever the GHA nodes provided for Windows. For python 3.10, I'd have to see if Aloys could make a new docker image as we don't have one yet with Py3.10.

meshula commented 2 years ago

Our CI is hiding in the PR workflow, here's an example ~ https://github.com/PixarAnimationStudios/OpenTimelineIO/runs/5555252482?check_suite_focus=true

meshula commented 2 years ago

and I see we haven't yet landed changed for py310, I thought we had.

xlietz commented 2 years ago

Oh now I see it! I should have looked in the actions tab to see that python-package.yml is the workflow name. I also just saw the CI update Remi Achard added in OpenEXR too which is using MSVC 2022 (17.1) - thanks for approving/merging that!

cary-ilm commented 2 years ago

Following up on this, is the CI where we think it should be? Can we close this out or is there more to sort out?

xlietz commented 2 years ago

I still need to add the Linux no-python builds to the Imath repo. But OpenEXR is updated so I think we can close this issue.