Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce that package argument defaults are applied, cleans up optional dependency convention #131271

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
25 changes: 24 additions & 1 deletion lib/customisation.nix
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,30 @@ rec {
callPackageWith = autoArgs: fn: args:
let
f = if lib.isFunction fn then fn else import fn;
auto = builtins.intersectAttrs (lib.functionArgs f) autoArgs;
funArgs = lib.functionArgs f;
auto = lib.mapAttrs (name: value:
if funArgs.${name}
then
# These definitions are nested under this conditional so that no
# thunks are allocated if no warning/error is thrown
let
file = if builtins.pathExists (fn + "/default.nix") then fn + "/default.nix" else fn;
location = if lib.isFunction fn then "a function (run with `--show-trace` to get hints as to which one)" else toString file;
message =
"In ${location}, the argument \"${name}\" has a default value (`${name} ? <default>`) which is not allowed "
+ "because the attribute \"${name}\" exists in the call scope, therefore overriding the default value. "
+ "If \"${name}\" should be a package configuration, changeable via `.override { ${name} = <value>; }`, "
+ "rename the argument to something that doesn't already exist. "
+ "If \"${name}\" should be optional dependency (commonly done with `${name} ? null`), remove the default value "
+ "and use an argument like `enableFeature ? true` to decide when to include \"${name}\" as a dependency.";
in
# We can't know the location of direct functions from within this
# call. Because this is very rare, we allow ourselves to throw an error,
# so that the backtrace can be used to figure out the location of it
if lib.isFunction fn then throw message
else lib.warn message value
else value
) (builtins.intersectAttrs funArgs autoArgs);
in makeOverridable f (auto // args);


Expand Down
2 changes: 1 addition & 1 deletion pkgs/applications/audio/audio-recorder/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
, pkg-config, intltool
, glib, dbus, gtk3, libappindicator-gtk3, gst_all_1
, librsvg, wrapGAppsHook
, pulseaudioSupport ? true, libpulseaudio ? null }:
, pulseaudioSupport ? true, libpulseaudio }:

