Cargo vendor regression for git sources containing `exclude = ["build.rs"]`

[dtolnay]

Problem

In Rust 1.79 and older releases, cargo vendor used to work for crates that have build scripts applying only to their source repo, and not published to crates.io. An example of such a crate is https://github.com/dtolnay/cxx/blob/1.0.124/macro/Cargo.toml#L8. The build script was correctly omitted from being vendored.

Since Rust 1.80, Cargo still does not vendor the build script, but now it inserts build = "build.rs" into the vendored Cargo.toml, which causes compilation of the vendored sources to fail.

Steps

Create an empty project with this Cargo manifest:

# Cargo.toml

[package]
name = "repro"
version = "0.0.0"
edition = "2021"
publish = false

[dependencies]
cxxbridge-macro = { git = "https://github.com/dtolnay/cxx", tag = "1.0.124" }

$ mkdir .cargo
$ cargo vendor > .cargo/config.toml
$ cargo check
...
   Compiling cxxbridge-macro v1.0.124 (https://github.com/dtolnay/cxx?tag=1.0.124#afd4aa3f)
error: couldn't read /path/to/vendor/vendor/cxxbridge-macro/build.rs: No such file or directory (os error 2)

error: could not compile `cxxbridge-macro` (build script) due to 1 previous error
warning: build failed, waiting for other jobs to finish...

Possible Solution(s)

Do not inject build = "build.rs" into the vendored manifest if build.rs is not being vendored.

Notes

Bisects to 1e60477.

Version

No response

[epage]

cxx-bridge-macro has an inferred build.rs

https://github.com/dtolnay/cxx/blob/master/macro%2FCargo.toml

We cover a lot of these cases for build.rs for package

cargo/tests/testsuite/package.rs

Line 3909 in fa64658

fn discovery_inferred_build_rs_included() {

But only had minimal tests for cargo vendor, like

cargo/tests/testsuite/vendor.rs

Line 886 in fa64658

fn git_deterministic() {

[epage]

We are using a PathSource to list the files to copy for vendor

cargo/src/cargo/ops/vendor.rs

Line 222 in fa64658

let paths = pathsource.list_files(pkg)?;

but only vendoring the normlalized git Cargo.toml

cargo/src/cargo/ops/vendor.rs

Line 363 in fa64658

let contents = pkg.manifest().to_normalized_contents()?;

This is not the first time we've been bitten by cargo vendor being almost but not quite like cargo package

cargo/src/cargo/ops/vendor.rs

Line 356 in fa64658

// When vendoring git dependencies, the manifest has not been normalized like it would be

While we might want to do a minimal change for backporting, we should evaluate packaging sources to fully unify these so we stop having these problems.

[weihanglo]

Cargo flavored UI test for the situation of excluding build script.

Details

#[cargo_test]
fn dont_copy_excluded_build_script_for_git_dep() {
    let git = git::new("git_dep", |p| {
        p.file(
            "Cargo.toml", 
            r#"
                [package]
                name = "git_dep"
                edition = "2021"
                exclude = ["build.rs"]
            "#,
        )
            .file("src/lib.rs", "")
            .file("build.rs", "fn main() {}")
    });

    let p = project()
        .file(
            "Cargo.toml",
            &format!(
                r#"
                    [package]
                    name = "foo"
                    edition = "2021"

                    [dependencies]
                    git_dep = {{ git = '{}' }}
                "#,
                git.url()
            ),
        )
        .file("src/lib.rs", "")
        .build();

    let output = p
        .cargo("vendor --respect-source-config")
        .exec_with_output()
        .unwrap();

    // Before 1.80.0 `package.build` expected to be false
    // After cargo#13713 it normalized Carg.toml before copying source from git repo.
    // Taht make `package.build=["build.rs"] get into Cargo.toml.
    //
    // While the behavior of vendoring a git source is not well-defined,
    // this is considered a regression in 1.80.0
    //
    // See cargo#14348 for more.
    assert_e2e().eq(
        p.read_file("vendor/git_dep/Cargo.toml"),
        str![[r##"
# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
#
# When uploading crates to the registry Cargo will automatically
# "normalize" Cargo.toml files for maximal compatibility
# with all versions of Cargo and also rewrite `path` dependencies
# to registry (e.g., crates.io) dependencies.
#
# If you are reading this file be aware that the original Cargo.toml
# will likely look very different (and much more reasonable).
# See Cargo.toml.orig for the original contents.

bin = []
example = []
test = []
bench = []

[package]
edition = "2021"
name = "git_dep"
build = "build.rs"
exclude = ["build.rs"]
autobins = false
autoexamples = false
autotests = false
autobenches = false
readme = false

[lib]
name = "git_dep"
path = "src/lib.rs"

"##]],
    );

    let output = String::from_utf8(output.stdout).unwrap();
    p.change_file(".cargo/config.toml", &output);

    // TODO: this should success when we fix cargo#14348.
    p.cargo("check")
        .with_status(101)
        .with_stderr_data(
            str![[r#"
[COMPILING] git_dep v0.0.0 ([ROOTURL]/git_dep#[..])
[ERROR] couldn't read [ROOT]/foo/vendor/git_dep/build.rs: [NOT_FOUND]

[ERROR] could not compile `git_dep` (build script) due to 1 previous error

"#]]
        )
        .run();
}

I am checking if there is any regression beyond build scripts.

[epage]

We have cargo package tests that would be good models, testing every build target type. We likely don't need to cover implicit vs explicit as well.