6 ms·

Precompiled binaries removed from serde v1.0.184

122 points by MallocVoidstar 3y ago

WirelessGigabit 3y ago
I think also one of the issues was that it wasn't opt-in, it wasn't deterministic from day one (I truly believe it should have), and it broke building when using git references as the detection was just triple based.
MallocVoidstar 3y ago
See https://news.ycombinator.com/item?id=37183361 https://news.ycombinator.com/item?id=37183361, https://news.ycombinator.com/item?id=37189462 https://news.ycombinator.com/item?id=37189462 for context.
joshka 3y ago
See also the PR: https://github.com/serde-rs/serde/pull/2590/files https://github.com/serde-rs/serde/pull/2590/files
deleted 3y ago
[deleted]
X-Istence 3y ago
This provides some additional context for why the pre-built binary was added: https://github.com/serde-rs/serde/issues/2584#issue-1858075241 https://github.com/serde-rs/serde/issues/2584#issue-18580752... Due to the way that the derive feature was causing the dependency chain in Cargo to be longer than necessary, thereby making it impossible for cargo to compile multiple crates in parallel.
- kzrdude 3y ago
  Isn't that a parallel concern? The dependency chain is a separate issue from just building serde_derive itself isn't it?
  - X-Istence 3y ago
    The dependency chain is what led to the longer compile times and why the prebuilt binary existed. It was to shave time off how long it would take to get serde with the "derive" feature enabled compiled.
veec_cas_tant 3y ago
See also: https://internals.rust-lang.org/t/pre-rfc-sandboxed-deterministic-reproducible-efficient-wasm-compilation-of-proc-macros/19359 https://internals.rust-lang.org/t/pre-rfc-sandboxed-determin... Authors comment: >"Someone else is always auditing the code and will save me from anything bad in a macro before it would ever run on my machines." (At one point serde_derive ran an untrusted binary for over 4 weeks across 12 releases before almost anyone became aware. This was plain-as-day code in the crate root; I am confident that professionally obfuscated malicious code would be undetected for years.) He got some hate for that in the replies but I think it is a great point. Pardon my snark, but every "security expert" that voiced their opinion over the last couple of days, yet failed to recognize the situation for almost a month, should reassess their reaction once the anger subsides. The current macro situation is dangerous (and wasteful), hopefully some real progress can be made.
- cesarb 3y ago
  > (At one point serde_derive ran an untrusted binary for over 4 weeks across 12 releases before almost anyone became aware. This was plain-as-day code in the crate root; I am confident that professionally obfuscated malicious code would be undetected for years.) > Pardon my snark, but every "security expert" that voiced their opinion over the last couple of days, yet failed to recognize the situation for almost a month, should reassess their reaction once the anger subsides. I'm not a "security expert", but I wondered why I hadn't noticed this situation, since I have the habit of always diffing between the previous and the current version of the crates in ~/.cargo/registry/src whenever cargo tells me it downloaded a new version of a dependency, and a new binary file would stick out like a sore thumb. But real life has intruded for me in the last few weeks, and the last serde_derive on my ~/.cargo is still 1.0.166, so it seems I luckily avoided all this mess. (Yes, I know that reviewing the code after it has already compiled is suboptimal, and obfuscated malicious code or even the "new dependency" trick could easily pass through my cursory inspection; but at least it can be reviewed by comparing it with the previous version, unlike a precompiled binary blob where even small changes can lead to a huge difference between one binary and the other, not to mention that comparing objdump disassembly output is painful.)
  - 3y ago
therein 3y ago
Nice to see the maintainer stop being so stubborn. I wish I could say this left a good taste in my mouth as a strong proponent of Rust. Especially after dtolnay banned me on the repo after I commented on the PR. Can't even give thumbs up on someone else's comment let alone open an issue or a PR. Interesting to see how glorified janitorial roles make people think they have some real power. Just take a look at https://github.com/andreisilviudragnea/serde-blocked/discussions/1 https://github.com/andreisilviudragnea/serde-blocked/discuss... David went on a full on power trip rampage, going as far as banning people who participated with emojis on that PR from the serde org on GitHub.
- Cloudef 3y ago
  I did not know you can block people on github. The blocked people can't even fork?
