MacOS workflow: Install harfbuzz normally again #2622

tobbi · 2023-09-04T08:31:05Z

See
https://github.com/orgs/Homebrew/discussions/4735#discussioncomment-6902146

See https://github.com/orgs/Homebrew/discussions/4735#discussioncomment-6902146

ZhongRuoyu

You may need to add

brew update
brew reinstall harfbuzz

to make sure you're using the newest bottle, instead of the one cached in the GitHub Actions runner image: https://github.com/SuperTux/supertux/actions/runs/6071036504/job/16468259452#step:3:24

The brew reinstall step shouldn't be needed when the runner image is updated, or when harfbuzz is bumped to a newer version. However, I do recommend that brew update be kept, because that makes sure the formula (package) information is up-to-date. (The GitHub runner image explicitly disables auto-update.) At the time of writing, cmake is already at 3.27.4, while the CI logs still incorrectly suggests that the already-installed cmake 3.27.3 is up to date.

ZhongRuoyu

Looks like you've hit another GitHub Actions runner image issue. Before that gets resolved, brew unlink python && brew link --overwrite python should work around it. (At Homebrew/brew we apply the same workaround: Homebrew/brew@298003b.)

tobbi · 2023-09-04T10:02:36Z

@ZhongRuoyu Still fails with the olink error. Any idea?

ZhongRuoyu · 2023-09-04T10:36:28Z

I downloaded the last successful artifact from https://github.com/SuperTux/supertux/actions/runs/6068886641 and had a look at SuperTux.app/Contents/Resources/Frameworks, where CPack's fixup_bundle copied the dylibs to. It turned out that it's libSDL2_image that links with libjpeg:

libSDL2_image-2.0.0.dylib:
	@executable_path/../Frameworks/libSDL2_image-2.0.0.dylib (compatibility version 601.0.0, current version 601.3.0)
	@executable_path/../Frameworks/libpng16.16.dylib (compatibility version 56.0.0, current version 56.0.0)
	@executable_path/../Frameworks/libjxl.0.8.dylib (compatibility version 0.8.0, current version 0.8.1)
	@executable_path/../Frameworks/libjpeg.8.dylib (compatibility version 8.0.0, current version 8.2.2)
	@executable_path/../Frameworks/libtiff.6.dylib (compatibility version 7.0.0, current version 7.0.0)
	@executable_path/../Frameworks/libavif.15.dylib (compatibility version 15.0.0, current version 15.0.1)
	@executable_path/../Frameworks/libwebp.7.dylib (compatibility version 9.0.0, current version 9.6.0)
	@executable_path/../Frameworks/libsharpyuv.0.dylib (compatibility version 1.0.0, current version 1.0.0)
	@executable_path/../Frameworks/libSDL2-2.0.0.dylib (compatibility version 2601.0.0, current version 2601.5.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.100.5)

sdl2_image was rebuilt two weeks ago (Homebrew/homebrew-core@7878f28) because of a dependency update. The rebuilt dylib has relocatable install names (the ones with @loader_path) in it, which CPack, again, failed to handle:

$ otool -L `brew --prefix sdl2_image`/lib/libSDL2_image.dylib
/usr/local/opt/sdl2_image/lib/libSDL2_image.dylib:
	/usr/local/opt/sdl2_image/lib/libSDL2_image-2.0.0.dylib (compatibility version 601.0.0, current version 601.3.0)
	@loader_path/../../../../opt/libpng/lib/libpng16.16.dylib (compatibility version 57.0.0, current version 57.0.0)
	@loader_path/../../../../opt/jpeg-xl/lib/libjxl.0.8.dylib (compatibility version 0.8.0, current version 0.8.2)
	@loader_path/../../../../opt/jpeg-turbo/lib/libjpeg.8.dylib (compatibility version 8.0.0, current version 8.3.2)
	@loader_path/../../../../opt/libtiff/lib/libtiff.6.dylib (compatibility version 7.0.0, current version 7.1.0)
	@loader_path/../../../../opt/libavif/lib/libavif.16.dylib (compatibility version 16.0.0, current version 16.0.0)
	@loader_path/../../../../opt/webp/lib/libwebp.7.dylib (compatibility version 9.0.0, current version 9.7.0)
	@loader_path/../../../../opt/sdl2/lib/libSDL2-2.0.0.dylib (compatibility version 2801.0.0, current version 2801.2.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1319.100.3)

