Skip to content

Update to a more recent rocket and rust toolchain#205

Merged
elegaanz merged 1 commit into
Plume-org:masterfrom
lthms:recent_rocket
Sep 8, 2018
Merged

Update to a more recent rocket and rust toolchain#205
elegaanz merged 1 commit into
Plume-org:masterfrom
lthms:recent_rocket

Conversation

@lthms
Copy link
Copy Markdown
Contributor

@lthms lthms commented Sep 7, 2018 •
edited
Loading

So, I have made some progress tonight, but unfortunately, this has become way more complicated than initially expected. The main reason is pretty simple: rocket has introduced some serious breaking changes related to the Uri type, and I am not familiar enough unfortunately to fix them easily.

I push this so you folks can have a look, maybe with enough familiarity this is trivial to fix.

Beware that this PR uses my forks of rocket_{i18n,csrf}. They are basically working, AFAICT at least, but I wanted to have a working Plume before creating PR there.

So, yeah. TL;DR: I tried and failed, but maybe what I achieved will help you?

To do before merging:

  • Fix the warnings
  • PR to rocket_i18n
  • PR to rocket_csrf

lthms
lthms commented Sep 7, 2018
View reviewed changes
Comment thread Cargo.toml Outdated
[dependencies.rocket_csrf]
git = "https://github.com/fdb-hiroshima/rocket_csrf"
rev = "896fcaf14bd85b3f8266c0201e5f61937d05aec9"
git = "https://github.com/lthms/rocket_csrf"
Copy link
Copy Markdown
Contributor Author

@lthms lthms Sep 7, 2018

Choose a reason for hiding this comment

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

So yeah, I just want to emphasize once again, just to be clear, that this is meant to be temporary, until my changes (or equivalent changes) are pushed to the rightful upstream.

@lthms
Copy link
Copy Markdown
Contributor Author

lthms commented Sep 7, 2018 •
edited
Loading

Related commits:

@elegaanz
Copy link
Copy Markdown
Member

elegaanz commented Sep 8, 2018

Thanks a lot for your help! Looks like a lot of errors are happening because of rwf2/Rocket@56c6a96, so adding .into() after uri! invocations should fix it.

@lthms
Copy link
Copy Markdown
Contributor Author

lthms commented Sep 8, 2018

I will try to do that, if that’s okay with you.

@elegaanz
Copy link
Copy Markdown
Member

elegaanz commented Sep 8, 2018

Yes, of course! If you have any question about the code, you know you can ask me 😉

@lthms
Copy link
Copy Markdown
Contributor Author

lthms commented Sep 8, 2018 •
edited
Loading

I’ve pushed a new commit that fixes the remaining errors. There remains a lot of warnings, though.

@lthms
Copy link
Copy Markdown
Contributor Author

lthms commented Sep 8, 2018

Some warnings are due to a change in rustc that affects diesel. The crate has already been updated, but not released.

@elegaanz
Copy link
Copy Markdown
Member

elegaanz commented Sep 8, 2018

@lthms Could you please use the git repository directly in the Cargo.toml then, with a pinned rev? We will go back to a stable version once it will be released.

@lthms lthms mentioned this pull request Sep 8, 2018
Merged
@lthms
Copy link
Copy Markdown
Contributor Author

lthms commented Sep 8, 2018

I think the PR of interest is diesel-rs/diesel#1787

So we need to depend on at least this revision. Unfortunately, I don't know if there are pending issues in current diesel master branch, so I have to admit I am not sure which revision to choose…

@elegaanz
Copy link
Copy Markdown
Member

elegaanz commented Sep 8, 2018

Another solution could be to ignore these warnings for the moment… https://github.com/rust-lang/crates.io/pull/1469/files

@lthms lthms mentioned this pull request Sep 8, 2018
Merged
@lthms
Copy link
Copy Markdown
Contributor Author

lthms commented Sep 8, 2018

Once Plume-org/rocket_csrf#1 is merged, I will update Cargo.toml to use the upstream rocket_csrf, squash my commits and I think it will be okay, at least from my side.

@lthms
deps: Update to a more recent rocket and rust toolchain
0ef4717 
With this patch, Plume will be use a more up-to-date revision of
Rocket, that works with nightly-2018-07-17. It may have been able to
make it work with a more recent revision, but it turns out rocket has
introduced several breaking changes so I’d rather fix those.

Besides updating rocket_i18n and rocket_csrf to use the same revision
than Plume, this patch deals with the new implementation of the
Uri<'_> type. It silents a class of warnings, to deal with a change in
rustc which affects diesel. This latter change should be reverted as
soon as diesel releases a new version of its crate.
@lthms lthms changed the title wip: Update to a more recent rocket and rust toolchain Update to a more recent rocket and rust toolchain Sep 8, 2018
@lthms
Copy link
Copy Markdown
Contributor Author

lthms commented Sep 8, 2018

From my perspective, this PR is now ready. I don’t think I have broken something, but I guess you never know :D.

@elegaanz elegaanz merged commit fe7f87c into Plume-org:master Sep 8, 2018
@lthms lthms deleted the recent_rocket branch September 8, 2018 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@trinity-1686a trinity-1686a trinity-1686a approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lthms @elegaanz @trinity-1686a