Conventions

[matklad]

r? @alexcrichton

I'd love to refactor our handing of inferring targets by convention, because it is super difficult to understand, and quite probably contains a couple of unintended/undocumented conventions (like src/foo.rs being a binary foo if there's no library in the package).

As a first step, I've just moved all the logic (600 loc) to a separate file from the toml.rs file, which used to be just huge (we even use in in IntelliJ Rust for performance testing :) ), and now it is "only" just above 1kloc :)

[matklad]

Ok, the next bunch of commits does some refactorings and deduplication. The main achievement so far is that we've got rid of normalize function which used a ton of impenetrable high order closure.

Also, some error messages are cleaned up a bit.

[matklad]

And the next bunch of commits actually makes some algorithmic changes:

• we no longer use different algorithms for inferring whole targets and for inferring a path for the target, of which only the name is know.

• we now warn about some conventions, which presumably shouldn't be conventions at all, but slipped due to the usage of two different algortithms from the previous bullet. This actually caused some churn in tests, because they used old conventions.

[alexcrichton]

This is looking great to me, thanks so much @matklad! Mind detailing the deprecation warnings here as well? (just in a comment). I've tagged this relnotes now but just want to make sure we know what to put in the release notes!

[matklad]

Mind detailing the deprecation warnings here as well? (just in a comment)

Sure! There are several cases, which I think are "accidental" and are deprecated here

• if you have a [lib] name = "foo" in your Cargo.toml, we treated src/foo.rs as a crate root. The fix is either to set path or to rename src/foo.rs to src/lib.rs

• if you don't have a library, but you have [bin] name = "foo", we treated src/foo.rs as a (binary) crate root. The fix is to add path or to move to src/bin/foo.rs.

• if you have a [bin] name = "bar", but your package's name is foo, we still treated src/main.rs or src/bin/main.rs as a bar's crate root, and now we warn. If binary names is the same as the package itself, then the behavior stays the same.

At least, these are all cases I intended to warn about, there maybe some unexpected stuff though!

However, almost all the changes to tests were to silence the warnings (eg, they compiled OK, but failed in with_stderr).

There was a single edge case where there is a genuine regression (and I really should have written about it before). In the tests, there was [lib] name = "foo" pattern, but src/foo.rs was generated by the build script from src/foo.rs.in. This happens because in "legacy mode" we check that file actually exists, before producing a warning, to avoid false positives. And in this case it's not really there, because the build script was not run yet. I ... don't know how to fix this neatly, but imo this is a really edgy edge case, and it was intended to be deprecated long ago:

cargo/tests/build.rs

Line 990 in 04d8a13

// this is testing that src/<pkg-name>.rs still works (for now)

:-)

[alexcrichton]

Ok that all sounds good to me and this seems like a great refactoring, so let's...

@bors: r+

[bors]

📌 Commit 71e6629 has been approved by alexcrichton

[bors]

⌛ Testing commit 71e6629 with merge 1566c92...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 1566c92 to master...