Allow path versions for dev-dependencies

[sgrif]

The motivating use case for this change is to allow publishing of crates whose tests rely on code not published in tests, or where the dependency can't otherwise be represented. The current workaround today is to comment out the dev-dependency line for publishing, Two concrete use cases are:

• Test code that must be a separate crate, but doesn't have any reason to be published:
  • cargo has some proc-macros that it uses for its test suite. This crate has no reason to otherwise be published.
• dev-dependencies that can't otherwise be represented:
  • diesel is split into multiple crates, one of which provides proc-macros. Prior to 1.0, the crate diesel_codegen (since renamed to diesel_derives) had a dependency on diesel. diesel used diesel_codegen in its doctests. Since this would present a dependency cycle using published versions, this could not be represented without path dependencies.

The following is required for this feature to land:

• Determine if we ever use the dependencies table other than for UI purposes or determining reverse dependencies
  • If no, we do not need to store these path dependencies at all, as we do not want to use them for reverse dependencies or display them in the UI
  • If yes, we will need to determine how best to store these. We may want to store them in a separate table -- otherwise we'll need to change the dependencies table to support crates that don't actually exist. No checklist item for this yet, since the exact steps will depend on what we use it for, and my guess is we don't need to store them.
• When a dev-dependency has a path field and request.env() != Env::Production do not reject it. Do not store it in the database.
• Validate that the paths for dev-dependencies does not refer to a path outside the .crate file.
  • Example impls might be "is relative and no segment is .." or "path == path.canoncialize()"
  • This doesn't need to be particularly clever, as long as it's simple and has clear intent. The common use case is path = "foo" or path = "src/foo".
  • We should not do any checks beyond the path, looking at .gitignore or the exclude field of Cargo.toml is out of scope for crates.io
• Allow * version ranges if the dependency has a path field
• Document this at https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#development-dependencies or somewhere else
• Remove the feature flag, begin allowing this in production

[ehuss]

I'm not sure this will require any changes on crates.io, so I may be a little confused. Cargo can just filter these entries (it's already enforcing it).

[alexcrichton]

Sorry for being a bit late to this myself, I think I've missed a lot of discussion that's happened already. It's definitely not my fault for jumping in earlier, and I apologize for that!

@ehuss has a PR for mostly this at rust-lang/cargo#7333, and the Cargo team was discussing that PR yesterday to see if it's ready to merge. We were under the impression that the previous consensus between Cargo/crates.io was to not merge that PR exactly-as-is, but rather start down the road of "multi crate *.crate files" where if a path dependency didn't have a version it would be included into the *.crate file and published to crates.io. After that the logic would be that you could still write version for dev-dependencies and it would verify that it's on crates.io, but if you didn't write version the dev-dependency would be automatically bundled to crates.io. Additionally, I believe the previous consensus was largely that you should have to go out of your way to publish a crate with tests that don't work on crates.io as sort of a guiding principle.

I'd like to personally push back on this and specifically attempt to make a case for the PR exactly as-is on Cargo. The PR's current implemented behavior looks like:

• If a [dev-dependency] has a version, it behaves exactly as today. The path is dropped, it's verified to exist on crates.io, and it's published to crates.io with the dependency.
• If a [dev-dependency] does not have version (and is path) then it's removed from the manifest that crates.io knows about during publication. This means Cargo/crates.io would silently allow dev-dependencies without version markers when publishing.

I personally feel that this is the ideal state to be in for publishing with [dev-dependencies]. The case for running tests from *.crate files sourced from crates.io is definitely not a use case of Cargo, and I'm led to believe the two main users of this are Crater and linux distributions running tests. I personally believe that these do not justify making the experience for publication harder for all developers by default.

One of the primary reasons I feel this way is that cargo test is virtually never the way to actually fully test a crate. I think of the hundreds of crates I maintain maybe one or two of them simply run cargo test in CI. Everything else either has specific tool dependencies in some form, there's features to toggle, environment variables to run slow tests, system services to set up, etc. While it's certainly nice that most crates run most tests via cargo test, this in no way provides 100% coverage, which neither Crater nor linux distributions AFAIK are going for.

There's a number of assorted other reasons that crates may not work with cargo test from *.crate files, such as include/exclude, depending on a repository file existing which is out of the publication tree, or things like that. These factors today already prevent cargo test from being run successfully. When adding this all up even today cargo test has a number of reasons it may not succeed, and while some require opt-in (include/exclude) others will implicitly by default prevent tests from being run (depending on system services, out-of-tree files, etc).

All of that basically adds up to that anything to do with dev-dependencies I don't think really changes the needle at all. All of crater's tests are best effort, and in general crater as a whole is simply best effort. I suspect that linux distributions are the same way in that they're best effort. Authors can always publish crates with versions in dev-dependencies to make these two use cases work, but I don't think we should place the onus on all authors to cater to these two use cases.

I'm curious what those in the previous discussions think about all this? Do y'all have thoughts on the state of the current PR on Cargo?

[carols10cents]

I was convinced by @joshtriplett's thoughts re: crater, so I defer to Josh.

[Mark-Simulacrum]

Coming over here form the Cargo PR; I was personally swayed by the argument that cargo test doesn't test everything anyway. I would like to get crater into a better place of still comparing build-pass without tests (i.e., without dev-deps being needed) for those crates where that's needed due to them not building their tests, but that seems like something that's largely up to crater, not crates.io/cargo teams.

So, though I'm not on either team so no need to block on me, but I am removing my concern regardless.

[sgrif]

We discussed this in the team meeting today. It looks like the implementation that Cargo is going with does not require any changes on our part regardless of the restrictions that are done. We came to the consensus that we're fine with deferring to the Cargo team on whatever decision they come to over this.