- samsari 3y ago
  >Interesting to see how glorified janitorial roles make people think they have some real power. >Just take a look at https://github.com/andreisilviudragnea/serde-blocked/discuss https://github.com/andreisilviudragnea/serde-blocked/discuss... >David went on a full on power trip rampage, going as far as banning people who participated with emojis on that PR from the serde org on GitHub. Yeah... I'm going to go ahead and assume there's a little more to it that you're telling us.
  - tentacleuno 3y ago
    Someone commented on the GitHub repo with very similar wording to the above[0]. I'm not sure if it's the same person as the OP, but I suspect it is. [0]: https://github.com/serde-rs/serde/issues/2593 https://github.com/serde-rs/serde/issues/2593
- rcxdude 3y ago
  even if it's a 'glorified janitorial role', you ain't paying him for it, so I don't really understand your objection to him not wanting to deal with your feedback on it (especially if it's just participating in a dogpile and not actually a useful contribution).
hyperman1 3y ago
I feel for him/her. When you provide a usefull service in OSS, this is the result of the slightest transgression, real or imagined. I also think this displays 'emperors clothes' behaviour. In a perfect world, many eyeballs make all bugs shallow, and long term, it actually mostly works. But short term, we collectively severely fall short of our ideals. I think part of the harshness an OSS contributor receives in this kind of situation comes from everyone realizing we've been caught with our pants down. It's human to shoot the messenger, but let's not forget this only happens because someone worked hard enough to make the whole world trust him or her.
oever 3y ago
> eventually we'd like to use a first-class precompiled macro if such a thing becomes supported by cargo / crates.io Distributing binaries is what distributions are for. It does not matter if it is for macros or for the rest of a crate. Anyone can write and publish a cargo plugin that provides binaries via a separate channel. https://crates.io/crates/cargo-prebuilt https://crates.io/crates/cargo-prebuilt has been around for a while. (I've not used it, just looked for it now)
mwcampbell 3y ago
Thanks to dtolnay for boldly doing what it took to make some real progress on the problem of proc macro compile times. It's unfortunate that we reacted the way we did, driving him to abort the experiment. I'm sorry I contributed to the pile-on with my first comment on the infamous GitHub issue, the one I directed specifically at him. I'll be slower to react next time.
- epage 3y ago
  I disagree that what was done with good but agree with matklad [0] about basic expectations of transparency and communication when you maintain de facto standards. This is too big of an "experiment" to run in production code on the ecosystem without first seeking input of drawing attention to it. If any of this was done to "push the conversation forward", it is done in an unhealthy way that burns a lot of bridges. As for making progress on the idea of improving build times, I agree with kayabaNerve from the Pre-RFC [1] > It's effectively impossible to fairly review this on its merit now, nor to say it isn't being reviewed on an accelerated time span than it would otherwise have been. [0] https://www.reddit.com/r/rust/comments/15va70a/serde_has_started_shipping_precompiled_binaries/jwuacsp/ https://www.reddit.com/r/rust/comments/15va70a/serde_has_sta... [1] https://internals.rust-lang.org/t/pre-rfc-sandboxed-deterministic-reproducible-efficient-wasm-compilation-of-proc-macros/19359/5?u=epage https://internals.rust-lang.org/t/pre-rfc-sandboxed-determin...
rcme 3y ago
I’ve seen a lot of complaints about this. I would love some insights into the author’s intentions, though. Why did they choose binary distribution? They must have had a reason.
- kzrdude 3y ago
  It makes a lot of users's crates faster to compile from scratch, and compile time is a common complaint in Rust. It could be celebrated as a big total ecosystem win for compilation speed, but other issues overshadowed the whole thing. Some gains and losses are elaborated here where it was added: https://github.com/serde-rs/serde/pull/2514 https://github.com/serde-rs/serde/pull/2514
