strip rust src lines during normalization #247
Conversation
| self.numbers_to_hide_are_rust_src = is_rust_src; | ||
| self.hide_numbers = 0; | ||
| while let Some(next_line) = self.all_lines.get(index + self.hide_numbers + 1) { |
There was a problem hiding this comment.
It seems to me that there was an off-by-one error here causing self.hide_numbers to be one higher than it needed to be. That didn't matter while the operation was just trimming leading numbers, resulted in a noop in the last line, but removing whole lines was more destructive.
| @@ -32,8 +32,5 @@ error[E0599]: the method `to_cxx_exception` exists for reference `&NonError`, bu | |||
| which is required by `&NonError: ToCxxExceptionDefault` | |||
| note: the trait `std::fmt::Display` must be implemented | |||
| --> $RUST/core/src/fmt/mod.rs | |||
| | | |||
| | pub trait Display { | |||
| | ^^^^^^^^^^^^^^^^^ | |||
| = note: this error originates in the macro `::cxx::map_rust_error_to_cxx_exception` (in Nightly builds, run with -Z macro-backtrace for more info) | |||
There was a problem hiding this comment.
E.g. this = note: line would be removed without the off-by-one adjustment.
There was a problem hiding this comment.
Thanks for the PR!
I would prefer not to make this change. The notes with which rustc annotates the standard library source snippet are potentially interesting to test. For example in that "rust-lib.rs" test touched by this PR: I would want to find out if that "doesn't satisfy `std::net::Ipv4Addr: quote::to_tokens::ToTokens`" message changes. Rustc occasionally does nutty things like _::_quote::to_tokens::ToTokens (rust-lang/rust#46991).
Hopefully the rust-src install bug is fixable in rustup.
|
Understood, I can see the benefit of both sides of the coin. We've unblocked the PyO3 CI but only by reverting the prior advice from #126 (comment) and removing the Unfortunately this means that users running our trybuild tests will hit a necessary failure until they install the Is there a way you would accept this PR as an optional mode, perhaps by additional configuration to set on |
|
I am glad you were able to unblock the PyO3 CI with PyO3/pyo3#3575. Also, surprised that deleting rust-toolchain.toml was necessary, because I have been using
I would be interested to find out if we can find any relevant difference between PyO3's CI and mine that would be exacerbating rust-lang/rustup#988. |
This adds a new normalization to
trybuildto remove src lines which originate fromrust-src. Therustupdefault profile does not contain therust-srccomponent, so I argue this normalization is justified as it makes thetrybuildoutputs consistent with what a user with a default rust install would see.This is a follow-up to #126, which was resolved with a working alternative at the time.
For some context: As of this week, the PyO3 CI has ground to a halt, there is a bad interaction between
rustupand GitHub Actions runner images which is causingrust-srccomponent to fail to install, see rust-lang/rustup#3530While there are several solutions we can employ to workaround the issue, this option seems like a potential plus for the ecosystem while also removing our need to install
rust-src.