OpenTimelineIO
OpenTimelineIO copied to clipboard
Add basic mingw_x86_64 build support
This is the minimal amount of changes required to add mingw_x86_64 support on windows and pass all the tests.
This pull request is mainly for discussion as to if this is a platform OTIO wishes to support. I haven't looked into adding the platform to CI yet, but feels like it might be a bit of a headache to get working.
I also haven't been able to get it to build in 32-bit mode yet, just in 64-bit.
Not being able to build on mingw has been mentioned here https://github.com/AcademySoftwareFoundation/OpenTimelineIO/discussions/1342#discussioncomment-3088730
I'm not entirely sure what we should test in CI. Maybe just test that it compiles and that it can be imported? Or maybe run the whole thing but just on one version of Python...
@JeanChristopheMorinPerso I wasn't sure what the best approach would be either. I guess we would want to pin the mingw MSYS2 version and run the unit test for its version of python.
I was going to see what other projects might be doing but haven't found anything yet. It doesn't seem to be a cibuildwheel supported platform either. I'll do a little more investigation.
Supporting this platform is probably not be worth the effort if there is no demand for it.
The reason I mentioned CI is that I don't think we have expertise to maintain it on the team, so CI would be the only way we'd know if something needs addressing, unless there was an active community around the target. Since the change is mostly detecting the platform and picking a generator, I wouldn't expect it to be overwhelming to maintain as long as otioview isn't part of it, and as long as it's as Mark suggested, pinned to a version in CI.
As a counterpoint, OpenEXR is an example project that is known to work with MinGW, and the community has been gracious enough to do the maintenance over the years. So perhaps a non-rigorous approach is fine, as long as we aren't making an over arching promise that it always works.
This could be a good topic to raise at an upcoming TSC meeting to take the temperature?
Yeah we can discuss it during the next TSC meeting.
I have a rough github actions building / testing example here. https://github.com/markreidvfx/otio-MinGW-w64-test
Getting it integrated into otio's build matrix might still be tricky, but I'll keep chipping away at it.
Codecov Report
Merging #1354 (7d5b4ec) into main (913f540) will not change coverage. The diff coverage is
100.00%.
@@ Coverage Diff @@
## main #1354 +/- ##
=======================================
Coverage 86.27% 86.27%
=======================================
Files 196 196
Lines 19865 19865
Branches 2309 2309
=======================================
Hits 17138 17138
Misses 2161 2161
Partials 566 566
| Flag | Coverage Δ | |
|---|---|---|
| py-unittests | 86.27% <100.00%> (ø) |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| tests/test_plugin_detection.py | 81.57% <ø> (ø) |
|
| tests/test_url_conversions.py | 90.47% <100.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 913f540...7d5b4ec. Read the comment docs.
Your last commit looks much better @markreidvfx !
I'm happier with it too! The only thing I haven't figured out is how to pin the MSYS2 version. There is some discussion of being able to do that here: https://github.com/msys2/setup-msys2/issues/180, but the feature has not been implemented.
@markreidvfx does the MSYS2 pinning represent a blocking fix? Is this otherwise ready to land? Thanks!
Pining the MSYS2 version is the only thing I haven't been able to solve.
From what I understand MSYS2 only updates their base packages once a year. Unless you run a pacman sync command to get updates on top of that (which I don't) you get the same base version for that period of time.
I personally think that it should be stable enough to test against and if it proves to be problematic, it is pretty easy to turn off the CI.