Skip to content
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.

Sign up for GitHub

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

luajit: fix cross build for 32 bit architectures #212760

Merged
merged 1 commit into from
Jan 31, 2023
Merged

luajit: fix cross build for 32 bit architectures #212760

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions pkgs/development/interpreters/luajit/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ let
++ optional enableVMAssertions "-DLUAJIT_USE_ASSERT"
++ optional deterministicStringIds "-DLUAJIT_SECURITY_STRID=0"
;

# LuaJIT requires build for 32bit architectures to be build on x86 not x86_64
# TODO support also other build architectures. The ideal way would be to use
# stdenv_32bit but that doesn't work due to host platform mismatch:
# https://github.com/NixOS/nixpkgs/issues/212494
buildStdenv = if buildPackages.stdenv.isx86_64 && stdenv.is32bit
then buildPackages.pkgsi686Linux.buildPackages.stdenv
else buildPackages.stdenv;
Comment on lines +60 to +62
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this looks good, I wonder if there is a more universal solution that works on other crosses.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I consider the luajit approach of requiring the bitness of the build host to match the build system pretty stupid. It is also the only package I know about that requires it (..although I have not looked for it. It is just from my practice with maintaining packages for OpenWrt.).

I am also afraid that this can't be easily achieved on other platforms. Only some CPUs have the ability to run 32-bit code alongside 64-bit. It is standard on x86_64 but not necessary on ARM or RISC-V (I think that one of the examples is Apple's chips).

I should have added a comment there that it is a speciality of luajit, not some common approach.


in
stdenv.mkDerivation rec {
pname = "luajit";
Expand Down Expand Up @@ -88,8 +97,7 @@ stdenv.mkDerivation rec {
"PREFIX=$(out)"
"DEFAULT_CC=cc"
"CROSS=${stdenv.cc.targetPrefix}"
# TODO: when pointer size differs, we would need e.g. -m32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The note should be preserved, this special casing only works x86_64.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except that this comment is almost only just for x86 because that is not some universal option. The only other applicable (also directly supported by nixpkgs) I found is PowerPC.

Nixpkgs provides, in general, only a single target compilers. That is why gcc_multi exists alongside gcc. That gcc_multi currently also hard depends on i686 and doesn't work on other platforms.

$ nix shell nixpkgs#pkgsCross.aarch64-multiplatform.stdenv.cc -c aarch64-unknown-linux-gnu-gcc -E -march=help -xc /dev/null
...
cc1: note: valid arguments are: armv8-a armv8.1-a armv8.2-a armv8.3-a armv8.4-a armv8.5-a
$ nix shell nixpkgs#pkgsCross.aarch64-multiplatform.stdenv_32bit.cc
error: Multilib aarch64-unknown-linux-gnu-stage-final-gcc-wrapper-9.5.0 not supported for ‘aarch64-linux’
(use '--show-trace' to show detailed location information)

I am going to add TODO at the other place instead of this one where support for other platforms should be required. The issue is that it won't be that easy for ARM for example because not every aarch64 supports for example armv7l.

The best practice here would be to use stdenv_32bit but that is broken as I described in #212494. I will suggest that in the TODO.

"HOST_CC=${buildPackages.stdenv.cc}/bin/cc"
"HOST_CC=${buildStdenv.cc}/bin/cc"
] ++ lib.optional enableJITDebugModule "INSTALL_LJLIBD=$(INSTALL_LMOD)";
enableParallelBuilding = true;
NIX_CFLAGS_COMPILE = XCFLAGS;
Expand Down