Fix Travis Windows build

[phansch]

Closes #3306

[mati865]

If that doesn't having binary representation of .rs, .stderr and test output could shed some light.

[mati865]

Strange, cat after dos2unix shows the line endings haven't changed (still are CRLF). dos2unix stdout is totally out of place and shows only a few lines.

[phansch]

I don't know what's going on with the cat 🐈 and dos2unix =(

Looking at the test output again, specifically the diffs: https://travis-ci.org/rust-lang-nursery/rust-clippy/jobs/451994929#L1102
They are indented by one space. This seems to come from compiletest-rs/src/runtest.rs#L2502, the Both case.

I guess I will try it with a patched compiletest-rs to print newline characters using {:?} instead of {}.

[phansch]

oh wait 🤦‍♂️! Looking at the past logs again, it looks like dos2unix only formats the first directory that's globbed from **/*?

dos2unix: converting file tests/ui/author/for_loop.stderr to Unix format...
dos2unix: converting file tests/ui/author/matches.stderr to Unix format...
dos2unix: converting file tests/ui/author/call.rs to Unix format...
dos2unix: converting file tests/ui/author/for_loop.rs to Unix format...
dos2unix: converting file tests/ui/author/matches.rs to Unix format...

I'm going to give it a try with some sort of find/xargs loop

[phansch]

Marked as WIP because this will need a rebase tomorrow and Travis doesn't start right now either. I'm pretty convinced that the updated find command will make the Windows UI tests pass.

[mati865]

Well done, looks better:

failures:
    [ui] ui\author.rs
    [ui] ui\author\call.rs
    [ui] ui\author\for_loop.rs
    [ui] ui\trailing_zeros.rs
test result: FAILED. 219 passed; 4 failed; 1 ignored; 0 measured; 0 filtered out

[phansch]

The UI and UI-Toml tests pass now. The dogfood tests are failing now:

++++ pwd
+++ CLIPPY='/c/Users/travis/build/rust-lang-nursery/rust-clippy/target/debug/cargo-clippy clippy'
+++ /c/Users/travis/build/rust-lang-nursery/rust-clippy/target/debug/cargo-clippy clippy --all-targets --all-features -- -D clippy::all -D clippy::internal -Dclippy::pedantic
error: failed to run `rustc` to learn about target-specific information
Caused by:
  process didn't exit successfully: `C:\Users\travis\build\rust-lang-nursery\rust-clippy\target\debug\clippy-driver.exe rustc - --crate-name ___ --print=file-names --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro` (exit code: 3221225781)

I wonder what's up with --crate-name ___?

[mati865]

Try adding toolchain bin dir to the PATH: #1422 (comment)
For Linux it's done with export LD_LIBRARY_PATH=$(rustc --print sysroot)/lib but Windows wants libs in the PATH and doesn't use LD_LIBRARY_PATH.

Other people also hit this issue: https://github.com/rust-lang-nursery/rust-clippy/search?q=3221225781&type=Issues

[flip1995]

In the appveyor config we add the binaries with https://github.com/rust-lang-nursery/rust-clippy/blob/3bb88775de5a6f8f5c5743dd11af6c820122ccc7/appveyor.yml#L27 to the PATH. The same should work with the libs.

[mati865]

Lib won't work for Windows, all dynamic libs are placed in /bin.

[phansch]

That worked! I now also removed the windows build from allow_failures. Thanks for your help =)

[flip1995]

Nice, windows passes now! Let's bring this on it's way, but still keep appveyor until travis windows is (more) stable (windows build time not linux build time times 2).

bors r+

[bors]

Build failed

• continuous-integration/travis-ci/push

[phansch]

huh, the windows build failed without error. I'm going to restart it.

for reference, because the log will be lost with the restart:

$ if [ -z ${INTEGRATION} ]; then
    if [ "$TRAVIS_OS_NAME" == "linux" ]; then
      . $HOME/.nvm/nvm.sh
      nvm install stable
      nvm use stable
      npm install remark-cli remark-lint
    fi
    if [ "$TRAVIS_OS_NAME" == "windows" ]; then
      choco install windows-sdk-10.0
      find tests/ui/* \( -name '*.rs' -or -name '*.stderr' -or -name '*.stdout' \) -exec dos2unix '{}' +
      find tests/ui-toml/* \( -name '*.rs' -or -name '*.stderr' -or -name '*.stdout' -or -name '*.toml' \) -exec dos2unix '{}' +
    fi
  fi
  

[phansch]

I guess the allow_failure wouldn't have worked either with that typo in there 🤦‍♂️

[flip1995]

Removing CARGO_INCREMENTAL=0 slowed down the build by another 8 minutes... Let's run a try run to see whether it works with bors and whether the build time was a coincidence.

@bors try

[bors]

⌛ Trying commit 876a7e1 with merge ebfc081...

[bors]

☀️ Try build successful - checks-travis, status-appveyor
Build commit: ebfc081

[phansch]

We could maybe skip the dogfood tests on windows? I'm not sure but I guess that's where most of the time is spent in the windows build.

[flip1995]

Build time is up by 8 minutes without CARGO_INCREMENTAL=0. So let's leave this in.

Removing dogfood is a good idea, since the results should be the same on windows and linux/macos.

---

To improve the build time of every build: Maybe we could also cache RTIM, to not have to install it every time?

[flip1995]

Without dogfood we now have about the same build times across the platforms.

@bors try

---

I think the important question before merging this is:
Should we still allow failures on windows builds or should we yolo it? 😄

[bors]

⌛ Trying commit ce2a7b0 with merge 25b51f8...

[bors]

☀️ Try build successful - checks-travis, status-appveyor
Build commit: 25b51f8

[flip1995]

@bors try

[bors]

⌛ Trying commit c61e4e1 with merge 2655637...

[flip1995]

Caching RTIM only saves 2-5 minutes of build time, but stores caches of the size of ~40MB. I don't think this is worth it.

[flip1995]

This should be ready for merge now. Last question:

Should we

1. allow failures on the windows build 👀
2. or yolo it? 🚀

[phansch]

If it works, it works 🤷‍♂️ Thanks for pushing this through!

@bors r=me,flip1995

[bors]

📌 Commit ce2a7b0 has been approved by me,flip1995

[bors]

⌛ Testing commit ce2a7b0 with merge 48b50e8...

[bors]

☀️ Test successful - checks-travis, status-appveyor
Approved by: me,flip1995
Pushing 48b50e8 to master...