We've temporarily disabled relocatable install names until we figure out a way to resolve similar issues (mostly with CPack). For now, I have dispatched a rebuild job for sdl2_image. I'll let you know when it's updated.

ZhongRuoyu · 2023-09-04T11:52:12Z

@tobbi I have rebuilt the affected libraries (that I recognised from the .pkg artifact; namely sdl2_image, libavif, brotli, and jpeg-xl), please try again; no modifications needed.

ZhongRuoyu · 2023-09-04T12:21:57Z

Somehow the updated bottle for jpeg-xl was not delivered in https://github.com/SuperTux/supertux/actions/runs/6071589928/job/16474229811#step:3:190 (see checksum update for Big Sur at Homebrew/homebrew-core#141358 -- the old one was downloaded). This could be due to GitHub Pages' caching (we host our formula API there). Would you please try rerunning the jobs @tobbi?

Sorry for the back and forth!

ZhongRuoyu · 2023-09-04T12:53:06Z

webp needs to be updated, too.

Actually, let me perform more tests in a separate supertux fork. I'll let you know when it builds.

ZhongRuoyu · 2023-09-05T15:14:54Z

@tobbi That turned out to be an unrelated CPack issue that's been there for a while: https://gitlab.kitware.com/cmake/cmake/-/issues/22557

What happened was: libwebp made their libsharpyuv requirement private in this commit, first released in 1.3.1. ~~The current GitHub Actions macOS 11 image is still on 1.3.0, but it was updated to the new version by doing brew update and brew install.~~

The direct consequence of that was the change in required install names:

(Before)

$ otool -L /usr/local/opt/sdl2_image/lib/libSDL2_image-2.0.0.dylib
/usr/local/opt/sdl2_image/lib/libSDL2_image-2.0.0.dylib:
	/usr/local/opt/sdl2_image/lib/libSDL2_image-2.0.0.dylib (compatibility version 601.0.0, current version 601.3.0)
	/usr/local/opt/libpng/lib/libpng16.16.dylib (compatibility version 56.0.0, current version 56.0.0)
	/usr/local/opt/jpeg-xl/lib/libjxl.0.8.dylib (compatibility version 0.8.0, current version 0.8.1)
	/usr/local/opt/jpeg-turbo/lib/libjpeg.8.dylib (compatibility version 8.0.0, current version 8.2.2)
	/usr/local/opt/libtiff/lib/libtiff.6.dylib (compatibility version 7.0.0, current version 7.0.0)
	/usr/local/opt/libavif/lib/libavif.15.dylib (compatibility version 15.0.0, current version 15.0.1)
	/usr/local/opt/webp/lib/libwebp.7.dylib (compatibility version 9.0.0, current version 9.6.0)
	/usr/local/opt/webp/lib/libsharpyuv.0.dylib (compatibility version 1.0.0, current version 1.0.0)
	/usr/local/opt/sdl2/lib/libSDL2-2.0.0.dylib (compatibility version 2601.0.0, current version 2601.5.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.100.5)

(After -- note that /usr/local/opt/webp/lib/libsharpyuv.0.dylib is no longer in the list.)

$ otool -L /usr/local/opt/sdl2_image/lib/libSDL2_image-2.0.0.dylib
/usr/local/opt/sdl2_image/lib/libSDL2_image-2.0.0.dylib:
	/usr/local/opt/sdl2_image/lib/libSDL2_image-2.0.0.dylib (compatibility version 601.0.0, current version 601.3.0)
	/usr/local/opt/libpng/lib/libpng16.16.dylib (compatibility version 57.0.0, current version 57.0.0)
	/usr/local/opt/jpeg-xl/lib/libjxl.0.8.dylib (compatibility version 0.8.0, current version 0.8.2)
	/usr/local/opt/jpeg-turbo/lib/libjpeg.8.dylib (compatibility version 8.0.0, current version 8.3.2)
	/usr/local/opt/libtiff/lib/libtiff.6.dylib (compatibility version 7.0.0, current version 7.1.0)
	/usr/local/opt/libavif/lib/libavif.16.dylib (compatibility version 16.0.0, current version 16.0.1)
	/usr/local/opt/webp/lib/libwebp.7.dylib (compatibility version 9.0.0, current version 9.7.0)
	/usr/local/opt/sdl2/lib/libSDL2-2.0.0.dylib (compatibility version 2801.0.0, current version 2801.2.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.100.5)

