A dependency on path = "." should have a good error message

[Eh2406]

In a package foo a [dev-dependencies] foo = { path = "." } should be an error.
As there is no use case for it, and it does not do what most people expect.
There are not many people that will try this, and there are few configurations that don't error already. So I don't think we need to get too fancy with the comparison. A check for a path "." would be a big step foreword.

[Eh2406]

cc #9245 and #3921 (comment)

[Nemo157]

Will probably fix #9454 too (by making it an error).

[yerke]

@rustbot claim

[yerke]

Does anyone have an example of a good error message we want to show in this case?

[Eh2406]

I don't have a good suggestion, but how does this sound:
"a project can not depend on itself as a path dependency"

[yerke]

@Eh2406 Great, thanks! It's better than "Something went wrong" I planned to use as a placeholder :)

[daxpedda]

This does have a good use case, it's a workaround for #2911 and works as intended as far as I've worked with it and tested it. I left a comment (#2911 (comment)) explaining why, I think, there was a misconception around it not working as intended.

[davidhewitt]

@daxpedda - OTOH, we learned recently in PyO3 that using path = . dependency to enable features required for tests is brittle. cargo will strip the path dependency when packaging, so users who download the .crate from crates.io later will no longer be able to run cargo test without enabling all those features.

[daxpedda]

so users who download the .crate from crates.io later will no longer be able to run cargo test without enabling all those features

Thanks for the heads up, I'm not really concerned though if tests work with cargo test through downloads from crates.io. In any case, I agree that this "feature" should be removed eventually.

[davidhewitt]

TBH I reached the same conclusion in discussion on PyO3/pyo3#1817

Having a way to specify features which are required for cargo test to work meaningfully is very convenient, and this self-dev-dependency trick definitely meets that need. I'd likewise be happy to replace it in PyO3 with something better, however that something better needs to be designed first...

[yerke]

Hi @daxpedda. You left a comment in #9702 saying that dependency on . should not be a hard error, but from your last comment in this issue I thought you agree that this should be removed.

@Eh2406 Do we have a decision on whether my PR is wanted or not? cc @ehuss and @alexcrichton as they are reviewing the PR in question.

[daxpedda]

I agree that it should eventually become an error, but currently it provides a workaround for a problem that has no other solution. Namely #2911.

Until there is a solution for this problem or a different workaround, I believe this should not become a hard error.

[davidhewitt]

We were able to remove the self-dev-dependency in PyO3 by using the CARGO_PRIMARY_PACKAGE env var, by using if option_env!("CARGO_PRIMARY_PACKAGE").is_some() to enable code paths only relevant for testing.

If CARGO_PRIMARY_PACKAGE were also set when executing build.rs then crates would be able to enable their own features / cfg based on this env var. I imagine this would be a more-than-sufficient alternative to #2911 (comment), thus enabling path = "." to become an error.

[Eh2406]

@yerke let me start off by apologizing. You deserve a prompt and knowledgeable response from me. This response is certainly not prompt and not as thorough as I would like it to be. In the time since I posted this issue, all the context has gone out of my head. I am doing my best to reconstruct it now.

For dependencies and build-dependencies a path="." already errors because it must create an infinite loop in the build order.
However things are more complicated for dev-dependencies. There always exists an implicit dev-dependency from the binarys/tests on the lib. The question is whether the path="." syntax unifies with that implicit edge or if it makes a new path package.
From my reading (but I have not gone back to test) different versions of Cargo have been inconsistent on the behavior of this syntax over time. Some versions it errored as building 2 incompatible versions of the same library, or it built it twice, or it unified.
Currently different aspects of cargo seem to have different opinions about what this means. (For example #9454, if you rename the path="." dependency it gets ignored and not marked as duplicate.) Generally tracking down these bugs and figuring out what is the correct behaviour is not worth the effort. Hence this issue to just ban the syntax.

On the other hand it happens to be that with resolver="2" the bugs line up to work ok as a workaround for #2911. Until a better work around is found (CARGO_PRIMARY_PACKAGE) or a permanent fix is implemented (rust-lang/rfcs#3020) one can argue that we should let it be.

I will bring it up at the next Cargo Team meeting, and get back to you.

[yerke]

@Eh2406 Thank you very much for getting back to me and explaining the context.

[Eh2406]

I did bring it up at last week's Cargo meeting and then forgot to write the update here 🤦 , Sorry.

If CARGO_PRIMARY_PACKAGE were also set when executing build.rs

build.rs can set rust features, but it can not turn on optional dependencies. So this is also only a workaround.

We agreed that we do want to ban this syntax, eventually. Given that the most pressing bugs so far reported is #9454 (not very pressing) and that people are using it to work around #2911 we decided to not make it a hard error at this time. This decision can and will be re evaluated when better alternatives are available or we find more pressing bugs.

@yerke your PR is significantly more robust then the code I had in mind. Thank you for your contribution! When we ban this behavior, it will be based on your code. I am sorry I was slow and confusing in my communications, and that I left E-help-wanted on something it turned out we were not ready to do.