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

Support normalization of $SRC_DIR #198

Closed
jebrosen opened this issue Nov 6, 2019 · 2 comments · Fixed by #244
Closed

Support normalization of $SRC_DIR #198

jebrosen opened this issue Nov 6, 2019 · 2 comments · Fixed by #244

Comments

@jebrosen
Copy link

jebrosen commented Nov 6, 2019

Rocket's CI has been broken for a while because of UI test diffs like this:

 error[E0277]: the trait bound `usize: rocket::response::Responder<'_>` is not satisfied
-  --> $DIR/responder-types.rs:38:13
-   |
-38 | fn foo() -> usize { 0 }
-   |             ^^^^^ the trait `rocket::response::Responder<'_>` is not implemented for `usize`
-   |
-   = note: required by `rocket::handler::<impl rocket::Outcome<rocket::Response<'r>, rocket::http::Status, rocket::Data>>::from`
+   --> $DIR/responder-types.rs:38:13
+    |
+38  | fn foo() -> usize { 0 }
+    |             ^^^^^ the trait `rocket::response::Responder<'_>` is not implemented for `usize`
+    | 
+   ::: /Users/runner/runners/2.159.2/work/1/s/core/lib/src/handler.rs:202:20
+    |
+202 |     pub fn from<T: Responder<'r>>(req: &Request<'_>, responder: T) -> Outcome<'r> {
+    |                    ------------- required by this bound in `rocket::handler::<impl rocket::Outcome<rocket::Response<'r>, rocket::http::Status, rocket::Data>>::from`

The new annotations appear to be introduced by rust-lang/rust#64151, in which $SRC_DIR has been used to normalize similarly added paths in the output (e.g. https://github.com/rust-lang/rust/pull/64151/files#diff-4b63aaabf76aa70969c04a1a4185c421R7).

$SRC_DIR normalization was added to rustc in rust-lang/rust#50943 but it doesn't appear to be supported in compiletest - this means that it is not currently possible for these errors to be properly tested both in CI and locally, as each run ends up with files in a different path.

If the same change from rust-lang/rust#50943 is likely to be usable (and accepted) here, I'd be happy to make that pull request.

@Manishearth
Copy link
Owner

Manishearth commented Nov 6, 2019 via email

@jebrosen
Copy link
Author

jebrosen commented Nov 9, 2019

It turns out it's more complicated than that. Copying the code used in rustc's version of compiletest doesn't work for me for two reasons:

  • We pass a relative path for src_base. This is in principle easy to fix by converting to an absolute path somewhere, but I'm wary of canonicalize for Windows adding \\?\ when it doesn't appear in error messages.
  • The parent of the parent of src_base is a suitable root in rustc, because it contains everything relevant. In rocket, the "root" ends up being another directory or two up. Using parent-of-parent would require projects using compiletest to make sure their directory layout fits.

I've made the change in the https://github.com/jebrosen/compiletest-rs/tree/normalize-srcdir in case it's useful to someone, but I think this is the wrong approach for me to pursue here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants