Regression report stable-2016-08-16 vs. beta-2016-08-26


#1

Root regressions, sorted by rank:

Full report.


#2

Breakdown:


#3

Hmm. That’s pretty large. I have to say that I don’t feel great about this. Particularly since that rule is hardly a popular one – nor the most crucial. Perhaps it was too ambitious to imagine changing it and instead it should remain a lint forever (it is, in its own way, a kind of lint anyhow). (cc @aturon)

I think we need to drill more into the process for these warning -> error transitions. For example, here, because this warning was so old, I don’t see that we did any crater runs of impact (can that be true? I feel like I do remember some investigation?). However, for https://github.com/rust-lang/rust/pull/34982, we did a crater run, and then I made a point of ensuring that every affected crate was at minimum notified and, if possible, had a pending PR fixing the problem. I believe that for https://github.com/rust-lang/rust/pull/34923 we were similarly diligent.


#4

It’s already a pretty big warning anyways. I actually find it quite useful during development that it’s just a warning and not an error. It tends to happen every once in a while and the fact that it does not immediately make the build fail I consider a nice feature.


#5

I also think it makes sense to revert this and rethink how to go forward on it. There’s no need to be hasty here.


#6

Thanks for digging through everything @eddyb.


#7

Abandoned projects maybe? private_in_public was reported as a warning since last autumn, i have hard time imagining how a project maintainer can compile his project and look at these warnings for so long and never silence them with #[allow(private_in_public)].[quote=“mitsuhiko, post:4, topic:3930”] I actually find it quite useful during development that it’s just a warning and not an error. [/quote] It’s still a warning, and it seems like it will proceed being a warning for a long time, it’s just deny-by-default now, no downstream projects affected and all that.


#8

Analysis of private_in_public errors:

  • 5 crates - private traits in bounds / where clauses - these are supposed to go away if RFC 1671 is accepted.
  • 10 crates - public reexports of private extern crates - this was fixed more recently (February 2016).
  • 12 crates - other legit private-in-public errors reported since December 2015.

#9

As the first crate in the listing (ugh, sorry!), Encoding 0.3 was in the development since early this year (I think) and I had a fix to the warning in the issue list but didn’t realize that the 0.2 branch would have the same problem. This is also partly because I haven’t set the warning to fail the CI test—but if I had a separate branch for 0.2 I wouldn’t be able to see that as new Rust releases do not trigger CI anyway. (So, essentially, same to a pre-1.0 situation.)

I’ve taken this incident as an opportunity to actually apply the fix to 0.3 branch and also published a quick fix as 0.2.33. Still have to think about the long-term maintenance though…


#10

@petrochenkov we talked about this in the core-team meeting. I think the conclusion was that we don’t feel comfortable with rolling out the private_in_public-deny change this way. We’d prefer to revert that change for the beta (since it’s an easy thing to do) and then we can have a separate discussion (i.e., now…) about precisely how we roll it out (do we need to notify crate authors? submit patches? should we just give up on making this a hard error altogether?)

Would you be up for making that PR?

One thing I don’t know is whether we think we should roll it back just on beta, or also on nightly (@brson, @alexcrichton, your thoughts on that?)


#11

I’d say on both nightly and beta. Reset things to the way they were just in case. But I think with the right preparation it’s reasonable to expect we’ll be confident to deploy to stable nov 10.


#12

Yeah I agree we should submit to nightly and we can then cherry-pick that to beta to fix both branches.


#13

OK. denying by default can be discussed again after deciding on RFC 1671. I’ll probably start sending PRs to affected crates in the meantime.


#14

Thanks. Sorry for the confusion, still feeling our way on this sort of thing!


#15

The situation when a crate is fixed on GitHub, but the update is not published to crates.io turns out to be pretty common - about half of private-in-public regressions in the list. @brson in particular fixed stdx but never published stdx-0.0.2 :slight_smile:


#16

Hah. Well, stdx is completely useless, but I’ll put it on my list of projects for which I need to publish new builds.


#17

The syntax error in rust-s3 was caused by #34600. We would have to partially unfix #34543 to make this error into a warning – c.f. this comment.


#18

rust-s3 has been updated to work with beta/nightly.


#19

Thanks for the investigation @jseyfried.