Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x.py test src/test/rustdoc tries to run src/tools/tidy as if it were HTML tidy #82501

Closed
jyn514 opened this issue Feb 24, 2021 · 4 comments · Fixed by #82507
Closed

x.py test src/test/rustdoc tries to run src/tools/tidy as if it were HTML tidy #82501

jyn514 opened this issue Feb 24, 2021 · 4 comments · Fixed by #82507
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. O-windows Operating system: Windows T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Feb 24, 2021

After #79370 I see this spam in x.py output:

thread 'main' panicked at 'need path to cargo', src\tools\tidy\src\main.rs:15:48
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

As I understand compiletest wants to run some third-party tool called tidy (with tidy --version), but it actually runs the rustbuild's tidy which doesn't support such command line use.

(Also this expensive probing happens unconditionally when running any test, even it is not related to rustdoc.)

Originally posted by @petrochenkov in #79370 (comment)

@jyn514 jyn514 added A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Feb 24, 2021
@jyn514
Copy link
Member Author

jyn514 commented Feb 24, 2021

I can't replicate this (specifically, confusing src/tools/tidy for /usr/bin/tidy) on Linux. @smmalis37 is on Windows and I think @petrochenkov is too. Can you post what your PATH environment variable is?

@jyn514 jyn514 added the O-windows Operating system: Windows label Feb 24, 2021
@smmalis37
Copy link
Contributor

smmalis37 commented Feb 24, 2021

Semicolons replaced with newlines for clarity:

C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.28.29333\bin\HostX64\x64
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\Common7\IDE\VC\VCPackages
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\Common7\IDE\CommonExtensions\Microsoft\TestWindow
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\Common7\IDE\CommonExtensions\Microsoft\TeamFoundation\Team Explorer
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\MSBuild\Current\bin\Roslyn
C:\Program Files (x86)\Windows Kits\10\bin\10.0.19041.0\x64
C:\Program Files (x86)\Windows Kits\10\bin\x64
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\\MSBuild\Current\Bin
C:\Windows\Microsoft.NET\Framework64\v4.0.30319
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\Common7\IDE\
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\Common7\Tools\
C:\Windows\system32
C:\Windows
C:\Windows\System32\Wbem
C:\Windows\System32\WindowsPowerShell\v1.0\
C:\Windows\System32\OpenSSH\
C:\Program Files\Microsoft VS Code\bin
C:\Program Files\Git\cmd
C:\Program Files (x86)\NVIDIA Corporation\PhysX\Common
C:\Program Files\dotnet\
C:\Program Files (x86)\WinMerge
C:\Program Files (x86)\dotnet\
C:\Users\smmal\.cargo\bin
C:\Users\smmal\AppData\Local\Microsoft\WindowsApps
C:\Users\smmal\.dotnet\tools
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\Common7\IDE\CommonExtensions\Microsoft\CMake\Ninja

Interestingly a where tidy turns up nothing and attempting to run tidy gives a not found as well. Is this something in the environment created by x.py?

@jyn514 jyn514 added the A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself label Feb 24, 2021
@jyn514
Copy link
Member Author

jyn514 commented Feb 25, 2021

This is indeed Windows only. This is the culprit:

rust/src/bootstrap/test.rs

Lines 1785 to 1792 in 1fdadbf

// The tests are going to run with the *target* libraries, so we need to
// ensure that those libraries show up in the LD_LIBRARY_PATH equivalent.
//
// Note that to run the compiler we need to run with the *host* libraries,
// but our wrapper scripts arrange for that to be the case anyway.
let mut dylib_path = dylib_path();
dylib_path.insert(0, PathBuf::from(&*builder.sysroot_libdir(compiler, target)));
cargo.env(dylib_path_var(), env::join_paths(&dylib_path).unwrap());

On windows, dylib_env_var() is PATH:
pub fn dylib_env_var() -> &'static str {
if cfg!(windows) {
"PATH"
so PATH is expanded to include stage0-bootstrap-tools\86_64-pc-windows-msvc\release\deps and stage0\bin. The strange thing is tidy isn't in either of those directories 😕 it's in stage0-bootstrap-tools\86_64-pc-windows-msvc\release (without deps).

Hopefully I can change this so that only commands that compiletest runs get this variable set, not compiletest itself. If not, maybe I could just rename the tidy binary.

@smmalis37
Copy link
Contributor

I think I figured it out. The tidy that's being run is in build\x86_64-pc-windows-msvc\stage0-tools-bin, which is the same location as compiletest.exe. That's how it's finding a tidy without one being anywhere in PATH.

Aaron1011 added a commit to Aaron1011/rust that referenced this issue Feb 25, 2021
Only look for HTML `tidy` when running rustdoc tests

This avoids printing lots of unnecessary errors, as well as making the
test suite slightly faster. This doesn't fix the windows bug tracked by rust-lang#82501, though.

r? `@petrochenkov`
@bors bors closed this as completed in 6c76dac Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. O-windows Operating system: Windows T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants