From a2eba21972972bbb58e5d9ea46c8bceda152b826 Mon Sep 17 00:00:00 2001 From: Andrew Marshall Date: Mon, 14 Oct 2024 21:43:58 -0400 Subject: [PATCH] firefox: Avoid unnecessarily overriding package When `cfg.package` is already wrapped, and wrapped without the `ExtensionSettings` key set, this would always add that key, even if its value was blank. This would result in `cfg.finalPackage` being a functionally-identical, but differently-input-addressed package. This is generally undesirable as it may result in multiple derivations being built, and also if the value of `cfg.package` is expected to be unchanged by the user (e.g. because they want it to be consistent between NixOS and HM configuration). Add a test to ensure this does not regress in the default case. Only test on newish stateVersion since the logic for `isWrapped` differs on older versions. --- modules/programs/firefox/mkFirefoxModule.nix | 13 +++++----- tests/modules/programs/firefox/common.nix | 1 + .../programs/firefox/final-package.nix | 25 +++++++++++++++++++ 3 files changed, 33 insertions(+), 6 deletions(-) create mode 100644 tests/modules/programs/firefox/final-package.nix diff --git a/modules/programs/firefox/mkFirefoxModule.nix b/modules/programs/firefox/mkFirefoxModule.nix index dd4920666..d595b87c2 100644 --- a/modules/programs/firefox/mkFirefoxModule.nix +++ b/modules/programs/firefox/mkFirefoxModule.nix @@ -799,12 +799,13 @@ in { finalPackage = wrapPackage cfg.package; policies = { - ExtensionSettings = listToAttrs (map (lang: - nameValuePair "langpack-${lang}@firefox.mozilla.org" { - installation_mode = "normal_installed"; - install_url = - "https://releases.mozilla.org/pub/firefox/releases/${cfg.package.version}/linux-x86_64/xpi/${lang}.xpi"; - }) cfg.languagePacks); + ExtensionSettings = lib.mkIf (cfg.languagePacks != [ ]) (listToAttrs (map + (lang: + nameValuePair "langpack-${lang}@firefox.mozilla.org" { + installation_mode = "normal_installed"; + install_url = + "https://releases.mozilla.org/pub/firefox/releases/${cfg.package.version}/linux-x86_64/xpi/${lang}.xpi"; + }) cfg.languagePacks)); }; }); } diff --git a/tests/modules/programs/firefox/common.nix b/tests/modules/programs/firefox/common.nix index 7d05382f1..ff6e86392 100644 --- a/tests/modules/programs/firefox/common.nix +++ b/tests/modules/programs/firefox/common.nix @@ -1,6 +1,7 @@ name: builtins.mapAttrs (test: module: import module [ "programs" name ]) { "${name}-deprecated-native-messenger" = ./deprecated-native-messenger.nix; + "${name}-final-package" = ./final-package.nix; "${name}-policies" = ./policies.nix; "${name}-profiles-bookmarks" = ./profiles/bookmarks; "${name}-profiles-containers" = ./profiles/containers; diff --git a/tests/modules/programs/firefox/final-package.nix b/tests/modules/programs/firefox/final-package.nix new file mode 100644 index 000000000..ec310ee33 --- /dev/null +++ b/tests/modules/programs/firefox/final-package.nix @@ -0,0 +1,25 @@ +modulePath: +{ config, lib, pkgs, ... }: + +let + + cfg = lib.getAttrFromPath modulePath config; + + firefoxMockOverlay = import ./setup-firefox-mock-overlay.nix modulePath; + +in { + imports = [ firefoxMockOverlay ]; + + config = lib.mkIf config.test.enableBig + (lib.setAttrByPath modulePath { enable = true; } // { + home.stateVersion = "19.09"; + + nmt.script = '' + package=${cfg.package} + finalPackage=${cfg.finalPackage} + if [[ $package != $finalPackage ]]; then + fail "Expected finalPackage ($finalPackage) to equal package ($package)" + fi + ''; + }); +}