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

std depends on cfg-if 1.0.0 and 0.1.10 #103365

Closed
chbaker0 opened this issue Oct 21, 2022 · 5 comments · Fixed by #103367
Closed

std depends on cfg-if 1.0.0 and 0.1.10 #103365

chbaker0 opened this issue Oct 21, 2022 · 5 comments · Fixed by #103367
Labels
C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@chbaker0
Copy link
Contributor

#101861 added cfg-if v1.0.0 as a std dependency.

std still depends on cfg-if v0.1.10:

$ cargo tree -i -p cfg-if@0.1.10
cfg-if v0.1.10
├── getrandom v0.1.14
│   ├── rand v0.7.3
│   │   [dev-dependencies]
│   │   └── std v0.0.0 (/usr/local/google/home/collinbaker/chromium/src/third_party/rust_src/src/library/std)
│   └── rand_core v0.5.1
│       ├── rand v0.7.3 (*)
│       └── rand_chacha v0.2.2
│           └── rand v0.7.3 (*)
├── panic_abort v0.0.0 (/usr/local/google/home/collinbaker/chromium/src/third_party/rust_src/src/library/panic_abort)
│   └── std v0.0.0 (/usr/local/google/home/collinbaker/chromium/src/third_party/rust_src/src/library/std)
└── unwind v0.0.0 (/usr/local/google/home/collinbaker/chromium/src/third_party/rust_src/src/library/unwind)
    └── std v0.0.0 (/usr/local/google/home/collinbaker/chromium/src/third_party/rust_src/src/library/std)

Depending on only one version would be nice. While it's a small library, it's still unnecessary bloat.

@chbaker0 chbaker0 added the C-bug Category: This is a bug. label Oct 21, 2022
@chbaker0
Copy link
Contributor Author

@rustbot label T-libs

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Oct 21, 2022
@thomcc
Copy link
Member

thomcc commented Oct 21, 2022

Note that this bloat is only exposed to users of -Zbuild-std so it doesn't matter much. I'd take a patch removing it from panic_abort and unwind though.

getrandom should only be used as a dev-dep, and also we can't easily update it for various unfortunate reasons, so I'm not sure how much I'd recommend pushing on that.

@chbaker0
Copy link
Contributor Author

Note that this bloat is only exposed to users of -Zbuild-std so it doesn't matter much. I'd take a patch removing it from panic_abort and unwind though.

not exactly true, it's shipped in the normal distribution:

$ ls .rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib
libaddr2line-74211b12b443811c.rlib          libmemchr-e89379089032cd0e.rlib             librustc-nightly_rt.tsan.a
libadler-ce60aba693c46806.rlib              libminiz_oxide-6d1ec8f1337db26d.rlib        librustc_std_workspace_alloc-b113a1d1429de7b6.rlib
liballoc-c6c03e024a2f1e46.rlib              libobject-4a45f4075ae80a22.rlib             librustc_std_workspace_core-522518611024dce5.rlib
libcfg_if-a51843dfc5fc8b4b.rlib             libpanic_abort-07057e0406e64bc5.rlib        librustc_std_workspace_std-013cf041d0880b37.rlib
libcfg_if-fcf994c37af81dc5.rlib             libpanic_unwind-15accacd1251d941.rlib       libstd-7b2106000b625742.rlib
libcompiler_builtins-b7c79d85cf21a511.rlib  libproc_macro-80378fa4c5002a0d.rlib         libstd-7b2106000b625742.so
libcore-05898138a596088a.rlib               libprofiler_builtins-f931f59836842a86.rlib  libstd_detect-bb8d1566a1046906.rlib
libgetopts-55c2848d703ec364.rlib            librustc_demangle-db4d651637ed1365.rlib     libtest-90e69e1db733a8fb.rlib
libgimli-1f391b051eb3b957.rlib              librustc-nightly_rt.asan.a                  libtest-90e69e1db733a8fb.so
libhashbrown-1791beb5b36e409b.rlib          librustc-nightly_rt.lsan.a                  libunicode_width-803f21b2ca81a772.rlib
liblibc-3e961d059b9bcde7.rlib               librustc-nightly_rt.msan.a                  libunwind-757bd6fa410f1121.rlib

Though it is pretty minor.

getrandom should only be used as a dev-dep, and also we can't easily update it for various unfortunate reasons, so I'm not sure how much I'd recommend pushing on that.

Locally I was able to update it from 0.1.14 -> 0.1.16, which depends on cfg-if 1.0.0. Will this break in ways I don't understand?

@thomcc
Copy link
Member

thomcc commented Oct 21, 2022

not exactly true, it's shipped in the normal distribution

Good point.

Locally I was able to update it from 0.1.14 -> 0.1.16, which depends on cfg-if 1.0.0. Will this break in ways I don't understand?

That's probably be fine. I think the issue is with getrandom 0.2 rather than the 0.1.x series -- whichever one requires wasm_bindgen for the wasm impl and compile_errors when it fails to resolve an impl (since std cannot depend on wasm-bindgen).

Feel free to r? @thomcc on a PR moving the stdlib to cfg-if v1.x.y.

@chbaker0
Copy link
Contributor Author

Cool! I put it up for discussion: #103367

@bors bors closed this as completed in dabb6c6 Nov 4, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 21, 2022
Update test's cfg-if dependency to 1.0

This change was mistakenly left out of rust-lang#103367

Finishes rust-lang#103365
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Feb 10, 2023
Remove std's transitive dependency on cfg-if 0.1

After rust-lang/rust#101946 this completes the move to cfg-if 1.0 by:
* Updating getrandom 0.1.14->0.1.16
* Updating panic_abort's and unwind's dep to cfg-if 1.0

Fixes rust-lang/rust#103365
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants