Refactor nix flake, based on overriding nixpkgs #345

MattSturgeon · 2025-01-20T08:07:58Z

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:

packaging/nix/flake.nix

beh-10257 · 2025-01-20T08:34:08Z

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))

beh-10257 · 2025-01-20T09:23:57Z

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

packaging/nix/package.nix

R1kaB3rN · 2025-01-20T18:00:18Z

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 · 2025-01-20T18:00:40Z

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

LovingMelody · 2025-01-20T20:51:56Z

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: "

packaging/nix/flake.nix

packaging/nix/umu-run.nix

packaging/nix/flake.nix

packaging/nix/override.nix

packaging/nix/package.nix

MattSturgeon · 2025-01-22T10:36:26Z

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 · 2025-01-22T10:50:06Z

@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 · 2025-01-22T11:14:48Z

So you think less testing is better?

R1kaB3rN · 2025-01-31T06:01:46Z

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

MattSturgeon · 2025-01-31T06:06:04Z

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.

packaging/nix/unwrapped.nix

beh-10257 · 2025-01-31T06:12:39Z

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

paths are broken is this whats intended to happen ??

beh-10257 · 2025-01-31T06:27:18Z

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

add these to install flags

I know its fixed but add this

umu-delta-install

beh-10257 · 2025-01-31T06:30:43Z

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

R1kaB3rN · 2025-01-31T06:33:45Z

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.

The umu-vendored make target depends on git submodules and is not needed now that we provide pyzstd as a nix package. Until now, it was disabled with a patch, however that broke in Open-Wine-Components#330. Instead of patching the makefile's `all` target, we can explicitly specify the targets we want to build/install using make flags. This is a temporary workaround until the Makefile can automatically disable the target when the dependency is already available. Maybe a make variable?

MattSturgeon · 2025-01-31T06:38:08Z

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.

beh-10257

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 · 2025-01-31T06:43:42Z

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.

https://github.com/Open-Wine-Components/umu-launcher/releases/tag/1.2.0 https://github.com/Open-Wine-Components/umu-launcher/releases/tag/1.2.1 Incorporates work done in: Open-Wine-Components/umu-launcher#345

https://github.com/Open-Wine-Components/umu-launcher/releases/tag/1.2.0 https://github.com/Open-Wine-Components/umu-launcher/releases/tag/1.2.1 https://github.com/Open-Wine-Components/umu-launcher/releases/tag/1.2.2 https://github.com/Open-Wine-Components/umu-launcher/releases/tag/1.2.3 Incorporates work done in: Open-Wine-Components/umu-launcher#345

MattSturgeon commented Jan 20, 2025

View reviewed changes

packaging/nix/flake.nix Outdated Show resolved Hide resolved

MattSturgeon force-pushed the main branch 2 times, most recently from eb39a88 to e9cd956 Compare January 20, 2025 12:41

MattSturgeon commented Jan 20, 2025

View reviewed changes

packaging/nix/package.nix Outdated Show resolved Hide resolved

LovingMelody suggested changes Jan 20, 2025

View reviewed changes

packaging/nix/flake.nix Outdated Show resolved Hide resolved

MattSturgeon force-pushed the main branch 2 times, most recently from 1f41ff2 to f0906bd Compare January 21, 2025 09:06

MattSturgeon commented Jan 21, 2025

View reviewed changes

packaging/nix/umu-run.nix Show resolved Hide resolved

MattSturgeon force-pushed the main branch 3 times, most recently from efaa609 to 26c9c70 Compare January 21, 2025 17:27

diniamo suggested changes Jan 21, 2025

View reviewed changes

packaging/nix/flake.nix Outdated Show resolved Hide resolved

packaging/nix/flake.nix Outdated Show resolved Hide resolved

packaging/nix/override.nix Outdated Show resolved Hide resolved

packaging/nix/package.nix Outdated Show resolved Hide resolved

MattSturgeon mentioned this pull request Jan 21, 2025

Specify alejandra as the nix formatter #350

Merged

MattSturgeon force-pushed the main branch from 26c9c70 to ff8c139 Compare January 21, 2025 18:37

MattSturgeon mentioned this pull request Jan 22, 2025

Add nix maintainers #351

Merged

MattSturgeon force-pushed the main branch from ff8c139 to 794f2ed Compare January 22, 2025 10:24

MattSturgeon marked this pull request as ready for review January 22, 2025 10:36

This comment has been minimized.

Sign in to view

MattSturgeon force-pushed the main branch from 794f2ed to 2d76ec0 Compare January 22, 2025 11:50

