Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
RFC: Packages as (optional) namespaces #3243
RFC: Packages as (optional) namespaces #3243
Changes from 6 commits
e590c72
5e6ad7c
d9d1556
e29aa55
3a0b099
4422adf
24db227
1986c3d
42046f8
2fff101
8e667ce
01d37b2
4501047
1f62468
1ca3e62
f7be349
9a144fb
a578a67
3396a2e
11c6354
cc5872d
65f7083
5c497c5
67ef913
56fae2e
482fae8
4c14f9c
d9a3c90
8bd7fbc
abde93f
7d77485
75dd867
0db096f
6cc8886
a93b1e4
656203f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option for disambiguation here could be namespaced features. If namespaced crates did not automatically create an unnamespaced feature like current optional dependencies do, then you would have to use
dep:foo/bar
to refer to it andfoo/bar
would unambiguously refer to the featurebar
on cratefoo
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am rather partial to this approach; it has a bunch of benefits:
@ <ident> /
triggers parsing as a namespaced path) though this might be too large a syntax changeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think it would be ideal for the same syntax to be used both in Cargo.toml and in source code. I'd like to suggest the usage of
:::
but I think it has the same issue that led to...
becoming..=
.Can you expand on why
@foo::bar
might be too large a syntax change?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it changes what counts as an identifier in Rust: it's a change at the token level, which in turn affects proc macros and such.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(it doesn't have to be at the token level tbh, but that does still complicate how paths work)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to have @rust-lang/lang input on this as a syntax change
(if this RFC starts going down this route I'll of course add them as an approving team)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does
@
mean in npm's context?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way npm does package names in a package.json is like
@scope/package@version
.Where scope is a existing organization on npm and
@version
a published version.Though they also allow you to use Github repos as shown in the image
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please don't introduce new keywords. I like the idea of namespace, but please take a simple path, or don't implement it at all until you find one. rust is already complex enough, I'm affraid we have to write things like this in the future:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a character that is currently not possible within a crate name does avoid issues like a bunch of crates already being published with a name that want as a root. For example, if your org uses something like
foo_bar
andfoo_baz
but someone else already published foo, you can't use those names. But you could for something like@foo/bar
.That said, I see @ehuss's comment below that cargo has already closed on support for
::
. Does that effectively mean this RFC is decided?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its been approved by 2 of the 3 related teams and is just waiting on enough votes from the last team.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional prior art: Microsoft had to add https://learn.microsoft.com/en-us/nuget/nuget-org/id-prefix-reservation to NuGet, since otherwise a ton of people were publishing things under
Microsoft.
andSystem.
.One interesting thing it allows is delegation of subnamespaces. If applied here, that could mean that if
serde-
were reserved then they could delegateserde-json
to a particular team, if they wanted to curate-but-not-own some of the sub-packages.EDIT: Not intending to say that's the model this should use—I haven't formed a position yet—just think it's good prior art for the RFC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like a bad argument since C# works pretty differently for packages, with the difference between top-level packages and modules not being as clear. Systems which use domain-style namespaces are a different story, since for example it's not clear whether
Category.Thing
is a single dependency by itself, or ifThing
is part of theCategory
dependency.In Rust, the top-level name is the dependency, period. So, you can be confident that
serde::json
is ajson
module within theserde
crate, andserde_json
is a crate by itself. Reserving theserde_
prefix for all crates feels like it's solving a problem that the solution created, since the only notion of ownership is for the rootserde
crate itself.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the problem is that in rust a crate can't be splitted while still indicating verifiable common ownership of the resulting cluster of crates, and that it will also stay that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't everything be grandfathered into a default namespace like
crates::serde
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because the current set of crates is the top level namespace