This didn't surface before because libSDL2_image was only rebuilt recently in Homebrew/homebrew-core@7878f28. Before that, the reference to libsharpyuv was still present.

CPack's fixup_bundle does an iterative search over the required dylibs, so without the direct requirement from libSDL2_image, it turned to rely on libwebp to find libsharpyuv. libwebp's requirements are (in both 1.3.0 and 1.3.1 versions):

otool -L /usr/local/opt/webp/lib/libwebp.7.dylib
/usr/local/opt/webp/lib/libwebp.7.dylib:
	/usr/local/opt/webp/lib/libwebp.7.dylib (compatibility version 9.0.0, current version 9.7.0)
	@rpath/libsharpyuv.0.dylib (compatibility version 1.0.0, current version 1.1.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.100.5)

And CPack failed to handle the @rpath-based install name, as discussed in the CMake issue.

Before this gets fixed on CMake's side, a workaround would be to run this after webp gets installed (i.e. after all brew commands):

install_name_tool -change \
  '@rpath/libsharpyuv.0.dylib' \
  /usr/local/opt/webp/lib/libsharpyuv.0.dylib \
  /usr/local/opt/webp/lib/libwebp.7.dylib

This effectively changes the @rpath-based install name to a full path; CPack does handle that.

tobbi · 2023-09-05T15:56:53Z

I'm confused: Why did this not surface before actions/runner-images#8078 emerged?

ZhongRuoyu · 2023-09-05T16:19:48Z

Why did this not surface before actions/runner-images#8078 emerged?

I made an incorrect claim in my earlier comment about the webp version in the GitHub Actions runner image; corrected that. The reason is that libSDL2_image was only rebulit recently in Homebrew/homebrew-core@7878f28. Before that, the reference to libsharpyuv was still present. Without brew update, the newest updates won't be fetched, so CI on master still works (for now, before the next runner image is put into use).

tobbi · 2023-09-05T19:53:08Z

I'm very confused as to where this leaves us.

ZhongRuoyu · 2023-09-06T03:15:35Z

