Skip to content

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

@Eh2406

Description

@Eh2406
Contributor

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.

Activity

added
A-diagnosticsArea: Error and warning messages generated by Cargo itself.
on May 27, 2021
Eh2406

Eh2406 commented on May 27, 2021

@Eh2406
ContributorAuthor
Nemo157

Nemo157 commented on May 28, 2021

@Nemo157
Member

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

changed the title [-]A dependacy on `path = "."` shuld have a good error mesege[/-] [+]A dependacy on `path = "."` shuld have a good error message[/+] on May 28, 2021
changed the title [-]A dependacy on `path = "."` shuld have a good error message[/-] [+]A dependency on `path = "."` should have a good error message[/+] on Jun 22, 2021
yerke

yerke commented on Jun 25, 2021

@yerke
Contributor

@rustbot claim

yerke

yerke commented on Jun 25, 2021

@yerke
Contributor

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

Eh2406

Eh2406 commented on Jun 28, 2021

@Eh2406
ContributorAuthor

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

yerke

yerke commented on Jun 28, 2021

@yerke
Contributor

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

4 remaining items

yerke

yerke commented on Jan 9, 2022

@yerke
Contributor

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

daxpedda commented on Jan 9, 2022

@daxpedda
Contributor

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

davidhewitt commented on Jan 16, 2022

@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

Eh2406 commented on Jan 18, 2022

@Eh2406
ContributorAuthor

@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

yerke commented on Jan 18, 2022

@yerke
Contributor

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

Eh2406

Eh2406 commented on Jan 25, 2022

@Eh2406
ContributorAuthor

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.

added
S-triageStatus: This issue is waiting on initial triage.
on Nov 20, 2024
jonaspleyer

jonaspleyer commented on Jan 27, 2025

@jonaspleyer

Sorry to bother again with this Issue. I am currently having the following problem: I have forked the crates_io_api crate and modified it such that I can use it in a WASM-specific crate. In order to use this package for publishing crates, I need to wait for my PR to be accepted and a new version to be published to crates.io.

In the meantime, I have created the crates_io_api-wasm-patch crate which is the same package but with the patch. For some of the tests, the full path use crates_io_api::Something is used.

I can create a dev-dependency like so:

[dev-dependencies]
crates_io_api = { version = "0.12.0", package = "crates_io_api-wasm-patch"}

Which brings all tests (including doctests!) to successfull completion. However, this approach will always test the version 0.12 on crates.io rather than the local version. It is also not enough to use a git path here since testing would then always require pushing to the repository. Thus we need to specify something like this:

[dev-dependencies]
crates_io_api = { package = "crates_io_api-wasm-patch", path="./" }

This works for regular tests but not for doc-tests. Please let me know if there are other workarounds which I can use. I would like to avoid to rename all paths crates_io_api::Something in all files.

weihanglo

weihanglo commented on Jan 27, 2025

@weihanglo
Member

Have you tried [patch] and did it work?

jonaspleyer

jonaspleyer commented on Jan 27, 2025

@jonaspleyer

Have you tried [patch] and did it work?

I have just tried the following:

[dev-dependencies]
crates_io_api = { package = "crates_io_api-wasm-patch", git = "https://github.com/jonaspleyer/crates-io-api"}

[patch.'https://github.com/jonaspleyer/crates-io-api']
crates_io_api = { package = "crates_io_api-wasm-patch", path = "./" }

I inserted a panic!() inside the main get method which is used in (almost) every test. Most regular tests are failing but all doctests which also do contain this command are still running. So it seems to be equivalent to the git = "..." approach which I am currently using.

Eh2406

Eh2406 commented on Jan 29, 2025

@Eh2406
ContributorAuthor

I think you want something more like:

[dev-dependencies]
crates_io_api = { version = "0.12.0", package = "crates_io_api-wasm-patch"}

[patch.crates-io]
crates_io_api-wasm-patch = { path = "./path-to-your-checkout" }

Specifically, the [dev-dependencies] stays looking exactly like it's pulling from crates-io. The patch section says that it should instead come from path = "./path-to-your-checkout".

If you need more help using the patch section please try one of the user forums like https://users.rust-lang.org/

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

Metadata

Metadata

Assignees

Labels

A-diagnosticsArea: Error and warning messages generated by Cargo itself.S-triageStatus: This issue is waiting on initial triage.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    Participants

    @epage@Nemo157@daxpedda@davidhewitt@Eh2406

    Issue actions

      A dependency on `path = "."` should have a good error message · Issue #9518 · rust-lang/cargo