Refactor nix flake, based on overriding nixpkgs

[MattSturgeon]

Now that there's a package being maintained in nixpkgs, it can be used as a starting point for the nix package here.

This is technically a breaking change, because the packages made available by the overlay are different and because the internals of the packages are different. But I've made efforts to avoid breaking changes to the public-facing API where practical.

Testing

If you have a nix flake setup as described in the README, you can simply replace your inputs.umu.url url string with:

umu.url = "github:MattSturgeon/umu-launcher?dir=packaging/nix"

Also, you may want to remove any follows targeting the umu input, or alternatively run nix flake update. Either way, the nixpkgs input must have NixOS/nixpkgs#369259 and NixOS/nixpkgs#375588.

You may occasionally need to update your lockfile (nix flake update) to get the latest changes from this PR.

Packages

• umu-launcher is now equivalent to the old umu and umu-run packages (combine.nix & umu-run.nix)
• umu-launcher-unwrapped is now equivalent to the old umu-launcher (umu-launcher.nix)
• umu-launcher-unwrapped is directly based on nixpkgs, overriding the relevant attrs to use this git repo instead of 1.1.4
• umu-launcher uses the overridden umu-launcher-unwrapped implicitly

Public API

The existing package-args API is still available, however warnings will be printed if it is used.

It consists of override-able package args:

• version
• truststore
• cbor2 (although this never had any effect)

A new API is proposed:

• version
• withTruststore
• withDeltaUpdates

The README has been updated to reflect the new package-args API.

Version

The package version is now based on the flake's shortRev, or dirtyShortRev if the flake is used locally and there are uncommitted changes.

This means users no longer need to override the version themselves.

The README has been updated to reflect this.

Nixpkgs

The new implementation relies on nixpkgs having the umu-launcher-unwrapped package added in NixOS/nixpkgs#369259 and a small fix from NixOS/nixpkgs#375588.

That package has been in:

• master since 2024-01-19
• nixpkgs-unstable since 2024-01-20
• nixos-unstable-small since 2024-01-20
• nixos-unstable since 2024-01-22

The nixpkgs maintainers are:

• @diniamo
• @MattSturgeon
• @LovingMelody

[bohanubis]

Looks good to me
Will look into making myself a nixpkgs maintainer

(Probably the problem was the combine.nix I didn't know I didn't need it at the time (my first flake after all))

[bohanubis]

Actually remove the warning for using umu-run aka the rename function

Using umu-run package is not documented or documented either way so yeah

[R1kaB3rN]

As for the flake changes, I cannot give a review there. Though I will say that I'm not seeing any issue with #343 and it doesn't affect the other packaging formats so you don't need to worry about it. Your change to the Makefile will only be a problem if the user or distribution maintainer overrides INSTALLER_ARGS. On that event, it will be their responsibility. And suppose INSTALLER_ARGS is a problem in another way. Most Linux distribution maintainers are competent enough to evaluate bad packaging practices before distributing the software to their users and will apply patches to workaround them. Again, it will be on them for not patching or failing to find the bad practices.

[R1kaB3rN]

cc @LovingMelody for review as the change directly affects nix-citizen.

[LovingMelody]

This doesn't seem to build?

umu-launcher/packaging/nix on  MattSturgeon/main:main 
at 15:49:15 ❯ nix build .\#umu-launcher .\#umu --override-input nixpkgs github:Nixos/nixpkgs/master
warning: not writing modified lock file of flake 'git+file:///home/melody/Development/umu-launcher?dir=packaging/nix':
• Updated input 'nixpkgs':
    'github:nixos/nixpkgs/538bf58cdd0cff89a93217837f4d87b6eb45ab2c?narHash=sha256-w78d7bi8hkCWVmGbuE5D9YRIJY27dbEBgwLsCRd8dV8%3D' (2025-01-20)
  → 'github:Nixos/nixpkgs/66aa98b29099c636622a9d9c18370f13701716f6?narHash=sha256-b4LNBx%2BScZY3TdYvdFckvuOD7L3RrQ06YUiNG6lmaeM%3D' (2025-01-20)
