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

Use stable libtest #165

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

gnzlbg
Copy link

@gnzlbg gnzlbg commented Mar 20, 2019

WIP: this uses my branch of stable libtest, that will be merged as part of rust-lang/libtest#11 . Once that is merged I'll update this PR to use libtest 0.0.2, but I wanted to make sure that CI here works fine first.

@mati865
Copy link
Contributor

mati865 commented Mar 20, 2019

error: pretty-printing failed in round 0 revision None
status: exit code: 1
command: "rustc" "-" "-Z" "unpretty=expanded" "--target" "x86_64-unknown-linux-gnu" "-L" "/tmp/compiletestGdXCMX/macro.stage-id.pretty.aux" "-L" "target/debug" "-L" "target/debug/deps"
stdout:
------------------------------------------
------------------------------------------
stderr:
------------------------------------------
error: the option `Z` is only accepted on the nightly compiler
------------------------------------------
thread '[pretty] pretty/macro.rs' panicked at 'explicit panic', /home/travis/build/laumann/compiletest-rs/src/runtest.rs:2632:9

@Munksgaard
Copy link
Collaborator

Our session-types library also broke due to the recent changes in compiletest. Your changes seem to fix some of the issue, but I'm still getting the following errors when trying to compile with this branch:

error[E0658]: `cfg(target_vendor)` is experimental and subject to change (see issue #29718)                       
    --> /home/munksgaard/.cargo/git/checkouts/libtest-ed9c19a14a678411/584b2b3/src/lib.rs:1052:9                  
     |                                                                                                            
1052 |     all(target_vendor = "fortanix", target_env = "sgx")                                                    
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^                                                                         
                                                                                                                  
error[E0658]: `cfg(target_vendor)` is experimental and subject to change (see issue #29718)                       
    --> /home/munksgaard/.cargo/git/checkouts/libtest-ed9c19a14a678411/584b2b3/src/lib.rs:1289:13                 
     |                                                                                                            
1289 |         all(target_vendor = "fortanix", target_env = "sgx")                                                
     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^                                

That's on a stable build by the way.

@mati865
Copy link
Contributor

mati865 commented Mar 21, 2019

@Munksgaard on which stable version?
cfg_target_vendor was stabilized in 1.33.0.

@Munksgaard
Copy link
Collaborator

Munksgaard commented Mar 21, 2019

@mati865, you're right, I was using an old stable version. After updating to 1.33.0 everything is dandy with compiletest_rs = { git = "https://github.com/gnzlbg/compiletest-rs", branch = "stable_libtest" } in my Cargo.toml. Thanks for pointing that out :-)

Copy link
Collaborator

@laumann laumann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removes the tester crate entirely, if I understand correctly?

@@ -1,6 +1,6 @@
[package]
name = "compiletest_rs"
version = "0.3.20"
version = "0.4.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't do version bumps in this commit, I do that separately.

@@ -2,13 +2,10 @@ language: rust
matrix:
include:
- rust: nightly
env: FEATURES=""
env: FEATURES="--features unstable"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about nightly for the feature name?

extern crate rustc;

#[cfg(unix)]
extern crate libc;
extern crate libtest as test;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the rename to libtest?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't want it to try and link with the local test crate that is unstable and broken now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But an as import will never change what gets linked. So why not keep extern crate libtest as test?

@Manishearth
Copy link
Owner

Is this ready to land? The PR description still says WIP.

@Manishearth
Copy link
Owner

Oh, the other PR needs to merge

@RalfJung
Copy link
Contributor

RalfJung commented Apr 7, 2019

Ping, any update on this?

@Manishearth
Copy link
Owner

We're currently reverting the libtest changes in Rust, but the path forward will likely involve landing this eventually, keeping it open for now.

@RalfJung
Copy link
Contributor

RalfJung commented Aug 4, 2019

What is the status of this? We just ran into another compiletest feature that hasn't been synced here yet (and it's been in rustc for more than 2 years).

@gnzlbg
Copy link
Author

gnzlbg commented Aug 5, 2019

Someone needs to replace compiletest in rust-lang/rust with this crate to avoid the duplication. I don't think anybody is working on that.

@RalfJung
Copy link
Contributor

RalfJung commented Aug 5, 2019

Interesting, thanks! Some time ago I was told that someone is working on it and it would happen right after the next Rust release. Oh well.

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 this pull request may close these issues.

None yet

6 participants