Add override function back to buildArmTrustedFirmware results for backward compatibility

[jmbaur]

The work done in 1898fb4 introduced a breaking change since the override function was removed. This adds back support for that, with an eval warning encouraging users to switch to the more extensible overrideAttrs for ATF related build parameters.

Things done

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Tested basic functionality of all binary files, usually in ./result/bin/.
• Nixpkgs Release Notes
  • Package update: when the change is major or breaking.
• NixOS Release Notes
  • Module addition: when adding a new NixOS module.
  • Module update: when the change is significant.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

---

Add a 👍 reaction to pull requests you find important.

[jmbaur]

@ofborg test optee

[emilazy]

with an eval warning encouraging users to switch to the more extensible overrideAttrs for ATF related build parameters.

This seems like a step in the wrong direction, from defined public API to monkey-patching internals. I would prefer to go the other way, as someone with an out-of-tree TF-A package.

[jmbaur]

with an eval warning encouraging users to switch to the more extensible overrideAttrs for ATF related build parameters.

This seems like a step in the wrong direction, from defined public API to monkey-patching internals. I would prefer to go the other way, as someone with an out-of-tree TF-A package.

The other way, as in use overrideAttrs? If so, I agree 100%, however the original commit I made broke eval on atf derivations that used override. This change is intended to provide backward compat for some amount of time, with the goal of removing override for the custom atf args altogether in favor of overrideAttrs

[emilazy]

No, overrideAttrs is a bad interface because it exposes internal implementation details of derivations and is correspondingly fragile, hence #421201 to cut down on its in‐tree use. Explicit options exposed via override are public API that can have their implementation adjusted without breaking downstream consumers. overrideAttrs is useful for quick out‐of‐tree patching hacks, but it shouldn’t be part of our recommended public interface for generic builder functions.

cc @alyssais

[jmbaur]

No, overrideAttrs is a bad interface because it exposes internal implementation details of derivations and is correspondingly fragile, hence #421201 to cut down on its in‐tree use. Explicit options exposed via override are public API that can have their implementation adjusted without breaking downstream consumers. overrideAttrs is useful for quick out‐of‐tree patching hacks, but it shouldn’t be part of our recommended public interface for generic builder functions.

cc @alyssais

Got it. Just read through the PR you linked, it's a super nuanced issue. I guess without getting too much into the weeds, the change in 1898fb4 is helpful because it allows for (IMO) nicer composition of options going into the ATF build without having the "extraThisOrThat" parameters for every single thing wrapping some underlying mkDerivation arg. It also properly allows for the dependencies of ATF itself to be overridden with override instead of overrideAttrs (and thus in-line with the goals in the PR you linked). It's a similar story for buildUBoot, where the dependencies used by that function are only able to be overridden using overrideAttrs. With regards to this PR, I guess the only way forward is to close this and revert 1898fb4, since it introduces eval failures for existing users of buildArmTrustedFirmware where override is also used on the resulting ATF derivation. Does that sound right?