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

guix: drop 32-bit android target #9528

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

tobtoht
Copy link
Collaborator

@tobtoht tobtoht commented Oct 22, 2024

According to @nahuhh these binaries haven't worked in years (see also: #8889 (comment)).

Android has mostly moved on to 64-bit: https://android.stackexchange.com/questions/253596

In a 2020 blog post ARM stated that "nearly 90 percent of today’s Android devices deploy a 64-bit capable version of the OS."

@MrCyjaneK
Copy link

@nahuhh / @tobtoht maybe those binaries didn't work, but wallet_api target is what is important for mobile wallets, even if that stats are accurate that's still 1 in 10 people who wouldn't be able to run monero wallet on their phone, at least not one based on the official code.

I'm strongly against removing support for something that is still supported by upstream (android still builds fine for 32bit arm, together with it still being supported target in the NDK), and something that is still being used.

@tobtoht tobtoht force-pushed the build_drop_32_arm_android branch from a057162 to ff0bb0f Compare October 23, 2024 13:51
@tobtoht tobtoht changed the title build: drop support for 32-bit android target release: drop 32-bit android target Oct 23, 2024
@tobtoht
Copy link
Collaborator Author

tobtoht commented Oct 23, 2024

@MrCyjaneK I reduced the scope to official releases.

Do you happen to have 32-bit ARM Android hardware? If so, could you test if the monerod release binary works?

@MrCyjaneK
Copy link

@tobtoht missed this comment, I can test it in a couple weeks when I'll be back at home, I do have one device. However I don't really see this as a required test as armv6 devices are rather low-end and wouldn't contribute much to the network anyway.. what I can confirm though is that wallet_api is working properly on armv6 devices.

@tobtoht
Copy link
Collaborator Author

tobtoht commented Nov 4, 2024

@MrCyjaneK Just to clarify, the intent of this PR is not to remove 32-bit android build support for wallet_api, that was a hasty mistake as you rightly pointed out. Rather, I'm questioning whether we should continue to release official binaries for this target. Shipping broken binaries is pointless, and fixing them might be a waste of time as the hardware fades into obscurity.

@tobtoht tobtoht force-pushed the build_drop_32_arm_android branch from ff0bb0f to 97a2043 Compare January 7, 2025 19:16
@tobtoht tobtoht changed the title release: drop 32-bit android target guix: drop 32-bit android target Jan 7, 2025
@tobtoht tobtoht force-pushed the build_drop_32_arm_android branch from 97a2043 to 2fcaf95 Compare January 7, 2025 19:26
@MrCyjaneK
Copy link

What I'm afraid of is that when this get merged then maintaining arm32 builds will become solely wallet developer responsibility, as it is currently with iOS.

Google currently supports arm32, and there are still last gen devices that are 32 bit only. If we run the CI anyway then I would prefer to keep it here. Especially since I can see that contrib/depends is being replaced with guix, removing that check will be a major pain in future when downstream will be forced to migrate over to a build system that doesn't guarantee the support of all non-EOL targets.

As for binary releases, sure, drop them I couldn't care less, but I am strongly against removing the check from CI (it's not our compute anyway), and if it causes problems this is a monero issue and not 32bit android issue, because that thing is still supported by NDK, and there are still 2 year old devices with 32bit arm CPU.

@tobtoht
Copy link
Collaborator Author

tobtoht commented Jan 9, 2025

Guix is not replacing depends. Guix simply provides a bootstrappable container environment in which we run depends. :)

This PR would only remove support for official builds. We can keep running arm32 depends builds in CI, so we make sure that we don't break support for downstream projects.

@MrCyjaneK
Copy link

Oh okay, that makes much more sense now.

@tobtoht tobtoht merged commit 90ca752 into monero-project:master Jan 14, 2025
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants