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

Remove use_std feature #657

Open
dtolnay opened this issue Jul 9, 2017 · 7 comments
Open

Remove use_std feature #657

dtolnay opened this issue Jul 9, 2017 · 7 comments

Comments

@dtolnay
Copy link
Member

dtolnay commented Jul 9, 2017

EDIT(by @gnzlbg): I'm repurposing this issue to track the removal of the now deprecated use_std cargo feature. The original text of this issue follows.


As recommended by https://github.com/brson/rust-api-guidelines#c-feature. This aligns better with the naming of implicit features inferred by Cargo.

@roblabla
Copy link
Contributor

What's the use-case for the use_std flag ? Couldn't the crate always be no_std? The only use of that feature flag is at https://github.com/rust-lang/libc/blob/master/src/unix/mod.rs#L286. What happens if we remove that check?

@alexcrichton
Copy link
Member

Changing it can be a breaking change in some subtle situations so it wasn't turned off by deafult, but for the next major release it'll be turned off by default.

@gnzlbg
Copy link
Contributor

gnzlbg commented Feb 21, 2019

Since this feature is enabled by default, this would only be a breaking change iff --no-default-features --features use_std is being used, which would be a weird configuration, since doing nothing has the same effect. Or if users are re-exporting the use_std feature, e.g., in Cargo.toml: use_std = [ "libc/use_std" ].

I have a proposal here.

We could add a std feature, in parallel with use_std, that is also enabled by default. Then, in the build.rs, we could translate use_std to std, and start warning on its usage. Internally, in the library, we would just use std as a feature.

We might not be able to remove use_std ever, but we can at least already implement a proper migration path, and start telling users to migrate to it.

The question is whether doing this is worth it or not.

gnzlbg added a commit to gnzlbg/libc that referenced this issue May 24, 2019
bors added a commit that referenced this issue May 24, 2019
Deprecate `use_std` cargo feature: use `std` instead .

Related to #657 .
bors added a commit that referenced this issue May 24, 2019
Deprecate `use_std` cargo feature: use `std` instead .

Related to #657 .
@gnzlbg gnzlbg changed the title Rename use_std feature to just std Remove use_std feature May 25, 2019
@gnzlbg
Copy link
Contributor

gnzlbg commented May 25, 2019

Now the std feature can be used as a cargo feature, and the use_std feature has been deprecated (produces a cargo warning). I'm repurposing this issue to track the removal of the use_std feature, which would be a breaking change that will probably have to wait till the next breaking libc release.

@mati865
Copy link
Contributor

mati865 commented Aug 6, 2019

Is std feature necessary?
It caused massive issues upgrading rand in Rust repo: rust-lang/rust#61393
More details at: rust-random/rand#852

Short summary:
Somehow when building Rust newer rand versions which use more of libc were failing during linking of rustc_main:

/usr/bin/ld: /usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/Scrt1.o: undefined reference to symbol '__libc_start_main@@GLIBC_2.2.5'
//lib/x86_64-linux-gnu/libc.so.6: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status

It was found this can be avoided by disabling std feature of libc causing it to pull libc.so and Rust built fine.

While it looks like rustbuild issue what is the point of not linking to C library by default? AFAIK it would not hurt if C library was linked twice, by libstd and libc.

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 6, 2019

Currently, there are three cases:

  • building libc when libstd is linked,
  • building libc when libstd is not linked.
    • building libc for the std library

The std feature should link libstd (although this is not happening anymore? libc is #![no_std]?), and IIUC should only be disabled for #![no_std] binaries or when building libstd itself.

I don't know why std is different from rustc-dep-of-std, AFAICT both should be equal. The rustc-dep-of-std feature requires no_core, but I don't know why this is the case, since libcore is always built when trying to build libc for libstd.

I think maybe we can unify these two. Have std always enabled by default, and have libstd disable it.

Is std feature necessary? While it looks like rustbuild issue what is the point of not linking to C library by default? AFAIK it would not hurt if C library was linked twice, by libstd and libc.

Does this hold for statically linked C libraries as well ? How does this interact with the crt-static target feature?

IIRC one shall not link the C libraries statically twice, and that's what the std feature avoided.

@mati865
Copy link
Contributor

mati865 commented Aug 7, 2019

std feature explicitly skips linking of C standard library:

libc/src/unix/mod.rs

Lines 296 to 299 in 7c8e397

} else if #[cfg(feature = "std")] {
// cargo build, don't pull in anything extra as the libstd dep
// already pulls in all libs.
} else if #[cfg(target_env = "musl")] {

If std binary depends on #[no_std] crate using libc = { version = "0.2", default-features = false } shouldn't it be considered as linking C standard library multiple times?

Although it may be oversimplified this example happen to work fine with dynamically linked glibc and statically dynamically linked musl: https://gist.github.com/mati865/b422b41f2c2784361091db78354d4bc8
Don't know about other targets.

@tgross35 tgross35 added this to the 1.0 milestone Aug 29, 2024
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

6 participants