-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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: EDK2, OVMF.fd: fixes for cross-compilation + dependencies + armv7l #53064
Conversation
# Given a stdenv, returns the edk2-valid arch. | ||
envToArch = env: | ||
if env.isi686 then | ||
"IA32" |
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.
These keep showing up. We probably should put them in lib/systems/ as uefiShortName
or something to avoid the duplication.
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.
They do come often in similar sounding names, but all-caps they're pretty much the only instances :/. These ones, though, seem to be really specific to their build system.
iso-image.nix
has those, which are not entirely equivalent, even ignoring case.
363 # Name used by UEFI for architectures.
364 targetArch =
365 if pkgs.stdenv.isi686 then
366 "ia32"
367 else if pkgs.stdenv.isx86_64 then
368 "x64"
369 else if pkgs.stdenv.isAarch64 then
370 "aa64"
371 else
372 throw "Unsupported architecture";
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.
Should still be a case on Nevermind. It's good as is, because the 32-bit ones are families of platform so there's no single string to case on.env.parsed.cpu.name
.
d2c27d5
to
f49e539
Compare
buildInputs = [ pythonEnv ] ++ | ||
stdenv.lib.optionals (attrs ? buildInputs) attrs.buildInputs; | ||
buildInputs = [] ++ attrs.buildInputs or []; | ||
nativeBuildInputs = [ buildPythonEnv buildPackages.iasl ] ++ attrs.nativeBuildInputs or []; |
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 do iasl
instead of buildPackages.iasl
here because of the splicing black magic. (See https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/splice.nix .)
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.
Actually, it's even more complex because the package using the edk should always live in a different bootstrapping phase (hence my suggestion above that OVMF should use buildPackages.edk2
). This means that this setup
really ought to be defined elsewhere so that anything used here can come from the phase of things using edk, rather than the phase providing edk. Otherwise all the splicing stuff / manual buildPackages
/ targetPackages
will make no sense.
What I just wrote probably in general makes no sense :). But it is exactly analogous how stdenv provided tools from the previous stage and lives in the same stage as things that use it, rather than being defined in the previous stage with the tools, and being used in the next stage. setup
is a wrapper around stdenv.mkDerivation
, so just as stdenv use buildPackages.gcc
, so should setup
use buildPackages.edk2
. But then it makes little sense to put as a passthru on edk2
since then we spuriously reference edk
to get setup
which just uses buildPackages.edk
+ all the splicing issues I referenced above.
|
||
build \ | ||
-n $NIX_BUILD_CORES \ | ||
${buildFlags} \ |
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 you use escapeShellArgs
to make this an array instead?
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 won't right now, I don't think this PR will ever be merged as-is, see the other comment where I explain I'm ending up reworking the edk2.setup
thing to work better and in a more generic fashion.
Though, I will on that branch.
pythonEnv = python2.withPackages(ps: [ps.tkinter]); | ||
|
||
targetArch = envToArch targetPlatform; | ||
hostArch = envToArch buildPlatform; |
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.
Should be hostPlatform
. It's not a good idea to get tricky and rename these!
In general, getting this stuff right because as far as I can tell from a cursory look at the docs, the EDK 2 is trying to be package manager for cross-compilation, and doing a poor job of it. The best results would probably the larger project of ripping it shreds and replacing as much stuff with nixpkgs packages as possible, and exposing the new chunks in some sort of |
In another branch, building-up from the cross-compilation branch, I'm working on making the EDK2 stuff more re-usable through making use of it to build a Raspberry Pi 3 UEFI implementation. I don't think I'm going as far as you're suggesting, but I think the end-result is better and easier to follow along. Though, this makes fixing things here annoying as some of the things are going to be removed :). Thanks for the review, it really helps get those things together. |
OK great! Happy to help with that one too. |
I've taken some of your Python-related changes and fixed cross-compilation of Python packages in #53123. At least, it mostly seems to work. |
@samueldr are you still working on this? If so, please rebase. |
I intend to work on this, haven't for a while, will rebase at that moment. If others are interested in working on this be my guest! |
Thank you for your contributions.
|
I converted it to a draft. |
I marked this as stale due to inactivity. → More info |
I'm not coming back to this exact PR ever again, so I'm closing it. |
THIS NEEDS FIXES CURRENTLY IN STAGING TO BUILD. It is why this is based on staging.
This also needs a good review by the peeps that know more about cross-compilation. Some of the fixes, especially the python-touching fixes, may not be optimal.
Motivation for this change
Well, get the OVMF EFI "floppy disks" (that's what .fd stands for?) for aarch64 and armv7l buildable with cross-compilation. Furthermore, get the armv7l build going too.
This is prep work for personal stuff, but hey, nixpkgs gets better here!
This was verified to work using
qemu-system-*
and the x86_64, aarch64 and armv7l NixOS iso. (The NixOS armv7l iso is another WIP thing, but it's really not good now.)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)