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

Do not rename packages on `cargo new`. #5013

Merged
merged 2 commits into from Feb 6, 2018

Conversation

Projects
None yet
@withoutboats
Copy link
Contributor

withoutboats commented Feb 5, 2018

Prior to this commit, packages beginning with rust or ending with
rs were renamed automatically when created, unless they were
binaries. The ostensible purpose of this code was to avoid people
uploading "redundant" names to crates.io, which is a repository of
Rust packages.

This behavior was overly opinionated. It is not cargo's
responsibility to discourage users from naming their packages any
particular way. Without a sound technical reasons why packages
cannot be named a certain way, cargo should not be intervening in
users' package naming decisions.

It also did this by automatically renaming the package for the
user, as opposed to erroring. Though it printed a message about
the behavior, it did not give the user a choice to abort the
process; to overrule cargo they had to delete the new project
and start again using the --name argument.

cargo new is many users' first entrypoint to the Rust ecosystem.
This behavior teaches a user that Rust is opinionated and magical,
both of which are divisive attributes for a tool, and attributes
which do not generally describe Rust's attitude toward things like
names and formatting.

If crates.io wishes to enforce that users not upload packages with
names like this, it should be enforced by crates.io at publish
time.

@rust-highfive

This comment has been minimized.

Copy link

rust-highfive commented Feb 5, 2018

r? @matklad

(rust_highfive has picked a reviewer for you, use r? to override)

@withoutboats withoutboats requested a review from alexcrichton Feb 5, 2018

withoutboats added some commits Feb 5, 2018

Do not rename packages on `cargo new`.
Prior to this commit, packages beginning with `rust` or ending with
`rs` were renamed automatically when created, unless they were
binaries. The ostensible purpose of this code was to avoid people
uploading "redundant" names to crates.io, which is a repository of
Rust packages.

This behavior was overly opinionated. It is not cargo's
responsibility to discourage users from naming their packages any
particular way. Without a sound technical reasons why packages
cannot be named a certain way, cargo should not be intervening in
users' package naming decisions.

It also did this by automatically renaming the package for the
user, as opposed to erroring. Though it printed a message about
the behavior, it did not give the user a choice to abort the
process; to overrule cargo they had to delete the new project
and start again using the `--name` argument.

`cargo new` is many users' first entrypoint to the Rust ecosystem.
This behavior teaches a user that Rust is opinionated and magical,
both of which are divisive attributes for a tool, and attributes
which do not generally describe Rust's attitude toward things like
names and formatting.

If crates.io wishes to enforce that users not upload packages with
names like this, it should be enforced by crates.io at publish
time.

@withoutboats withoutboats force-pushed the withoutboats:no-rust-rename branch from 1bd3f60 to e101b92 Feb 5, 2018

@matklad

This comment has been minimized.

Copy link
Member

matklad commented Feb 6, 2018

Funny enough, I myself was once frustrated by “you can’t name package lang-x” error, when I was learning some language (Clojure perhaps? I don’t remember).

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 6, 2018

📌 Commit e101b92 has been approved by matklad

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 6, 2018

⌛️ Testing commit e101b92 with merge a997689...

bors added a commit that referenced this pull request Feb 6, 2018

Auto merge of #5013 - withoutboats:no-rust-rename, r=matklad
Do not rename packages on `cargo new`.

Prior to this commit, packages beginning with `rust` or ending with
`rs` were renamed automatically when created, unless they were
binaries. The ostensible purpose of this code was to avoid people
uploading "redundant" names to crates.io, which is a repository of
Rust packages.

This behavior was overly opinionated. It is not cargo's
responsibility to discourage users from naming their packages any
particular way. Without a sound technical reasons why packages
cannot be named a certain way, cargo should not be intervening in
users' package naming decisions.

It also did this by automatically renaming the package for the
user, as opposed to erroring. Though it printed a message about
the behavior, it did not give the user a choice to abort the
process; to overrule cargo they had to delete the new project
and start again using the `--name` argument.

`cargo new` is many users' first entrypoint to the Rust ecosystem.
This behavior teaches a user that Rust is opinionated and magical,
both of which are divisive attributes for a tool, and attributes
which do not generally describe Rust's attitude toward things like
names and formatting.

If crates.io wishes to enforce that users not upload packages with
names like this, it should be enforced by crates.io at publish
time.
@matklad

This comment has been minimized.

Copy link
Member

matklad commented Feb 6, 2018

Hm, this also reminds me about “cargo new should create a binary by default”. Have we actually decided something about that issue? The discussion seems inconclusive: https://internals.rust-lang.org/t/should-cargo-new-default-to-applications/3772/27.

IntelliJ has been defaulting to “—bin” for more than a year now, and it is one of the few places where IntelliJ overrides Cargo defaults.

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 6, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: matklad
Pushing a997689 to master...

@bors bors merged commit e101b92 into rust-lang:master Feb 6, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@nox

This comment has been minimized.

Copy link
Contributor

nox commented Feb 7, 2018

I liked this feature and I'm kinda surprised this was accepted so fast. The change seems even more opinionated than the behaviour it removed.

@matklad

This comment has been minimized.

Copy link
Member

matklad commented Feb 7, 2018

@nox could you elaborate why do you think this feature is useful?

To me, arguments "Cargo should not scare new users by silently doing something different" and "validation should be strict and belong to crates.io" seem very compelling.

cc @rust-lang/cargo

EDIT: to make it clear, I don't think we need any validation here, but, if we indeed chose to forbid some names, we need to forbid them for real.