error:
       … in the condition of the assert statement
         at /nix/store/ad8slsrhg8mz40jl69nx0z1blpcs4dyq-source/lib/customisation.nix:417:9:
          416|       drvPath =
          417|         assert condition;
             |         ^
          418|         drv.drvPath;

       … while evaluating a branch condition
         at /nix/store/ad8slsrhg8mz40jl69nx0z1blpcs4dyq-source/pkgs/stdenv/generic/check-meta.nix:504:6:
          503|       inherit (validity) valid;
          504|   in if validity ? handled then validity else validity // {
             |      ^
          505|       # Throw an error if trying to evaluate a non-valid derivation

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

       error: attempt to call something which is not a function but a set: { type = "derivation"; LANG = «thunk»; __ignoreNulls = true; __structuredAttrs = false; all = «thunk»; args = «thunk»; buildInputs = «thunk»; builder = «thunk»; cmakeFlags = «thunk»; configureFlags = «thunk»; «45 attributes elided» }
       at /nix/store/6ghygpfc3ciyp8hznp30cghv1dxlij9q-source/packaging/nix/flake.nix:21:9:
           20|         pkgs: name:
           21|         pkgs.${name} or throw (
             |         ^
           22|           "umu-launcher: "

[MattSturgeon]

I've refactored and cleaned things up following feedback.

To demonstrate everything working:

nix repl output

$ nix repl

Nix 2.24.11
Type :? for help.

nix-repl> :lf .
Added 15 variables.

nix-repl> :b packages.x86_64-linux.default

This derivation produced the following outputs:
  out -> /nix/store/c1qn3pm1hfnsqiqbbnfv98dfz7f6ff0s-umu-launcher-git.2025.01.22.10.21.40

nix-repl> :b packages.x86_64-linux.umu
evaluation warning: `packages.x86_64-linux.umu` has been renamed to `packages.x86_64-linux.umu-launcher`

This derivation produced the following outputs:
  out -> /nix/store/c1qn3pm1hfnsqiqbbnfv98dfz7f6ff0s-umu-launcher-git.2025.01.22.10.21.40

nix-repl> :b packages.x86_64-linux.default.override { cbor2 = false; }
evaluation warning: umu-launcher: the argument `cbor2` has never had any effect. The new argument `withDeltaUpdates` should be used instead.

This derivation produced the following outputs:
  out -> /nix/store/c1qn3pm1hfnsqiqbbnfv98dfz7f6ff0s-umu-launcher-git.2025.01.22.10.21.40

nix-repl> :b packages.x86_64-linux.default.override { truststore = false; }
evaluation warning: umu-launcher: the argument `truststore` has been renamed to `withTruststore`.

This derivation produced the following outputs:
  out -> /nix/store/z0c1z9dvngcc39m2igbbhr6jpmq71adf-umu-launcher-git.2025.01.22.10.21.40

nix-repl> :b packages.x86_64-linux.default.override { withTruststore = false; }

This derivation produced the following outputs:
  out -> /nix/store/z0c1z9dvngcc39m2igbbhr6jpmq71adf-umu-launcher-git.2025.01.22.10.21.40

nix-repl> :b packages.x86_64-linux.default.override { withDeltaUpdates = false; }

This derivation produced the following outputs:
  out -> /nix/store/iz5jc3il84m3lrw2hdic2yq8vy1645f1-umu-launcher-git.2025.01.22.10.21.40

nix-repl> :b packages.x86_64-linux.default.override { foo = false; }
error:
       … while calling a functor (an attribute set with a '__functor' attribute)

       error: function 'anonymous lambda' called with unexpected argument 'foo'
       at /nix/store/bf13bgpnl10dphhj8kcv5sy34kh2d7z8-source/packaging/nix/package.nix:1:1:
            1| {
             | ^
            2|   # Dependencies:

Note how overriding cbor2 or truststore produce appropriate warnings.

---

As per #345 (comment) I've refactored to avoid constructing an instance of nixpkgs. By using nixpkgs.legacyPackages.<system>, it is far more likely that the same instance of nixpkgs already exists in memory, especially if end-users are configuring follows.

As per #345 (comment), I've refactored and cleaned up the package-args API. The new args are withTruststore and withDeltaChanges.

As per #345 (comment), I've greatly reduced the boilerplate needed for deprecation warnings. I've still ensured breaking changes are avoided where possible.

I haven't switched to nixos-unstable, because I noticed the current flake is targeting nixpkgs-unstable. Given this isn't a system flake, nixpkgs-unstable is probably more suitable anyway and even if not, I don't see changing the input as in-scope for this PR.

NixOS/nixpkgs#369259 is now available in all unstable channels, so I've marked #345 (comment) resolved.

I've listed changes in detail in the commit message, including breaking down the breaking changes I was unable to avoid.

This is now ready for review.

[diniamo]

@MattSturgeon nixos-unstable should be used nonetheless. The names are slightly confusing, the actual difference to nixpkgs-unstable is the amount of tests ran (with nixos-unstable having more).

[diniamo]

So you think less testing is better?

[R1kaB3rN]

Yeah, so if the zstd library is included in the package already then it should be good.

[MattSturgeon]

Yeah, so if the zstd library is included in the package already then it should be good.

It is a buildInput of pyzstd. So it will be installed as a transitive dependency, but without checking, IDK if it will be directly accessible for umu-launcher on its LD_LIBRARY_PATH...

No harm in explicitly adding it back to umu-launcher-unwrapped's inputs too if we're unsure.

I've checked that the package builds as-is, but I'm not familiar enough to know what can/should be checked at runtime.

[bohanubis]

@MattSturgeon I have a question why is nix build ./#umu not working

paths are broken is this whats intended to happen ??

[bohanubis]

      "umu-install"
      "umu-launcher-install"
      "umu-delta-install"

add these to install flags

I know its fixed but add this

umu-delta-install

[bohanubis]

yeah probably
same for umu-delta I guess if we are both wrong @R1kaB3rN will tell us afterword

[R1kaB3rN]

add these to install flags

Should "umu-delta-install" be conditional on withDeltaUpdates?

diff --git a/packaging/nix/unwrapped.nix b/packaging/nix/unwrapped.nix
index fb646b5..11ec5b0 100644
--- a/packaging/nix/unwrapped.nix
+++ b/packaging/nix/unwrapped.nix
@@ -39,9 +39,12 @@ umu-launcher-unwrapped.overridePythonAttrs (prev: {
       "umu-dist"
       "umu-docs"
       "umu-launcher"
-      "umu-delta"
       "umu-install"
       "umu-launcher-install"
+    ]
+    ++ lib.optionals withDeltaUpdates [
+      "umu-delta"
+      "umu-delta-install"
     ];
 
   nativeBuildInputs =

No, umu-delta-install should not be conditional with withDeltaUpdates.

[MattSturgeon]

No, umu-delta-install should not be conditional with withDeltaUpdates.

To clarify, both umu-delta and umu-delta-install should always be installed, regardless of withDeltaUpdates?

If so, this should be resolved.

[bohanubis]

looks good thanks for doing the work I was not in a situation and still isn't in a situation to dedicate this much time thanks again @MattSturgeon

[R1kaB3rN]

No, umu-delta-install should not be conditional with withDeltaUpdates.

To clarify, both umu-delta and umu-delta-install should always be installed, regardless of withDeltaUpdates?

If so, this should be resolved.

Yes, both of those targets are required regardless of withDeltaUpdates.