Skip to content
This repository has been archived by the owner on Jan 6, 2023. It is now read-only.

cargo: update users to 0.8 #35

Merged
merged 1 commit into from
Oct 17, 2018
Merged

Conversation

lucab
Copy link
Contributor

@lucab lucab commented Oct 15, 2018

No description provided.

@lucab lucab requested a review from sdemos October 15, 2018 11:19
@lucab
Copy link
Contributor Author

lucab commented Oct 15, 2018

/cc @sdemos for review

@sdemos
Copy link
Contributor

sdemos commented Oct 15, 2018

hmm that's odd, it looks like the api changed slightly. the string it's returning is from the ffi library. it looks like newer rust handles that correctly but the version we are using in CL doesn't?

@sdemos
Copy link
Contributor

sdemos commented Oct 15, 2018

actually it turns out that we are on rust 1.28 now (coreos/coreos-overlay#3378) so we should actually just bump the ci version and see if that fixes it

@lucab
Copy link
Contributor Author

lucab commented Oct 15, 2018

Ah, I didn't see this locally as I have a 1.29 (where this is fine). I'll check tomorrow and see if I manage to make it work on 1.26 too. My guess is that it may be rust-lang/rust#51178, which is new in 1.29 anyway.

@sdemos
Copy link
Contributor

sdemos commented Oct 15, 2018

alright, that makes sense. fwiw we will be bumping up to 1.29 at some point apparently per @dm0- (pending some eclass work), but it seems like the fix is probably as simple as changing it to "root".into(), so we can probably just do that.

@dm0-
Copy link

dm0- commented Oct 15, 2018

The CL Rust 1.29 upgrade is coreos/portage-stable#688 which depends on coreos/portage-stable#690. Assuming 690 gets reviewed this week, it will be in the alpha next Tuesday.

@dm0-
Copy link

dm0- commented Oct 15, 2018

The dependency PR was merged, and I'm running one last test build on the Rust update. CL should be on 1.29 tomorrow.

@lucab
Copy link
Contributor Author

lucab commented Oct 16, 2018

@sdemos @dm0- thanks both, but I prefer not to hard-require latest toolchain (that would haunt me back in rpm land, pretty sure). I've adjusted this PR so that it's green on 1.26. I didn't realize I was using some newer Rust features, glad that travis caught that!

Tailor is still dead tough.

Copy link
Contributor

@sdemos sdemos left a comment

Choose a reason for hiding this comment

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

that works for me. everything looks good.

@lucab lucab merged commit f38f3d5 into coreos:master Oct 17, 2018
@lucab lucab deleted the ups/cargo-update-users branch October 17, 2018 07:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants