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

Pin unicode-normalization for Rust 1.33 CI build #572

Closed
wants to merge 1 commit into from

Conversation

mbrubeck
Copy link
Contributor

See #566 for details.

@o0Ignition0o
Copy link
Contributor

o0Ignition0o commented Dec 10, 2019

In order to not have MaybeUninit I think we want to have precise flags on smallvec as well as on unicode-normalization

I have just tested cargo update --p unicode-normalization --precise 0.1.9 && cargo update -p smallvec --precise 0.6.12 && cargo test --all-features --all with rustc 1.33.0 and it seemed to work on my machine

Edit:
I have just tested cargo update --p unicode-normalization --precise 0.1.9 && cargo test --all-features --all with rustc 1.33.0 and it worked as well

@mbrubeck
Copy link
Contributor Author

Any smallvec 0.6 version is compatible with Rust 1.33, so pinning unicode-normalization to 0.1.9 should be sufficient.

I'm not sure why the failing Travis build is using unicode-normalization 0.1.11. The script is working correctly locally...

@o0Ignition0o
Copy link
Contributor

o0Ignition0o commented Dec 10, 2019

Oh that's weird indeed, I was pretty confident smallvec version = "0.6.13" carried a backport of maybeuninit.

Sorry for the noise
Edit: I might be miss reading or missunderstanding this diff

@mbrubeck
Copy link
Contributor Author

smallvec 0.6.13 uses the maybe-uninit crate which is compatible with Rust 1.20 and later.

@o0Ignition0o
Copy link
Contributor

Oh ok, thanks for pointing it out to me ! :)

@mbrubeck
Copy link
Contributor Author

I think the previous error was caused by rust-lang/cargo#6904. Trying a workaround.

@o0Ignition0o
Copy link
Contributor

o0Ignition0o commented Dec 10, 2019

And it passed ! 🎉
So if I get your force push correctly, cargo update actually caused a cargo.lock to be created, which works around the bug you refered to ?

@mbrubeck
Copy link
Contributor Author

So if I get your force push correctly, cargo update actually caused a cargo.lock to be created, which works around the bug you refered to ?

Right.

@SimonSapin
Copy link
Member

Thanks for the PR but this approach feels dissatisfying to me. I think I’d prefer either to have this requirement in idna’s Cargo.toml, or to stop testing on 1.33 and do 1.36 instead. I’m still not sure which is best, though.

SimonSapin added a commit that referenced this pull request Jan 2, 2020
… because `unicode-normalization` did.

Fixes #566
Closes #572
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