cargo icon indicating copy to clipboard operation
cargo copied to clipboard

dependencies warning control, overriding path heuristic

Open ijackson opened this issue 5 years ago • 8 comments

Describe the problem you are trying to solve

I am working on a project with upstream dependencies, some of which I have had to edit locally. So (pending an upstream release, and sometimes even pending locally committing the upstream code) I have edited my Cargo.toml to have a path dependency on the upstream.

However, the upstream has a number of warnings from rustc.

When I compile the same crate as a non-path dependency, cargo gets the version from crates.io - which has the same warnings - but the warnings are suppressed. Evidently cargo treats the use of a path dependency as an indication that I am a developer of the dependency and therefore want to see the warnings. (I failed to find a discussion of this in the cargo documentation.)

Describe the solution you'd like

This path dependency heuristic is a good rule of thumb. Usually it will be right. But it would be good if there were a way to override it.

I suggest an additional entry in the dependency, alongside the path key. propagate_warnings maybe. The default would be false for non-path dependencies, and true for path dependencies, but it could be overridden by the depending crate.

Warnings would be shown to the user if all of the dependency links from the toplevel to the relevant place had propagate_warnings. (I haven't checked but presumably this is what cargo does already, only just checking for path dependencies.)

In my scenario this would mean that I would see the warnings if I ran cargo build in the directory of my dependency, but not in the directory of my own project. That seems right to me.

Notes

It seems that a git dependency suppresses the warnings. So I could use a git dependency instead, as a workaround. In my situation this is less than ideal, because it means I must always be sure to commit all my edits to the dependency. For another user it might well be useful to enable the warnings.

Another possibility would be some kind of global configuration to specify which crates to print warnings for. That would be independently useful but it would be less helpful in my specific situation.

ijackson avatar Jul 25 '20 18:07 ijackson

Have you tried using the [replace] or [patch] sections instead of a path dep? They may work better for the use case of fixing an upstream bug.

Eh2406 avatar Jul 25 '20 20:07 Eh2406

@Eh2406 [patch] with a path dependency will also issue warnings.

ehuss avatar Jul 25 '20 20:07 ehuss

How many things can I get wrong in one day? :-P

Just ignore me. Sorry for the noise.

Eh2406 avatar Jul 25 '20 21:07 Eh2406

It might be reasonable to simply not warn for [patch] dependencies (or [replace]). Whenever I've used that I've never really wanted warnings and almost always get a lot to deal with anyway.

alexcrichton avatar Jul 28 '20 17:07 alexcrichton

I'd like to take this on. Any pointers to how and where cargo decides to suppress warnings when building dependencies?

Edited to add I never actually did start on this and don't plan to now.

rtsuk avatar Aug 10 '21 16:08 rtsuk

Yay, thanks for looking at this. I'm afraid I don't have any tips for navigating the inside of cargo. But I can say what the behaviour appears to be:

Basically, path dependencies are treated like the toplevel crate and have warnings enabled. Warnings are suppressed for dependencies specified as git= and for crates.io. I assume that this is done transitively, so that in principle if there's a git dependency which has path dependencies within the git tree, the warnings are suppressed for those too. crates.io doesn't allow path dependencies; IDK what happens if you specify a registry that contains things that look like path dependencies. (I assume cargo must do some laundering here or there might be a security risk.)

IDK what [patch] and [replace] do.

Overall it appears that this "you see warnings" thing is a property of a dependency edge. Currently, just from the type of the edge. @alexcrichton is suggesting that [patch] and [replace] should be treated differently. I am suggesting an ability to explicitly control the warnings-propagation-ness of a dependency edge (in the depending package, where the dependency is specified).

I hope this is of some use...

ijackson avatar Aug 10 '21 20:08 ijackson

The decision to show warnings is controlled by show_warnings. However, whether or not something comes from [patch] is lost by the time it is needed, so I think adding that support will be hard. Unfortunately I don't have the time to look into it in more detail.

ehuss avatar Aug 20 '21 19:08 ehuss

I just encountered this and found this behavior surprising. Since I was doing no local patching of the dependency, the suggestions regarding [patch]/[replace] aren't helpful to me.

My naive expectation is that the transition from crates.io dependency -> remote git dependency -> local path dependency should be doable without any change to the warnings that are printed.

ericseppanen avatar Aug 18 '22 19:08 ericseppanen

Just found this issue and would definitely appreciate some workaround for this, if only so that it's easier to develop a change for a dependency before submitting a PR upstream. Thank you for everyone's work on such a great system!

mattklein123 avatar Jun 29 '23 14:06 mattklein123

See. https://github.com/rust-lang/cargo/issues/12115#issuecomment-1546337961

Yes, one of the future possibilities listed in the RFC is for cargo to have configurable lints.

weihanglo avatar Jun 29 '23 14:06 weihanglo