Conversation
There was a problem hiding this comment.
Pull request overview
Updates the flake to newer upstream inputs and reconciles local overlays/NixOS configuration with those updates, including pinning a local Transmission 4 package definition.
Changes:
- Add a local
transmission_4derivation (Transmission 4.0.6) with patch/backport logic and an AppArmor profile output. - Update the overlay to use the local Transmission derivation and add temporary Darwin Python package backports.
- Adjust NixOS host/service configuration (servarr systemd tweaks; auto-upgrade persistence) and refresh
flake.lock.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
pkgs/transmission_4/default.nix |
Introduces a locally pinned Transmission 4.0.6 package with patches, dependency cleanup, and AppArmor output. |
overlays/default.nix |
Switches Transmission pinning to the local derivation; adds Darwin-only python backports/overrides. |
nixos/srvarrvm/default.nix |
Refactors systemd service tweaks (UMask + mount requirements) for servarr-related services. |
nixos/default.nix |
Changes system.autoUpgrade.persistent behavior for the fleet default. |
flake.lock |
Updates pinned input revisions/hashes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mkdir $apparmor | ||
| cat >$apparmor/bin.transmission-daemon <<EOF | ||
| include <tunables/global> | ||
| $out/bin/transmission-daemon { | ||
| include <abstractions/base> |
There was a problem hiding this comment.
postInstall writes the AppArmor profile to $apparmor/bin.transmission-daemon, but it only creates $apparmor (not $apparmor/bin). This will fail with “No such file or directory” during the build. Create the bin subdirectory (or use an install helper that creates parent dirs) before redirecting into the file.
| # Excluding gtest since it is hardcoded to vendored version. The rest of the listed libraries are not packaged. | ||
| pushd third-party | ||
| for f in *; do | ||
| if [[ ! $f =~ googletest|wildmat|fast_float|wide-integer|jsonsl|libutp ]]; then |
There was a problem hiding this comment.
The third-party cleanup loop explicitly keeps libutp vendored, but buildInputs also includes the system libutp package. This is confusing and may be redundant or accidentally pull in the wrong implementation. Either remove vendored third-party/libutp to force the system library (and keep libutp in buildInputs), or drop the libutp dependency (and adjust the comment about “not packaged” accordingly).
| if [[ ! $f =~ googletest|wildmat|fast_float|wide-integer|jsonsl|libutp ]]; then | |
| if [[ ! $f =~ googletest|wildmat|fast_float|wide-integer|jsonsl ]]; then |
| miniupnpc, | ||
| dht, | ||
| libnatpmp, | ||
| libiconv, |
There was a problem hiding this comment.
libiconv is listed as a function argument but never referenced in this derivation. If it’s actually required (commonly on Darwin), add it to the appropriate buildInputs/platform-specific inputs; otherwise remove it from the argument list to avoid dead parameters.
| libiconv, |
| llmAgentsPkgs = inputs.llm-agents.packages.${prev.system}; | ||
| releaseTransmission = | ||
| if prev.lib.strings.hasPrefix "4.0." pkgsRelease.transmission_4.version then | ||
| pkgsRelease.transmission_4 | ||
| else | ||
| throw "Expected transmission_4 from nixpkgs-25_11 to be 4.0.x, got ${pkgsRelease.transmission_4.version}"; | ||
| # Temporary Darwin firefox wrapper backport: | ||
| # in our pinned nixpkgs revision, wrapper logic copies only *.dylib symlinks. | ||
| # Some shared libraries are Mach-O dylibs without that suffix, which leaves them | ||
| # symlinked and breaks runtime features (e.g. Crypto API / media codecs). | ||
| # Drop this once nixpkgs PR #488112 (or equivalent) is in the pinned input. | ||
| patchFirefoxDarwinWrapper = | ||
| pkg: | ||
| pkg.overrideAttrs (old: { | ||
| buildCommand = old.buildCommand + '' | ||
| # Backport nixpkgs#488112 with otool-based dylib detection. | ||
| cd "$out/Applications/Firefox.app" | ||
|
|
||
| find . -type l -print0 | while IFS= read -r -d $'\0' file; do | ||
| case "$(basename "$file")" in | ||
| omni.ja) | ||
| ;; | ||
| *) | ||
| otool -l "$file" 2>/dev/null | grep -q 'LC_ID_DYLIB' || continue | ||
| ;; | ||
| esac | ||
|
|
||
| target="$(readlink -f "$file")" | ||
| rm "$file" | ||
| cp "$target" "$file" | ||
| done | ||
| ''; | ||
| }); | ||
| releaseTransmission = prev.callPackage ../pkgs/transmission_4/default.nix { }; | ||
| in |
There was a problem hiding this comment.
releaseTransmission is now defined via prev.callPackage ../pkgs/transmission_4/default.nix { }, so it’s no longer sourced from pkgsRelease. Consider renaming this binding (or adding an inline note) so it’s clear this is a locally pinned Transmission derivation rather than the release channel package.
| @@ -77,7 +77,7 @@ in | |||
| ]; | |||
| dates = lib.mkDefault "Sat 03:00"; | |||
| randomizedDelaySec = "45min"; | |||
There was a problem hiding this comment.
Changing system.autoUpgrade.persistent from true to false changes upgrade semantics: missed upgrade runs (e.g., during downtime) will no longer execute on the next boot. If the intent is to avoid “upgrade immediately after reboot” behavior, consider adding a short comment here explaining the rationale so it’s not accidentally reverted later.
| randomizedDelaySec = "45min"; | |
| randomizedDelaySec = "45min"; | |
| # Intentionally non-persistent: missed runs during downtime should NOT | |
| # execute immediately on next boot; upgrades run only in the scheduled window. |
No description provided.