rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.37k stars 12.72k forks source link

regression: evcxr tests fail #61566

Closed Mark-Simulacrum closed 5 years ago

Mark-Simulacrum commented 5 years ago

The code here is, I believe, attempting to perform some form of eval-like operation on Rust code and specifically is now generating evcxr_variable_store.put_variable::<i32>(stringify!(a), a); instead of evcxr_variable_store.put_variable::<String>(stringify!(a), a); which is making the tests in this crate fail on beta.

pnkfelix commented 5 years ago

I'll look at this.

davidlattimore commented 5 years ago

Thanks for pointing this out. It's quite likely that there's some implementation detail I'm depending on. i.e. whatever rustc changed that caused this to break is quite possibly a reasonable thing to have changed. I'll try to have a look over the weekend

davidlattimore commented 5 years ago

Looks like the changes that caused evcxr problems were both changes in --error-format=json. 1) JSON errors are now written to stdout instead of stderr. 2) The JSON content changed. Looks like it got wrapped in a new outer object. e.g. before:

{
    "message": "the trait bound `std::string::String: std::marker::Copy` is not satisfied",
    "code": {
        "code": "E0277",
    }
    ...
}

After:

{
    "reason": "compiler-message",
    "package_id": "...",
    ...
    "message": {
        "message": "the trait bound `std::string::String: std::marker::Copy` is not satisfied",
        "code": {
            "code": "E0277",
        }
    ...
   }
}

Both these changes are easy enough to work with.

davidlattimore commented 5 years ago

Correction, it seems that rustc's behaviour is unchanged, but Cargo now intercepts the JSON errors emitted by rustc on stderr, wraps them in its own JSON and re-emits them on stdout. Observed by running the following command in a crate with an error in it. cargo +beta rustc -- --error-format=json

I've pushed a new version of evcxr that makes it tolerant of these changes

pnkfelix commented 5 years ago

Sounds like this is fixed with evcxr version 0.3.4, in that the crate was relying on a detail of the JSON error format that was not meant to be treated as stable.

https://github.com/google/evcxr/blob/master/RELEASE_NOTES.md#version-034

Thanks for looking into this and the quick follow-up, @davidlattimore

Closing as not-a-bug.