stdenv.mkDerivation rec {
pname = "audio-recorder";
Expand Down
2 changes: 1 addition & 1 deletion pkgs/applications/audio/axoloti/libusb1.nix
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{ stdenv, lib, fetchurl, pkg-config, systemd ? null, libobjc, IOKit, fetchpatch }:
{ stdenv, lib, fetchurl, pkg-config, systemd, libobjc, IOKit, fetchpatch }:

stdenv.mkDerivation rec {
name = "libusb-1.0.19";
Expand Down
6 changes: 3 additions & 3 deletions pkgs/applications/audio/carla/default.nix
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{ lib, stdenv, fetchFromGitHub, alsa-lib, file, fluidsynth, jack2,
liblo, libpulseaudio, libsndfile, pkg-config, python3Packages,
which, withFrontend ? true,
withQt ? true, qtbase ? null, wrapQtAppsHook ? null,
withGtk2 ? true, gtk2 ? null,
withGtk3 ? true, gtk3 ? null }:
withQt ? true, qtbase, wrapQtAppsHook,
withGtk2 ? true, gtk2,
withGtk3 ? true, gtk3 }:

with lib;

Expand Down
48 changes: 24 additions & 24 deletions pkgs/applications/audio/cmus/default.nix
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
{ config, lib, stdenv, fetchFromGitHub, runCommand, ncurses, pkg-config
, libiconv, CoreAudio, AudioUnit

, alsaSupport ? stdenv.isLinux, alsa-lib ? null
, alsaSupport ? stdenv.isLinux, alsa-lib
# simple fallback for everyone else
, aoSupport ? !stdenv.isLinux, libao ? null
, jackSupport ? false, libjack ? null
, samplerateSupport ? jackSupport, libsamplerate ? null
, ossSupport ? false, alsa-oss ? null
, pulseaudioSupport ? config.pulseaudio or false, libpulseaudio ? null
, mprisSupport ? stdenv.isLinux, systemd ? null
, aoSupport ? !stdenv.isLinux, libao
, jackSupport ? false, libjack
, samplerateSupport ? jackSupport, libsamplerate
, ossSupport ? false, alsa-oss
, pulseaudioSupport ? config.pulseaudio or false, libpulseaudio
, mprisSupport ? stdenv.isLinux, systemd

# TODO: add these
#, artsSupport
Expand All @@ -17,23 +17,23 @@
#, sunSupport
#, waveoutSupport

, cddbSupport ? true, libcddb ? null
, cdioSupport ? true, libcdio ? null, libcdio-paranoia ? null
, cueSupport ? true, libcue ? null
, discidSupport ? (!stdenv.isDarwin), libdiscid ? null
, ffmpegSupport ? true, ffmpeg ? null
, flacSupport ? true, flac ? null
, madSupport ? true, libmad ? null
, mikmodSupport ? true, libmikmod ? null
, modplugSupport ? true, libmodplug ? null
, mpcSupport ? true, libmpcdec ? null
, tremorSupport ? false, tremor ? null
, vorbisSupport ? true, libvorbis ? null
, wavpackSupport ? true, wavpack ? null
, opusSupport ? true, opusfile ? null

, aacSupport ? false, faad2 ? null # already handled by ffmpeg
, mp4Support ? false, mp4v2 ? null # ffmpeg does support mp4 better
, cddbSupport ? true, libcddb
, cdioSupport ? true, libcdio, libcdio-paranoia
, cueSupport ? true, libcue
, discidSupport ? (!stdenv.isDarwin), libdiscid
, ffmpegSupport ? true, ffmpeg
, flacSupport ? true, flac
, madSupport ? true, libmad
, mikmodSupport ? true, libmikmod
, modplugSupport ? true, libmodplug
, mpcSupport ? true, libmpcdec
, tremorSupport ? false, tremor
, vorbisSupport ? true, libvorbis
, wavpackSupport ? true, wavpack
, opusSupport ? true, opusfile

, aacSupport ? false, faad2 # already handled by ffmpeg
, mp4Support ? false, mp4v2 # ffmpeg does support mp4 better

# not in nixpkgs
#, vtxSupport ? true, libayemu ? null
Expand Down
20 changes: 10 additions & 10 deletions pkgs/applications/audio/csound/default.nix
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{ lib, stdenv, fetchFromGitHub, cmake, libsndfile, libsamplerate, flex, bison, boost, gettext
, alsa-lib ? null
, libpulseaudio ? null
, libjack2 ? null
, liblo ? null
, ladspa-sdk ? null
, fluidsynth ? null
# , gmm ? null # opcodes don't build with gmm 5.1
, eigen ? null
, curl ? null
, alsa-lib
, libpulseaudio
, libjack2
, liblo
, ladspa-sdk
, fluidsynth
# , gmm # opcodes don't build with gmm 5.1
, eigen
, curl
, tcltk ? null
, fltk ? null
, fltk
}:

stdenv.mkDerivation rec {
Expand Down
42 changes: 21 additions & 21 deletions pkgs/applications/audio/deadbeef/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,32 @@
, pkg-config
, jansson
# deadbeef can use either gtk2 or gtk3
, gtk2Support ? false, gtk2 ? null
, gtk3Support ? true, gtk3 ? null, gsettings-desktop-schemas ? null, wrapGAppsHook ? null
, gtk2Support ? false, gtk2
, gtk3Support ? true, gtk3, gsettings-desktop-schemas, wrapGAppsHook
# input plugins
, vorbisSupport ? true, libvorbis ? null
, mp123Support ? true, libmad ? null
, flacSupport ? true, flac ? null
, wavSupport ? true, libsndfile ? null
, cdaSupport ? true, libcdio ? null, libcddb ? null
, aacSupport ? true, faad2 ? null
, opusSupport ? true, opusfile ? null
, wavpackSupport ? false, wavpack ? null
, ffmpegSupport ? false, ffmpeg ? null
, apeSupport ? true, yasm ? null
, vorbisSupport ? true, libvorbis
, mp123Support ? true, libmad
, flacSupport ? true, flac
, wavSupport ? true, libsndfile
, cdaSupport ? true, libcdio, libcddb
, aacSupport ? true, faad2
, opusSupport ? true, opusfile
, wavpackSupport ? false, wavpack
, ffmpegSupport ? false, ffmpeg
, apeSupport ? true, yasm
# misc plugins
, zipSupport ? true, libzip ? null
, artworkSupport ? true, imlib2 ? null
, hotkeysSupport ? true, libX11 ? null
, osdSupport ? true, dbus ? null
, zipSupport ? true, libzip
, artworkSupport ? true, imlib2
, hotkeysSupport ? true, libX11
, osdSupport ? true, dbus
# output plugins
, alsaSupport ? true, alsa-lib ? null
, pulseSupport ? config.pulseaudio or stdenv.isLinux, libpulseaudio ? null
, alsaSupport ? true, alsa-lib
, pulseSupport ? config.pulseaudio or stdenv.isLinux, libpulseaudio
# effect plugins
, resamplerSupport ? true, libsamplerate ? null
, overloadSupport ? true, zlib ? null
, resamplerSupport ? true, libsamplerate
, overloadSupport ? true, zlib
# transports
, remoteSupport ? true, curl ? null
, remoteSupport ? true, curl
}:

assert gtk2Support || gtk3Support;
Expand Down
6 changes: 3 additions & 3 deletions pkgs/applications/audio/fmit/default.nix
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{ lib, mkDerivation, fetchFromGitHub, fftw, qtbase, qtmultimedia, qmake, itstool, wrapQtAppsHook
, alsaSupport ? true, alsa-lib ? null
, jackSupport ? false, libjack2 ? null
, portaudioSupport ? false, portaudio ? null }:
, alsaSupport ? true, alsa-lib
, jackSupport ? false, libjack2
, portaudioSupport ? false, portaudio }:

assert alsaSupport -> alsa-lib != null;
assert jackSupport -> libjack2 != null;
Comment on lines 6 to 7
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are we doing with those? Are you gonna remove them, too or will this be another cleanup PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using null to indicate absence of a dependency can still make sense, which can be done with .override or overlays. So these checks would still give early errors in some cases.

Personally I'm not sure how useful these assertions are though, maybe we should think about removing them at some point.

Copy link
Member

@SuperSandro2000 SuperSandro2000 Jul 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am personally for removing them. I don't think they are useful or people normally run into these.

Expand Down
4 changes: 2 additions & 2 deletions pkgs/applications/audio/librespot/default.nix
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{ lib, fetchFromGitHub, rustPlatform, pkg-config, openssl, withRodio ? true
, withALSA ? true, alsa-lib ? null, withPulseAudio ? false, libpulseaudio ? null
, withPortAudio ? false, portaudio ? null }:
, withALSA ? true, alsa-lib, withPulseAudio ? false, libpulseaudio
, withPortAudio ? false, portaudio }:

