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

determine what can be done about the polyfills that were re-hidden in r28 #2102

Open
DanAlbert opened this issue Nov 11, 2024 · 9 comments
Open

Comments

@DanAlbert
Copy link
Member

Description

#2081 reported issues with the blanket exposure of all the stuff previously hidden by versioner. We've rehidden those things behind __ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__, but it'd be nice to be able to re-expose the polyfills even outside that mode since those should make things better for everyone. At least one library (libconfuse) had issues with that though. Someone will need to go and figure out why so we can determine if adding unconditional polyfills is something we can pursue or not.

@enh-google
Copy link
Collaborator

Someone will need to go and figure out why so we can determine if adding unconditional polyfills is something we can pursue or not.

this makes it sound like having polyfills is a new thing, but it isn't --- the only new part was "should we try harder to polyfill?". the new thing here is really "we tried to build a bunch of random stuff that we didn't historically".

i'd also point out that ripping off bandaids like this is best done sooner rather than later. our historical mistake here has been waiting until we know there are a lot of projects that want the polyfill, which is then by definition "too late". the existence of exactly one [known] instance is in some ways ideal: (a) proof that it's useful but (b) not a serious obstacle to causing some localized pain for the good of everyone else.

(speaking of bandaids, if we have any types other than pthread_barrier_t that are currently guarded by #ifs, maybe we should fix all of them at once too? i have no idea how to search for that though :-( )

determine what can be done about the polyfills that were re-hidden in r28

polyfills plural? it was only reallocarray(), no?

@DanAlbert
Copy link
Member Author

the new thing here is really "we tried to build a bunch of random stuff that we didn't historically".

The new part is "and it broke". We haven't been building those things, but other people have.

i'd also point out that ripping off bandaids like this is best done sooner rather than later.

This isn't a band-aid that needs to be removed though. The polyfill is only hidden in the "prefer compatibility with stuff that doesn't do things the Android way" mode.

@enh-google
Copy link
Collaborator

The new part is "and it broke". We haven't been building those things, but other people have.

it seems highly unlikely that none of our existing polyfills ever broke anything.

This isn't a band-aid that needs to be removed though. The polyfill is only hidden in the "prefer compatibility with stuff that doesn't do things the Android way" mode.

yeah, that's an interesting argument ... i don't know whether i'd imagined a future of "Android code uses weak symbols, with the traditional option mainly being for autoconf stuff", but it's maybe not the worst idea...

@DanAlbert
Copy link
Member Author

The new part is "and it broke". We haven't been building those things, but other people have.

it seems highly unlikely that none of our existing polyfills ever broke anything.

At least within the scope of stuff in vcpkg, I think that's what the evidence says though, unless @dg0yt has been fixing all the others that vcpkg ran into and we just weren't hearing about them. Beyond that there's no way to know, of course, but I don't think the probable existence of problems we don't know about is an argument against fixing the ones we're certain of?

This isn't a band-aid that needs to be removed though. The polyfill is only hidden in the "prefer compatibility with stuff that doesn't do things the Android way" mode.

yeah, that's an interesting argument ... i don't know whether i'd imagined a future of "Android code uses weak symbols, with the traditional option mainly being for autoconf stuff", but it's maybe not the worst idea...

That's exactly the advice I've been giving people, and the approach I've been taking in the samples. The docs don't say it quite so explicitly, but it's definitely alluded to: https://developer.android.com/ndk/guides/using-newer-apis#why_isnt_this_the_default. I can't imagine why anyone actively writing Android code would use the other mode, aside from never needing any APIs beyond their minSdkVersion so they just haven't gotten around to flipping the switch yet.

@enh-google
Copy link
Collaborator

Beyond that there's no way to know, of course, but I don't think the probable existence of problems we don't know about is an argument against fixing the ones we're certain of?

i genuinely don't know --- this is definitely a "one man's problem is another man's feature" case, after all :-(

That's exactly the advice I've been giving people, and the approach I've been taking in the samples. The docs don't say it quite so explicitly, but it's definitely alluded to: https://developer.android.com/ndk/guides/using-newer-apis#why_isnt_this_the_default. I can't imagine why anyone actively writing Android code would use the other mode, aside from never needing any APIs beyond their minSdkVersion so they just haven't gotten around to flipping the switch yet.

yeah, maybe the "fix" here is to just make that more explicit (and for future polyfills to be predicated on weak symbols by default)...

@enh-google
Copy link
Collaborator

(though it would still be interesting to know why autoconf gets this wrong. it seems like fixing that would make the world a better place, even if it takes a decade for everyone to update their autoconf.)

@dg0yt
Copy link

dg0yt commented Nov 14, 2024

AFAICT the focus shouldn't be on autoconf but on gnulib, the actual source of polyfill widely used in autoconf-based packages. As mentioned in the other issue, gnulib is not a standalone library, but vendored into packages. So some things could be adressed with changes in gnulib (renaming their replacements). But it may take a while for it being available in releases of relevant packages.

@DanAlbert
Copy link
Member Author

It's probably both, but I think you're right that gnulib is a big part of it. That's also the one where I'm not sure there are going to be simple upstream fixes.

@enh-google
Copy link
Collaborator

oh, i'd just assumed that gnulib used autoconf, but it seems you're right ... it doesn't look like it does :-(

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

No branches or pull requests

3 participants