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

Old version (1.x) of the slog crate can't compile #47781

Closed
pietroalbini opened this issue Jan 26, 2018 · 6 comments
Closed

Old version (1.x) of the slog crate can't compile #47781

pietroalbini opened this issue Jan 26, 2018 · 6 comments
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Milestone

Comments

@pietroalbini
Copy link
Member

This was caught in crater thanks to shadowsocks (crater log), which uses an old release of slog (1.0). This works fine on stable, but fails on beta and nightly.

   Compiling slog v1.3.2
error[E0277]: the trait bound `*mut core::ops::Fn() + 'static: core::marker::Sync` is not satisfied in `core::fmt::Arguments<'static>`
   --> /home/pietro/.cargo/registry/src/github.com-1ecc6299db9ec823/slog-1.3.2/src/ser.rs:228:6
    |
228 | impl SyncSerialize for fmt::Arguments<'static> {}
    |      ^^^^^^^^^^^^^ `*mut core::ops::Fn() + 'static` cannot be shared between threads safely
    |
    = help: within `core::fmt::Arguments<'static>`, the trait `core::marker::Sync` is not implemented for `*mut core::ops::Fn() + 'static`
    = note: required because it appears within the type `core::marker::PhantomData<*mut core::ops::Fn() + 'static>`
    = note: required because it appears within the type `core::fmt::Void`
    = note: required because it appears within the type `&'static core::fmt::Void`
    = note: required because it appears within the type `core::fmt::ArgumentV1<'static>`
    = note: required because it appears within the type `[core::fmt::ArgumentV1<'static>]`
    = note: required because it appears within the type `&'static [core::fmt::ArgumentV1<'static>]`
    = note: required because it appears within the type `core::fmt::Arguments<'static>`

cc @dpc

@pietroalbini pietroalbini added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. C-bug Category: This is a bug. labels Jan 26, 2018
@pietroalbini pietroalbini added this to the 1.24 milestone Jan 26, 2018
@dtolnay dtolnay added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Jan 26, 2018
@dtolnay
Copy link
Member

dtolnay commented Jan 26, 2018

I believe this is expected from the soundness fix in #45198. The fix exposed legitimately unsound use of fmt::Arguments in those versions of slog. We coordinated with the slog author to publish a fix in slog before landing the fix on the rustc side.

@dtolnay dtolnay closed this as completed Jan 26, 2018
@kennytm
Copy link
Member

kennytm commented Jan 26, 2018

shadowsocks's slog dependency needs to be updated from ~1.3 to 1.7.1, or preferably to 2.1.1.

cc @loggerhead.

@pietroalbini
Copy link
Member Author

bomgar/rust-oauth-proxy (crater log) and epage/relint (crater log) also depend on an old version of slog, which needs to be updated to 1.7.1 or 2.1.1. cc @bomgar @epage

woodpecker also regressed, not related to slog though (crater log). cc @niamster

@epage
Copy link
Contributor

epage commented Jan 26, 2018

Don't worry about relint atm. Its not yet in a usable state and I've not been working to get it there with some of the other stuff on my plate.

If there isn't a nice way for you all to resolve it, I can do whatever you recommend to appease crater.

@pietroalbini
Copy link
Member Author

If you don't plan to fix it it's fine: crater is currently marking it as regressed because it builds fine on stable but fails on beta, and after the next stable is released it should stop complaining about it.

@loggerhead
Copy link
Contributor

Thanks, I have changed the version of slog to fix this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants