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.

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

fix compileDTS overlay for raspberry-pi #754

Merged
merged 2 commits into from
Oct 10, 2023

Conversation

gador
Copy link
Contributor

@gador gador commented Oct 8, 2023

Due to the recent change in NixOS/nixpkgs#251898
the evaluation of my raspberry-pi-4 config is now broken with the following error:

error:
       … while calling the 'derivationStrict' builtin

         at /builtin/derivation.nix:9:12: (source not available)

       … while evaluating derivation 'nixos-system-kari-23.11.20231007.a1decd5'
         whose name attribute is located at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/pkgs/stdenv/generic/make-derivation.nix:300:7

       … while evaluating attribute 'buildCommand' of derivation 'nixos-system-kari-23.11.20231007.a1decd5'

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/nixos/modules/system/activation/top-level.nix:53:5:

           52|     passAsFile = [ "extraDependencies" ];
           53|     buildCommand = systemBuilder;
             |     ^
           54|

       (stack trace truncated; use '--show-trace' to show the full trace)

       error: attribute 'compileDTS' missing

       at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/nixos/modules/hardware/device-tree.nix:91:7:

           90|       in
           91|       pkgs.deviceTree.compileDTS {
             |       ^
           92|         name = "${o.name}-dtbo";
The full stack trace is:
error:
       … while calling the 'derivationStrict' builtin

         at /builtin/derivation.nix:9:12: (source not available)

       … while evaluating derivation 'nixos-system-kari-23.11.20231007.a1decd5'
         whose name attribute is located at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/pkgs/stdenv/generic/make-derivation.nix:300:7

       … while evaluating attribute 'buildCommand' of derivation 'nixos-system-kari-23.11.20231007.a1decd5'

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/nixos/modules/system/activation/top-level.nix:53:5:

           52|     passAsFile = [ "extraDependencies" ];
           53|     buildCommand = systemBuilder;
             |     ^
           54|

       … while calling 'g'

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/attrsets.nix:599:19:

          598|           g =
          599|             name: value:
             |                   ^
          600|             if isAttrs value && cond value

       … from call site

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/attrsets.nix:602:20:

          601|               then recurse (path ++ [name]) value
          602|               else f (path ++ [name]) value;
             |                    ^
          603|         in mapAttrs g;

       … while calling anonymous lambda

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/modules.nix:242:72:

          241|           # For definitions that have an associated option
          242|           declaredConfig = mapAttrsRecursiveCond (v: ! isOption v) (_: v: v.value) options;
             |                                                                        ^
          243|

       … while evaluating the option `system.systemBuilderCommands':

       … while calling anonymous lambda

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/modules.nix:822:28:

          821|         # Process mkMerge and mkIf properties.
          822|         defs' = concatMap (m:
             |                            ^
          823|           map (value: { inherit (m) file; inherit value; }) (builtins.addErrorContext "while evaluating definitions from `${m.file}':" (dischargeProperties m.value))

       … while evaluating definitions from `/nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/nixos/modules/system/boot/kernel.nix':

       … from call site

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/modules.nix:823:137:

          822|         defs' = concatMap (m:
          823|           map (value: { inherit (m) file; inherit value; }) (builtins.addErrorContext "while evaluating definitions from `${m.file}':" (dischargeProperties m.value))
             |                                                                                                                                         ^
          824|         ) defs;

       … while calling 'dischargeProperties'

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/modules.nix:894:25:

          893|   */
          894|   dischargeProperties = def:
             |                         ^
          895|     if def._type or "" == "merge" then

       … from call site

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/modules.nix:900:11:

          899|         if def.condition then
          900|           dischargeProperties def.content
             |           ^
          901|         else

       … while calling 'dischargeProperties'

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/modules.nix:894:25:

          893|   */
          894|   dischargeProperties = def:
             |                         ^
          895|     if def._type or "" == "merge" then

       … from call site

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/nixos/modules/system/boot/kernel.nix:331:15:

          330|             ln -s ${config.system.modulesTree} $out/kernel-modules
          331|             ${optionalString (config.hardware.deviceTree.package != null) ''
             |               ^
          332|               ln -s ${config.hardware.deviceTree.package} $out/dtbs

       … while calling 'optionalString'

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/strings.nix:243:5:

          242|     # String to return if condition is true
          243|     string: if cond then string else "";
             |     ^
          244|

       … while calling 'g'

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/attrsets.nix:599:19:

          598|           g =
          599|             name: value:
             |                   ^
          600|             if isAttrs value && cond value

       … from call site

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/attrsets.nix:602:20:

          601|               then recurse (path ++ [name]) value
          602|               else f (path ++ [name]) value;
             |                    ^
          603|         in mapAttrs g;

       … while calling anonymous lambda

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/modules.nix:242:72:

          241|           # For definitions that have an associated option
          242|           declaredConfig = mapAttrsRecursiveCond (v: ! isOption v) (_: v: v.value) options;
             |                                                                        ^
          243|

       … while evaluating the option `hardware.deviceTree.package':

       … while calling anonymous lambda

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/modules.nix:844:17:

          843|       if isDefined then
          844|         if all (def: type.check def.value) defsFinal then type.merge loc defsFinal
             |                 ^
          845|         else let allInvalid = filter (def: ! type.check def.value) defsFinal;

       … from call site

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/modules.nix:844:22:

          843|       if isDefined then
          844|         if all (def: type.check def.value) defsFinal then type.merge loc defsFinal
             |                      ^
          845|         else let allInvalid = filter (def: ! type.check def.value) defsFinal;

       … while calling 'check'

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/types.nix:619:15:

          618|       descriptionClass = "conjunction";
          619|       check = x: x == null || elemType.check x;
             |               ^
          620|       merge = loc: defs:

       … from call site

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/types.nix:619:31:

          618|       descriptionClass = "conjunction";
          619|       check = x: x == null || elemType.check x;
             |                               ^
          620|       merge = loc: defs:

       … while calling 'check'

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/types.nix:493:15:

          492|       descriptionClass = "noun";
          493|       check = x: isStringLike x && builtins.substring 0 1 (toString x) == "/";
             |               ^
          494|       merge = mergeEqualOption;

       … while evaluating derivation 'device-tree-overlays'
         whose name attribute is located at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/pkgs/stdenv/generic/make-derivation.nix:300:7

       … while evaluating attribute 'buildCommand' of derivation 'device-tree-overlays'

         at /nix/store/b7xl2abnwi94znl0hpqk43z41m9l5spa-source/raspberry-pi/4/apply-overlays-dtmerge.nix:8:3:

            7|   nativeBuildInputs = [ dtc libraspberrypi ];
            8|   buildCommand = let
             |   ^
            9|     overlays = toList overlays';

       … from call site

         at /nix/store/b7xl2abnwi94znl0hpqk43z41m9l5spa-source/raspberry-pi/4/apply-overlays-dtmerge.nix:21:9:

           20|
           21|       ${flip (concatMapStringsSep "\n") overlays (o: ''
             |         ^
           22|       overlayCompat="$(fdtget -t s "${o.dtboFile}" / compatible)"

       … while calling 'flip'

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/trivial.nix:138:16:

          137|   */
          138|   flip = f: a: b: f b a;
             |                ^
          139|

       … from call site

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/trivial.nix:138:19:

          137|   */
          138|   flip = f: a: b: f b a;
             |                   ^
          139|

       … while calling 'concatMapStringsSep'

         at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/lib/strings.nix:117:5:

          116|     # List of input strings
          117|     list: concatStringsSep sep (map f list);
             |     ^
          118|

       … while calling anonymous lambda

         at /nix/store/b7xl2abnwi94znl0hpqk43z41m9l5spa-source/raspberry-pi/4/apply-overlays-dtmerge.nix:21:51:

           20|
           21|       ${flip (concatMapStringsSep "\n") overlays (o: ''
             |                                                   ^
           22|       overlayCompat="$(fdtget -t s "${o.dtboFile}" / compatible)"

       error: attribute 'compileDTS' missing

       at /nix/store/6bs7cvmiiv6792bs2mbhigmr0jgaj9ma-source/nixos/modules/hardware/device-tree.nix:91:7:

           90|       in
           91|       pkgs.deviceTree.compileDTS {
             |       ^
           92|         name = "${o.name}-dtbo";
Description of changes

due to NixOS/nixpkgs#251898 the compileDTS function has been moved to device-tree
nixos-hardware has an own overlay for the raspberry-pi which needs the compileDTS function now, too.

This commit adds it.

Things done
  • Tested the changes in your own NixOS Configuration
  • Tested the changes end-to-end by using your fork of nixos-hardware and
    importing it via <nixos-hardware> or Flake input

Please note: This has not yet been tested IRL on my raspberry-pi. But the evaluation and compilation of my NixOS config now doesn't throw an error.
If anyone is willing to test it, please do so and report back.

due to NixOS/nixpkgs#251898
the compileDTS function has been moved to `device-tree`

nixos-hardware has an own overlay for the raspberry-pi
which needs the `compileDTS` function now, too.

This commit adds it.

Signed-off-by: Florian Brandes <florian.brandes@posteo.de>
Copy link
Contributor

@Majiir Majiir left a comment

Choose a reason for hiding this comment

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

Are we sure that any overlay is needed for device-tree? A few fixes have been merged in nixpkgs since this fork was created.

For reference, this is the cleaned-up diff between the nixpkgs and nixos-hardware versions:

--- a/pkgs/os-specific/linux/device-tree/default.nix
+++ b/pkgs/os-specific/linux/device-tree/default.nix
@@ -1,4 +1,4 @@
-{ lib, stdenv, stdenvNoCC, dtc }:
+{ lib, stdenv, stdenvNoCC, dtc, libraspberrypi }:
 
 with lib; {
   # Compile single Device Tree overlay source
@@ -26,7 +26,7 @@ with lib; {
 
   applyOverlays = (base: overlays': stdenvNoCC.mkDerivation {
     name = "device-tree-overlays";
-    nativeBuildInputs = [ dtc ];
+    nativeBuildInputs = [ dtc libraspberrypi ];
     buildCommand = let
       overlays = toList overlays';
     in ''
@@ -46,7 +46,7 @@ with lib; {
         # skip incompatible and non-matching overlays
         if [[ ! "$dtbCompat" =~ "$overlayCompat" ]]; then
           echo "Skipping overlay ${o.name}: incompatible with $(basename "$dtb")"
-        elif ${if (o.filter == null) then "false" else ''
+        elif ${if ((o.filter or null) == null) then "false" else ''
           [[ "''${dtb//${o.filter}/}" ==  "$dtb" ]]
         ''}
         then
@@ -54,9 +54,15 @@ with lib; {
         else
           echo -n "Applying overlay ${o.name} to $(basename "$dtb")... "
           mv "$dtb"{,.in}
-          fdtoverlay -o "$dtb" -i "$dtb.in" "${o.dtboFile}"
+
+          # dtmerge requires a .dtbo ext for dtbo files, otherwise it adds it to the given file implicitly
+          dtboWithExt="$TMPDIR/$(basename "${o.dtboFile}").dtbo"
+          cp -r ${o.dtboFile} "$dtboWithExt"
+
+          dtmerge "$dtb.in" "$dtb" "$dtboWithExt"
+
           echo "ok"
-          rm "$dtb.in"
+          rm "$dtb.in" "$dtboWithExt"
         fi
         '')}

raspberry-pi/4/pkgs-overlays.nix Outdated Show resolved Hide resolved
@gador
Copy link
Contributor Author

gador commented Oct 8, 2023

@Majiir awesome, thanks, I simplified the fix.

Are we sure that any overlay is needed for device-tree? A few fixes have been merged in nixpkgs since this fork was created.

This is a good question. There are subtle differences in there.
Maybe @ipetkov or @carlossless could chime in?

But this is maybe a matter for another PR. This (now small) change at least allows building the config again with the current nixpkgs

Co-authored-by: Majiir Paktu <majiir@nabaal.net>
Signed-off-by: Florian Brandes <florian.brandes@posteo.de>
@carlossless
Copy link
Contributor

carlossless commented Oct 10, 2023

@gador @Majiir the main function of that overlay (in the nix sense) is to use dtmerge instead of fdtoverlay for merging the device tree overlays. dtmerge was needed because the overlays (example) that rpi distributes were not compatible with fdtoverlay.

I don't remember the specifics about what exactly is not supported by fdtoverlay and I'm not aware if anything changed.

I won't have time for this myself in the coming weeks, but it could be tested by removing the package overlay, turning on the poe-hat overlay and seeing if it builds.

@Mic92
Copy link
Member

Mic92 commented Oct 10, 2023

Let me know when this is good to be merged.

@gador
Copy link
Contributor Author

gador commented Oct 10, 2023

@carlossless thanks for the feedback! I guess one can have a look at removing the custom dtmerge function at a later point?

I just tested this PR in my config on my raspberry pi 4 with POE+ hat. Works just fine

@yu-re-ka yu-re-ka merged commit c2bbfcf into NixOS:master Oct 10, 2023
2 checks passed
bct added a commit to bct/nix-config that referenced this pull request Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants