Open leonardo-m opened 5 years ago
gnzlbg
commented
5 years ago Windows CI is broken with the same error. It would be great if someone with access to a windows machine could give us a hand getting this up and running again.
In the meantime, if you have installed jemalloc on windows yourself, you can use the JEMALLOC_OVERRIDE environment variable to pick it up.
SimonSapin
commented
5 years ago I have access to a Windows machine. What is the appropriate C toolchain to install? I think it’s looking for gcc.exe which I don’t have. (Servo on Windows is built with MSVC.)
running: "sh" "/c/Users/Simon/projects/jemallocator/target/x86_64-pc-windows-gnu/debug/build/jemalloc-sys-fc11e55a94735c81/out/jemalloc/configure" "--disable-cxx" "--with-jemalloc-prefix=_rjem_" "--with-private-namespace=_rjem_" "--host=x86_64-w64-mingw32" "--build=x86_64-pc-win32" "--prefix=C:\\Users\\Simon\\projects\\jemallocator\\target\\x86_64-pc-windows-gnu\\debug\\build\\jemalloc-sys-fc11e55a94735c81\\out"
checking for xsltproc... /usr/bin/xsltproc
checking for x86_64-w64-mingw32-gcc... gcc.exe
checking whether the C compiler works... no
--- stderr
configure: error: in `/c/Users/Simon/projects/jemallocator/target/x86_64-pc-windows-gnu/debug/build/jemalloc-sys-fc11e55a94735c81/out/build':
configure: error: C compiler cannot create executablesjemalloc itself can be built with both, msvc and mingw, the build.rs AFAICT never worked properly with either because rust-lang/rust never used it on windows
this is jemalloc's appveyor: https://github.com/jemalloc/jemalloc/blob/dev/.appveyor.yml
It uses MSYS2 and defines:
CPU on all builds to either i686 or x86_64, MYSTEM on all builds to MINGW32 or MINGW64MSVC on msvc builds only to either amd64 or x86SimonSapin
commented
5 years ago After a round of choco install mingw I get to:
$ cargo build --target x86_64-pc-windows-gnu
[…]
checking build system type...
--- stderr
Invalid configuration `x86_64-pc-win32': system `win32' not recognized
configure: error: /bin/sh /c/Users/Simon/projects/jemallocator/target/x86_64-pc-windows-gnu/debug/build/jemalloc-sys-fc11e55a94735c81/out/jemalloc/build-aux/config.sub x86_64-pc-win32 failedhttps://github.com/alexcrichton/jemallocator/pull/95 should fix some path concatenation issues, but I don't know if you are getting there. Maybe you can try with that branch? There is a function in the build script that transforms the names of the windows targets before calling configure for some reason.
gnzlbg
commented
5 years ago It might be worth it to detect windows in the script, and depending on the target and toolchain, define inside the build.rs the CPU, MSVC, ... environment variables that the ./configure script might be using
SimonSapin
commented
5 years ago Same, with #95.
ErichDonGubler
commented
5 years ago @leonardo-m: If we could get your test code it'd reduce the barrier to entry for reproducing the problem even further!
gnzlbg
commented
5 years ago @ErichDonGubler AFAIK the library just doesn't build. That is:
git clone git@github.com:alexcrichton/jemallocator.git
cd jemallocator/jemalloc-sys
cargo buildshould reproduce the issue on windows, pretty much independently of the windows target used.
The appveyor build bots hit this and they use MSYS2, and we don't have travis-ci windows build bots yet, but i've played a bit with those and they all hit these issues under GIT BASH. This happens for all targets: {x86_64,i686}-pc-windows-{gnu,msvc}.
ErichDonGubler
commented
5 years ago Even with the msvc targets, for some reason the build.rs is using an sh command invocation -- this is obviously wrong very odd. :\ I'm pretty sure that this is the source of the error that OP posted -- sh is the file not being found. I'm not familiar with how or even if jemalloc builds on MSVC, so I'm not familiar yet with what would need to change there to actually make it work with the msvc targets.
With the gnu targets, jemalloc fails compilation with several errors similar to this:
In file included from include/jemalloc/internal/jemalloc_preamble.h:5:0,
from <snip>/jemallocator/target/debug/build/jemalloc-sys-174892ce6f5bf465/out/jemalloc/src/jemalloc.c:2:
<snip>/jemallocator/target/debug/build/jemalloc-sys-174892ce6f5bf465/out/jemalloc/include/jemalloc/internal/jemalloc_internal_decls.h:22:14: fatal error: sys/syscall.h: No such file or directory
# include <sys/syscall.h>
^~~~~~~~~~~~~~~This is expected to fail on MSYS2 because it uses other POSIX APIs to be cross-platform. I'm not with the APIs exposed by syscall.h.
gnzlbg
commented
5 years ago Even with the msvc targets, for some reason the build.rs is using an sh command invocation -- this is obviously wrong.
Yes, the build.rs unconditionally uses sh to run the configure script.
Looking at jemalloc's MSVC build bots upstream (e.g. see https://ci.appveyor.com/project/jasone/jemalloc/builds/20163016/job/e2xha8rtrqbudxg3?fullLog=true#L69) it appears that they invoke bash -c ... on windows to run autoconf and configure. I don't know if we can do something like this in the build.rs [0] - it would have to work on both MSYS2 (for appveyor) and GIT BASH (for travis-ci) at least.
Is there a better way of doing this ?
With the gnu targets, jemalloc fails compilation with several errors similar to this:
That's weird - could you build with cargo build -vv and upload the output to a gist ?
[0] on unixes' we ship a configure script in jemalloc-sys/configure/configure, but we are also able to build with autoconf and validate that the result of autoconf matches the one in jemalloc-sys/configure/configure. on windows we probably should not require autoconf, but we should support it - whether we might need to ship a different configure file, I don't know.
ErichDonGubler
commented
5 years ago @gnzlbg:
Is there a better way of doing this ?
Well, okay, it might just be me that's wrong. By a large margin CMake has been the de-facto standard for cross-platform compilations I've seen, but I've only worked with more "modern" C/C++ codebases recently. jemalloc has been around for a long time, so I could definitely understand why it simply expects to lean on nix tooling like autoconf. To me it's odd to require using nix tools to compile things with the MSVC toolchain nowadays, but I definitely don't consider myself "experienced". So, I'll edit my "obviously wrong" comment above. Whatever the case, I'd definitely be in favor of trying to remove the need for *nix tools if possible.
That's weird - could you build with cargo build -vv and upload the output to a gist ?
Would a few <detail> tags here work? I can still make a gist if you want.
rustup showcargo build -vvgnzlbg
commented
5 years ago To me it's odd to require using *nix tools to compile things with the MSVC toolchain nowadays,
It is, there is an issue open about using CMake with jemalloc, but it is a non-trivial amount of work.
I'd definitely be in favor of trying to remove the need for *nix tools if possible.
Yes, this is why we try to ship a configure (or more) that work on the hosts, so that autoconf is not required and one only needs sh, which is nowadays available almost everywhere. jemalloc upstream uses bash though, so maybe upgrading from sh to bash could work under MSYS2?
Would a few
tags here work?
Sure, thanks! So one weird thing is this:
PREFIX : C:\Users\egubler\workspace\personal\bug-reports\jemallocator\target\debug\build\jemalloc-sys-39a05dc36974796a\out
BINDIR : C:Usersegublerworkspacepersonalbug-reportsjemallocatortargetdebugbuildjemalloc-sys-39a05dc36974796aout/bin
DATADIR : C:\Users\egubler\workspace\personal\bug-reports\jemallocator\target\debug\build\jemalloc-sys-39a05dc36974796a\out/share
INCLUDEDIR : C:Usersegublerworkspacepersonalbug-reportsjemallocatortargetdebugbuildjemalloc-sys-39a05dc36974796aout/include
LIBDIR : C:Usersegublerworkspacepersonalbug-reportsjemallocatortargetdebugbuildjemalloc-sys-39a05dc36974796aout/lib
MANDIR : C:\Users\egubler\workspace\personal\bug-reports\jemallocator\target\debug\build\jemalloc-sys-39a05dc36974796a\out/share/man
srcroot : /c/Users/egubler/workspace/personal/bug-reports/jemallocator/target/debug/build/jemalloc-sys-39a05dc36974796a/out/jemalloc/
abs_srcroot : /c/Users/egubler/workspace/personal/bug-reports/jemallocator/target/debug/build/jemalloc-sys-39a05dc36974796a/out/jemalloc/
objroot :
abs_objroot : /c/Users/egubler/workspace/personal/bug-reports/jemallocator/target/debug/build/jemalloc-sys-39a05dc36974796a/out/build/Some paths appear right, some use the /c/Users/... syntax, and some are missing the slashes /, including the include paths, which would explain why the headers are not being found. I don't know what's in charge of setting the INCLUDEDIR (is it the build.rs? jemalloc's config script, the users?), but it appears very wrong, same for LIBDIR.
ErichDonGubler
commented
5 years ago @gnzlbg: Yeah, the same applies to BINDIR too. Let me investigate that...
ErichDonGubler
commented
5 years ago @gnzlbg: I don't believe these paths are making a difference. I forced the paths to contain forward slashes with this patch:
diff --git a/jemalloc-sys/build.rs b/jemalloc-sys/build.rs
index 6909f96..d672ca9 100644
--- a/jemalloc-sys/build.rs
+++ b/jemalloc-sys/build.rs
@@ -298,7 +298,9 @@ fn main() {
cmd.arg(format!("--host={}", gnu_target(&target)));
cmd.arg(format!("--build={}", gnu_target(&host)));
- cmd.arg(format!("--prefix={}", out_dir.display()));
+ let mut out_dir_string = out_dir.display().to_string();
+ out_dir_string = out_dir_string.replace("\\", "/");
+ cmd.arg(format!("--prefix={}", out_dir_string));
run(&mut cmd);
...and now they print like this:
PREFIX : C:/msys64/home/K0RYU/workspace/bug-reports/jemallocator/target/debug/build/jemalloc-sys-1566e241d4cd2bb0/out
BINDIR : C:/msys64/home/K0RYU/workspace/bug-reports/jemallocator/target/debug/build/jemalloc-sys-1566e241d4cd2bb0/out/bin
DATADIR : C:/msys64/home/K0RYU/workspace/bug-reports/jemallocator/target/debug/build/jemalloc-sys-1566e241d4cd2bb0/out/share
INCLUDEDIR : C:/msys64/home/K0RYU/workspace/bug-reports/jemallocator/target/debug/build/jemalloc-sys-1566e241d4cd2bb0/out/include
LIBDIR : C:/msys64/home/K0RYU/workspace/bug-reports/jemallocator/target/debug/build/jemalloc-sys-1566e241d4cd2bb0/out/lib
MANDIR : C:/msys64/home/K0RYU/workspace/bug-reports/jemallocator/target/debug/build/jemalloc-sys-1566e241d4cd2bb0/out/share/man...but I'm getting the same compilation errors anyway -- that makes sense, since these are technically output folders and depend on a finished compilation first! #include <sys/syscall.h> is simply not expected to work on MSYS2/Git Bash platforms (which are really the same thing, but the latter has a subset of functionality). mingw upstream even says this on their wiki (emphasis mine):
Unlike Cygwin, MinGW doesn't provide Linux or Unix system calls or a POSIX emulation layer. Some POSIX compatibility is provided by the supported runtime library, msvcrt.dll. A few additional functions are provided to help with portability. However, most likely a POSIX application would need to be ported to use Windows APIs in order to compile with MinGW, just as you would to compile with MSVC or Borland or Watcom C++.
jemalloc upstream uses bash though, so maybe upgrading from sh to bash could work under MSYS2?
I doubt that this would make a meaningful difference for the autoconf usages, but it WOULD let you use jemalloc-sys/jemalloc/runtests.sh out-of-the-box. Looking at the jemalloc-sys/jemalloc/scripts/gen_run_tests.py file that it uses to generate test code for bash, I don't see anything that immediately looks like it shouldn't just run in sh, but forcing sh would require either upstream changes or a patch for this crate.
ErichDonGubler
commented
5 years ago So, to me, the real question is: why is jemalloc, as jemallocator's build.rs is configuring it, expecting POSIX syscalls right now?
gnzlbg
commented
5 years ago @ErichDonGubler
<sys/syscall.h> is simply not expected to work on MSYS2/Git Bash platforms
note that jemalloc's own CI uses appveyor with MSYS2 (https://github.com/jemalloc/jemalloc/blob/dev/.appveyor.yml), and the only thing they do is bash -c "autoconf", bash -c "./configure", mingw32-make -k check, and that works out of the box, both for gnu and msvc toolchains and for both x86_64 and i686 targets.
ErichDonGubler
commented
5 years ago @gnzlbg: So, I decided to take your suggestion and explore what upstream does with Appveyor. I've managed to make things compile and have eliminated all but one issue getting this to work on my fork's Appveyor -- I hope to have enough time in the next couple of days to resolve the issue and make a PR. :)
ErichDonGubler
commented
5 years ago I have managed to make gnu targets compile with a few more changes! Would you like me to make a PR with just gnu targets, or would you want me to roll in msvc targets too?
gnzlbg
commented
5 years ago @ErichDonGubler Sure, a PR that allows the gnu targets to work would be a great improvement already :) You can always send a PR that fixed the msvc targets later! Thanks for looking into this!
I see that x86_64-pc-windows-gnu is supported, but when I try to use it in a test program I get the error messages (it can't find the specified file):
https://gist.github.com/rust-play/a29bb45bd2e6077616d1a5256072d4fb