rustPlatform.buildRustPackage rec {
pname = "librespot";
Expand Down
8 changes: 4 additions & 4 deletions pkgs/applications/audio/lmms/default.nix
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{ lib, fetchFromGitHub, cmake, pkg-config, alsa-lib ? null, fftwFloat, fltk13
, fluidsynth_1 ? null, lame ? null, libgig ? null, libjack2 ? null, libpulseaudio ? null
, libsamplerate, libsoundio ? null, libsndfile, libvorbis ? null, portaudio ? null
, qtbase, qtx11extras, qttools, SDL ? null, mkDerivation }:
{ lib, fetchFromGitHub, cmake, pkg-config, alsa-lib, fftwFloat, fltk13
, fluidsynth_1, lame, libgig, libjack2, libpulseaudio
, libsamplerate, libsoundio, libsndfile, libvorbis, portaudio
, qtbase, qtx11extras, qttools, SDL, mkDerivation }:

mkDerivation rec {
pname = "lmms";
Expand Down
4 changes: 2 additions & 2 deletions pkgs/applications/audio/ncmpcpp/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
, icu
, curl
, outputsSupport ? true # outputs screen
, visualizerSupport ? false, fftw ? null # visualizer screen
, visualizerSupport ? false, fftw # visualizer screen
, clockSupport ? true # clock screen
, taglibSupport ? true, taglib ? null # tag editor
, taglibSupport ? true, taglib # tag editor
}:

assert visualizerSupport -> (fftw != null);
Expand Down
8 changes: 4 additions & 4 deletions pkgs/applications/audio/ncspot/default.nix
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{ stdenv, lib, fetchFromGitHub, rustPlatform, pkg-config, ncurses, openssl, libiconv
, withALSA ? true, alsa-lib ? null
, withPulseAudio ? false, libpulseaudio ? null
, withPortAudio ? false, portaudio ? null
, withMPRIS ? false, dbus ? null
, withALSA ? true, alsa-lib
, withPulseAudio ? false, libpulseaudio
, withPortAudio ? false, portaudio
, withMPRIS ? false, dbus
}:

let
Expand Down
12 changes: 6 additions & 6 deletions pkgs/applications/audio/pulseaudio-dlna/default.nix
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{ fetchFromGitHub, lib, pythonPackages
, mp3Support ? true, lame ? null
, opusSupport ? true, opusTools ? null
, faacSupport ? false, faac ? null
, flacSupport ? true, flac ? null
, soxSupport ? true, sox ? null
, vorbisSupport ? true, vorbis-tools ? null
, mp3Support ? true, lame
, opusSupport ? true, opusTools
, faacSupport ? false, faac
, flacSupport ? true, flac
, soxSupport ? true, sox
, vorbisSupport ? true, vorbis-tools
}:

assert mp3Support -> lame != null;
Expand Down
4 changes: 2 additions & 2 deletions pkgs/applications/audio/quodlibet/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
gst_all_1, withGstPlugins ? true,
xineBackend ? false, xine-lib,
withDbusPython ? false, withPyInotify ? false, withMusicBrainzNgs ? false, withPahoMqtt ? false,
webkitgtk ? null,
keybinder3 ? null, gtksourceview ? null, libmodplug ? null, kakasi ? null, libappindicator-gtk3 ? null }:
webkitgtk,
keybinder3, gtksourceview, libmodplug, kakasi, libappindicator-gtk3 }:

