Cargo: Make doctest compile / run cwd consistent with other test kinds

Created on 17 Dec 2020  路  13Comments  路  Source: rust-lang/cargo

In #8954 I changed the cwd that rustdoc itself is invoked to be similar to how rustc is invoked.

This is needed to get correct workspace-relative file paths, at least for -Z instrument-coverage purposes (https://github.com/rust-lang/rust/issues/79417) (maybe also debuginfo, but I haven鈥檛 checked).

The problem is that this also changes the cwd in which the doctests themselves are executed, which is sadly a regression from stable :-(

CC @jan-auer @alexcrichton @richkadel maybe @jyn514

I have a small example here with unit, integration and doctests, running it with:

stable:

  • rustc --crate-name child --edition=2018 child/src/lib.rs [鈥
  • rustdoc --edition=2018 --crate-type lib --test /Users/swatinem/Coding/cargo-test-cwd/child/src/lib.rs [鈥

nightly:

  • rustc --crate-name child --edition=2018 child/src/lib.rs [鈥
  • rustdoc --edition=2018 --crate-type lib --crate-name child --test child/src/lib.rs [鈥

Full output below, but it shows that unit and integration tests are run with the crate directory as CWD, not the workspace, but the compiler itself is invoked from the workspace.
I think we need a way to tell rustdoc to use a specific cwd when running the doctests vs when compiling them.


cargo +stable test --workspace --verbose -- --nocapture > stable.txt 2>&1 

   Compiling root v0.1.0 (/Users/swatinem/Coding/cargo-test-cwd)
   Compiling child v0.1.0 (/Users/swatinem/Coding/cargo-test-cwd/child)
     Running `rustc --crate-name root --edition=2018 src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=2 -C metadata=b9fe8482c61999e8 -C extra-filename=-b9fe8482c61999e8 --out-dir /Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C incremental=/Users/swatinem/Coding/cargo-test-cwd/target/debug/incremental -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C target-cpu=native`
     Running `rustc --crate-name child --edition=2018 child/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=2 -C metadata=51b38cc0eeaf6f95 -C extra-filename=-51b38cc0eeaf6f95 --out-dir /Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C incremental=/Users/swatinem/Coding/cargo-test-cwd/target/debug/incremental -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C target-cpu=native`
     Running `rustc --crate-name root --edition=2018 src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 --test -C metadata=8d2e496c07b655ba -C extra-filename=-8d2e496c07b655ba --out-dir /Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C incremental=/Users/swatinem/Coding/cargo-test-cwd/target/debug/incremental -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C target-cpu=native`
     Running `rustc --crate-name child --edition=2018 child/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 --test -C metadata=2c1e3a1656f1d0d3 -C extra-filename=-2c1e3a1656f1d0d3 --out-dir /Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C incremental=/Users/swatinem/Coding/cargo-test-cwd/target/debug/incremental -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C target-cpu=native`
     Running `rustc --crate-name integration --edition=2018 tests/integration.rs --error-format=json --json=diagnostic-rendered-ansi --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 --test -C metadata=d55edb18df8de4ee -C extra-filename=-d55edb18df8de4ee --out-dir /Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C incremental=/Users/swatinem/Coding/cargo-test-cwd/target/debug/incremental -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps --extern root=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/libroot-b9fe8482c61999e8.rlib -C target-cpu=native`
     Running `rustc --crate-name integration --edition=2018 child/tests/integration.rs --error-format=json --json=diagnostic-rendered-ansi --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 --test -C metadata=a20222878ab3a217 -C extra-filename=-a20222878ab3a217 --out-dir /Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C incremental=/Users/swatinem/Coding/cargo-test-cwd/target/debug/incremental -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps --extern child=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/libchild-51b38cc0eeaf6f95.rlib -C target-cpu=native`
    Finished test [unoptimized + debuginfo] target(s) in 1.19s
     Running `/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/child-2c1e3a1656f1d0d3 --nocapture`

running 1 test
unit test child: /Users/swatinem/Coding/cargo-test-cwd/child
test unit_cwd ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running `/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/integration-a20222878ab3a217 --nocapture`

running 1 test
integration test child: /Users/swatinem/Coding/cargo-test-cwd/child
test integration_cwd ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running `/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/root-8d2e496c07b655ba --nocapture`

running 1 test
unit test root: /Users/swatinem/Coding/cargo-test-cwd
test unit_cwd ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running `/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/integration-d55edb18df8de4ee --nocapture`

running 1 test
integration test root: /Users/swatinem/Coding/cargo-test-cwd
test integration_cwd ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

   Doc-tests child
     Running `rustdoc --edition=2018 --crate-type lib --test /Users/swatinem/Coding/cargo-test-cwd/child/src/lib.rs --crate-name child -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps --test-args --nocapture --extern child=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/libchild-51b38cc0eeaf6f95.rlib -C embed-bitcode=no`

running 1 test
test src/lib.rs - (line 1) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

   Doc-tests root
     Running `rustdoc --edition=2018 --crate-type lib --test /Users/swatinem/Coding/cargo-test-cwd/src/lib.rs --crate-name root -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps --test-args --nocapture --extern root=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/libroot-b9fe8482c61999e8.rlib -C embed-bitcode=no`

running 1 test
test src/lib.rs - (line 1) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out


cargo +nightly test --workspace --verbose -- --nocapture > nightly.txt 2>&1 

   Compiling root v0.1.0 (/Users/swatinem/Coding/cargo-test-cwd)
   Compiling child v0.1.0 (/Users/swatinem/Coding/cargo-test-cwd/child)
     Running `rustc --crate-name root --edition=2018 src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=2 -C metadata=763164469cdcead1 -C extra-filename=-763164469cdcead1 --out-dir /Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C incremental=/Users/swatinem/Coding/cargo-test-cwd/target/debug/incremental -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C target-cpu=native`
     Running `rustc --crate-name root --edition=2018 src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 --test -C metadata=9644a0ca393294d3 -C extra-filename=-9644a0ca393294d3 --out-dir /Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C incremental=/Users/swatinem/Coding/cargo-test-cwd/target/debug/incremental -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C target-cpu=native`
     Running `rustc --crate-name child --edition=2018 child/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=2 -C metadata=3da7cb1692c9969f -C extra-filename=-3da7cb1692c9969f --out-dir /Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C incremental=/Users/swatinem/Coding/cargo-test-cwd/target/debug/incremental -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C target-cpu=native`
     Running `rustc --crate-name child --edition=2018 child/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 --test -C metadata=b1c958d1677c12d4 -C extra-filename=-b1c958d1677c12d4 --out-dir /Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C incremental=/Users/swatinem/Coding/cargo-test-cwd/target/debug/incremental -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C target-cpu=native`
     Running `rustc --crate-name integration --edition=2018 tests/integration.rs --error-format=json --json=diagnostic-rendered-ansi --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 --test -C metadata=3efdbdc44398e0ef -C extra-filename=-3efdbdc44398e0ef --out-dir /Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C incremental=/Users/swatinem/Coding/cargo-test-cwd/target/debug/incremental -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps --extern root=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/libroot-763164469cdcead1.rlib -C target-cpu=native`
     Running `rustc --crate-name integration --edition=2018 child/tests/integration.rs --error-format=json --json=diagnostic-rendered-ansi --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 --test -C metadata=f9944916cab4691d -C extra-filename=-f9944916cab4691d --out-dir /Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -C incremental=/Users/swatinem/Coding/cargo-test-cwd/target/debug/incremental -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps --extern child=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/libchild-3da7cb1692c9969f.rlib -C target-cpu=native`
    Finished test [unoptimized + debuginfo] target(s) in 1.12s
     Running `/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/child-b1c958d1677c12d4 --nocapture`

running 1 test
unit test child: /Users/swatinem/Coding/cargo-test-cwd/child
test unit_cwd ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running `/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/integration-f9944916cab4691d --nocapture`

running 1 test
integration test child: /Users/swatinem/Coding/cargo-test-cwd/child
test integration_cwd ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running `/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/root-9644a0ca393294d3 --nocapture`

running 1 test
unit test root: /Users/swatinem/Coding/cargo-test-cwd
test unit_cwd ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running `/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/integration-3efdbdc44398e0ef --nocapture`

running 1 test
integration test root: /Users/swatinem/Coding/cargo-test-cwd
test integration_cwd ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests root
     Running `rustdoc --edition=2018 --crate-type lib --crate-name root --test src/lib.rs -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps --test-args --nocapture --extern root=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/libroot-763164469cdcead1.rlib -C embed-bitcode=no`

running 1 test
test src/lib.rs - (line 1) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.25s

   Doc-tests child
     Running `rustdoc --edition=2018 --crate-type lib --crate-name child --test child/src/lib.rs -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps -L dependency=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps --test-args --nocapture --extern child=/Users/swatinem/Coding/cargo-test-cwd/target/debug/deps/libchild-3da7cb1692c9969f.rlib -C embed-bitcode=no`

running 1 test
test child/src/lib.rs - (line 1) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.24s

A-doctests C-bug
Source
picture Swatinem

Most helpful comment

@Swatinem is there a way to get the relative path from the workspace root to the doctest root via API, during the InstrumentCoverage MIR transform, like you get the line offset?

Unless I'm mistaken, nothing in the coverage map implementation really cares what the actual filename is during compile time.

What's important is that llvm-cov can find the files, so relative paths only depend on where llvm-cov is run from, and that should be the workspace root.

You could force absolute paths for doctests, but I think it's cleaner to just prepend the doctest root path with the path from workspace root to doctest root, the same way you offset the line numbers, for example:

    let start_line = source_map.doctest_offset_line(&source_file.name, start_line);
    let end_line = source_map.doctest_offset_line(&source_file.name, end_line);
    let file_name = source_map.doctest_offset_file_name(&source_file.name, file_name);
    CodeRegion {
        file_name,
        start_line: start_line as u32,
        start_col: start_col.to_u32() + 1,
        end_line: end_line as u32,
        end_col: end_col.to_u32() + 1,
    }

I don't think anything else needs to change.

picture richkadel on 31 Dec 2020
👍2

All 13 comments

I think @jan-auer and me were both very eager to create this, this is basically a duplicate of: #8992

Since one workaround for this is to use env!("CARGO_MANIFEST_DIR") inside the doctest, maybe that is something that rustdoc itself can use as the cwd when running the tests, does that sound like a good solution? @jyn514

Swatinem picture Swatinem on 17 Dec 2020
😄1

Can you explain why -Z instrument-coverage needs workspace relative paths, and why rustdoc behaves differently from rustc?

jyn514 picture jyn514 on 30 Dec 2020

Can you explain why -Z instrument-coverage needs workspace relative paths, and why rustdoc behaves differently from rustc?

I don鈥檛 know exactly why workspace relative paths are needed, just that the llvm-based tools need them to have proper coverage mappings.

For example when I run my testcases from https://github.com/Swatinem/fucov, I get the following lcov output:

SF:src\lib.rs
FN:1,_RNvNvCs4fqI2P2rA04_8rust_out4main28__doctest_main_src_lib_rs_1_0
FN:4,_RNvNvCs4fqI2P2rA04_8rust_out4main28__doctest_main_src_lib_rs_3_0
[鈥nip鈥

Note however that the test on line 3 comes from crate-d, and there is a doctest on line 1 of crate-a, crate-b and crate-c, but those are not being destinguished.

When I switch the llvm tools to html output, I get the following:

error: src\lib.rs: no such file or directory
warning: The file 'src\lib.rs' isn't covered.
error: src\mod_with_file_doctest.rs: no such file or directory
warning: The file 'src\mod_with_file_doctest.rs' isn't covered.
error: src\somemod\mod.rs: no such file or directory
warning: The file 'src\somemod\mod.rs' isn't covered.
error: src\somemod\someothermod\mod.rs: no such file or directory
warning: The file 'src\somemod\someothermod\mod.rs' isn't covered.

Especially the src\lib.rs exists basically 4 times in the workspace, so the llvm tools have no idea whats going on.

Other test types are covered properly in the workspace setting, just not doctests.

Swatinem picture Swatinem on 30 Dec 2020

Ok, so basically rustdoc has always behaved differently from rustc and people depended on that fact? That's really unfortunate :/ I think doing this only when running the test would still run into the same issue: people could be opening relative file system paths at runtime, and that would break if you changed the working directory. I still think the best path forward is to figure out why LLVM depends on the current working directory and maybe tell it to use paths relative to the workspace instead.

jyn514 picture jyn514 on 30 Dec 2020

I think the crucial detail is the difference between compiling and then running the doctest. For coverage, it is apparently required to have the doc tests compiled from the workspace root. I'm not that familiar with the LLVM tooling myself, but I'm assuming they are using DWARF's relative paths, which are always based on the "compilation directory", which is more-or-less the CWD of the compiler invocation. This is all defined at compile time.

Contrary to this, cargo already runs all other tests in the respective crate root, which may be a workspace member. That means, for unit and integration tests, there is already an expectation that by default relative file paths are always evaluated from Cargo.toml, and I think the same should hold for doc tests.

@Swatinem please correct me if I'm wrong in any of the above.

jan-auer picture jan-auer on 31 Dec 2020

Contrary to this, cargo already runs all other tests in the respective crate root, which may be a workspace member. That means, for unit and integration tests, there is already an expectation that by default relative file paths are always evaluated from Cargo.toml, and I think the same should hold for doc tests.

Sure, but that would be a breaking change. At the very least it would need an edition, but I'm still not convinced there's not a way to pass this information to LLVM in another way.

jyn514 picture jyn514 on 31 Dec 2020

I would propose to maybe add a new CLI flag to rustdoc that controls the runtime cwd, while keeping the current compile-time cwd behavior.
As the next step, re-land https://github.com/rust-lang/cargo/pull/8954 that uses this new flag to have a different compile-time and runtime cwd, making code-coverage happy without breaking current runtime cwd expectations.

Does that sound good? @jyn514

Also, I think @jan-auer is correct that the llvm tools probably use DWARF and similar debuginfo for all this. I haven鈥檛 looked at how this all works in detail, but that would at least explain why you need to provide all your compiled objects (executables) to the coverage tools.

Swatinem picture Swatinem on 31 Dec 2020

As the next step, re-land #8954 that uses this new flag to have a different compile-time and runtime cwd, making code-coverage happy without breaking current runtime cwd expectations.

If there's a way to do this so that changing the directory is opt-in and only happens when collecting coverage information, I would be ok with this. Otherwise, it's still a breaking change IMO.

jyn514 picture jyn514 on 31 Dec 2020

Otherwise, it's still a breaking change IMO.

Explain please.

Also, is all of this specified/documented somewhere how these things should behave?

Swatinem picture Swatinem on 31 Dec 2020

Explain please.

Previously, rustdoc would run tests in the crate directory, and the behavior of that was observable in doc tests (with std::fs, etc). After your proposed change, it will run in the workspace root, which could break doctests.

This is probably more on the T-cargo side than the T-rustdoc side, and if the cargo team is willing to make this change I'll defer to them, but I'm still concerned about the breaking change.

Also, is all of this specified/documented somewhere how these things should behave?

Probably not, one of the issues of rustdoc is that a lot of people depend on the implementation rather than the documented behavior; this is one of the reasons intra-doc links had to be stabilized, because even though they were unstable half the ecosystem was using them: https://github.com/rust-lang/rust/issues/63305.

jyn514 picture jyn514 on 31 Dec 2020

After your proposed change, it will run in the workspace root, which could break doctests.

Thats why my original PR was reverted. My proposal would be to keep the current runtime behavior, while changing the compile-time cwd, specifically to not break this.

Swatinem picture Swatinem on 31 Dec 2020

btw, changing the compile-time cwd would probably be observable when using the unstable --persist-doctests flag, which is a requirement for code coverage anyway because you need the executables.

Swatinem picture Swatinem on 31 Dec 2020

@Swatinem is there a way to get the relative path from the workspace root to the doctest root via API, during the InstrumentCoverage MIR transform, like you get the line offset?

Unless I'm mistaken, nothing in the coverage map implementation really cares what the actual filename is during compile time.

What's important is that llvm-cov can find the files, so relative paths only depend on where llvm-cov is run from, and that should be the workspace root.

You could force absolute paths for doctests, but I think it's cleaner to just prepend the doctest root path with the path from workspace root to doctest root, the same way you offset the line numbers, for example:

    let start_line = source_map.doctest_offset_line(&source_file.name, start_line);
    let end_line = source_map.doctest_offset_line(&source_file.name, end_line);
    let file_name = source_map.doctest_offset_file_name(&source_file.name, file_name);
    CodeRegion {
        file_name,
        start_line: start_line as u32,
        start_col: start_col.to_u32() + 1,
        end_line: end_line as u32,
        end_col: end_col.to_u32() + 1,
    }

I don't think anything else needs to change.

richkadel picture richkadel on 31 Dec 2020
👍2
Was this page helpful?
0 / 5 - 0 ratings

Related issues

mathstuf picture mathstuf  路  3Comments
ckaran picture ckaran  路  3Comments
ehuss picture ehuss  路  3Comments
fprijate picture fprijate  路  3Comments
dotnetspec picture dotnetspec  路  3Comments