search

android / ndk

The Android Native Development Kit
1.99k stars 257 forks source link

remove explicit API guards from bionic #2058

Closed DanAlbert closed 2 months ago

DanAlbert commented 2 months ago

Description

WEAK_API_DEFS doesn't work for anything in libc because we've got a tool that adds explicit API guards wherever there's an __INTRODUCED_IN (but only in bionic). That was a workaround we needed back when we supported GCC and could rely on __attribute__((availability)) everywhere, but that's long gone.

https://r.android.com/3236544 removes that. Can see from the one explicit guard I put back that there is potential for source compat break here. I'm planning to add explicit guards back wherever they're need to keep from breaking common 3p libraries, but there's no way to find those proactively.

The extremely conservative approach that wouldn't risk any source compat breaks but would still let us remove the tool that's supporting this (so still reduces our maintenance costs, but unfortunately would give up the advantage of making WEAK_API_DEFS work everywhere) would be to check in the preprocessed headers with the explicit guards. That can be the last resort, but it'd be much better if WEAK_API_DEFS can also benefit from this.

I'm feeling ambitious today so aiming for r28, but if the change doesn't land in time for beta 1 it'll get pushed to r29.

licy183 commented 2 months ago

Should API guards be added to some structs?

gnulib checks whether the struct timezone_t is declared and determines whether to use mktime_z and its friends provided by GNU. If API Level < 35, NDK declares the timezone_t but doesn't declare mktime_z, and compilation failure will occur.

enh-google commented 2 months ago

Should API guards be added to some structs?

we'll probably have to do some, yes.

gnulib checks whether the struct timezone_t is declared and determines whether to use mktime_z and its friends provided by GNU. If API Level < 35, NDK declares the timezone_t but doesn't declare mktime_z, and compilation failure will occur.

if you work on gnulib, "please fix your configure file to check for the functions you want, rather than making potentially invalid assumptions like this" ... if you don't, yeah, this is the kind of thing we'll want to work around in the short term (but probably still file bugs with the libraries that have broken assumptions like this).

DanAlbert commented 2 months ago

Yes, please fix those problems if you're able. If we hide structs or constants in the NDK it's a hack to keep those projects building that prevents other projects from using those types.