non-blocking build error reported in example code since 0d62ae2 about cargo HOT 7 OPEN

@Muscraft Here is a test demonstrating it:

#[cargo_test]
fn invalid_name_in_git_repo() {
    // Checks to ignore invalid package name in git repository.
    let git_project = git::new("bar", |p| {
        p.file("Cargo.toml", &basic_manifest("bar", "1.0.0"))
            .file("src/lib.rs", "")
            .file(
                "dont_look_at_me/Cargo.toml",
                r#"
                    [package]
                    name = "{{project_name}}"
                    version = "1.0.0"
                    edition = "2021"
                "#,
            )
            .file("dont_look_at_me/src/lib.rs", "")
    });
    let p = project()
        .file(
            "Cargo.toml",
            &format!(
                r#"
                    [package]
                    name = "foo"
                    version = "1.0.0"
                    edition = "2021"

                    [dependencies]
                    bar = {{ git = "{}" }}
                "#,
                git_project.url()
            ),
        )
        .file("src/lib.rs", "")
        .build();
    p.cargo("fetch")
        .with_stderr(
            "\
[UPDATING] git repository `[..]`
error: invalid character `{` in package name: `{{project_name}}`, \
the first character must be a Unicode XID start character (most letters or `_`)
 --> [..]/dont_look_at_me/Cargo.toml:3:28
  |
3 |                     name = \"{{project_name}}\"
  |                            ^^^^^^^^^^^^^^^^^^
  |
[LOCKING] 2 packages
",
        )
        .run();
}

I would generally expect it to ignore packages it cannot parse, unless it is unable to find the requested package. Errors are supposed to be deferred here and not shown unless it fails. Perhaps the new diagnostics aren't getting accumulated?

from cargo.

Can you share the repo where this happened or create an example? It would help in debugging what is going on.

from cargo.

Sorry, unfortunately it's a private corp repo :/ I may be able to do an example, tho I am not quite sure what all causes this...
Give me a little bit and I'll see what I can do.

from cargo.

@Muscraft Here is a test demonstrating it:

Thanks so much, that's better and faster than what I would've done!

from cargo.

So from what I understand of this situation is that this is superficial though misleading in a very negative way.

As for the change. We switched from reporting the error exclusively through Result to printing the error immediately and then using a special error to say that we already reported it. This means that when we try to capture and ignore the error, we don't fail but we still print.

In terms of fixes.

• We could switch things so our new error report is done through Result, like before.
  • We might run into conflicts between annotate-snippets and cargo both wanting to report error:
  • This will likely make error recovery (ie reporting multiple errors, not just the first) more difficult
• We could use cargo-util-schema or just toml to pre-parse the data to see if its a manifest we care about
  • I'm assuming this would require other changes as I asume we gather everything and then figure out what we care about
  • Likely missing some cases where this would cause us problems since its been a bit since I've delved into this machinery
• We could pass in a trait object for reporting diagnostics and change out the implementation in this case
• Write to buffers and replay the result

from cargo.

We could use cargo-util-schema or just toml to pre-parse the data to see if its a manifest we care about

Looking a bit more into this. Doing this would likely also fix:

• #6211 (for certain classes of errors)
• #10752 which blocks turning that warning into an error (see also #13629)

The idea would be that we walk the filesystem, finding Cargo.tomls and indexing them by package.name. When querying a git source, we would take the name from the Dependency, look up the item (warning on duplicate) and then do a full load, caching it.

The biggest problem is with cargo install --git. In that case, select_pkg loads all packages to discover bins, much like we do with workspaces (#4830 would expand this further).

What if we changed read_packages to stop walking on a Cargo.toml and instead do the normal workspace loading logic?

from cargo.

One way to reduce the sympton is to reduce how much we walk in git sources.

In #10752 (comment) we talked about a breadcrumb to switch our walking from recursive directory walking to workspace loading. This would reduce the chance of seeing duplicate package name warnings and reduce the parse errors reported because most of those are likely coming from test or example cases underneath a workspace.

I wonder if we could switch to this model without requiring a breadcrumb...

from cargo.