Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upAutomatically Usable External Crates #2088
Conversation
cramertj
added some commits
Jul 19, 2017
cramertj
added
the
T-lang
label
Jul 28, 2017
sfackler
reviewed
Jul 28, 2017
| # Cargo.toml: | ||
| name = "my_crate" | ||
| version = "0.1.0" | ||
| authors = ["Me" <me@mail.com>] |
This comment has been minimized.
This comment has been minimized.
sfackler
reviewed
Jul 28, 2017
| external dependency appears anywhere within the crate. | ||
| For example, if `rand = "0.3"` is listed as a dependency in Cargo.toml | ||
| and `extern crate rand;` appears somewhere in the crate being compiled, | ||
| then no implicit `extern crate rand;` will be added. |
This comment has been minimized.
This comment has been minimized.
sfackler
Jul 28, 2017
Member
Do we need this special case? It seems like an "if a tree falls in the forest" kind of thing.
This comment has been minimized.
This comment has been minimized.
cramertj
Jul 28, 2017
Author
Member
My personal preference would be that users avoid extern crate as much as possible. In a world with no extern crate, this rule doesn't really matter at all.
I know that there are other people who will disagree, though, and I think that this check will allow them to continue keeping a tight lock on where their external dependencies are used.
This comment has been minimized.
This comment has been minimized.
aturon
Jul 28, 2017
Member
I feel like this portion of the RFC is under-specified, and could also use a bit more analysis around rationale.
re: under-specification: "appears anywhere within the crate" is a bit ambiguous. Are we taking cfg into account? Macros?
re: rationale: you mention back-compat and wanting to control scoping. It might be useful to separate those concerns a bit. Is there something more minimal we could do strictly for back-compat?
This comment has been minimized.
This comment has been minimized.
vorner
Jul 29, 2017
I think it is actually impossible to check for „anywhere within the crate“. Or, if I have an example (file in examples) using the library and that one uses an external crate, does it count? How does the rustc compiling the library know that, without being informed about the examples? Or does a crate mean one unit of compilation, therefore the example would be considered a separate crate?
This comment has been minimized.
This comment has been minimized.
cramertj
Jul 29, 2017
Author
Member
By "anywhere within the crate" I meant "anywhere within the crate source currently being compiled after macro expansion" (including cfgs). This isn't perfect, but I think it's the only truly viable option. I suppose in order for macros to be imported correctly in a macros 2.0 world, you'd have to already know at least some set of external crates that you were going to import. I'll have to think about this-- my gut reaction is to say that external macros expanding to extern crate; could be made an error, but maybe that's too drastic / surprising.
Overall, I feel like I'm pretty open to suggestions on this front. My goal was to support backcompat and leave an "out" for people who had special use cases for extern crate-- perhaps I should have instead focused merely on backcompat.
This comment has been minimized.
This comment has been minimized.
vorner
Jul 29, 2017
Hmm. That dependency on cfgs, that can make a crate to auto-include or not, might be a source of unexpected failures on different configurations.
#[cfg(a_feature)]
mod submodule {
extern crate a_crate;
}
fn main() {
a_crate::do_something();
}This will stop compiling if the a_feature is activated, but otherwise will compile just fine. And the a_feature can be hidden somewhere deep.
I don't have any new proposal (I think there are some in the discussion), but I find this behaviour a bit complex and magical.
This comment has been minimized.
This comment has been minimized.
le-jzr
Jul 29, 2017
•
@cramertj For backcompat, just leave extern crate working as it does now. Even currently, it's not an error to have extern crate in the root and then also extern crate in several submodules. With the RFC, extern crate lib; and use lib; should be mostly interchangeable in their basic forms.
This comment has been minimized.
This comment has been minimized.
le-jzr
Jul 29, 2017
•
The motivation for extern crate anywhere disabling the feature is just about not polluting namespace. We've already established that the RFC shouldn't result in automatic inclusion in local namespace. It can possibly be further specified that the automatic path links can be shadowed by root modules (which would make the shadowed crates only accessible via extern crate, but that's not a problem, and it makes everything work backwards compatibly).
sfackler
reviewed
Jul 28, 2017
| - Don't do this. | ||
| - Specify external dependencies using only `extern crate`, rather than only | ||
| `Cargo.toml`, by using `extern crate foo = "0.2";` or similar. This would |
This comment has been minimized.
This comment has been minimized.
sfackler
Jul 28, 2017
Member
In the olden days, the pre-Cargo build tool behaved kind of like this, interestingly.
This comment has been minimized.
This comment has been minimized.
Stebalien
Aug 2, 2017
Contributor
We can do something even simpler. If cargo finds a missing dependency, it can offer to add the latest version from crates.io to Cargo.toml (pinning it to ^latestVersion).
sfackler
reviewed
Jul 28, 2017
| Creating a root-level item named `std` will prevent `std` from being included, | ||
| and will trigger a warning. | ||
| It will still be necessary to use the `extern crate` syntax when using |
This comment has been minimized.
This comment has been minimized.
sfackler
Jul 28, 2017
Member
I think this is actually kind of a drawback - if there's a long enough period where this RFC is implemented but macros 2.0 isn't, then newcomers to the language won't know about extern crate at all until they want to use a macro. Hopefully that gap won't be all that long though?
This comment has been minimized.
This comment has been minimized.
cramertj
Jul 28, 2017
Author
Member
Yes, it's definitely annoying. Since we do have a plan to migrate away from #[macro_use] in the relatively near future, I think it's livable, but the overlap period of "extern crate just for macros" is frustrating.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
pengowen123
Jul 29, 2017
pub extern crate could potentially be replaced with pub use crate. This could cause problems with having the same item imported twice though.
This comment has been minimized.
This comment has been minimized.
|
Big |
This comment has been minimized.
This comment has been minimized.
|
Not sure if it needs to be called out in the RFC specifically, but it'd be good to update the unused extern crate lint to to work for implicit extern crates as well so you know if you're depending on a crate you no longer use. |
This comment has been minimized.
This comment has been minimized.
|
Small nit-pick: I would use "deduce" instead of "infer" (or, alternatively, something along the lines of "automatic Inference, in its generality, would mean e.g. having In fact, it's "simply" a different way to specify dependencies (via the command-line), which we perhaps should have done a long time ago. We used to have extern crate rand = "*";
extern crate glutin = "https://github.com/tomaka/glutin#next";What we have right now in stable Rust is the most boring/verbose approach: a list of dependencies in |
petrochenkov
reviewed
Jul 28, 2017
| `extern crate` in order to have more fine grained control-- say, if they wanted | ||
| to import an external crate only inside an inner module. | ||
| No automatic import will occur if an `extern crate` declaration for the same | ||
| external dependency appears anywhere within the crate. |
This comment has been minimized.
This comment has been minimized.
petrochenkov
Jul 28, 2017
Contributor
Within the crate before macro expansion or after macro expansion? :)
With explicit extern crates crate names are injected into modules immediately and unconditionally, and can be used in macros before waiting for them to be expanded, so it's reasonable to make this check immediately before expanding anything as well.
On the other hand, if some macro expands to an extern crate item, then you'll have a conflict instead of an override.
(The same issue exists for "items such as modules, types, or functions that conflict with the names of implicitly imported crates will cause the implicit extern crate declaration to be removed")
This comment has been minimized.
This comment has been minimized.
petrochenkov
Jul 28, 2017
•
Contributor
It may be possible to reuse the rules from glob imports - they also "step aside" when conflict with explicitly written names and interactions with macros were already figured out in recent name resolution RFCs.
The "extern crate declaration for the same external dependency appears anywhere within the crate" rule doesn't fit into the glob import analogy though, but I'm not sure how useful it is.
This comment has been minimized.
This comment has been minimized.
|
Can RLS/racer/Intellij jump into the toml files on "Go To Definition"? If yes, than I can live with this. |
This comment has been minimized.
This comment has been minimized.
|
Please no. Big Furthermore, this will make the behaviour way worse for crates named With the change, when I want to read a small example, I won't be able to scan the I'd very much prefer to have inferred Cargo.toml, meaning having |
est31
reviewed
Jul 28, 2017
| `Cargo.toml`, by using `extern crate foo = "0.2";` or similar. This would | ||
| require either `Cargo` or `rustc` to first scan the source before determining | ||
| the build dependencies of the existing code, a system which requires fairly | ||
| tight coupling between a build system and `rustc`, and which would almost |
This comment has been minimized.
This comment has been minimized.
est31
Jul 28, 2017
Contributor
I don't think any "tight" coupling is needed for this: just add a mode to rustc which when called only scans for extern crate statements and dumps them via json. Surely the ride is a bit bumpy because deps don't exist yet so no proc macros or similar are supported, but this is not a "tight coupling". External build systems that want to support the new mode can just write a parser for that json code.
This comment has been minimized.
This comment has been minimized.
le-jzr
commented
Jul 28, 2017
This solution would be problematic when the external crate is used by multiple modules in a crate. You would either have to tag each with a version (which would complicate updating the version), or you'd need to single out one module as "the one true declaration", and have all others |
This comment has been minimized.
This comment has been minimized.
Does doing extern crate for the same crate in multiple modules work? Interesting. If it does, I'd just say that all others should
Note that I don't think that the current solution is bad in any way, its very nice in fact. My most preferred option would be to not do anything and leave stuff as is. I just said that I prefer implied |
This comment has been minimized.
This comment has been minimized.
|
Huuuuuuge thumbs up from me; this is a big win, IMO. |
This comment has been minimized.
This comment has been minimized.
|
I'm not a huge fan of this and my arguments go along the lines of @est31 (except wishing no inference in either way. I'd like to add that there's plenty of scenarios where the dependency list in cargo absolutely doesn't match the one used in the lib and the binaries. Changing this would make it unclear what is used in what. I like Rust for it's explicitness, even to the point where it is a little more verbose and I see a thrust towards becoming more and more implicit. I'm also unsure about the interaction with other build systems. Finally, I see how this could introduce a case where you can accidentally depend. I don't believe it's simpler and easier, just less verbose. |
This comment has been minimized.
This comment has been minimized.
le-jzr
commented
Jul 28, 2017
•
|
@est31 The irregularity of using IMO it would be best if |
This comment has been minimized.
This comment has been minimized.
|
@le-jzr I don't have a major problem with |
This comment has been minimized.
This comment has been minimized.
le-jzr
commented
Jul 28, 2017
•
|
Actually, ignore most of what I said. After some experimenting I'm forced to conclude I had incorrect understanding of how |
This comment has been minimized.
This comment has been minimized.
|
@skade More often then not though in Rust source code Really my only concern about this proposal is macro imports and the possible conflicts between naming there across crates if things are implicitly imported. |
This comment has been minimized.
This comment has been minimized.
le-jzr
commented
Jul 28, 2017
•
|
Just to be clear on what It's entirely module level, but it actually attaches the external crate as a private sub-module of the module it's used in? Is this the correct understanding? |
This comment has been minimized.
This comment has been minimized.
|
@mgattozzi To clarify, in macros 2.0, macro imports work just like normal items. You can |
This comment has been minimized.
This comment has been minimized.
dpc
commented
Jul 28, 2017
|
I am initially skeptical. I have one question. Are we currently being warned of unused |
This comment has been minimized.
This comment has been minimized.
|
@dpc there's currently a lint for unused extern crate which was recently turned form allow by default to warn by default. Not sure if it's on beta or just nightly though. I would very much like to see it updated to handle implicit crate usage as well - I've definitely had a couple of unused Cargo-level dependencies before by accident. |
This comment has been minimized.
This comment has been minimized.
|
+1, but I'd like ways to turn this off both in |
This comment has been minimized.
This comment has been minimized.
rpjohnst
commented
Jul 28, 2017
•
|
I like this idea because it matches Cargo in its convention-over-configuration approach. The vast majority of For reference, it also seems to go well with my proposal for minimal implicit modules. |
This comment has been minimized.
This comment has been minimized.
Yeah, I meant only the |
Vurich
referenced this pull request
Aug 5, 2017
Closed
Replace usages of ::std::env::temp_dir with tempdir crate in tests #6215
This comment has been minimized.
This comment has been minimized.
Vurich
commented
Aug 5, 2017
|
I really like @le-jzr's proposal, especially if you also have |
This comment has been minimized.
This comment has been minimized.
|
I want to propose an alternative to I'm a great fan of the concept of separating namespaces, and if this is done, I want to give this RFC my |
This comment has been minimized.
This comment has been minimized.
|
I've read the discussion in "Revisiting Rust’s modules, part 2" and now I agree that |
This comment has been minimized.
This comment has been minimized.
|
Please don't call it |
This comment has been minimized.
This comment has been minimized.
mathstuf
commented
Aug 11, 2017
|
So I haven't read every comment here, but I have skimmed and searched it. I like doing this inside of my root module: mod crates {
pub extern crate dep1;
pub extern crate dep2;
pub extern crate dep3;
pub extern crate dep4;
}So that when crate mismatches appear, it is obvious that I don't really care about the default as long as:
|
withoutboats
added
the
Ergonomics Initiative
label
Aug 14, 2017
This comment has been minimized.
This comment has been minimized.
CAD97
commented
Aug 15, 2017
|
@mathstuf see also #2108 which avoids some of the problems you're avoiding with that arrangement. I would also like to make sure that we can still import crates under a module; UNIC has a lot of large data tables that it exposes. Under the understanding that most users will only need a small subset of the available tables, logical groups are each exported as a separate crate. It is much better if, if you want to use It does not have to happen right now, and could be added later (and probably should go through its own RFC), but I would like to see the [dependencies]
unic_ucd_age = { version = "0.5", alias = "unic::ucd::age" }
unic_ucd_category = { version = "0.5", alias = "unic::ucd::category" }would result in the equivalent mod unic {
mod ucd {
extern crate unic_ucd_age as age;
extern crate unic_ucd_category as category;
}
} |
This comment has been minimized.
This comment has been minimized.
mathstuf
commented
Aug 15, 2017
|
I had actually just read through that RFC yesterday. IMO, these two together would satisfy my needs (though I'm not the biggest fan of autoimport, the explicit list should be sufficient for me as well). Not sure whether I'd like it as much if either landed without the other though… |
This comment has been minimized.
This comment has been minimized.
|
I'm strongly against this RFC, for all the reasons listed by @est31 in his various comments. If I see To me, this RFC is yet again one that optimises for writability rather than readability, I don't think that's a good thing. |
This comment has been minimized.
This comment has been minimized.
mathstuf
commented
Aug 21, 2017
This comment has been minimized.
This comment has been minimized.
|
@mathstuf Good point, but I have more concerns about that other RFC, so I decided to comment here about my gripes with that one in isolation of the other. |
This comment has been minimized.
This comment has been minimized.
Korvox
commented
Aug 21, 2017
|
@nox While I don't fully like the |
This comment has been minimized.
This comment has been minimized.
|
@Korvox this specific proposal only proposes to eliminate There has been an earlier proposal to mount automatically imported crates inside The modules RFC is still in flux and might drop the ambiguity fix altogether or might not be adopted. So I continue to |
This comment has been minimized.
This comment has been minimized.
rpjohnst
commented
Aug 23, 2017
|
The new modules RFC keeps the ambiguity fix, and it seems one of the least controversial parts of it. It also continues to cite this RFC as a major piece of itself. |
This comment has been minimized.
This comment has been minimized.
|
Some thoughts:
|
This comment has been minimized.
This comment has been minimized.
The ecosystem will be split anyway, because
I'm not pleased at all. The
I think there is a possibility to keep the general model of The two primary issues are:
(For reference, a bikeshedding thread with a poll for specific syntax - https://internals.rust-lang.org/t/poll-which-other-crate-relative-path-syntax-do-you-prefer/5744.) |
This comment has been minimized.
This comment has been minimized.
epdtry
commented
Aug 24, 2017
|
I'd like to see a clearer statement in the RFC text regarding the future of |
This comment has been minimized.
This comment has been minimized.
I think this part of the RFC should be removed. First, it is not clear what “phasing out” is intended to mean. An “unnecessary In other words, I’m disagreeing with @aturon’s comment at #2088 (comment). In general, just because we can turn a warning into a hard error in a new epoch/checkpoint doesn’t mean we should. I think “dislike of old things” shouldn’t be a sufficient reason. Edit: I should mention that, other than this detail, this RFC looks good and I’m if favor of doing this. |
This comment has been minimized.
This comment has been minimized.
I'd personally be okay with something along those lines. This seems highly related to module-system proposals to add a The one downside is that it makes paths to external items more verbose than under the current proposal. It's still an improvement on the current system, though, since one can just add |
This comment has been minimized.
This comment has been minimized.
#1400 can help with this as well (IIRC, it even was mentioned in some earlier "module reform" proposal). |
This comment has been minimized.
This comment has been minimized.
|
I'd definitely be less opposed to this RFC if all the implicit crates were put under a special namespace, and an implicit crate was only considered to be pulled in if and only if you referred to a symbol from the implicit crate. Also yes please to #1400, that's such a basic quality of life ergonomic improvement that I'm amazed we're wasting so much time on other things instead of pushing forward on nested use paths. |
This comment has been minimized.
This comment has been minimized.
rpjohnst
commented
Aug 24, 2017
|
If we're going to add a prefix to paths, I definitely prefer adding something like |
This comment has been minimized.
This comment has been minimized.
|
@rpjohnst let x = a::b::c;you still don't know if EDIT: Oh wait, non-
If "other languages" is C++ (which I think should be prioritized when deciding on syntactic compatibilities), then Rust is already consistent - both use |
This comment has been minimized.
This comment has been minimized.
rpjohnst
commented
Aug 25, 2017
•
|
That's not the comparison I was making. My point is that in C++, C#, Java, etc. absolute paths (whether they begin with In those languages, however, you don't usually wind up with both a library and a binary with the same "crate" name, while in Rust you often do. So, instead of using Putting |
This comment has been minimized.
This comment has been minimized.
|
Thanks everyone for the great discussion! I've learned a lot from the conversation here, and it's helped me to identify several of the key flaws in the original proposal. The reliance upon the In light of the lessons learned here, I'm closing this RFC in favor of @aturon's new paths and visibility RFC. This RFC is the latest and last iteration of the modules proposals. It takes a much more conservative, straightforward approach to cleaning up paths and visibility in Rust. Under the new proposal, Please take a look at the new proposal and leave your feedback there. |
cramertj commentedJul 28, 2017
•
edited
This RFC reduces redundant boilerplate when including external crates. With this change, projects using Cargo (or other build systems using the same mechanism) will no longer have to specify extern crate: dependencies added to Cargo.toml will be automatically useable. We continue to support extern crate for backwards compatibility with the option of phasing it out in future Rust epochs.
Rendered