rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.77k stars 12.5k forks source link

Android NDK r25b changes will break developers using r22b or older #103673

Closed alex-pinkus closed 1 year ago

alex-pinkus commented 1 year ago

Discussion continued from #102332.

Background

Changes that were merged in #102332, slated for 1.66.0, will update the CI build scripts so that all Android targets[^1] use the r25b release[^2] of the Android NDK to build Rust binaries, moving from r15c. The r25b release is the newest version of the NDK at time of writing (Oct 2022), and is a Long Term Support release (LTS)[^3]. r15c predates LTS designation, is over five years old, and is past the end of its support life. Rust's Android platform support document[^4], merged on 24 September 2022, indicates support for the most recent LTS release of the NDK, so the changes in #102332 are in line with that document.

Currently, Rust developers who wish to use an android-ndk toolchain newer than r22b must use build-time hacks to do so. This is because the Rust standard library uses libgcc for its unwinder implementation on Android, but libgcc is not included in new versions of the NDK. When using r23b or newer in a Rust library, a developer will see ld: error: unable to find library -lgcc. #85806 added logic to select between libgcc or libunwind, but this logic runs when building Rust itself, not when compiling the downstream application, so it is mainly useful with the nightly flag -Z build-std. Since older versions of the NDK do not have libunwind, and newer versions do not have libgcc, support for r23 (and newer) is mutually exclusive from support for r22 (and older), unless a developer applies a manual workaround[^5]. This workaround, for building r25b, with a binary release that uses libgcc[^6], involves a fake libgcc.a containing INPUT(-lunwind). One common form of this workaround^7 is used in the ring crate and referenced from setup scripts for flutter_rust_bridge; a slightly modified form can be found in cargo-apk.

Problem

When the updated CI script lands in a stable release, the Rust binary releases for Android targets will compute has_unwind=true, and link against libunwind instead of libgcc, causing every project that builds using NDK version r22b or older to fail with the message error: cannot find -lunwind. Because building with r23b or newer requires either nightly Rust or a build-script workaround, it is likely that most Android projects using Rust today are using r22b or older and will therefore be broken by this change. One comment notes that Firefox will be broken[^8]; a quick GitHub search also uncovers libraries like openssl-src among others that build using older NDK versions on CI.

Depending on the version that a project must be upgraded from, going to r23b can be a non-trivial effort. The process for obtaining a toolchain changed in r19[^9], so a developer must rewrite their CI compilation scripts. One commenter noted seeing compilation issues in a codebase containing C++ headers[^10], although they were able to resolve the issues. These are not reasons to never upgrade, but they indicate that a surprise NDK upgrade may be unwelcome work requiring more than just a version number bump.

Options

Currently, an undetermined (but probably large) subset of the Android Rust ecosystem will find itself broken on December 15th, when Rust version 1.66.0 comes out. What should be done about it? Some ideas, in no particular order:

1. Roll back to version r15c by reverting the change

Reverting the change is the simplest option, and buys an indefinite amount of time to define a more graceful deprecation strategy. Doing so, however, prolongs the usage of a build tool that's well past end of life, and means that any features introduced in the last 5 years aren't available to Rust developers.

2. Downgrade to a newer toolchain that is older than r23

