search

NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
16.46k stars 12.95k forks source link

Common pattern of `.${stdenv.hostPlatform.system} or (throw "Unsupported platform: ${stdenv.hostPlatform.system}")` should be encapsulated in library function #314224

Open pluiedev opened 1 month ago

pluiedev commented 1 month ago

Currently there's a lot of code in Nixpkgs that selectively use a different src/hash/rev/etc. depending on the target system. This is especially common in proprietary software that have different download links for pre-built binaries, for instance, in UT1999, the url and hash are different for each given platform:

let
  # ...
  srcs = {
    x86_64-linux = fetchurl {
      url = "https://github.com/OldUnreal/UnrealTournamentPatches/releases/download/v${version}/OldUnreal-UTPatch${version}-Linux-amd64.tar.bz2";
      hash = "sha256-aoGzWuakwN/OL4+xUq8WEpd2c1rrNN/DkffI2vDVGjs=";
    };
    aarch64-linux = fetchurl {
      url = "https://github.com/OldUnreal/UnrealTournamentPatches/releases/download/v${version}/OldUnreal-UTPatch${version}-Linux-arm64.tar.bz2";
      hash = "sha256-2e9lHB12jLTR8UYofLWL7gg0qb2IqFk6eND3T5VqAx0=";
    };
    i686-linux = fetchurl {
      url = "https://github.com/OldUnreal/UnrealTournamentPatches/releases/download/v${version}/OldUnreal-UTPatch${version}-Linux-x86.tar.bz2";
      hash = "sha256-1JsFKuAAj/LtYvOUPFu0Hn+zvY3riW0YlJbLd4UnaKU=";
    };
    x86_64-darwin = fetchurl {
      url = "https://github.com/OldUnreal/UnrealTournamentPatches/releases/download/v${version}/OldUnreal-UTPatch${version}-macOS-Sonoma.dmg";
      hash = "sha256-TbhJbOH4E5WOb6XR9dmqLkXziK3/CzhNjd1ypBkkmvw=";
    };
  };
in
stdenv.mkDerivation {
  src = srcs.${stdenv.hostPlatform.system} or (throw "Unsupported system: ${stdenv.hostPlatform.system}");
  # ...
}

Searching through the entire repository, there are 69 occurrences of this pattern, given by running rg -l '.\$\{stdenv.hostPlatform.system\} or \(throw' | wc -l. Without the or (throw part, it's more like 205 occurrences. In my opinion, this already makes it common enough to justify having a library function that simplifies this.

List of files that feature this pattern 
pkgs/applications/audio/touchosc/default.nix
pkgs/applications/blockchains/trezor-suite/default.nix
pkgs/applications/gis/udig/default.nix
pkgs/applications/graphics/kodelife/default.nix
pkgs/applications/misc/keeweb/default.nix
pkgs/applications/networking/browsers/mullvad-browser/default.nix
pkgs/applications/networking/browsers/tor-browser/default.nix
pkgs/applications/networking/browsers/vivaldi/default.nix
pkgs/applications/networking/instant-messengers/armcord/default.nix
pkgs/applications/networking/instant-messengers/qq/default.nix
pkgs/applications/networking/mullvad-vpn/default.nix
pkgs/applications/networking/p2p/frostwire/default.nix
pkgs/applications/networking/remote/teamviewer/default.nix
pkgs/applications/networking/resilio-sync/default.nix
pkgs/applications/science/math/scilab-bin/default.nix
pkgs/applications/version-management/p4v/default.nix
pkgs/applications/video/streamlink-twitch-gui/bin.nix
pkgs/by-name/cu/cups-kyocera-3500-4500/package.nix
pkgs/by-name/go/goofcord/package.nix
pkgs/by-name/ou/outfox/package.nix
pkgs/by-name/ph/photonvision/package.nix
pkgs/by-name/pu/pulsar/package.nix
pkgs/by-name/st/stereotool/package.nix
pkgs/by-name/ut/ut1999/package.nix
pkgs/by-name/wi/windmill/package.nix
pkgs/by-name/ye/yesplaymusic/package.nix
pkgs/development/compilers/ccl/default.nix
pkgs/development/compilers/gcc-arm-embedded/10/default.nix
pkgs/development/compilers/gcc-arm-embedded/11/default.nix
pkgs/development/compilers/gcc-arm-embedded/12/default.nix
pkgs/development/compilers/gcc-arm-embedded/13/default.nix
pkgs/development/compilers/gcc-arm-embedded/6/default.nix
pkgs/development/compilers/gcc-arm-embedded/7/default.nix
pkgs/development/compilers/gcc-arm-embedded/8/default.nix
pkgs/development/compilers/gcc-arm-embedded/9/default.nix
pkgs/development/compilers/jetbrains-jdk/default.nix
pkgs/development/compilers/jetbrains-jdk/jcef.nix
pkgs/development/compilers/julia/1.6-bin.nix
pkgs/development/compilers/julia/generic-bin.nix
pkgs/development/compilers/mozart/binary.nix
pkgs/development/compilers/oraclejdk/jdk-linux-base.nix
pkgs/development/libraries/libcef/default.nix
pkgs/development/python-modules/debugpy/default.nix
pkgs/development/python-modules/tensorflow/default.nix
pkgs/development/tools/azure-static-sites-client/default.nix
pkgs/development/tools/confluent-cli/default.nix
pkgs/development/tools/misc/blackfire/default.nix
pkgs/development/tools/misc/segger-ozone/default.nix
pkgs/development/web/postman/linux.nix
pkgs/games/katawa-shoujo/default.nix
pkgs/misc/cups/drivers/kyodialog/default.nix
pkgs/misc/drivers/pantum-driver/default.nix
pkgs/misc/urbit/default.nix
pkgs/os-specific/linux/broadcom-sta/default.nix
pkgs/servers/keycloak/keycloak-metrics-spi/default.nix
pkgs/servers/monitoring/grafana/default.nix
pkgs/servers/search/elasticsearch/7.x.nix
pkgs/servers/sql/cockroachdb/cockroachdb-bin.nix
pkgs/servers/ums/default.nix
pkgs/tools/admin/ibmcloud-cli/default.nix
pkgs/tools/archivers/rar/default.nix
pkgs/tools/bootloaders/refind/default.nix
pkgs/tools/inputmethods/droidmote/default.nix
pkgs/tools/misc/archi/default.nix
pkgs/tools/misc/logstash/7.x.nix
pkgs/tools/networking/cloudflare-warp/default.nix
pkgs/tools/networking/ookla-speedtest/default.nix
pkgs/tools/security/enpass/default.nix
pkgs/tools/security/sonar-scanner-cli/default.nix
Scrumplex commented 1 month ago

Perhaps this could be something that is handled by mkDerivation? As it is right now, this pattern makes overriding sources much harder, as they will be different depending on the platform.

Aleksanaa commented 1 month ago

Perhaps this could be something that is handled by mkDerivation?

You are right, maybe we should have a unified pattern like srcForSystem.

Aleksanaa commented 1 month ago

Related: https://github.com/NixOS/nixpkgs/issues/293838

eclairevoyant commented 1 month ago

Perhaps this could be something that is handled by mkDerivation?

You are right, maybe we should have a unified pattern like srcForSystem.

This is not as important for src, but for version it definitely is needed if versions differ by platform. See #313741. Hence srcForSystem is too specific to src.

eclairevoyant commented 1 month ago

Also after considering this further, this seems like unnecessary boilerplate. If a platform isn't listed in meta.platforms then we shouldn't have to eval src or version in the first place.

I'd rather just remove the need for such a throw at all.