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

minizip: fix building for Android API level < 24 #26260

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kambala-decapitator
Copy link
Contributor

Summary

Changes to recipe: minizip/all

Motivation

Fixes building for Android API level < 24 again.

Details

An improvement over #15532. Not sure how/why it worked with Conan v1 previously, but now another adjustment is needed (maybe something changed in NDK, was testing with r27c now).

I've also opened PR in upstream: madler/zlib#1025, but Conan patch includes only one change from it as otherwise it'd require more patches (that line has changed between versions), the other one is already handled by the injected IOAPI_NO_64 macro.

Also moved applying patches to source().


@jcar87
Copy link
Contributor

jcar87 commented Dec 30, 2024

Thanks!

I can see multiple PRs that aim to target older Android API levels - what's the rationale for this? we're unlikely to support this unless there's a strong reason. As far as I can see:

  • API level 24 was introduced in Android 7.0 released in the second half of 2016 - https://developer.android.com/tools/releases/platforms#7.0. It's unlikely that any Android devine (at least, smartphone) released before that is still supported by the manufacturer
  • The google play store does not accept anything lower than API level 34 for new apps
  • The current version of Android (Android 15) will not allow installing apks that target APIs 24 or older

We're trying to understand what the motivation is - as this imposes a higher maintenance burden on us, which has an impact on the entire community - and while we are appreciative of teams still target very old versions, it may not be worth for the Conan Center maintainers and the community to be burdened with this, unless it's something that happens to a wider user base

@kambala-decapitator
Copy link
Contributor Author

kambala-decapitator commented Jan 2, 2025

Thank you for the comment, @jcar87

I can see multiple PRs that aim to target older Android API levels - what's the rationale for this?

our project uses Conan to manage dependencies and supports Android 21+. Currently I'm in the process of migrating to v2, so I'm fixing anything I find in the recipes that prevents me from building packages.

It's unlikely that any Android devine (at least, smartphone) released before that is still supported by the manufacturer

it's true. But we're trying to make the project accessible to the widest audience possible.

The google play store does not accept anything lower than API level 34 for new apps

sorry, seems you're confusing SDK version (max version of APIs that can be used, SDK that you build your code against; Google docs call it target API level) and API level (minimum OS version that can run your code). In Apple world it's similar: you build against a modern SDK (say, macOS 15), but your app runs on deployment target (minimum OS version) you specify (currently the earliest possible with the latest Xcode is macOS 10.13).

Btw Conan doesn't model SDK version.

The current version of Android (Android 15) will not allow installing apks that target APIs 24 or older

I guess you're talking about this. But it's also about target API level, i.e. SDK version that you build against, just like I explained above.

Until now the only restriction on the minimum API level (besides what NDK states) was Google disabling its Google Services for old devices, e.g. recently they've dropped the services for API level < 21. And if you don't use them, you can keep lower API level if you need it, e.g. 19 or 16.

this imposes a higher maintenance burden on us

what kind of maintenance do you mean exactly? AFAIK CCI doesn't build Android packages (just like Apple non-macOS), so its support is entirely on the community.

@jcar87
Copy link
Contributor

jcar87 commented Jan 2, 2025

what kind of maintenance do you mean exactly? AFAIK CCI doesn't build Android packages (just like Apple non-macOS), so its support is entirely on the community.

By resources I mean:

  • CI resources: any PR to change recipes will still be validated on the main platforms (Windows, Linux, Mac) - regardless of whether the changes affect those platforms or not
  • Reviewer resources: we still need to validate these changes work
  • Ongoing maintenance: it means the Conan team bears the support if anything doesn't work

It's unlikely that any Android devine (at least, smartphone) released before that is still supported by the manufacturer

it's true. But we're trying to make the project accessible to the widest audience possible.

yes - but this being too old an Android version makes us question whether in that wide audience there's actual uses. We don't want to support anything that is very unlikely to be in use - or that shouldn't be used according to Android's own guidelines

Assuming this is correct: https://composables.com/android-distribution-chart
Android 7.0 would be at 1% - also need to keep in mind that Android 7.0, wherever it is used - is no longer receiving any support or security updates and is at risk. So we're just trying to find an answer to the question: should Conan Center support Android 7 in 2025, and if so: why?

@kambala-decapitator
Copy link
Contributor Author

@jcar87 thank you for the response!

I can't insist on merging PRs about old Android versions of course, but since they require only a few lines to change, in my view it's affordable. Fortunately, most of the changes can be done on profile level as well, so it's still solvable even without having the changes in the recipes.

If in the end you're going to reject this PR, you could also close #26240 and require dropping the same adjustment in #26245.

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.

2 participants