search

graalvm / mandrel-packaging

6 stars 8 forks source link

Test mac-os builds both on Intel and Apple M #415

Closed zakkak closed 3 months ago

zakkak commented 3 months ago

macos-13 is the latest intel-based runner, see https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories

jerboaa commented 3 months ago

Looks like we need something like https://github.com/graalvm/mandrel/pull/735 as well.

zakkak commented 3 months ago

Looks like we need something like graalvm/mandrel#735 as well.

Done in https://github.com/graalvm/mandrel-packaging/pull/416

Karm commented 3 months ago

@zakkak I had to rename lib/static/darwin-arm64 to lib/static/darwin-aarch64 too :man_shrugging:

working with 23.1 branch

zakkak commented 3 months ago

@zakkak I had to rename lib/static/darwin-arm64 to lib/static/darwin-aarch64 too 🤷‍♂️

working with 23.1 branch

That sounds like an upstream bug, shouldn't upstream use the same naming as OpenJDK?

Also any idea why the smoke tests seem to work without this renaming in this case?

Karm commented 3 months ago

@zakkak

@zakkak I had to rename lib/static/darwin-arm64 to lib/static/darwin-aarch64 too 🤷‍♂️ working with 23.1 branch

That sounds like an upstream bug, shouldn't upstream use the same naming as OpenJDK?

I have seen these in the wild, JNI Java libs, projects, JDK, GraalVM, uname, arch etc.:

osx-aarch64
osx-arm
osx-arm64
mac-aarch64
mac-arm
mac-arm64
macos-aarch64
macos-arm
macos-arm64
darwin-aarch64
darwin-arm
darwin-arm64

The ISA is apparently interchangeably called both Aarch64 and ARM64. It seems that MacOS pretty consistently reports "arm64" though. Both via file command examining Mach-0 executables and with uname and arch. The kernel is called "Darwin" in uname, but $(sw_vers -productName) tells you "macOS".

Temurin 21 uses "lib/static/darwin-arm64", although it names its tarballs "aarch64_mac". GraalVM CE uses "lib/static/darwin-aarch64" and they call their tarballs "macos-aarch64".

Also any idea why the smoke tests seem to work without this renaming in this case?

My hunch is that this naming mayhem is so prevalent a problem that the GHA runners have some kind of aliasing in place to mitigate failing on such trivial naming steps? When I am done with the baremetal ones, I'll take a look.

Karm commented 3 months ago

@jerboaa @zakkak What we need to agree on is whether we copy what Temurin does, both in naming the lib dir and in naming the tarballs, or whether we copy what GraalVM CE does.

The one who suggests we solve this by creating a third naming scheme wins a t-shirt :)

zakkak commented 3 months ago

@jerboaa @zakkak What we need to agree on is whether we copy what Temurin does, both in naming the lib dir and in naming the tarballs, or whether we copy what GraalVM CE does.

The one who suggests we solve this by creating a third naming scheme wins a t-shirt :)

I would love to see the print on that T-shirt but I will be logical and not suggest a third naming scheme :)

Although I would like to go with the Temurin naming scheme I am afraid the easiest path is to go with the GraalVM CE naming scheme as I expect mx to expect/generate the names and I don't think it's worth the effort to try and detect where and how that happens. Furthermore, less deviation from upstream seems better to me in general (we could also ask upstream for the reasoning behind the different naming).

zakkak commented 3 months ago

Also any idea why the smoke tests seem to work without this renaming in this case?

It turns out it's still building for x86_64

C compiler: cc (apple, x86_64, 15.0.0)

https://github.com/graalvm/mandrel-packaging/actions/runs/9256122608/job/25461451554?pr=415#step:9:520

Switching to draft... I need to change more things (like the openjdk we download) and make sure rosetta is not taking over and emulating things.

Karm commented 3 months ago

@zakkak Just to confirm, the build I tried was on a system without Rosetta, so it works, but for the naming....

zakkak commented 3 months ago

@zakkak Just to confirm, the build I tried was on a system without Rosetta, so it works, but for the naming....

Yes, I fixed it in the github action now as well, but it looks like this renaming needs to be done by build.java instead. I'll give it some more thought and try to understand why GraalVM CE uses a different naming scheme.

zakkak commented 3 months ago

That sounds like an upstream bug, shouldn't upstream use the same naming as OpenJDK?

So it looks like a Temurin issue after all. I opened https://github.com/adoptium/temurin-build/pull/3827 to fix this.