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

Update to rust master #1085

Merged
merged 17 commits into from
Dec 25, 2014
Merged

Update to rust master #1085

merged 17 commits into from
Dec 25, 2014

Conversation

geomaster
Copy link
Contributor

rust-lang/rust#19253 and rust-lang/rust@25f8051 have introduced changes
to the namespacing within the std::collections::hash_map, breaking some
of Cargo code which imported these.

rust-lang/rust@cf350ea, implementing changes proposed by RFC #344, have
also broken some code which relies on hash_set::SetItems (now renamed to
hash_set::Iter).

This PR fixes the incompatibilities: imports of
std::collections::hash_map::{Occupied, Vacant} have been replaced by
imports of std::collections::hash_map::Entry::{Occupied, Vacant} and one
instance where the SetItems has been used was replaced by the proper
usage of Iter.

rust-lang/rust#19253 and rust-lang/rust@25f8051 have introduced changes
to the namespacing within the std::collections::hash_map, breaking some
of Cargo code which imported these.

rust-lang/rust@cf350ea, implementing changes proposed by RFC rust-lang#344, have
also broken some code which relies on hash_set::SetItems (now renamed to
hash_set::Iter).

This commit fixes the incompatibilities: imports of
std::collections::hash_map::{Occupied, Vacant} have been replaced by
imports of std::collections::hash_map::Entry::{Occupied, Vacant} and one
instance where the SetItems has been used was replaced by the proper
usage of Iter.
@alexcrichton
Copy link
Member

Cargo actually builds with a pinned version of the rust compiler, so you'll also have to update the date listed in src/rustversion.txt as well (you can see if travis failed/passed to see if it works). Thanks for this!

@geomaster
Copy link
Contributor Author

@alexcrichton My rustc reports rustc 0.13.0-dev (2f3cff695 2014-12-22 17:11:37 +0000) for --version. Do I just write 2014-12-22 into src/rustversion.txt? Pardon my ignorance, this is the first time I'm contributing to a project on here so I'm kind of clueless :)

Latest changes require that we build with a later Rust nightly
@alexcrichton
Copy link
Member

Yep, that should do it!

@geomaster
Copy link
Contributor Author

Okay, bumping the version triggered errors in Cargo's dependencies caused by the same breaking change. I fixed these locally, so I will submit pull requests to these libraries and we have to wait for them to be merged in order for this to build. Also, this triggers a compiler bug, further preventing the build in rustc that seems to be fixed by rust-lang/rust@c5aaa8c but is not yet in the 2014-12-22 nightly. So in any case we have to wait.

@alexcrichton
Copy link
Member

If you add this to Cargo.toml locally you should be able to get past the compiler bug:

[profile.dev]
debug = false

I wouldn't commit that to this PR, but it should help out working locally!

@geomaster
Copy link
Contributor Author

It seems that the issues in docopt/docopt.rs and servo/rust-url that prevented them from building on the latest rustc have been resolved. Fixing Cargo.toml seemed easy enough, but the registry library requires curl-rust, which in turn demands rust-url 0.2.0, and since rust-url doesn't have a release that accompanies the required commit (servo/rust-url@13be975), Cargo fetches 0.2.6 for curl-rust which is an older release that doesn't build on current rustc. Also the compiler bug still keeps Travis unhappy, not only here but on most other Rust projects that trigger it. What should I do now?

@alexcrichton
Copy link
Member

Today's nightly has fixed the debuginfo ICE and I think a number of crates have all been updated, perhaps today's nightly could work?

Updated to use the latest 2014-12-23 nightly and newest versions of
crates url and docopt.rs
@geomaster
Copy link
Contributor Author

What does this mean?

failed to select a version for `url` (required by `cargo`):
all possible versions conflict with previously selected versions of `url`
version 0.2.6 in use by url v0.2.6
possible versions to select: [0.2.7]

On my machine it fetches the right version of that library but I bomb against an error in docopt.rs:

