Call for Discussion: why and how much should we care about the `.override` interfaces? (Part I: Duckstation and PCSX2)

Development Nixpkgs Architecture
1

From time to time I refactor the expressions in Nixpkgs, whether mine or someone else’s.

The reasons are multiple:

  • cosmetic formatting

  • finalAttrs design pattern

  • removing or banishing bad language constructs such as rec and nested with, according to the Best Practices

  • migration to by-name hierarchy

  • Technical decisions based on intuition more than a technical and weary examination

In this last category, recently I am unifying the .override interface (a.k.a. the input set) of some packages.
I have started with bochs, and it is working fine (except unrelated, typical errors on Darwin), but two specific cases were a bit polemic.

For now I will leave the other case to a new post. In this I will talk about the Duckstation update PR

Triggered by an update PR by SuperSamus, I migrated and refactored the expression that builds Duckstation. In order to do it, I modified the expression to use callPackage instead of qt6.callPackage as before.

Further, the new Duckstation release required the application of a patch, not in the Duckstation code but rather into a dependency, namely shaderc.

That being said, originally SuperSamus built the patched shaderc inside the Duckstation expression. Basically, the caller provides a pristine shaderc, so that the expression patches and builds shaderc before building duckstation itself.

Instead of this approach, I preferred to keep the expression “agnostic” about its inputs: duckstation receives whatever shaderc is provided, while the caller (at top-level) provides the correctly patched shaderc.

Supersandro2000 raised two objections about this.

  • The first was about callPackage VS qt6.callPackage.
  • The second was about patched shaderc inside VS outside the expression.

Summarizing my decisions, I chose callPackage in order to make the expression compatible with by-name hierarchy, and I patched shaderc outside duckstation because it keeps the override interface cleaner and predictable.

This post is too long already, I will vent my full opinion tomorrow. For now I expect I have provided a clear picture about what is going on.

Calling @SuperSamus @Sandro as participants of the conversation.

4 Likes
2

I believe this is standard practice as well. To me it follows the same logic as passing electron and then specifying a specific version in all-packages.nix, which appears to be preferred over specifying, say, electron_30 directly as function arg.

3

Making the expression totally agnostic about its input only amounts to sharding the domain knowledge IMO. I’m in favor of specifying the supported version in the expression, but would love it if it could be done in a way that yields a stable override interface.

4

I’ve made some progress towards changing callPackage here, but pay attention mostly to the first line of hello/package.nix. Before we do anything radical like that PR, I think we need to create a migration path away from the current callPackage definition. The PR shows a possible way to achieve that in makeOverridable (which is basically the half of callPackage where the problem is). It allows a package file to bring its own call package, bootstrapping off the legacy callPackage.

That PR doesn’t actually solve the problem yet, but I think it could be solved with a user facing syntax like:

{ callPackageWithDefaults }: callPackageWithDefaults
({ git_minimal }: { git = git_minimal; })
({
  git 
}:

stdenv.mkDerivation {
  # ...
})

or even

{ callPackageWithDefaults }: callPackageWithDefaults ({
  pkgs,
  git ? pkgs.git_minimal
}:

stdenv.mkDerivation {
  # ...
})

So this would achieve the desired effect of having usable default values defined inside package files, without breaking existing callPackage invocations, and it would provide a path forward for more cleanups, and better support for packages that aren’t typical mkDerivation packages.

4 Likes