Skip to content

First implementation of Rust 2018 modules.#332

Merged
emilio merged 2 commits intomozilla:masterfrom
Gankra:rust-2018-2
May 2, 2019
Merged

First implementation of Rust 2018 modules.#332
emilio merged 2 commits intomozilla:masterfrom
Gankra:rust-2018-2

Conversation

@Gankra
Copy link
Copy Markdown
Contributor

@Gankra Gankra commented Apr 30, 2019

This changes our implementation to always ignore extern crates, favouring
collecting metadata from cargo metadata (the Cargo.toml) and Cargo.lock.

Future work might be needed to make this more robust.

Closes #331

@Gankra
Copy link
Copy Markdown
Contributor Author

Gankra commented May 1, 2019

@hamchapman how's this? (I think I was accidentally relying on non-lexical lifetimes being turned on in nightly)

@hamchapman
Copy link
Copy Markdown

Yep, all working very nicely for me now! Thanks 👍

@Gankra
Copy link
Copy Markdown
Contributor Author

Gankra commented May 1, 2019

Confirmed that using this patch I can upgrade gfx/wr to rust 2018 and build firefox. Still need a real release to do a try build to test all configs properly, though.

Copy link
Copy Markdown
Collaborator

@emilio emilio left a comment

Choose a reason for hiding this comment

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

It looks sane. I'm a bit concerned about scanning the lockfile over and over, but if this is a problem in practice we can optimize it, and you'll get some nice build-time regression bugs to remind you of that :)


let mut output = vec![];

if let Some(dependencies) = dependencies {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: may be nicer to:

let dependencies = match dependencies {
    Some(deps) => deps,
    None => return vec![],
};

Then deindent everything below.

Gankra added 2 commits May 2, 2019 11:52
This changes our implementation to always ignore `extern crates`, favouring
collecting metadata from `cargo metadata` (the Cargo.toml) and `Cargo.lock`.

Future work might be needed to make this more robust.
Copy link
Copy Markdown
Collaborator

@emilio emilio left a comment

Choose a reason for hiding this comment

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

LGTM

@emilio
Copy link
Copy Markdown
Collaborator

emilio commented May 2, 2019

It'd be great if @eqrion does a pass over it or what not, but I think this is good to merge.

Thanks a lot @gankro!

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.

Plan For Refactor: Ignore Extern Crates Completely

3 participants