All that the install_name_tool workaround does is (sort-of) "normalise" the install name to a path, just like the other transitive dependencies (see the libSDL2_image output above for example). This is not required in usual cases, and its purpose is only to help CPack find the (now-made transitive) dependency libsharpyuv of dependency libwebp, which CPack is not able to perform currently. Not much change otherwise, as far as I can tell. webp should still work; so should the SuperTux.app bundle. (One note is that the install names may need to be adjusted when the soversion of either libwebp or libsharpyuv is bumped, but that's rather uncommon.)

As for when this workaround can be removed: probably not until that CPack issue is resolved. One takeaway from the previous CPack failures (for which Homebrew temporarily disabled relocatable install names) is that CPack does not always handle these less-seen cases.

.github/workflows/macos.yml

Vankata453

Take this review with a grain of salt, since I do not understand MacOS compilation. However, seems like the workflow finishes, so that's good.

tobbi · 2023-09-24T16:34:02Z

Who really has any idea about Mac OS workflows. Hint: I do not

Vankata453 · 2023-09-24T16:34:50Z

To be fair, at least you've used MacOS and have compiled ST on it. I haven't. :D

mrkubax10 · 2023-09-24T18:35:35Z

Take this review with a grain of salt

Same, I used macOS (Hackintosh Yosemite actually) only once...

tobbi · 2023-09-25T14:55:17Z

@ZhongRuoyu Thanks for all your help!

ZhongRuoyu · 2023-09-25T15:06:32Z

No problem! Glad it worked for you again.

CMake's `fixup_bundle` does not recognise install names with `@rpath`, so it fails while bundling `libjxl_cms` as `libjxl`'s dependency. Make CMake happy by changing its install name to a full path instead. See also: SuperTux#2622 (comment)

CMake's `fixup_bundle` does not recognise install names with `@rpath`, so it fails while bundling `libjxl_cms` as `libjxl`'s dependency. Make CMake happy by changing its install name to a full path instead. See also: #2622 (comment)

MacOS workflow: Install harfbuzz normally again

280a1a2

See https://github.com/orgs/Homebrew/discussions/4735#discussioncomment-6902146

ZhongRuoyu reviewed Sep 4, 2023

View reviewed changes

Address #2622 (review)

d96a52f

ZhongRuoyu reviewed Sep 4, 2023

View reviewed changes

Address #2622 (review)

368dbba

mrkubax10 added os:macos status:needs-review Work needs to be reviewed by other people category:tests type:bugfix Pull Requests that fix bugs. labels Sep 4, 2023

tobbi marked this pull request as draft September 4, 2023 10:17

Address more comments

ba4422e

ZhongRuoyu reviewed Sep 23, 2023

View reviewed changes

.github/workflows/macos.yml Outdated Show resolved Hide resolved

tobbi added 2 commits September 23, 2023 20:56

Get rid of temporary harfbuzz fixes

8197230

Install harfbuzz

c6bf2c8

tobbi marked this pull request as ready for review September 23, 2023 20:18

tobbi requested review from Rusty-Box, mrkubax10 and Vankata453 and removed request for Rusty-Box September 23, 2023 20:28

Vankata453 approved these changes Sep 24, 2023

View reviewed changes

tobbi merged commit fda1823 into master Sep 24, 2023
32 checks passed

mrkubax10 removed the status:needs-review Work needs to be reviewed by other people label Oct 7, 2023

mrkubax10 deleted the harfbuzz-fix-again branch January 4, 2024 13:17

ZhongRuoyu mentioned this pull request Feb 13, 2024

Fix macOS CI #2779

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MacOS workflow: Install harfbuzz normally again #2622

MacOS workflow: Install harfbuzz normally again #2622

tobbi commented Sep 4, 2023

ZhongRuoyu left a comment

ZhongRuoyu left a comment •

edited

Loading

tobbi commented Sep 4, 2023

ZhongRuoyu commented Sep 4, 2023 •

edited

Loading

ZhongRuoyu commented Sep 4, 2023 •

edited

Loading

ZhongRuoyu commented Sep 4, 2023 •

edited

Loading

ZhongRuoyu commented Sep 4, 2023

ZhongRuoyu commented Sep 5, 2023 •

edited

Loading

tobbi commented Sep 5, 2023

ZhongRuoyu commented Sep 5, 2023 •

edited

Loading

tobbi commented Sep 5, 2023

ZhongRuoyu commented Sep 6, 2023 •

edited

Loading

Vankata453 left a comment

tobbi commented Sep 24, 2023 •

edited

Loading

Vankata453 commented Sep 24, 2023

mrkubax10 commented Sep 24, 2023 •

edited

Loading

tobbi commented Sep 25, 2023

ZhongRuoyu commented Sep 25, 2023

MacOS workflow: Install harfbuzz normally again #2622

MacOS workflow: Install harfbuzz normally again #2622

Conversation

tobbi commented Sep 4, 2023

ZhongRuoyu left a comment

Choose a reason for hiding this comment

ZhongRuoyu left a comment • edited Loading

Choose a reason for hiding this comment

tobbi commented Sep 4, 2023

ZhongRuoyu commented Sep 4, 2023 • edited Loading

ZhongRuoyu commented Sep 4, 2023 • edited Loading

ZhongRuoyu commented Sep 4, 2023 • edited Loading

ZhongRuoyu commented Sep 4, 2023

ZhongRuoyu commented Sep 5, 2023 • edited Loading

tobbi commented Sep 5, 2023

ZhongRuoyu commented Sep 5, 2023 • edited Loading

tobbi commented Sep 5, 2023

ZhongRuoyu commented Sep 6, 2023 • edited Loading

Vankata453 left a comment

Choose a reason for hiding this comment

tobbi commented Sep 24, 2023 • edited Loading

Vankata453 commented Sep 24, 2023

mrkubax10 commented Sep 24, 2023 • edited Loading

tobbi commented Sep 25, 2023

ZhongRuoyu commented Sep 25, 2023

ZhongRuoyu left a comment •

edited

Loading

ZhongRuoyu commented Sep 4, 2023 •

edited

Loading

ZhongRuoyu commented Sep 4, 2023 •

edited

Loading

ZhongRuoyu commented Sep 4, 2023 •

edited

Loading

ZhongRuoyu commented Sep 5, 2023 •

edited

Loading

ZhongRuoyu commented Sep 5, 2023 •

edited

Loading

ZhongRuoyu commented Sep 6, 2023 •

edited

Loading

tobbi commented Sep 24, 2023 •

edited

Loading

mrkubax10 commented Sep 24, 2023 •

edited

Loading