Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Features/apple m1 #144

Merged
merged 14 commits into from
Jan 8, 2023
Merged

Features/apple m1 #144

merged 14 commits into from
Jan 8, 2023

Conversation

Bluebugs
Copy link
Contributor

Description:

This make fyne-cross actually able to try to compile a Fyne application on Mac Arm silicon. This doesn't make it actually work due to a bug in qemu used by both docker and podman to emulate amd64 on arm64 architecture.

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

@Bluebugs Bluebugs requested a review from lucor November 18, 2022 21:57
Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just not sure about this...
By landing the PR you get docs that imply it should work and it won't... before this we don't indicate that - so it could be confusing for users...

internal/command/docker.go Outdated Show resolved Hide resolved
@lucor
Copy link
Member

lucor commented Nov 20, 2022

Nice work. Agreed the bug with podman need to be addressed...
Have you by any chance tested if this patch work with docker using the binfmt qemu emulator or has the same bug ?

@Bluebugs
Copy link
Contributor Author

Have you by any chance tested if this patch work with docker using the binfmt qemu emulator or has the same bug ?

Yes, docker and podman both use qemu and binfmt in the same way. So same bug :-(

@Bluebugs
Copy link
Contributor Author

I have tried building image for arm64 and it work. The only image which get weird are the Linux amd64 and Linux arm64. Amd64 on amd64 doesn't need to cross compile and arm64 on arm64 doesn't need to cross compile, but when running on the opposing architecture they do need to. So Linux amd64 and Linux arm64 are inherently different image which make things weird from a multi arch container point of view. Not to sure yet how to fix that.

@Bluebugs
Copy link
Contributor Author

I have not updated the Makefile that build the image to make them build multi arch as I think they will have to be different for podman and docker. Might be work for another PR.

@Bluebugs
Copy link
Contributor Author

I'm just not sure about this... By landing the PR you get docs that imply it should work and it won't... before this we don't indicate that - so it could be confusing for users...

If we can prepare multi architecture image, it will work for all fyne-cross target except linux-arm64 and linux-amd64 at the moment. We need to work on those image. Not to sure yet of the strategy, but we might be able to fix it by the release time of the next fyne-cross.

@andydotxyz
Copy link
Member

I'm a bit lost sorry, it sounds like a much bigger change than what I imagined from the PR title (a fix to the darwin image).

@andydotxyz andydotxyz closed this Nov 22, 2022
@andydotxyz
Copy link
Member

Didn't mean to close sorry...

@andydotxyz andydotxyz reopened this Nov 22, 2022
@Bluebugs
Copy link
Contributor Author

I'm a bit lost sorry, it sounds like a much bigger change than what I imagined from the PR title (a fix to the darwin image).

I guess there is to much update going on here. So support on Mac M1 was broken with fyne-cross as it is currently released. This PR does now work with locally made multi arch image directly from the Dockerfile provided in tree. There is only two exceptions that require additional work, the linux/amd64 and the linux/arm64 image which I think should be worked on a different PR.

@andydotxyz
Copy link
Member

But don't linux/amd64 and the linux/arm64 work before this PR?

@Bluebugs
Copy link
Contributor Author

Nothing worked before this PR on Mac M1. It may have worked at some point, but both podman and docker have had to use qemu or Apple has stopped providing rosetta ability to them somehow. And since that change, fyne-cross is not working on Mac arm silicon (and on other arm silicon).

@andydotxyz
Copy link
Member

I am confused more now, sorry.

it will work for all fyne-cross target except linux-arm64 and linux-amd64

Does this mean that those items don't work for all platforms calling it, or just those with a M1 host?
My concern for the regression is that it sounded like those targets were now broken on (non-darwin) hosts where it worked before...

@Bluebugs
Copy link
Contributor Author

This is just impacting arm host. So mostly M1 developer are impacted, but if you do have a ARM laptop it would be impacted too (like the pinebook for example).

@andydotxyz
Copy link
Member

This is just impacting arm host. So mostly M1 developer are impacted, but if you do have a ARM laptop it would be impacted too (like the pinebook for example).

I can't quite parse. Is this "only darwin arm" or "any arm" hosts?
If it is the latter then it's a regression surely?

@Bluebugs
Copy link
Contributor Author

I can't quite parse. Is this "only darwin arm" or "any arm" hosts? If it is the latter then it's a regression surely?

We did not support, as far as I know, using arm as host and the only one that seems to have worked at some point in time was Mac M1, but that one as surely stopped. When I say host, I mean the host calling fyne-cross to do the build.

@andydotxyz
Copy link
Member

I can't quite parse. Is this "only darwin arm" or "any arm" hosts? If it is the latter then it's a regression surely?

We did not support, as far as I know, using arm as host and the only one that seems to have worked at some point in time was Mac M1, but that one as surely stopped. When I say host, I mean the host calling fyne-cross to do the build.

@lucor do you know what was supported before this time? I admit I am out of my depth here.

@lucor
Copy link
Member

lucor commented Nov 23, 2022

To be honest, I don't recall. Probably it is a use case we have not fully tested.
By sure we were able to build macOS app on arm64 host, targeting amd64 and arm64 since on macOS fyne-cross fallback to the native build via Fyne CLI when building from darwin (docker is not involved)

Anyway, we should start supporting multiarch docker images. At least for amd64 and arm64.

I'll try to test in parallel if the new images based on zig could help making them easier to build/maintain.

In the meantime this PR could help allowing to run arm64 images on amd64 architectures via qemu.

@Bluebugs do you have any reference to the qemu issue that is blocking this PR ?

@Bluebugs
Copy link
Contributor Author

This PR is build on the idea that we will release arm64 image for next major release of fyne-cross as the support for emulating amd64 on arm64 is currently broken due to what seems to be a bug in qemu as it impact both docker and podman ( docker/for-mac#5123 (comment) ). So qemu is not blocking this PR.

If we want to go with qemu and not release arm64 image, I will need to remove the if block at line 115 in docker.go from this PR. But it is to be noted, that all the image I tried trigger a segv in qemu due to the bug above, so I am not eager to follow this path.

Most docker image we have can already be build for arm64, the exception being linux/amd64 and linux/arm64 (maybe also android). The issue with those two is that they assume no cross compilation for one and cross compilation for the other. I feel like it is likely that a zig image would solve all of that in one swoop.

@lucor fyne does have access to a M1 remotely and I could setup docker on it if that would be interesting to you.

@lucor
Copy link
Member

lucor commented Nov 23, 2022

This PR is build on the idea that we will release arm64 image for next major release of fyne-cross as the support for emulating amd64 on arm64 is currently broken due to what seems to be a bug in qemu as it impact both docker and podman ( docker/for-mac#5123 (comment) ). So qemu is not blocking this PR.

If we want to go with qemu and not release arm64 image, I will need to remove the if block at line 115 in docker.go from this PR. But it is to be noted, that all the image I tried trigger a segv in qemu due to the bug above, so I am not eager to follow this path.

I see, sorry I misread the PR. I was thinking it was a patch to work with qemu not the multiarch ones...

Most docker image we have can already be build for arm64, the exception being linux/amd64 and linux/arm64 (maybe also android). The issue with those two is that they assume no cross compilation for one and cross compilation for the other. I feel like it is likely that a zig image would solve all of that in one swoop.

Yes, I know. We need to figure out that. Hoping zig image would solve.

@lucor fyne does have access to a M1 remotely and I could setup docker on it if that would be interesting to you.

Thanks :-)

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I understand this now.
Is @lucor happy too?

@Bluebugs Bluebugs merged commit 47d4040 into develop Jan 8, 2023
@Bluebugs Bluebugs deleted the features/apple-m1 branch January 8, 2023 18:47
@andydotxyz
Copy link
Member

Sorry @lucor i think we didn't wait for your final thoughts on this, are you happy as-is?

@lucor
Copy link
Member

lucor commented Jan 9, 2023

No worries, LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants