lanzaboote-stub: init at 0.3.0 - #231951
RaitoBezarius wants to merge 4 commits into
Conversation
111edf3 to
c9fb579
Compare
c9fb579 to
e1c2349
Compare
e1c2349 to
c521016
Compare
This comment was marked as duplicate.
This comment was marked as duplicate.
|
#235230 is done; reviewing this PR is next on my list. |
5586367 to
e2723f0
Compare
It's only on
Oh you don't need to wait for #235230 and in fact you shouldn't, since it causes a mass-rebuild. Just please check that the CI it adds still passes with (the final version of) this PR. If it doesn't pass either it found a bug in your PR, or your PR found a bug in the new tests -- either way let me know so we're aware. I'm thinking ahead to a scenario where this PR merges ahead of the 23.11 branch-off but #235230 doesn't. |
Github needs a 😭 reaction/vote |
What about the following? It's gross, but as long as LLVM has merged the change into LLVM17 it's effectively just a different way of backporting that change: diff --git a/pkgs/development/compilers/llvm/16/llvm/default.nix b/pkgs/development/compilers/llvm/16/llvm/default.nix
index 9475b5c99485..c5bcfd35c5bf 100644
--- a/pkgs/development/compilers/llvm/16/llvm/default.nix
+++ b/pkgs/development/compilers/llvm/16/llvm/default.nix
@@ -36,6 +36,10 @@
let
inherit (lib) optional optionals optionalString;
+ hackThemTriplesForGreatJustice = triple: {
+ "aarch64-unknown-uefi" = "aarch64-windows";
+ }.${triple} or triple;
+
# Used when creating a version-suffixed symlink of libLLVM.dylib
shortVersion = with lib;
concatStringsSep "." (take 1 (splitString "." release_version));
@@ -308,8 +312,8 @@ in
"-DLLVM_INSTALL_UTILS=ON" # Needed by rustc
"-DLLVM_BUILD_TESTS=${if doCheck then "ON" else "OFF"}"
"-DLLVM_ENABLE_FFI=ON"
- "-DLLVM_HOST_TRIPLE=${stdenv.hostPlatform.config}"
- "-DLLVM_DEFAULT_TARGET_TRIPLE=${stdenv.hostPlatform.config}"
+ "-DLLVM_HOST_TRIPLE=${hackThemTriplesForGreatJustice stdenv.hostPlatform.config}"
+ "-DLLVM_DEFAULT_TARGET_TRIPLE=${hackThemTriplesForGreatJustice stdenv.hostPlatform.config}"
"-DLLVM_ENABLE_DUMP=ON"
] ++ optionals stdenv.hostPlatform.isStatic [
# Disables building of shared libs, -fPIC is still injected by cc-wrapper
|
Last I remember, LLVM 17 was basically ready I think? Can we push it over the line? |
|
LLVM17 has only the basic changes, things like data layout of the target
description are wrong. I'd need to wait LLVM18 if I started working on this
myself.
Le lun. 30 oct. 2023, 09:13, Alyssa Ross ***@***.***> a
écrit :
… I think backporting the Triple commit seems fair, but I don't mind waiting
for LLVM17 though it's tiring.
Last I remember, LLVM 17 was basically ready I think? Can we push it over
the line?
—
Reply to this email directly, view it on GitHub
<#231951 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACMZRGT6GSLYAZDC2ANLNLYB5ORDAVCNFSM6AAAAAAYBODJ3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBUGY4DAMJQGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
| patches = [ | ||
| ./support-uefi-triples.patch | ||
| ]; | ||
|
|
There was a problem hiding this comment.
I'm flattered that you included my patch but since it hasn't been merged upstream I don't think we should do this yet.
Instead I've added *-*-uefi to the exception-list: 54ddd6e 20a384e in #235230
If gnu-config merges the patch then we can just drop the exception. If they don't, then the fallout is limited to having an extra exception rather than having to back out the patch (which causes a total rebuild of the entire packageset).
There was a problem hiding this comment.
Pushed a17e894 to this branch to drop the patch. Speeds up CI since gnu-config is part of stdenv.
…/5495945c851a7bc9770eda5a3ed63c25 This commit fixes the CI failures with PR 231951: - https://gist.github.com/GrahamcOfBorg/5495945c851a7bc9770eda5a3ed63c25
| homepage = "https://github.com/nix-community/lanzaboote"; | ||
| license = licenses.gpl3Only; | ||
| mainProgram = "lanzaboote_stub.efi"; | ||
| platforms = [ "x86_64-uefi" "i686-uefi" "aarch64-uefi" ]; |
There was a problem hiding this comment.
| platforms = [ "x86_64-uefi" "i686-uefi" "aarch64-uefi" ]; | |
| hydraPlatforms = [ lib.systems.inspect.platformPatterns.isUefi ]; |
| platforms = [ "x86_64-uefi" "i686-uefi" "aarch64-uefi" ]; | ||
| # i686: Builtins errors | ||
| # aarch64: compile fine but... | ||
| broken = stdenv.isi686 || stdenv.isAarch64; |
There was a problem hiding this comment.
| broken = stdenv.isi686 || stdenv.isAarch64; | |
| broken = with stdenv.hostPlatform; isi686 || isAarch64; |
| linker = "lld"; | ||
| }; | ||
|
|
||
|
|
| redox = filterDoubles predicates.isRedox; | ||
| windows = filterDoubles predicates.isWindows; | ||
| genode = filterDoubles predicates.isGenode; | ||
| uefi = filterDoubles predicates.isEfiEnvironment; |
There was a problem hiding this comment.
| uefi = filterDoubles predicates.isEfiEnvironment; | |
| uefi = filterDoubles predicates.isUefi; |
| isMusl = with abis; map (a: { abi = a; }) [ musl musleabi musleabihf muslabin32 muslabi64 ]; | ||
| isUClibc = with abis; map (a: { abi = a; }) [ uclibc uclibceabi uclibceabihf ]; | ||
|
|
||
| isEfiEnvironment = [ |
There was a problem hiding this comment.
| isEfiEnvironment = [ | |
| isUefi = [ |
| mainProgram = "lanzaboote_stub.efi"; | ||
| platforms = [ "x86_64-uefi" "i686-uefi" "aarch64-uefi" ]; | ||
| # i686: Builtins errors | ||
| # aarch64: compile fine but... |
There was a problem hiding this comment.
| # aarch64: compile fine but... | |
| # aarch64: compile fine but unable to test on real hardware |
There was a problem hiding this comment.
shouldn't it be # aarch64: compiles fine but unable to test on real hardware?
|
If you rebase this to a more recent staging (or switch branch to |
I just need to do it when I don't feel like I will fuck up :D. |
|
Is there any progress on this? :) |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/installation-medium-has-invalid-secure-boot-signature/58952/6 |
|
This pull requests is stalled. |
Can I ask you the courtesy to ping the author of PR whom you close PRs? (Or put the documentation of that new nixpkgs process in your closing messages?) Thank you. |
|
Well, do you want to work on this? You said me in several occasions that you don't want to contribute to nixpkgs anymore and working on your NixOS fork. I do not think we should add too many processes to nixpkgs because it will make it too hard to understand how to make changes, so please just re-open the pull request again if you want to work on this. Otherwise just delete the branch. |
Again, all of this has nothing to do with my request. Simply ask before doing what when it comes to other people's work is what I would like you to do. Thank you. |
|
Well, usually I ask them to re-open their pull request if they want to continue to work on the pull request. But I was 95% sure you would want not do that, so I kept it at that. |
If you want to make up your own rules, please consider discussing it with others before applying them out of the blue, especially when it comes to the work of others. "Closing then asking to re-open" is not courtesy, it's just you doing whatever you think is acceptable to do at any time, and you are continuing to shove your view. |
Description of changes
This brings https://github.com/nix-community/lanzaboote UEFI stub in-tree.
Dependents:
Dependencies:
Related:
Current design
Stub and tool are packaged in the global hierarchy as:
lanzaboote-toolandlanzaboote-uefi-stub, no assumption on the system are encoded inside their package respectively.It is expected to build the UEFI stub using a UEFI system with a UEFI compiler.
The linker is a flavored linker which will propagate adequately the
lldWindows-style driver for arguments.lanzaboote-toolis supposed to be wrapped to know about itslanzaboote-stub, unfortunately, this is impossible to achieve inside of nixpkgs, multiple attempts were proposed to the reviewers in the next section and were all rejected. Therefore, the only solution is to give up on having a wrappedlanzaboote-tooland propose a semi-wrappedlanzaboote-tooland expect the user to understand they have to export theLANZABOOTE_STUBenvironment variable to know about the stub itself.This is what we do in the systemd-boot NixOS module ourselves and we expect to work in many cases, either case, author would like to move this PR forward ideally and not concern themselves further with the semi wrapping issue stemming from the impossibility to build a package as a data via cross-compilation inside of nixpkgs, alas.
What has been tried?
pkgs/hierarchy: NACK because it reimports nixpkgs inside nixpkgs.uefiPkgsfor cross compilation inside nixpkgs without a direct reimport: NACK because it's too complicated and does not compose well.Some funny questions
It seems like this PR tries to move to make UEFI a proper target in nixpkgs, but there's already plenty of "data as UEFI binaries" inside of nixpkgs without any cross compilation involved per se:
Ideally, we should treat the same way as in this PR, I suppose this will be very hard given their build system.
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)