klaussilveira 3y ago
Is there a way to force cargo to compile dependencies instead of using binaries? Also, any way to force vendoring?

I think also one of the issues was that it wasn't opt-in, it wasn't deterministic from day one (I truly believe it should have), and it broke building when using git references as the detection was just triple based.

MallocVoidstar 3y ago

See https://news.ycombinator.com/item?id=37183361 https://news.ycombinator.com/item?id=37183361, https://news.ycombinator.com/item?id=37189462 https://news.ycombinator.com/item?id=37189462 for context.

joshka 3y ago

See also the PR: https://github.com/serde-rs/serde/pull/2590/files https://github.com/serde-rs/serde/pull/2590/files

deleted 3y ago

[deleted]

X-Istence 3y ago

This provides some additional context for why the pre-built binary was added: https://github.com/serde-rs/serde/issues/2584#issue-1858075241 https://github.com/serde-rs/serde/issues/2584#issue-18580752... Due to the way that the derive feature was causing the dependency chain in Cargo to be longer than necessary, thereby making it impossible for cargo to compile multiple crates in parallel.

kzrdude 3y ago
Isn't that a parallel concern? The dependency chain is a separate issue from just building serde_derive itself isn't it?
- X-Istence 3y ago
  The dependency chain is what led to the longer compile times and why the prebuilt binary existed. It was to shave time off how long it would take to get serde with the "derive" feature enabled compiled.

veec_cas_tant 3y ago

See also: https://internals.rust-lang.org/t/pre-rfc-sandboxed-deterministic-reproducible-efficient-wasm-compilation-of-proc-macros/19359 https://internals.rust-lang.org/t/pre-rfc-sandboxed-determin... Authors comment: >"Someone else is always auditing the code and will save me from anything bad in a macro before it would ever run on my machines." (At one point serde_derive ran an untrusted binary for over 4 weeks across 12 releases before almost anyone became aware. This was plain-as-day code in the crate root; I am confident that professionally obfuscated malicious code would be undetected for years.) He got some hate for that in the replies but I think it is a great point. Pardon my snark, but every "security expert" that voiced their opinion over the last couple of days, yet failed to recognize the situation for almost a month, should reassess their reaction once the anger subsides. The current macro situation is dangerous (and wasteful), hopefully some real progress can be made.

cesarb 3y ago
> (At one point serde_derive ran an untrusted binary for over 4 weeks across 12 releases before almost anyone became aware. This was plain-as-day code in the crate root; I am confident that professionally obfuscated malicious code would be undetected for years.) > Pardon my snark, but every "security expert" that voiced their opinion over the last couple of days, yet failed to recognize the situation for almost a month, should reassess their reaction once the anger subsides. I'm not a "security expert", but I wondered why I hadn't noticed this situation, since I have the habit of always diffing between the previous and the current version of the crates in ~/.cargo/registry/src whenever cargo tells me it downloaded a new version of a dependency, and a new binary file would stick out like a sore thumb. But real life has intruded for me in the last few weeks, and the last serde_derive on my ~/.cargo is still 1.0.166, so it seems I luckily avoided all this mess. (Yes, I know that reviewing the code after it has already compiled is suboptimal, and obfuscated malicious code or even the "new dependency" trick could easily pass through my cursory inspection; but at least it can be reviewed by comparing it with the previous version, unlike a precompiled binary blob where even small changes can lead to a huge difference between one binary and the other, not to mention that comparing objdump disassembly output is painful.)
- 3y ago

therein 3y ago

Nice to see the maintainer stop being so stubborn. I wish I could say this left a good taste in my mouth as a strong proponent of Rust. Especially after dtolnay banned me on the repo after I commented on the PR. Can't even give thumbs up on someone else's comment let alone open an issue or a PR. Interesting to see how glorified janitorial roles make people think they have some real power. Just take a look at https://github.com/andreisilviudragnea/serde-blocked/discussions/1 https://github.com/andreisilviudragnea/serde-blocked/discuss... David went on a full on power trip rampage, going as far as banning people who participated with emojis on that PR from the serde org on GitHub.