let optionals = lib.optionals; in
python3.pkgs.buildPythonApplication rec {
Expand Down
2 changes: 1 addition & 1 deletion pkgs/applications/audio/rosegarden/default.nix
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{ lib, stdenv, fetchurl, cmake, makedepend, perl, pkg-config, qttools, wrapQtAppsHook
, dssi, fftwSinglePrec, ladspaH, ladspaPlugins, libjack2, alsa-lib
, liblo, libsamplerate, libsndfile, lirc ? null, lrdf, qtbase }:
, liblo, libsamplerate, libsndfile, lirc, lrdf, qtbase }:

stdenv.mkDerivation (rec {
version = "20.12";
Expand Down
2 changes: 1 addition & 1 deletion pkgs/applications/blockchains/bitcoin-abc.nix
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{ lib, stdenv, mkDerivation, fetchFromGitHub, pkg-config, cmake, openssl, db53, boost
, zlib, miniupnpc, qtbase ? null , qttools ? null, util-linux, protobuf, qrencode, libevent
, zlib, miniupnpc, qtbase, qttools, util-linux, protobuf, qrencode, libevent
, withGui, python3, jemalloc, zeromq4 }:

with lib;
Expand Down
2 changes: 1 addition & 1 deletion pkgs/applications/blockchains/bitcoin-classic.nix
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{ lib, stdenv, fetchFromGitHub, pkg-config, autoreconfHook, openssl, db48, boost
, zlib, miniupnpc, qtbase ? null, qttools ? null, util-linux, protobuf, qrencode, libevent
SuperSandro2000 marked this conversation as resolved.
Show resolved Hide resolved
, zlib, miniupnpc, qtbase, qttools, util-linux, protobuf, qrencode, libevent
, withGui }:

with lib;
Expand Down
6 changes: 3 additions & 3 deletions pkgs/applications/blockchains/bitcoin-gold.nix
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
, zeromq
, libsodium
, withGui
, qtbase ? null
, qttools ? null
, wrapQtAppsHook ? null
, qtbase
, qttools
, wrapQtAppsHook
}:

with lib;
Expand Down
2 changes: 1 addition & 1 deletion pkgs/applications/blockchains/bitcoin-unlimited.nix
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{ lib, stdenv, fetchFromGitHub, pkg-config, autoreconfHook, openssl, db48, boost
, zlib, miniupnpc, util-linux, protobuf, qrencode, libevent, python3
, withGui, wrapQtAppsHook ? null, qtbase ? null, qttools ? null
, withGui, wrapQtAppsHook, qtbase, qttools
, Foundation, ApplicationServices, AppKit }:

with lib;
Expand Down
6 changes: 3 additions & 3 deletions pkgs/applications/blockchains/bitcoin.nix
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
, pkg-config
, util-linux
, hexdump
, wrapQtAppsHook ? null
, wrapQtAppsHook
, boost
, libevent
, miniupnpc
Expand All @@ -14,8 +14,8 @@
, db48
, sqlite
, qrencode
, qtbase ? null
, qttools ? null
, qtbase
, qttools
, python3
, nixosTests
, withGui
Expand Down
6 changes: 3 additions & 3 deletions pkgs/applications/blockchains/digibyte.nix
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
, hexdump
, zeromq
, withGui
, qtbase ? null
, qttools ? null
, wrapQtAppsHook ? null
, qtbase
, qttools
, wrapQtAppsHook
}:

with lib;
Expand Down
6 changes: 3 additions & 3 deletions pkgs/applications/blockchains/elements.nix
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
, pkg-config
, util-linux
, hexdump
, wrapQtAppsHook ? null
, wrapQtAppsHook
, boost
, libevent
, miniupnpc
Expand All @@ -14,8 +14,8 @@
, db48
, sqlite
, qrencode
, qtbase ? null
, qttools ? null
, qtbase
, qttools
, python3
, openssl
, withGui
Expand Down
4 changes: 2 additions & 2 deletions pkgs/applications/blockchains/pivx.nix
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{ fetchFromGitHub, lib, stdenv, pkg-config, autoreconfHook, wrapQtAppsHook ? null
{ fetchFromGitHub, lib, stdenv, pkg-config, autoreconfHook, wrapQtAppsHook
, openssl, db48, boost, zlib, miniupnpc, gmp
, qrencode, glib, protobuf, yasm, libevent
, util-linux, qtbase ? null, qttools ? null
, util-linux, qtbase, qttools
, enableUpnp ? false
, disableWallet ? false
, disableDaemon ? false
Expand Down
Loading