/home/geomaster/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/docopt-0.6.18/src/lib.rs:486:41: 486:54 error: attempt to bound type parameter with a nonexistent trait `StrAllocating`
/home/geomaster/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/docopt-0.6.18/src/lib.rs:486                where I: Iterator<S>, S: StrAllocating {

Fixing this (it was caused by rust-lang/rust@9b99436) I am greeted with:

/home/geomaster/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/docopt-0.6.18/src/parse.rs:385:30: 386:47 error: if and else have incompatible types: expected `fn(collections::vec::Vec<parse::Pattern>) -> parse::Pattern {parse::Optional}`, found `fn(collections::vec::Vec<parse::Pattern>) -> parse::Pattern {parse::Sequence}` (expected fn item, found a different fn item)
/home/geomaster/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/docopt-0.6.18/src/parse.rs:385                     let mk = if self.cur() == "]" { Optional }
/home/geomaster/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/docopt-0.6.18/src/parse.rs:386                              else { Sequence };
/home/geomaster/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/docopt-0.6.18/src/parse.rs:390:35: 390:38 error: mismatched types: expected `&u8`, found `collections::vec::Vec<parse::Pattern>` (expected &-ptr, found struct collections::vec::Vec)
/home/geomaster/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/docopt-0.6.18/src/parse.rs:390                             Ok(mk(seq))
                                                                                                                                 ^~~
/home/geomaster/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/docopt-0.6.18/src/parse.rs:390:29: 390:40 error: mismatched types: expected `core::result::Result<parse::Pattern, collections::string::String>`, found `core::result::Result<u8, _>` (expected enum parse::Pattern, found u8)
/home/geomaster/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/docopt-0.6.18/src/parse.rs:390                             Ok(mk(seq))
                                                                                                                           ^~~~~~~~~~~
<std macros>:5:9: 5:22 error: mismatched types: expected `&u8`, found `collections::vec::Vec<parse::Pattern>` (expected &-ptr, found struct collections::vec::Vec)
<std macros>:5         xs.into_vec()
                       ^~~~~~~~~~~~~
<std macros>:1:1: 8:2 note: in expansion of vec!
/home/geomaster/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/docopt-0.6.18/src/parse.rs:393:35: 393:58 note: expansion site
/home/geomaster/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/docopt-0.6.18/src/parse.rs:393:29: 393:59 error: mismatched types: expected `core::result::Result<parse::Pattern, collections::string::String>`, found `core::result::Result<u8, _>` (expected enum parse::Pattern, found u8)
/home/geomaster/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/docopt-0.6.18/src/parse.rs:393                             Ok(mk(vec!(Alternates(alts))))

I'm not sure which change causes this, I'm not very fluent with Rust, so any help would be appreciated.

As per rust-lang/rust@b04bc5c, move all Encodable and Decodable deriving
modes to RustcEncodable and RustcDecodable, and also extern the
rustc-serialize crate instead of the former serialize one.
As per rust-lang/rust@f8cfd24, rename references to slice::Items to
slice::Iter and slice::MutItems to slice::IterMut
As per rust-lang/rust@9b99436, the return type of from_utf8 has been
changed from Option to Result. Consequently, update code which relied on
this return type to work with Ok(...) and Err(...) instead of Some(...)
and None
rustc complains that we cannot assign two different fn's to a single
object within an if-else block. I was not able to figure out what causes
this; so deferring the execution of the particular function is a
reasonable fallback. Not very elegant and should be changed to something
with less boilerplate
Bump to newest versions of several libraries
Copy lev_distance.rs verbatim from Rust since str::lev_distance has been
deprecated by rust-lang/rust@9b99436 with no replacement and change code
to use it instead of the deprecated function
Apparently .repeat() for str and .into_string() have been deprecated,
replace them with suggested alternatives
@geomaster
Copy link
Contributor Author

I have updated the code so it now works with all the new breaking changes in Rust, with no deprecation warnings. However, some tests fail on the CI server. If you have any clue as to why this could have happened, assistance would be welcome.

@@ -201,7 +201,6 @@ fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package,
// a real job. Packages which are *not* compiled still have their jobs
// executed, but only if the work is fresh. This is to preserve their
// artifacts if any exist.
let job = if compiled {Job::new} else {Job::noop};
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to keep this up here if possible (keep the logic duplication to a minimum). You should be able to keep it up here with something like:

let job = if compiled {
    Job::new as fn(Work, Work) -> Job
} else {
    Job::noop as fn(Work, Work) -> Job
};

@geomaster
Copy link
Contributor Author

Thanks, will fix tomorrow, it's 6AM here now so kinda time to go to bed. What concerns me more are these two failed tests. I'm not sure if it's a regression caused by using a newer rustc or I screwed up something.

@alexcrichton
Copy link
Member

It looks like these tests are failing because of a bug in rustdoc. Feel free to update the expected output to what rustdoc currently outputs, I'll just be sure to update it back once rustdoc is fixed!

And again, thanks so much for this!

As suggested by @alexcrichton, reverted back to the old way of handling
both Job::new and Job::noop that avoids ugly constructs and code
duplication
Two tests (test_with_deep_lib_dep and lib_with_standard_name) fail due
to a bug in rustdoc (see rust-lang/rust#20183) so temporarily changing
these so they pass. Be sure to change it back when the fix lands in the
current rust nightly
@geomaster
Copy link
Contributor Author

All done. The CI builds pass. You may want to perform some more review to ensure that I didn't do anything erroneous (I am again saying that the stuff in these commits is the first Rust code I've ever written so be extra careful), but oh well, it compiles fine, the tests pass, the changes were pretty much heavylifting with no much thought involved, so I guess it can be merged. The commits are a bit of a clusterfuck so some rebasing and renaming has to be done. If anything needs to be fixed still just let me know.

@alexcrichton
Copy link
Member

Awesome work, thanks so much!

bors added a commit that referenced this pull request Dec 25, 2014
rust-lang/rust#19253 and rust-lang/rust@25f8051 have introduced changes
to the namespacing within the std::collections::hash_map, breaking some
of Cargo code which imported these.

rust-lang/rust@cf350ea, implementing changes proposed by RFC #344, have
also broken some code which relies on hash_set::SetItems (now renamed to
hash_set::Iter).

This PR fixes the incompatibilities: imports of
std::collections::hash_map::{Occupied, Vacant} have been replaced by
imports of std::collections::hash_map::Entry::{Occupied, Vacant} and one
instance where the SetItems has been used was replaced by the proper
usage of Iter.
@bors bors merged commit c55a992 into rust-lang:master Dec 25, 2014
@geomaster
Copy link
Contributor Author

Great! My first pull request ever - merged! :) Happy holidays @alexcrichton and everyone else, thank you for the wonderful work you did on Rust and related projects!

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.

3 participants