Hide iconv functions on Apple devices behind a feature flag

[winterqt]

See #2870 and #2944 for context. I believe the comment in the code explains the changes well enough, but if you'd like me to add a description to the commit as well (or adjust any wording etc.), let me know.

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) soon.

Please see the contribution instructions for more information.

[winterqt]

What would be the best path forward for libc-test? Should I just add the apple-iconv to its dependency on libc, or do something more complex?

[JohnTitor]

Uhm, I hesitate to add a platform feature flag as we support a lot of platforms and the flag management can be complex easily 😱(we've done a similar thing to Rust versions and it introduces a ton of complexity, huh).
Perhaps we should have asked that those functions be separated into a platform-specific crate rather than added to the libc crate.
@rust-lang/crate-maintainers Any thoughts on this?

(By the way, this is a breaking change and I'd like to ship it in a major release.)

[Amanieu]

I still don't understand what the problem is with linking against iconv on apple targets. This library is guaranteed to always be present, so it shouldn't cause any issues?

[winterqt]

@Amanieu See @thomcc's excellent comment here for some info.

[joshtriplett]

👍 for just dropping these functions entirely in a subsequent major release, if we ever make a major release.

[winterqt]

@JohnTitor @joshtriplett Do you think there's any chance of a major release happening within, say, a few months? A year? Just curious, I appreciate your insights, thanks! 😊

[JohnTitor]

We're going to make a major change to musl in #3068 and I guess we could publish this along with that PR. It will be merged once FCP passes.
I cannot tell the exact date and time for that, as it requires agreement by team members, but it could happen within a few months (hopefully).

[thomcc]

See @thomcc's excellent comment

That was about libstd's usage. I'd don't love that we link to it in libc, but there are reasons I haven't pursued this.

I still don't understand what the problem is with linking against iconv on apple targets. This library is guaranteed to always be present, so it shouldn't cause any issues?

It's mostly compelling since it's practically unused¹, and linking with it means it must be available on the host, which breaks some minimalist (or perhaps naïve) cross-compilation setups, where a full SDK is not available, but a linker and a libSystem stub are (for example, this is the case with zig's cross compilation support, unless you bring along the .tbd files you need). IOW, the library is guaranteed to be present on the target, but not on the host, and it is needed in both places (as linking a mach-o requires the linker know which dylib provides each symbol).

Concretely, this probably is one of the first things hit by folks who try to cross compile rust to macOS by using zig {cc,ld} directly. I'm unsure how reasonable it is to expect cross compilation to work in such cases, and users will probably hit something after this anyway, and are likely better off with something like cargo-zigbuild anyway, which shaves off these kinds of sharp edges (it bundles libiconv.tbd for this reason).

---

In any case, I don't think we should removing the items (even if they're nearly unused), and I really don't think we should add a feature flag for this. I do think it's annoying that Rust users may have to cart around semi-vestigial libiconv.tbd in their cross compilation setup, even if they don't need it (but perhaps if it weren't that it might be something else²). So, I have two thoughts here:

1. If we care about that kind of use case, then we should just change the PR to remove the #[link] attribute and deprecate the items (with a message explaining a fix). I don't know if this is technically breaking, but it seems like it unlikely to break anybody.

2. If we don't care, then we probably should wait until raw-dylib is supported on macOS (which sadly seems stalled). At that point, we could change the attribute to #[link(name = "iconv", kind = "raw-dylib")]. Then (IIUC) we'll generate the tbd automatically when compiling, avoiding such problems. That said, it may be quite a while before that is available on stable.

I do think the hassle this causes in practice is not nearly worth it given how few people (if any) depend on it, but I also think libc should not make the breaking API changes it does (altho I'm unsure how much the link removal would count...). So I don't know.

Footnotes

1. virtually nobody on github use libc's declarations, and a large amount of the code that does want to call iconv seems to explicitly link anyway. Of course, neither of those searches are absolute proof of anything, but it's a ballpark idea of usage. ↩

2. IOW, perhaps it's better that cross compilation does not work out of the box. I don't think we want to make any guarantees that it will (even in std), nor do we want the bug reports that could come if it worked and then we began linking to another system library. ↩

[JohnTitor]

That's insightful, thank you!

1. If we care about that kind of use case, then we should just change the PR to remove the #[link] attribute and deprecate the items (with a message explaining a fix).

Do you mean we should deprecate items first and then remove them? If so, how about deprecating on 0.2.x and removing on 0.3.x (along with time64 change)?

[workingjubilee]

Given that it seems much more likely that we will be pushing out a 0.3 release sometime "soon" (highly relatively speaking): Yeah, we should probably instead do a deprecation and doc(hidden).