-
Notifications
You must be signed in to change notification settings - Fork 137
Conversation
These crates weren't getting much value out of `failure`, and the use of proc macros to generate what is effectively a match statement on enum variants which returns a string literal adds a lot to compile times. Furthermore, `failure` is still using the non-1.0 proc macro crates. It seems there's upstream work to get this fixed[1], but since we're not really getting value out of the proc macros as-is, there's not much reason to bother. [1]: rust-lang-deprecated/failure#319
These crates weren't getting much value out of `failure`, and the use of proc macros to generate what is effectively a match statement on enum variants which returns a string literal adds a lot to compile times. Furthermore, `failure` is still using the non-1.0 proc macro crates. It seems there's upstream work to get this fixed[1], but since we're not really getting value out of the proc macros as-is, there's not much reason to bother. [1]: rust-lang-deprecated/failure#319
The syn change has been released in 1.0.3. Any news on tis @infinity0 ? |
This PR is now ready to go in, just waiting approval. |
Hmm seems like I need to update the tests too, I'll need to find some time for that later. |
@davidbarsky @mitsuhiko this is ready to go in. |
Hi @infinity0 , how is it going? Do you need any help? |
I had some issues getting Travis CI to work with uMatrix and wasn't yet able to see why the CI test failed. (I'm at CCC Camp right now with too many other distractions.) |
@infinity0 Happy hacking. Ask away if you need any help |
But Minimum supported rustc version of If we upgrade |
@koushiro failure has had no release in 8 months. People wanting to support old versions of rust are free to stick with 0.1.5 as they've been doing for the past 8 months, imo... |
Could someone paste the Travis CI error here please? I still can't see it. Too much spaghetti javascript. |
@infinity0 rust 1.18
stableWorker information system_info betaworker_info nightlyWorker information system_info |
The CI is now passing, except on rust 1.18 which we have to drop anyways. @davidbarsky @mitsuhiko this is ready to go in, can you merge and release to crates.io? |
@infinity0 IMO, the PR should be merged into version 0.2 (not worth releasing 0.2 for these breaking changes), but the development of version 0.2 is currently stuck on #296. |
Are these changes breaking? I thought it was just a straightforward upgrade to proc_macro2. I didn't change anything API-related. |
It does not change the API, but it is not available for rustc 1.18~1.30. |
@infinity0 @koushiro Hi all! I think that due to the centrality of the Failure crate, this version bump be be extremely impactful to the ecosystem at large. I don't think we should do any minimum I would personally be be more comfortable merging and releasing these changes if they came out as part of the 0.2 alpha & beta releases. I also—weakly—believe that with the merging of rust-lang/rust#60852, Additionally, feel free to fork & rebase atop of #296. I'd like to be able to get Failure 0.2, even an alpha, out. |
The issue however, and why I did this PR in the first place, is that other parts of the ecosystem have already moved to proc_macro2 v1, which breaks the build for crates that pull in both proc_macro v1 and v0.x.x. I can't remember the details now, but I'm pretty sure core language tools like cargo are already affected by this. |
Perhaps a decent compromise could be to merge this PR as v0.2 and then release the next version as v0.3? |
It doesn't break, it just turns out that we have to build both v1 and v0.x.y - takes more time + increased code bloat - but generally it's mostly fine |
@davidbarsky i tried to get backtrace added to std error a few months back and back then I mostly just stalled on not being sure what a sensible public API for backtrace would be in std. I would like to just sprint on that over a few days together with others because I think this could help the ecosystem quite a bit at this point. |
This is the case for most crates but I'm pretty sure proc_macro2 is a special case that makes things much more likely to break when mixing different versions. I can't remember what specific condition causes the errors now, but I remember seeing some rustc error messages along the lines of "incompatible symbols, did you use two different versions of the same crate?" |
I've got a project with both proc-macro2 0.4.30 and 1.0.2, and it's building fine (if slower). I suspect the way to get it to break is if you try to actually link both of them into the same binary (NOTE: not the final binary, in the same proc macro binary), which doesn't happen in typical usage. |
Nothing's special with proc_macro2 here. Errors like this trigger when you happen to import a certain symbol from few different versions of a crate, typically via reexports |
Serde has recently published a release that bumped the MSRV from rustc 1.15 to rustc 1.31 in a bugfix release. Given that Failure "only" has 3,825,432 downloads, while Serde has 13,467,675 million downloads, I feel comfortable following in the footsteps of Serde and doing an MSRV bump to 1.31 if @mitsuhiko doesn't have any opposition. |
No objections |
Nice! @infinity0, would you like to bump the MSRV to 1.31 in Travis/any documentation? |
@infinity0 ping? If @infinity0 doesn't show up soonish, can a member of rust-lang-nursery just run with this, please? |
This PR is pretty important to keep out duplicate dependencies in your build tree, so the sooner we can get this merged the merrier. 😄 |
I've gone ahead and opened #329, which basically takes these commits and bumps the MSRV. |
This PR updates the `RELEASES.md` to capture the changes introduced in #319 and #329. Thanks to @djc and @infinity0 for their help!
This PR updates the `RELEASES.md` to capture the changes introduced in #319 and #329. Thanks to @djc and @infinity0 for their help!
Requires dtolnay/syn#696 to first be merged and released.
It is possible to do this without that PR, by moving the added API as a utility function in this crate, but I thought that this would be much less tidy.