MattSturgeon changed the title ~~Base nix flake on nixpkgs package~~ Refactor nix flake, based on overriding nixpkgs Jan 22, 2025

R1kaB3rN requested a review from LovingMelody January 22, 2025 18:58

R1kaB3rN requested a review from beh-10257 January 31, 2025 06:01

MattSturgeon commented Jan 31, 2025

View reviewed changes

packaging/nix/unwrapped.nix Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

MattSturgeon force-pushed the main branch from bd89c8b to 1c1df13 Compare January 31, 2025 06:23

This comment was marked as resolved.

Sign in to view

MattSturgeon force-pushed the main branch from 1c1df13 to 1e1f582 Compare January 31, 2025 06:32

MattSturgeon force-pushed the main branch from 1e1f582 to d69fe8b Compare January 31, 2025 06:34

beh-10257 approved these changes Jan 31, 2025

View reviewed changes

R1kaB3rN approved these changes Jan 31, 2025

View reviewed changes

R1kaB3rN merged commit bc7667d into Open-Wine-Components:main Jan 31, 2025
12 checks passed

LovingMelody mentioned this pull request Feb 1, 2025

umu -> umu-launcher: switch to npins & rename to umu-launcher fufexan/nix-gaming#233

Merged

MattSturgeon mentioned this pull request Feb 14, 2025

umu-launcher: 1.1.4 -> 1.2.5 & init pyzstd NixOS/nixpkgs#381975

Merged

11 tasks

MattSturgeon mentioned this pull request Feb 25, 2025

build(nix): drop support for outdated nixpkgs revisions #402

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor nix flake, based on overriding nixpkgs #345

Refactor nix flake, based on overriding nixpkgs #345

MattSturgeon commented Jan 20, 2025 •

edited

Loading

beh-10257 commented Jan 20, 2025

beh-10257 commented Jan 20, 2025

R1kaB3rN commented Jan 20, 2025

R1kaB3rN commented Jan 20, 2025

LovingMelody commented Jan 20, 2025

MattSturgeon commented Jan 22, 2025 •

edited

Loading

diniamo commented Jan 22, 2025

This comment has been minimized.

diniamo commented Jan 22, 2025

This comment has been minimized.

R1kaB3rN commented Jan 31, 2025

MattSturgeon commented Jan 31, 2025

beh-10257 commented Jan 31, 2025 •

edited

Loading

This comment was marked as resolved.

This comment was marked as resolved.

beh-10257 commented Jan 31, 2025 •

edited

Loading

This comment was marked as resolved.

beh-10257 commented Jan 31, 2025

R1kaB3rN commented Jan 31, 2025

MattSturgeon commented Jan 31, 2025 •

edited

Loading

beh-10257 left a comment

R1kaB3rN commented Jan 31, 2025 •

edited

Loading

Refactor nix flake, based on overriding nixpkgs #345

Refactor nix flake, based on overriding nixpkgs #345

Conversation

MattSturgeon commented Jan 20, 2025 • edited Loading

Testing

Packages

Public API

Version

Nixpkgs

beh-10257 commented Jan 20, 2025

beh-10257 commented Jan 20, 2025

R1kaB3rN commented Jan 20, 2025

R1kaB3rN commented Jan 20, 2025

LovingMelody commented Jan 20, 2025

MattSturgeon commented Jan 22, 2025 • edited Loading

diniamo commented Jan 22, 2025

This comment has been minimized.

diniamo commented Jan 22, 2025

This comment has been minimized.

R1kaB3rN commented Jan 31, 2025

MattSturgeon commented Jan 31, 2025

beh-10257 commented Jan 31, 2025 • edited Loading

This comment was marked as resolved.

This comment was marked as resolved.

beh-10257 commented Jan 31, 2025 • edited Loading

This comment was marked as resolved.

beh-10257 commented Jan 31, 2025

R1kaB3rN commented Jan 31, 2025

MattSturgeon commented Jan 31, 2025 • edited Loading

beh-10257 left a comment

Choose a reason for hiding this comment

R1kaB3rN commented Jan 31, 2025 • edited Loading

MattSturgeon commented Jan 20, 2025 •

edited

Loading

MattSturgeon commented Jan 22, 2025 •

edited

Loading

beh-10257 commented Jan 31, 2025 •

edited

Loading

beh-10257 commented Jan 31, 2025 •

edited

Loading

MattSturgeon commented Jan 31, 2025 •

edited

Loading

R1kaB3rN commented Jan 31, 2025 •

edited

Loading