Fix parallel make check
#24537
Fix parallel make check
#24537
Conversation
|
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
|
I think there's still a problem if |
|
@bors r+ rollup Nice fixes! |
|
📌 Commit 566d653 has been approved by |
Thanks for letting me know. If I have time, I'll look into it. |
|
⌛ Testing commit 566d653 with merge 9da3545... |
|
💔 Test failed - auto-linux-64-x-android-t |
This required fixing the `pretty-rpass-full` tests to have the same `$$(CSREQ$(1)_T_$(2)_H_$(3))` dependencies as the `rpass-full` and `cfail-full` tests. It also required fixing the `run-make/simd-ffi` test to use unique names for its output files.
|
@bors: r- |
|
This seems to be causing the rollup to fail, due to android. Any ideas? |
|
I would think that fulldeps tests shouldn't run on cross-compiled targets, because our build system doesn't compile LLVM for those targets. I'll look into it. |
|
The fulldeps tests require the rustc crates to be built, but there are two kinds of fulldeps tests:
When I submitted this PR, I was only thinking about the non-plugin tests. In fact, for the We depend upon Currently, if someone attempts to run The immediate fix for this PR is to do the same thing with the pretty fulldeps tests that we already do with the other fulldeps tests. At one point, we were skipping fulldeps tests for target-only triples, using the |
|
I think it should also be possible to just skip fulldeps tests whenever the target is not equal to the host as those can't link to the host libraries anyway (as they probably weren't built). |
|
That's definitely possible -- it was implemented in this commit on April 2 last year, then disabled in this commit two weeks later. The problem with the idea is: for cross-compiles, about half of the fulldeps tests are verifying that I did notice that the second commit I linked removed a bunch of |
|
Hm I think that the removal of |
|
Yes, the removing of the I'm trying to change the fulldeps |
|
Sounds good to me, thanks @rprichard! |
|
I added a couple of commits that switch over to using |
| @@ -8,6 +8,7 @@ | |||
| // option. This file may not be copied, modified, or distributed | |||
| // except according to those terms. | |||
|
|
|||
| // ignore-cross-compile | |||
There was a problem hiding this comment.
did this one pass previously?
There was a problem hiding this comment.
I believe the // ignore-test line in that file means that the test is unconditionally skipped. I verified the the test is skipped on one of the Linux buildbot builders. In any case, it needs // ignore-cross-compile because it uses the syntax crate.
There was a problem hiding this comment.
ah, I see. I'm trying to get this test compiling, but it's pretty busted :(
|
So, the appropriate title of this PR is now: "Fix cross-compiling |
|
Adding the It will make paths a bit longer, which shouldn't be a problem on Unix, but Windows might be a concern, given its 260-character limit. The mode is frequently redundant with the directory name of the test source file, but omitting it seems hazardous. e.g. |
|
Feel free to throw it in, that sounds like a good fix! |
|
I think I should also squash my two commits touching |
The current code attempts to define the PRETTY_DEPS$(1)_H_$(3)_pretty-rpass-full variable, which does not work, because $(1) and $(3) are not inside a function. Moreover, there is a test (run-pass-fulldeps/compiler-calls.rs) that uses rustc_driver, which is not an indirect dependency of librustc or libsyntax. Listing all the dependencies will be hard to maintain, but there's a better way to do this... As with the rpass-full and cfail-full tests, add dependencies using the $$(CSREQ$(1)_T_$(3)_H_$(3)) variable, which includes the complete set of host and target crates, built for a particular stage and host. We use T_$(3), not T_$(2), because we only build LLVM for host triples (not target triples), so we can only build rustc_llvm for host triples. The fulldeps tests that use plugins need host rustc crates, whereas fulldeps tests that link against rustc and run should be skipped for cross-compilation (such as Android). Fixes rust-lang#22021
These tests fail, in general, for cross-compilation, because they require the rustc crates to exist for the target, and they don't. We can't compile them for the target unless we also compile LLVM for the target (we don't). Android is a subset of cross-compilation. The other fulldeps tests, on the other hand, work fine for cross-compilation, and in fact, are verifying that rustc correctly searches for a host plugin crate, not a target plugin crate.
The problem is that rustdoc searches for external crates using the host triple, not the target triple. It's actually unclear to me whether this is correct behavior or not, but it is necessary to get cross-compiled tests working.
The run-pass and pretty run-pass tests could run concurrently, and if they do, they need to keep their output segregated. This change might be overkill. We need the suffix for the `pretty` mode, but we might not need it otherwise. The `debuginfo-lldb` and `debuginfo-gdb` modes look like they could also need it, but the current `tests.mk` file happens not to enable both lldb and gdb at the same time, for incidental reasons.
rprichard
force-pushed
the
fix-parallel-check
branch
from
528472e to
38d26d8
Compare
|
I squashed my two aux dirs now look like this: |
This required fixing the `pretty-rpass-full` tests to have the same `$$(CSREQ$(1)_T_$(2)_H_$(3))` dependencies as the `rpass-full` and `cfail-full` tests. It also required fixing the `run-make/simd-ffi` test to use unique names for its output files.
Sniped from @rprichard's work in rust-lang#24537. r? @alexcrichton
Sniped from @rprichard's work in rust-lang#24537. r? @alexcrichton
Sniped from @rprichard's work in rust-lang#24537. r? @alexcrichton
Sniped from @rprichard's work in #24537. r? @alexcrichton
This required fixing the
pretty-rpass-fulltests to have the same$$(CSREQ$(1)_T_$(2)_H_$(3))dependencies as therpass-fullandcfail-fulltests. It also required fixing therun-make/simd-ffitest to use unique names for its output files.