With a net-new change to use r21e (newest compatible previous LTS version, released January 2021) or r22b (released March 2021)[^11], the binaries produced by CI could take advantage of a significantly newer toolchain and retain compatibility with older toolchains downstream. A Rust binary compiled using r22b can be linked into an application using a toolchain as old as r10e (and likely older, but I didn't have an older NDK at hand).

However, both r21e and r22b are still considered obsolete and unsupported. Upgrading to one of these versions means that breakage will still occur later, when we do decide to upgrade.

3. Warn users about breakage over a longer time period

On the r25b pull request, @jonhoo mentioned that a blog post would help alert users to the change. @jonhoo compared this to the change in minimum linux-gnu versions that happened in August^12 and the accompanying announcement[^13]. That change was pre-warned over several releases; in order to take a similar action here, the r25b PR would need to be reverted. While this impacts just the Android targets, rather than tier-1 linux targets as the glibc bump did, the impact on those developers is much more severe. By raising awareness of the issue and defining a timeline for the change, the blog post would help to reduce impact and limit unpleasant surprises.

4. Make no change, but still warn users about the looming breakage

Even if no action is taken to delay this change, a blog post will be a prudent way to alert developers that the change is happening in six weeks. If it is safe to recommend the INPUT(-lunwind) workaround for broad use, the blog post could recommend immediate migration to r23 or newer before the change hits stable.


Conclusion

As time marches on, developer tools evolve, and long-term support versions fade into unsupported obsolescence. It's natural and expected for Rust to drop support for older releases of the Android NDK, as with any other toolchain or environment that has passed end of life.

However, the current implementation will require a big-bang migration to r23+ for any project wishing to adopt 1.66.0. Some developers may have adopted r23, if they are attentive to Rust community workarounds, or brave enough to run their CI only on nightly. It's likely that many have not. In the interest of preserving existing project compatibility, it may be prudent to attempt a graceful migration to libunwind wherever possible, which can be done after still upgrading to a version that's newer than the current one. By giving developers ample warning that allows them to adopt today's workaround, the Rust project can keep the ecosystem current without causing sudden surprises.

[^1]: Android targets: arm-linux-androideabi, armv7-linux-androideabi, thumbv7neon-linux-androideabi-ndk, i686-linux-android-ndk, aarch64-linux-android, x86_64-linux-android [^2]: Android NDK r25 revision history. [^3]: NDK release process, detailing the implications of LTS releases. [^4]: Platform support for *-linux-android and *-linux-androideabi and PR: #101780 [^5]: libunwind availability in the Android NDK: Screen Shot 2022-10-27 at 9 18 15 PM [^6]: Apparent first attestation of the INPUT(-lunwind) workaround

[^8]: Comment from @glandium indicating Firefox breakage [^9]: Standalone toolchain (deprecated) documentation [^10]: Comment reporting compilation errors caused by the upgrade [^11]: In order to use r22b, the logic in library/unwind/build.rs will also need to prefer libgcc over libunwind, since r22b includes libunwind.a within the C++ STL in arm-linux-androideabi only, for reasons that are not clear.

[^13]: Increasing the glibc and Linux kernel requirements

Mark-Simulacrum commented 1 year ago

Nominating for compiler discussion.

I would probably be somewhat in favor of fully rolling back the bump on soon to be beta 1.66 (we branch today), and landing a bump (edit: on master) only to r21/r22 so we retain compatibility for slightly longer.

We should in parallel issue a blog post with our strategy for this bump, and ideally a strategy for future bumps. It looks like Android's LTS releases are only supported until the next one is issued (once a year roughly) which means there's not really an overlap period in which we can stick with a supported copy until user's migrate. This situation definitely reminds me of our similar challenges on BSDs.

tmandry commented 1 year ago

I see a couple issues here:

Difficulty in moving between ndk versions

I think we should strive to match the experience of the ndk itself here. In particular if there is no ndk release with both libgcc and libunwind, the Rust project shouldn't go out of its way to ease the upgrade experience beyond giving people enough warning. (If someone wants to contribute such a soft transition path and it's not a lot of maintenance overhead, that's fine.)

Still, matching the ndk experience requires being in line with the expected upgrade cadence of ndk releases. I think this is ultimately up to the Android target maintainers.

Tying rustup releases to ndk and target API levels

It doesn't seem ideal to have your version of Rust coupled to your target API level, especially on a frequently updating platform such as Android. (Unless you can mix code targeting different API levels in the same binary, in which case it might be fine?) I expect to see similar issues with Fuchsia, where the goal is to update the platform even more frequently.

Thinking out loud a bit, this makes me wonder if we should just point people toward -Z build-std and stabilize the functionality in some capacity. This gives people maximum flexibility to mix and match ndk versions, set target API levels, and manage upgrade paths on their own. On my machine it's not that much overhead (+~17s to initial build time), but I would like to be conscious of the impact to people who are building on slower laptops.

alex-pinkus commented 1 year ago

On point 1: Totally agree that the Rust project isn't the right place to ease the android-ndk upgrade experience in general. Is there clear direction somewhere on what "enough warning" means? With the glibc version bump, the blog post was published almost two months in advance.

On point 2: In the general case, at least, rustup does fairly well with staying uncoupled from ndk versions. As I mentioned, a distribution of rust that uses ndk r22b will build fine against an application using as old as r10e, and possibly older. This weird situation is more a problem of the android-ndk than it is a problem of anything Rust has done. (As a side note, with target API levels, I understand the story to be a bit more complex, with the current min of android-14 changing to android-19 with this version bump, which restricts the possible targets the application can declare, but users can declare a lower min or target if their ndk supports it, even if that causes issues for the stdlib).

Rust is fairly unique here because, if you combine points 1 and 2, every project that uses Rust has a dependency by definition that will break them when it upgrades. C++ doesn't have this problem because their stdlib gets distributed as part of the ndk. Theoretically, a random C++ library might also have this problem if they link against libgcc, but no one library is going to have the kind of reach across C++ that the Rust stdlib has across Rust. It's also not uncommon in my experience for Android C++ projects to download their dependencies from source anyway (fresco is a good example) and build them directly.

Pointing people toward a stabilized version of -Z build-std definitely seems like a preferred solution here. Initial builds are going to have other far greater bottlenecks, and as an added bonus, Android developers are often also binary-size-conscious which build-std would help with. It's not clear to me whether you're suggesting that as a prerequisite for the ndk-bump or just as a useful tool in the future. Is build-std far enough along that some form of it could be stabilized in the right timeframe?

apiraino commented 1 year ago

I think there is enough meat here to fire up an MCP FCP. Similar situation were handled that way. An MCP was suggested in the first place by @jyn514 in this comment.

Anyway, this will be discussed tomorrow. Between the previous discussion on Zulip and this issue comments, I think there is already great context provided.

alex-pinkus commented 1 year ago

@apiraino, just wondering, did the aforementioned discussion take place (sounds like it would have been on Nov 3)? Any outcomes to share?

apiraino commented 1 year ago

@alex-pinkus correct - the team wanted to discuss this issue but unfortunately time ran out. This week (on Thu, 10th) the topic will be again on the table and hopefully it will be discussed (and a comment posted here for visibility).

chriswailes commented 1 year ago

An announcement sounds good to me, even if that means delaying the update for a bit. However, I feel that updating to a different unsupported NDK would be a step sideways at best. This is mainly because it would buy us very little (as far as I'm aware) and it would cause churn for developers already using custom NDK setups.

Building against an older NDK leaves us with two (related) issues though:

  1. Because the NDK is no longer supported the Rust community needs to paper over any issues
  2. We're unable to test against NDK betas, file patches against the NDK, or request point releases for breaking issues

I'm also concerned about burdening new developers who'd like to use modern NDKs for their projects but are required to take extra steps to do so... but I can't really quantify that in any meaningful way so I guess i'll just mention it.

Going forward I think it'd be ideal to integrate the NDK betas into the nightly/beta rustc builds. This would allow developers time to prepare for the changes, especially if a blog post is timed with the release containing the updated NDK.

As for supporting multiple APIs in the same binary, I've just finished a rough draft of an RFC I hope to PR next week for API version tests in cfg. This should allow for the mixing of code targeting different API levels to live in the same codebase.

apiraino commented 1 year ago

visited during T-compiler meeting on Zulip. Two the main points:

@rustbot label -I-compiler-nominated

chriswailes commented 1 year ago

To clarify, how hard is it going to be for developers to update their Rust projects to the new NDK if they are using something newer than r15 but older than r25b? For Android's Rust toolchain I've updated the NDK twice (23->24->25) and haven't had an issue with any of the transitions (no changes needed to build system or source).

chriswailes commented 1 year ago

Sorry to keep popping in here, but do we know why Firefox is stuck on an older version of the NDK? Is it for a reason beyond Rust's dependency on an older version? If Firefox is using an out-of-support NDK for other reasons I think that may be their bigger issue and I'm not sure that's a good reason to holding back on updating the official Rust builds. If, however, Firefox is having to use an older NDK to support using Rust this seems like this would fix the issue for them.

alex-pinkus commented 1 year ago

With the NDK updates you're referring to for Android's Rust toolchain, you were building in-tree, right? As in, you don't use a prebuilt rust stdlib for the target, you produce one as part of the AOSP build?

I don't have visibility into the firefox project and their reasons, but with the rustup-vended stdlib linking against libgcc, I suspect that a lot of projects are "stuck". While we'll fix that when we complete this upgrade, we'll break all of those projects all at once to do so. The upgrade won't be hard (at worst, it's removing any make_standalone_toolchain invocations, rewriting paths, and fixing the kinds of rare compilation issues that have been reported), but that won't stop it from being disruptive.

chriswailes commented 1 year ago

Yes, we build the stdlib as part of our Rust toolchain distribution. This doesn't occur during the AOSP build itself, we just do a "release" following upstream releases. I can see how the situation you're describing is different.

I 100% agree that we should notify developers about this issue, even if that means delaying the update for a bit. I just wanted to understand the concerns about developer experience a bit more concretely. Thanks for the information.

complexspaces commented 1 year ago

To clarify, how hard is it going to be for developers to update their Rust projects to the new NDK if they are using something newer than r15 but older than r25b?

I saw this issue and became curious, so I tried updating 1Password's NDK toolchain from r22b to r25b. I seem to have ran into an issue with cargo-ndk where it look to make a bad assumption about the mapping from NDK version to target version used in all of the NDK's various x86_64-linux-android<n>-clang toolchains. This just seemed to mostly work before since 21-24 exist, but 25 doesn't. I can't imagine that we would be the only people to run into this when trying, so this might add more ecosystem friction migrating.

chriswailes commented 1 year ago

Thanks for testing that out. I wonder if most crates use custom detection logic like that or if they rely on cc. I do have a RP up to fix Android compiler detection for r25.

alex-pinkus commented 1 year ago

I published a revert for the r25b change: #104628.

dvc94ch commented 1 year ago

just downloaded beta to try this out and now this. really think the rust project should reconsider it's position on zero breakage for mobile targets. seems to be serving rust well for desktop/server.

when working on mobile google/apple don't hesitate to break stuff. when working on mobile with rust, rust makes sure the bandaid is ripped of excruciatingly slowly prolonging the pain for everyone. looking at all the discussions around this topic, it also cost the rust project about a week of productivity.

it is always possible for firefox to stick on an older rust version until they migrated their codebase to a newer ndk.

dvc94ch commented 1 year ago

and I guess one other thing to point out is that rust updates do regularly cause breakage. whenever clippy adds a new lint, any ci that runs clippy will break. so pretending that rust updates don't ever under no circumstance cause breakage is not entirely true.

alex-pinkus commented 1 year ago

Are you unable to use the INPUT(-lunwind) workaround described in my original writeup?

As a side note, I’m not aware of anyone advocating for “zero breakage for mobile targets.” My writeup closed with this sentence:

By giving developers ample warning that allows them to adopt today's workaround, the Rust project can keep the ecosystem current without causing sudden surprises.

dvc94ch commented 1 year ago

I've been using this workaround for a long time. I was planning on finally removing it when I noticed that the PR had been reverted.

dvc94ch commented 1 year ago

but I don't see how requiring people to use a workaround is an improvement over not requiring people to use a workaround. instead of everyone using workarounds wouldn't it be more efficient to have people maintain their codebases?

dvc94ch commented 1 year ago

I guess a different way of saying it is that with or without this PR people will have additional work.

  1. everyone needs to implement workaround
  2. all codebases need to be updated
  3. everyone needs to remove workaround

why not just do 2? seems like much more efficient. the reason it wasn't done was because there was no policy on ndk updates. now that there is a policy on ndk updates that says the last LTS ndk should be used, you're violating your own policy.

alex-pinkus commented 1 year ago

I think you and I agree on the long-term goal of allowing the ecosystem to operate on a newer NDK. I suspect others do as well, although I cannot speak for them.

alex-pinkus commented 1 year ago

I guess a different way of saying it is that with or without this PR people will have additional work. [...] why not just do 2?

Personally, I see it slightly differently from just trying to minimize the number of steps. Let's suppose there are two types of developers:

  1. A developer who wishes to execute the fewest number of steps possible, even if it means that they are broken at some point due to Rust releases and must take action with priority.
  2. A developer who wishes to make changes on their own schedule, and not be in a situation where a version comes out and they immediately must migrate, even if they must take intermediate steps to make that happen.

If we had issued no warning whatsoever, and just rolled out 1.66 with libunwind replacing libgcc, we would be satisfying group 1. Everyone would need to take one action, and they would need to do it urgently or see their Rust compiler become out of date. Worse still, anyone in group 2 would be caught by surprise on doing this. They must temporarily join "group 1" in order to take urgent changes.

If we issue some warning and give developers time to migrate, we are satisfying group 2, and we are not requiring anyone in "group 1" to join "group 2". Anyone in group 1 can feel free not to apply the workaround, and then migrate when they are forced to. They will be making an informed choice to execute only 1 step, as they desire to do, at a time that they have been warned it will occur. Yes, this will happen later than it would otherwise, but we've already had five years on an older NDK version -- it doesn't seem to me like another two months cause significant headache.

Ensuring that developers maintain their software seems to me to require that we cater to both group 1 and group 2. A pattern I find myself in often, for instance, is that my bandwidth for contributions varies over time; sometimes I have more time to devote to open-source, and sometimes I have less. This means I fit better in a "group 2" model -- and if I worked in an ecosystem that required me to be "group 1," I would probably not be keen to continue contributing.

What do you think? Do you find that you're always in group 1, or does it vary from time to time for you as well?

jyn514 commented 1 year ago

FWIW I am not sure that the original revert was justified, given that we document the old NDK is unsupported. But given that the original revert happened I don't have objections to a blog post + few months of warning.

I think my larger worry is that we have no consistent policy for which changes are considered breaking and which aren't; if something similar happened in the future (e.g. dropping support for Windows XP) would we also require a blog post and deprecation period first? IMO the revert set a bad precedent.

jyn514 commented 1 year ago

To be clear, I have no problem with a blog post if it's suggested before the break happens. My only concern is that we backed out a change that broke an unsupported platform, and I worry that adds pressure to back out future changes that break unsupported platforms.

dvc94ch commented 1 year ago

well in this case the developer in case 2 has not been paying attention, the problem isn't new and the ndk fairly old. you'll always catch someone offguard that hasn't been paying attention.

alex-pinkus commented 1 year ago

Maybe it would be most useful to enumerate the steps that are still pending for this issue. I'll summarize them as I see them, but others please chime in if there's anything I missed:

  1. FCP/MCP? There's been allusion to bringing this as a change proposal of some kind (MCP suggested but discarded here, reiterated but suggesting FCP here, mentioned again on Zulip here). The discussion on the Apple OS issue that @jyn514 linked to implies to me that there's at a minimum some documentation exercise, and probably still debate to be had.
  2. Blog post announcement. First suggested here. We could use the glibc bump as a guide here. This could help guide users to one workaround or another if they wished to migrate ahead of time.
  3. Revert of the revert. Once there's been sufficient lead time, someone will need to reintroduce the r25b upgrade.

For # 1, my read of this Zulip discussion is that @pnkfelix has the pen on something, but I'm not sure what scope of discussion that refers to (or if that was referring to actions that have already taken place). That seems like the right place to raise concerns about process or precedent.

For # 2, I'm happy to kick something off, but I may not be the right person to do so. Since I'm not the author of the upgrade PR, nor a maintainer of the Android targets, nor a member of any Rust team, I don't want to steal thunder from someone who is better suited to write this.

My interpretation is that # 3 is blocked on # 2 but that # 1 can happen in parallel.

Are there any pending actions missing from this list?

dvc94ch commented 1 year ago

Rereading your previous argument that "as long as you use a very old out dated sdk you don't need to apply the workaround" is a bit off. Especially since staying current with rust versions is in your argument of paramount importance.

So your argument suits the very very narrow set of people that are ok with a very old ndk but need the latest rust.

The defacto thing that happened is everyone updated their ndk, everyone applied the workaround, everyone waited patiently for the rust project to decide to update their ndk and then once it was all done the project was sent into a permanent state of libo like has happened before because of some hold outs that will probably never update their code anyway. Why Firefox didn't manage to update as claimed previously is a real mystery. Maybe Mozilla is broke?

dvc94ch commented 1 year ago

Ah some more background might be required for people less familiar with android. The ndk maintains backwards compatibility well, so updating the ndk is rarely a problem. When I said google breaks stuff without hesitation I was referring to supporting newer android versions.

One example was when they decided that the rtnetlink Linux interface was suddenly going to be unsupported by enabling some new selinux policy. As long as you target the same android version you should be able to update the ndk fairly painlessly.

chriswailes commented 1 year ago

I have several concerns about this whole process.

  1. This change is being held to an undocumented standard of "announcement". If there is a preexisting policy on how to alert developers to a change that's fine and I'm more than happy to comply, but here Android development (and users) are suffering as we hash out the policy in real time. Without a written policy anyone can show up and demand an announcement, just like is happening here.
  2. The Android Platform Team specifically asked for an explicit schedule for the revert/announce/revert-revert timing and a statement of policy for future change announcements before the revert occurred. We seemed to get a positive response to that at the time but then the revert went ahead anyway.
  3. The Android NDK update policy was reviewed by the languages and compilers team in multiple meetings. At no point was any opposition brought up to the policy. I'd prefer not to re-litigate the topic because of issues that this might cause to developers using unsupported NDKs. We have to get into a good state eventually and the sooner the better.

IMHO we are delaying an update that most developers want, and those that will be most inconvenienced by this are using not one but TWO unsupported tools: the old NDK directly and Rust's usage of the NDK. At a certain point if you do unsupported things you should not expect support. Not only that, projects that use Rust are actually restricted in the features they can access due to the current state of things. In summary, it seems like we're doing a lot of work to avoid some people having to delete a file sooner rather than later and the longer we keep things the way they are the more people are going to have to apply workarounds and do other unsupported things.

alex-pinkus commented 1 year ago

Apologies on any process miss here. I kicked off the revert nine days after T-compiler called it "for now the wise choice to do", having seen no new movement or concerns on this issue, and apparent consensus (including from you) on "delaying the update" (which requires a revert by definition). The only intervening discussion since then that I saw was on clarifying the impact. I'm not a usual contributor here, so if there's something I missed looking at process-wise, I apologize, and I would appreciate knowing how to do that better next time.

What do you think the right steps are from here?

glandium commented 1 year ago

Let me ask this: why can't the rust compiler accommodate for both? Clang does.

chriswailes commented 1 year ago

I'm new here as well so I'm still figuring out the ropes to :-) I appreciate your desire to notify developers, but I had hoped to avoid the situation we're in now where this update is delayed indefinitely until a policy conversation (and no one likes having to make policy decisions) is resolved.

Going forward I think it would be a good idea to come up with a Rust-wide announcement policy for changes. If that is going to be a bigger topic and take a while we should make a one-off decision for this announcement (say, announce now and release in 1.67), stick to that for this update, and then comply with any future policy including no requirement if no policy is decided upon (which isn't to say there wouldn't be an announcement).

chriswailes commented 1 year ago

Let me ask this: why can't the rust compiler accommodate for both? Clang does.

The compiler can accommodate both but libraries such as libc and cc need to make decisions based on the NDK in use. There is an RFC in the works to allow libraries to support multiple OS/libc/etc. versions.

jonhoo commented 1 year ago

The Android NDK update policy was reviewed by the languages and compilers team in multiple meetings.

I think the root of the process failure is perhaps here — while the update policy was agreed upon by the relevant teams, it wasn't (to my knowledge at least) circulated with users of Rust, and so a fairly vital group of stakeholders wasn't consulted, which in turn could have given early warning of the issues that'd crop up come release time.

I agree with you that we shouldn't block updating the NDK on deciding on an announcement policy. As you say, we should just do the announcements for the NDK update (probably following roughly the same timeline as the glibc announcements), and then decide on the policy independently.

I do suggest though that we should block future changes to Rust's platform update/support policy on deciding on the announcement/notification strategy for such changes.

chriswailes commented 1 year ago

So, who is able to decide when this feature can be submitted and release an announcement? We seem to be stuck in limbo.

jonhoo commented 1 year ago

I think the best next step is for someone to write a first draft of an announcement of the change. That's most likely to get the ball rolling.

chriswailes commented 1 year ago

"In Rust the NDK used when targeting the Android platform will be updated to r25b. This will increase the minimum supported API level from 15 to 19. If you are currently using an NDK newer than the previous version used by Rust (r17) you will likely need to adjust your build to remove any workarounds you might have regarding libunwind."

@alex-pinkus Is there anything else you think should be added?

glandium commented 1 year ago

As someone currently stuck on r21d, your message doesn't tell me my builds will break with that new version of rust.

This will increase the minimum supported API level from 15 to 19.

huh, wait what?

tmandry commented 1 year ago

@rustbot label I-compiler-nominated

Besides a revert, the compiler team said on this issue: (https://github.com/rust-lang/rust/issues/103673#issuecomment-1310574653)

  • seems that the path to a FCP is appropriate to get more context and collective understanding

But it isn't clear who is kicking off the FCP, or if anything else needs to be done before that point. The questions around next steps are summed up in https://github.com/rust-lang/rust/issues/103673#issuecomment-1328332144.

I'm nominating so the compiler team can address this in their next meeting (hopefully; a week from now). In the meantime I think efforts to hash out the wording on an announcement should continue.

smeenai commented 1 year ago

This will increase the minimum supported API level from 15 to 19.

huh, wait what?

NDK r24 dropped support for API levels below 19: https://github.com/android/ndk/wiki/Changelog-r24

chriswailes commented 1 year ago

If you are currently using an NDK newer than the previous version used by Rust (r17) you will need to adjust your build to remove any workarounds you might have regarding libunwind.

@glandium Does removing "likely" from the above sentence help? If not, do you have a suggestion on an alternative wording?

glandium commented 1 year ago

I don't think it does. The main difference for Firefox is that libunwind is now necessary to build (it's added to the linker command line) while it wasn't before, and libunwind is not in NDKs before... I don't know what version, but it's not in the one we're using.

smeenai commented 1 year ago

r23 added and switched to using LLVM libunwind (along with compiler-rt) for all architectures. Prior to that, it was only used for armv7 (and the NDK only provided it for that architecture); other architectures used libgcc for unwinding. https://reviews.llvm.org/D96403 was the relevant Clang change.

chriswailes commented 1 year ago

@glandium I'm not entirely sure what you're asking for here.

As someone currently stuck on r21d, your message doesn't tell me my builds will break with that new version of rust.

I changed the message to indicate that builds will break. Are you also asking for a detailed explanation of why they will break to be included? Again, if you could provide the language you'd like to see in the announcement that would be very helpful.

dvc94ch commented 1 year ago

Why will builds break? Most builds won't break, there's only a few builds that will have an issue

chriswailes commented 1 year ago

It might break due to

workarounds you might have regarding libunwind.

But I can expand on that a bit more. I'm currently creating a pull request to the blog to help move things along.

chriswailes commented 1 year ago

PR for announcement created here.

alex-pinkus commented 1 year ago

Thanks for opening up the PR! I'll add my comments there.

I think the breakage @glandium is referring to is the same one I'm concerned about: anyone using a currently-supported NDK version (i.e. r22 or below) will be unable to build until they switch their NDK version. In other words, anyone not currently applying a workaround will be broken.

~I'll share a suggestion on the PR for how we might message that without seeming too gloomy.~ Edit: @chriswailes was too fast for me and already addressed this on the PR -- thank you!

pnkfelix commented 1 year ago

@rustbot assign @pnkfelix

(the T-compiler had previously talked about needing an FCP here, but I failed to follow up on it at that time. Self assigning now as a forcing function.)