More jemalloc fixes #38675
More jemalloc fixes #38675
Conversation
infinity0
commented
Dec 29, 2016
infinity0
commented
- Disable jemalloc on s390x as well (closes ppc64le and s390x 1.13.0 binaries segfault when trying to build 1.14.0 #38596)
- Disable jemalloc tests on platforms where it is disabled (closes 1.14.0 tests fail (1 run-pass, 1 compile-fail) on arm64 #38612)
|
(rust_highfive has picked a reviewer for you, use r? to override) |
infinity0
force-pushed
the
more-jemalloc-fixes
branch
2 times, most recently
from
6aa7710
to
6ffa107
Compare
| @@ -10,7 +10,7 @@ | |||
|
|
|||
| #![feature(alloc_jemalloc)] | |||
|
|
|||
| #[cfg(any(target_os = "linux", target_os = "macos"))] | |||
| #[cfg(any(all(target_os = "linux", not(any(target_arch = "aarch64", target_arch = "mips", target_arch = "mips64", target_arch = "powerpc", target_arch = "powerpc64", target_arch = "s390x"))), target_os = "macos"))] | |||
There was a problem hiding this comment.
This seems like it's getting out of hand, perhaps you can just configure the test to only run on x86_64 linux and OSX?
There was a problem hiding this comment.
What is the point of a testsuite if you're not running it? If users of non-x86 platforms are supposed to disable the testsuite, they shouldn't be really trusting any of the generated binaries, to be honest.
There was a problem hiding this comment.
In this particular case, we're simply disabling jemalloc-related tests that are not applicable on those platforms, because rust doesn't use jemalloc there. But in general, I do hope rust starts running tests on these other platforms too.
| @@ -29,9 +29,9 @@ extern crate allocator_dylib2; | |||
| // ensure we get the same error. | |||
| // | |||
| // So long as we CI linux/OSX we should be good. | |||
| #[cfg(any(target_os = "linux", target_os = "macos"))] | |||
| #[cfg(any(all(target_os = "linux", not(any(target_arch = "aarch64", target_arch = "mips", target_arch = "mips64", target_arch = "powerpc", target_arch = "powerpc64", target_arch = "s390x"))), target_os = "macos"))] | |||
There was a problem hiding this comment.
This also seems like it's getting out of hand, can we configure this to basically only run the test on x86_64 linux and OSX as well?
|
This seems reasonable to me (modulo @alexcrichton's comment), though this code is not really my area of expertise. |
|
r=me once @alexcrichton is satisfied, so: |
|
Thanks for keeping these platforms running @infinity0 |
infinity0
force-pushed
the
more-jemalloc-fixes
branch
from
6ffa107
to
cadebc7
Compare
|
Updated to specifically enable on x86 and x86_64 only. (I did it the other way around originally to match what is done in |
|
@bors: r+ |
|
📌 Commit cadebc7 has been approved by |
|
☀️ Test successful - status-appveyor, status-travis |