@nox

This comment has been minimized.

Copy link
Contributor

nox commented Feb 7, 2018

Validation on crates.io only moves the frustration to even later, when the code compiles and runs but suddenly you can't publish because your package is rejected. What is the purpose of having a rust- prefix in a crate name? Do we know whether not doing that anymore would be less or more confusing than keeping the current behaviour?

And more generally, where do we draw the line for breaking changes (this is one) landing without discussion (AFAIK)?

@matklad

This comment has been minimized.

Copy link
Member

matklad commented Feb 7, 2018

What is the purpose of having a rust- prefix in a crate name?

I think there's a high chance that the very first crate a new Rust programmer creates is rust-hello-world or something similar. At least, my very first crate was named rustraytracer :-)

And I am certainly sure that not silently renaming the crate in some cases would be a less confusing behavior? Like, this just removes special case, so it must be less confusing?

And more generally, where do we draw the line for breaking changes (this is one) landing without discussion (AFAIK)?

Why do you think this is a breaking change? Like, it certainly is a slight behavioral change, and, technically, any change can be considered breaking, but I don't think anyone really depends on cargo new rust-foo giving a warning?

As I considered it a "minor, non breaking change" and I found the motivation very compelling (I've hit a similar problem in some other language, so I might be biased), I decided that "two people think that this is a good idea" is a strong enough consensus in this particular case.

I certainly would want to back out this change if it indeed breaks some code, or if other folks are sure that the current behavior is better :-)

@nox

This comment has been minimized.

Copy link
Contributor

nox commented Feb 7, 2018

@matklad

This comment has been minimized.

Copy link
Member

matklad commented Feb 7, 2018

Hm, so I looks like I was wrong in my assessment of this change as "minor and non breaking" :-(

Let's see what other folks from @rust-lang/cargo think!

Thanks for calling this out, @nox!

@carols10cents

This comment has been minimized.

Copy link
Member

carols10cents commented Feb 7, 2018

I am in favor of this change, I see this as a bug that produces unexpected behavior that we are fixing. This is aimed at eliminating frustration for new users; existing users know enough to name their directories rust-foo and their packages foo by hand.

I am also against adding validations to crates.io preventing rust-* named crates from being published; I don't think it's a problem.

@nox

This comment has been minimized.

Copy link
Contributor

nox commented Feb 7, 2018

(This change should at least be listed in the next release's changelog, if it stays.)

@matklad matklad added the relnotes label Feb 7, 2018

@aturon

This comment has been minimized.

Copy link
Member

aturon commented Feb 7, 2018

@nox, fwiw, this change was discussed with the entire Cargo team, but was not deemed significant enough to merit going through a full RFC process. The teams need the ability to make small decisions without huge process overhead.

@joshtriplett

This comment has been minimized.

Copy link
Member

joshtriplett commented Feb 7, 2018

I'm in favor of this change.

However, I would propose that we announce it in This Week in Rust while it's still in nightly, and give people plenty of time to offer feedback before it becomes stable.

Also, we might want to warn about names that crates.io won't accept, as a heads-up.

@withoutboats

This comment has been minimized.

Copy link
Contributor

withoutboats commented Feb 7, 2018

I also agree with Carol that crates.io should not reject these names either. My comment about crates.io was that that on upload is the correct place to put this validation if we actually wanted to have it.

@withoutboats

This comment has been minimized.

Copy link
Contributor

withoutboats commented Feb 7, 2018

Hm, this also reminds me about “cargo new should create a binary by default”. Have we actually decided something about that issue? The discussion seems inconclusive: https://internals.rust-lang.org/t/should-cargo-new-default-to-applications/3772/27.

I think this is also a good idea, for the same reason I laid out in 2016 - new users are much more likely want bin, and users who are not new know how to solve the problem. If we want to make that change, it would be best to bundle these changes in the same release.

@daboross

This comment has been minimized.

Copy link
Contributor

daboross commented Feb 14, 2018

If we change the default to creating a binary, could we add this back in as a warning when making a library crate?

I don't think this matters at all when making a binary, but it seems useful to remind that 'rust-' is redundant when someone starts a new library.

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Feb 14, 2018

If we do add a warning, it could remind that the package name for crates.io doesn’t have to be the same as the repository name. For example, https://crates.io/crates/url is not prefixed but https://github.com/servo/rust-url is.

@stbuehler

This comment has been minimized.

Copy link

stbuehler commented Feb 14, 2018

I don't see how it is useful to anyone if the crate names have rust/rs prefixes/suffixes.
I don't want those prefixes/suffixes neither on crates.io nor in my binary names...

Using those prefixes/suffixes in repository/folder names on the other hand is clearly useful.

(I'd also prefer if crates.io would enforce package name == crate name. Hi there, xml-rs!)

@WiSaGaN

This comment has been minimized.

Copy link
Contributor

WiSaGaN commented Feb 21, 2018

I would probably feel OK either way if this starts as the default. This change though creates confusion for existing users by asking user to notice and relearn the default after several years without significant benefit.

I feel that there are increasingly more changes that do not take into account the cost of changing widely accepted behavior. This will frustrate users that want a "boring" and predictable language since they will constantly see surprises in the new compiler version, which requires them to follow closely to what the community does or read every release notes, and adapt existing tool and documentation constantly.

@matklad matklad referenced this pull request Feb 21, 2018

Merged

New defaults to bin #5029

@alexcrichton alexcrichton referenced this pull request Mar 22, 2018

Merged

1.25 announcement #237

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment