libsdl-org / SDL_image

Image decoding for many popular formats for Simple Directmedia Layer.
zlib License
559 stars 182 forks source link

CI: macos ninja setup failure #447

Closed sezero closed 5 months ago

sezero commented 5 months ago

We use turtlesec-no/get-ninja for installing ninja to CI host and it is failing on current macOS 14 hosts. Upstream issue (where someone made a fork with a workaround): https://github.com/turtlesec-no/get-ninja/issues/4

This is so with SDL_image, SDL_mixer, SDL_net and sdl2-compat. On the other hand, we use ashutoshvarma/setup-ninja in SDL_ttf and SDL_rtf, and it doesn't seem to be failing.

Should we switch to using ashutoshvarma/setup-ninja for SDL_image, SDL_mixer, SDL_net and sdl2-compat? Or should we switch to using the fork of turtlesec-no/get-ninja? OR: Why aren't we just doing a simple brew install for it?

CC: @madebr

madebr commented 5 months ago

The fork is much simpler and has less dependencies. Let's make the switch.

sezero commented 5 months ago

OK then. We should do the same for all satellite libs and sdl2-compat, as well.

sezero commented 5 months ago

Same issue on SDL2 and release branches of satellite libs, btw.

madebr commented 5 months ago

I went around all satellite libraries and switched the action everywhere, meanwhile also fixing the dylib current/compatibility version <=> soversion thing.

There are 2 macos/arm64 issues left:

sezero commented 5 months ago

vendored libmpg123 fails to build. Perhaps we need to bump the vendored version.

Before blindly bumping it, I looked at optimize.h line 198, it errors if REAL_IS_FIXED is defined, because cmake_host_system_information isn't setting HAVE_FPU? Why?

madebr commented 5 months ago

Looks like a CMake bug to me. https://github.com/madebr/throwaway/actions/runs/8915872718/job/24486263066#step:3:11

I created an upstream bug report.

The mpg123 cmake script already contains an exception for MSVC, so it looks like the test has also failed there in the past.

sezero commented 5 months ago

OK, so we should add an APPLE exception then?

madebr commented 5 months ago

Yeah, I'm looking into creating a patch for current main atm. It should be easy to backport.

madebr commented 5 months ago

Done, and also applied to mpg123 of SDL2.

sezero commented 5 months ago

macos/arm64 issues left:

the build-time SDL_image test fails (because of the relative recent libjxl)

The CI log says brew is installing version 0.10.2 of libjxl. Do we have any other runners that use system libjxl and at least with version >=0.10.0 ?

P.S.: The libjxl + macos issue should be noted in a new ticket?

madebr commented 5 months ago

The CI log says brew is installing version 0.10.2 of libjxl. Do we have any other runners that use system libjxl and at least with version >=0.10.0 ?

P.S.: The libjxl + macos issue should be noted in a new ticket?

Yes, a new ticket should be created.

Only toolchain that has libjxl>=0.10 is msys2.

edit: their website mentions 0.10.2, but our runners only use 0.9.1.

sezero commented 5 months ago

Created https://github.com/libsdl-org/SDL_image/issues/450

Closing this one.