Cloudef 3y ago
I did not know you can block people on github. The blocked people can't even fork?
samsari 3y ago
>Interesting to see how glorified janitorial roles make people think they have some real power. >Just take a look at https://github.com/andreisilviudragnea/serde-blocked/discuss https://github.com/andreisilviudragnea/serde-blocked/discuss... >David went on a full on power trip rampage, going as far as banning people who participated with emojis on that PR from the serde org on GitHub. Yeah... I'm going to go ahead and assume there's a little more to it that you're telling us.
- tentacleuno 3y ago
  Someone commented on the GitHub repo with very similar wording to the above[0]. I'm not sure if it's the same person as the OP, but I suspect it is. [0]: https://github.com/serde-rs/serde/issues/2593 https://github.com/serde-rs/serde/issues/2593
rcxdude 3y ago
even if it's a 'glorified janitorial role', you ain't paying him for it, so I don't really understand your objection to him not wanting to deal with your feedback on it (especially if it's just participating in a dogpile and not actually a useful contribution).

hyperman1 3y ago

I feel for him/her. When you provide a usefull service in OSS, this is the result of the slightest transgression, real or imagined. I also think this displays 'emperors clothes' behaviour. In a perfect world, many eyeballs make all bugs shallow, and long term, it actually mostly works. But short term, we collectively severely fall short of our ideals. I think part of the harshness an OSS contributor receives in this kind of situation comes from everyone realizing we've been caught with our pants down. It's human to shoot the messenger, but let's not forget this only happens because someone worked hard enough to make the whole world trust him or her.

oever 3y ago

> eventually we'd like to use a first-class precompiled macro if such a thing becomes supported by cargo / crates.io Distributing binaries is what distributions are for. It does not matter if it is for macros or for the rest of a crate. Anyone can write and publish a cargo plugin that provides binaries via a separate channel. https://crates.io/crates/cargo-prebuilt https://crates.io/crates/cargo-prebuilt has been around for a while. (I've not used it, just looked for it now)

mwcampbell 3y ago

Thanks to dtolnay for boldly doing what it took to make some real progress on the problem of proc macro compile times. It's unfortunate that we reacted the way we did, driving him to abort the experiment. I'm sorry I contributed to the pile-on with my first comment on the infamous GitHub issue, the one I directed specifically at him. I'll be slower to react next time.

epage 3y ago
I disagree that what was done with good but agree with matklad [0] about basic expectations of transparency and communication when you maintain de facto standards. This is too big of an "experiment" to run in production code on the ecosystem without first seeking input of drawing attention to it. If any of this was done to "push the conversation forward", it is done in an unhealthy way that burns a lot of bridges. As for making progress on the idea of improving build times, I agree with kayabaNerve from the Pre-RFC [1] > It's effectively impossible to fairly review this on its merit now, nor to say it isn't being reviewed on an accelerated time span than it would otherwise have been. [0] https://www.reddit.com/r/rust/comments/15va70a/serde_has_started_shipping_precompiled_binaries/jwuacsp/ https://www.reddit.com/r/rust/comments/15va70a/serde_has_sta... [1] https://internals.rust-lang.org/t/pre-rfc-sandboxed-deterministic-reproducible-efficient-wasm-compilation-of-proc-macros/19359/5?u=epage https://internals.rust-lang.org/t/pre-rfc-sandboxed-determin...

rcme 3y ago

I’ve seen a lot of complaints about this. I would love some insights into the author’s intentions, though. Why did they choose binary distribution? They must have had a reason.

kzrdude 3y ago
It makes a lot of users's crates faster to compile from scratch, and compile time is a common complaint in Rust. It could be celebrated as a big total ecosystem win for compilation speed, but other issues overshadowed the whole thing. Some gains and losses are elaborated here where it was added: https://github.com/serde-rs/serde/pull/2514 https://github.com/serde-rs/serde/pull/2514

klaussilveira 3y ago

Is there a way to force cargo to compile dependencies instead of using binaries? Also, any way to force vendoring?