-
-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
[WIP] Rust cross compilation #50866
[WIP] Rust cross compilation #50866
Conversation
Still compiling. Cross-compiled LLVM is also still broken. |
Success on x86_64-linux (full log) Attempted: libpfm, openssl Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: libpfm, openssl Partial log (click to expand)
|
nativeBuildInputs = [unzip]; | ||
propagatedNativeBuildInputs = [ findXMLCatalogs ]; | ||
nativeBuildInputs = [ unzip ]; | ||
propagatedBuildInputs = [ findXMLCatalogs ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a actually a bug I think. "propagatedNativeBuildInputs" should be correct. Will have a fix soon hopefully.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I can wait a bit, I just get the things working properly and not blocking on nitpicks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
findXMLCatalogs
does sound like a tool that should be propagatedNativeBuildInputs
.
@@ -105,6 +105,10 @@ let | |||
''; | |||
|
|||
postFixup = '' | |||
# fixes cross-compilation to pick the right perl, | |||
# since perl is also in nativeBuildInputs | |||
(sed -i -e '1 s!^.*$!${perl}/bin/perl!' $bin/bin/c_rehash) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't think this would be a problem now that #49608 is merged. Do you know where this comes from? It also looks like openssl supports passing in HASHBANGPERL to do this explicity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HASHBANGPERL
is only supported in openssl 1.1, but we still use openssl 1.0.2 in most places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the stdenv was broken. Since this is the second time I think that change has a bit too much impact to merge into staging directly, we should run it through a separate jobset first to get an overview of the breakages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I probably don't need this change to make cross-compiling rust packages work. So I will split it of probably later.
Timed out, unknown build status on x86_64-darwin (full log) Attempted: openssl The following builds were skipped because they don't evaluate on x86_64-darwin: libpfm Partial log (click to expand)
|
Mhm, I currently stuck at this fix-point stuff that is used to create
|
Build inputs are clean now.
I think most cross-compile fixes from this pr are actually not needed. |
Compilation works now!
|
The first working
|
"-DTARGET_TRIPLE=${stdenv.hostPlatform.config}" | ||
|
||
"-DLLVM_DEFAULT_TARGET_TRIPLE=${stdenv.targetPlatform.config}" | ||
"-DLLVM_TARGETS_TO_BUILD=${llvmTarget stdenv.hostPlatform};${llvmTarget stdenv.targetPlatform}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do this for the other LLVMs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will split this off into a different pull request: #51226
|
||
"${setBuild}.jemalloc=${hostJemalloc}/lib/libjemalloc_pic.a" | ||
"${setHost}.jemalloc=${hostJemalloc}/lib/libjemalloc_pic.a" | ||
"${setTarget}.jemalloc=${targetJemalloc}/lib/libjemalloc_pic.a" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ericson2314 this one seems actually undefined, when I try to build nix-build default.nix -A pkgsCross.aarch64-multiplatform.rustc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's OK. For now. I need to change something in booter.nix to avoid these issues. Need to throw in a buildPackages
to get the cross compiler vs cross compiler compiler, which is the immediate need anyways.
Is this just blocked on ofborg? |
It also requires to fix a few cargo packages that depend on |
If you want to list those, I'm happy to divide + conquer on fixing that + eval failures. |
I will not have a look at this the next few days, so you could just fix the
This should probably also mentioned in the changelog since this also affects out-of-tree rust packages. |
We talked briefly about this on IRC. I think it should action be changed back to |
should "mv ${releaseDir} target/" after |
@@ -36,21 +37,22 @@ let | |||
cargoDepsCopy="$sourceRoot/${cargoVendorDir}" | |||
''; | |||
|
|||
ccForBuild="${buildPackages.stdenv.cc}/bin/${buildPackages.stdenv.cc.targetPrefix}cc"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The build cc is almost never prefixed. I suppose there could be some weird case where it needs to be though.
I rebased this on the latest nixos-unstable and did the renaming for |
@illegalprime can you just open a new pull request? |
We can get rid of |
and because people might have overwritten |
will do! I believe |
You don't need to bisect. This was a commit where I introduced C.UTF-8: #56143 (comment) |
We could temporarily drop the new locale when cross-compiling or something like that, right? EDIT: I mean, we worked without it until recently, so it shouldn't be too bad... |
@vcunat that might work also it would break other packages that assumes we have it, which might be python packages rather soon. |
the problem is |
It is not too complicated to call localedef manually: |
thanks for linking that @Mic92! bisecting is weird, maybe I don't understand it enough or all of the merge commits are making it complicated but this didn't find the right commit:
however this did:
picking 600 commits in the past was just arbitrary and I'm lucky it found the commit , but how should one do this bisect in the future? do i have to flatten out the history to do it right or is my understanding just poor? |
Are you sure that |
Well, better than breaking glibc build already :-) It's why I meant it as a temporary workaround. The "assumes we have it" part won't be portable to non-glibc anyway, but perhaps those don't need it or something... |
Superseded by #56540 |
Status
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)