Cargo emits confusing errors when trying to use example crates

[mejrs]

Problem

Cargo emits confusing errors when trying to use example crates

Steps

When you have an example crate, and define it in the main crate's cargo.toml with

[[example]]
name = "example1"
path = "examples/example1"

commands like cargo check --examples gives useless errors like:

• error: couldn't read examples/example1: Access is denied. (os error 5) on Windows,
• error: couldn't read examples/example1: Is a directory (os error 21) on Ubuntu/MacOS

Possible Solution(s)

It would be nice if this would Just Work. But otherwise it would be friendlier to nudge users to something like

[[example]]
name = "example1"
path = "examples/example1/src/main.rs"

rather than emitting useless error messages

Notes

No response

Version

cargo 1.57.0 (b2e52d7ca 2021-10-21)
release: 1.57.0
commit-hash: b2e52d7cab0a286ee9fcc0c17510b1e72fcb53eb
commit-date: 2021-10-21
host: x86_64-pc-windows-msvc
libgit2: 1.3.0 (sys:0.13.23 vendored)
libcurl: 7.79.1-DEV (sys:0.4.49+curl-7.79.1 vendored ssl:Schannel)
os: Windows 10.0.19043 (Windows 10 Home) [64-bit]

[epage]

I've accepted this assuming:

But otherwise it would be friendlier to nudge users to something like

[[example]]
name = "example1"
path = "examples/example1/src/main.rs"

rather than emitting useless error messages

[TanmayArya-1p]

I'd like to take a go at this. I'm new to contributing to cargo but i'd like to try working on this one. @rustbot claim

[epage]

As a heads up

• We'll expect tests to be written. We tend to do end-to-end tests. For manifests, we tend to put error cases in the misnamed bad_config.rs
• We'll want this to cover all relevant build target types
• We've found it helpful to write tests first, with them passing, showing the current behavior. Then do the feature work on a follow up commit. This makes it easy to see how behavior changed through the diff. For more, see https://doc.crates.io/contrib/process/working-on-cargo.html#submitting-a-pull-request

[TanmayArya-1p]

Turns out I made a major oversight trying to solve this issue :(

My initial approach ( #16329 ) was to add a simple validation check within each of the to_target functions in targets.rs that verify if the path specified is a valid source file.
I soon realised that these to_target functions are executed on every run of cargo regardless of what's being built. This means that a simple cargo build that has nothing to do with examples or tests will still call the to_target functions on each and every TomlTarget including all example and test targets.
This causes cargo build to fail when trying to build crates fetched from the registry without an /examples directory but with explicitly specified examples in their manifest. This is also the reason why the CI fails in my PR.

So as i see it, the paths of targets in the manifest need to be validated lazily if and only when the user requests their compilation.
As far as i know, one way of doing this is validating each root Unit from the BuildContext generated here after parsing the command line options and the manifest. The same could also be done inside of BuildRunner.

There might be a much simpler way to solve this that I don't see. Would love to hear feedback from others.
Also, I'd like to rework on this issue again once an approach is decided to solve this. @rustbot unclaim

[epage]

I'd recommend looking for where the error is reported and check the cases there.

[TanmayArya-1p]

 error: couldn't read examples/example1: Is a directory (os error 21)

After doing some digging I discovered that, this error originates from rustc. The build targets are being passed on by the BuildRunner without being validated in Cargo all the way till Executor::exec where it calls rustc.
I feel like we need to validate each Target somewhere earlier in the pipeline, BuildRunner seems to be somewhat an appropriate place for this. If it's ok, I'll try implementing this and test this in my fork to see how it behaves in the CI.

[epage]

Sure