Skip to content

Conversation

@mazunki
Copy link
Contributor

@mazunki mazunki commented Oct 30, 2025

I'm splitting up #2325 into standalone PRs for easier reviewing.

This was the only dependency which wasn't using default.nix in our list. It should be easier to manage our targets when everything follows the same structure.

@mazunki
Copy link
Contributor Author

mazunki commented Oct 30, 2025

Tests are passing.

A quick search for libfdt suggests this is only relevant for aarch64, and there are no tests surrounding it.

@mazunki
Copy link
Contributor Author

mazunki commented Oct 31, 2025

@elstr-512 , does this help you?

@elstr-512
Copy link

@elstr-512 , does this help you?

Probably! My current solution isn't great, I'm just inserting the libfdt/dtc dependency in overlay.nix.

   ...
      aarch64_inputs =
          if self.stdenv.targetPlatform.system == "aarch64-linux" then [
            prev.pkgsStatic.dtc
          ]
          else [];
   ...

@mazunki
Copy link
Contributor Author

mazunki commented Nov 2, 2025

Let me know if this is missing something. I briefly saw you had a compiler too on your end which this doesn't provide. I suppose that's important?

@MagnusS
Copy link
Member

MagnusS commented Nov 3, 2025

From @elstr-512's branch it looks like we can use dtc from nixpkgs here so we don't have to maintain our own nix build for libfdt. I suggest we close this PR for now, then we can revisit when @elstr-512 submits the ARM build patches. It looks like ./deps/libfdt could then be removed completely.

@alfreb
Copy link
Contributor

alfreb commented Nov 8, 2025

Yea, I agree, if we can use one from nixpkgs let's do that instead. But thanks @mazunki - getting a dependency with nix is better than getting it with cmake, so your solution would be an improvement as well.

@alfreb alfreb closed this Nov 8, 2025
@mazunki
Copy link
Contributor Author

mazunki commented Nov 8, 2025

Understandable. I did see it existed upstream, but was concerned that if we needed to patch libfdt or upstream changed some behaviour in an update we would need to do this anyway. Either way, this is also good.

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.

4 participants