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

lib/trivial: add assertMsg and assertMsgOneOf #44136

Merged
merged 4 commits into from
Sep 6, 2018
Merged
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
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions lib/asserts.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
{ lib }:

rec {

/* Print a trace message if pred is false.
Intended to be used to augment asserts with helpful error messages.

Example:
assertMsg false "nope"
=> false
stderr> trace: nope

assert (assertMsg ("foo" == "bar") "foo is not bar, silly"); ""
stderr> trace: foo is not bar, silly
stderr> assert failed at …

Type:
assertMsg :: Bool -> String -> Bool
*/
# TODO(Profpatsch): add tests that check stderr
assertMsg = pred: msg:
if pred
then true
else builtins.trace msg false;
Copy link
Member

Choose a reason for hiding this comment

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

What about else throw "Assertion failed: ${msg}" instead? Using trace will log with lvlError and thus might be filtered out (eg. on Hydra, where only C++ exceptions are captured), throw on the other hand is fatal (at least until used with tryEval, but the same can be used for assertions anyway, so no need to abort).

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds like a good improvement to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Wait, how would that look in practice? Would it still be used like

assert (assertMsg ("foo" == "bar") "foo is not bar, silly"); ""

Copy link
Member

Choose a reason for hiding this comment

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

Sure, the difference is just that assert here essentially becomes a no-op if pred doesn't match, because throw already takes care of raising an error before the actual assertion fails.

Copy link
Member Author

Choose a reason for hiding this comment

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

Then the API stays the same, I see. I think we should do that.


/* Specialized `assertMsg` for checking if val is one of the elements
of a list. Useful for checking enums.

Example:
let sslLibrary = "libressl"
in assertOneOf "sslLibrary" sslLibrary [ "openssl" "bearssl" ]
=> false
stderr> trace: sslLibrary must be one of "openssl", "bearssl", but is: "libressl"

Type:
assertOneOf :: String -> ComparableVal -> List ComparableVal -> Bool
*/
assertOneOf = name: val: xs: assertMsg
(lib.elem val xs)
"${name} must be one of ${
lib.generators.toPretty {} xs}, but is: ${
lib.generators.toPretty {} val}";

}
6 changes: 4 additions & 2 deletions lib/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,11 @@ let
systems = callLibs ./systems;

# misc
asserts = callLibs ./asserts.nix;
debug = callLibs ./debug.nix;

generators = callLibs ./generators.nix;
misc = callLibs ./deprecated.nix;

# domain-specific
fetchers = callLibs ./fetchers.nix;

Expand All @@ -60,7 +61,6 @@ let
boolToString mergeAttrs flip mapNullable inNixShell min max
importJSON warn info nixpkgsVersion version mod compare
splitByAndCompare functionArgs setFunctionArgs isFunction;

inherit (fixedPoints) fix fix' extends composeExtensions
makeExtensible makeExtensibleWithCustomName;
inherit (attrsets) attrByPath hasAttrByPath setAttrByPath
Expand Down Expand Up @@ -117,6 +117,8 @@ let
unknownModule mkOption;
inherit (types) isType setType defaultTypeMerge defaultFunctor
isOptionType mkOptionType;
inherit (asserts)
assertMsg assertOneOf;
inherit (debug) addErrorContextToAttrs traceIf traceVal traceValFn
traceXMLVal traceXMLValMarked traceSeq traceSeqN traceValSeq
traceValSeqFn traceValSeqN traceValSeqNFn traceShowVal
Expand Down
7 changes: 5 additions & 2 deletions lib/lists.nix
Original file line number Diff line number Diff line change
Expand Up @@ -509,15 +509,18 @@ rec {
=> 3
*/
last = list:
assert list != []; elemAt list (length list - 1);
assert lib.assertMsg (list != []) "lists.last: list must not be empty!";
elemAt list (length list - 1);

/* Return all elements but the last

Example:
init [ 1 2 3 ]
=> [ 1 2 ]
*/
init = list: assert list != []; take (length list - 1) list;
init = list:
assert lib.assertMsg (list != []) "lists.init: list must not be empty!";
take (length list - 1) list;


/* return the image of the cross product of some lists by a function
Expand Down
7 changes: 5 additions & 2 deletions lib/strings.nix
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ rec {
components = splitString "/" url;
filename = lib.last components;
name = builtins.head (splitString sep filename);
in assert name != filename; name;
in assert name != filename; name;

/* Create an --{enable,disable}-<feat> string that can be passed to
standard GNU Autoconf scripts.
Expand Down Expand Up @@ -459,7 +459,10 @@ rec {
strw = lib.stringLength str;
reqWidth = width - (lib.stringLength filler);
in
assert strw <= width;
assert lib.assertMsg (strw <= width)
"fixedWidthString: requested string length (${
toString width}) must not be shorter than actual length (${
toString strw})";
if strw == width then str else filler + fixedWidthString reqWidth filler str;

/* Format a number adding leading zeroes up to fixed width.
Expand Down
2 changes: 1 addition & 1 deletion lib/trivial.nix
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ rec {
builtins.fromJSON (builtins.readFile path);


## Warnings and asserts
## Warnings

/* See https://github.com/NixOS/nix/issues/749. Eventually we'd like these
to expand to Nix builtins that carry metadata so that Nix can filter out
Expand Down
8 changes: 6 additions & 2 deletions lib/types.nix
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ rec {
let
betweenDesc = lowest: highest:
"${toString lowest} and ${toString highest} (both inclusive)";
between = lowest: highest: assert lowest <= highest;
between = lowest: highest:
assert lib.assertMsg (lowest <= highest)
"ints.between: lowest must be smaller than highest";
addCheck int (x: x >= lowest && x <= highest) // {
name = "intBetween";
description = "integer between ${betweenDesc lowest highest}";
Expand Down Expand Up @@ -439,7 +441,9 @@ rec {
# Either value of type `finalType` or `coercedType`, the latter is
# converted to `finalType` using `coerceFunc`.
coercedTo = coercedType: coerceFunc: finalType:
assert coercedType.getSubModules == null;
assert lib.assertMsg (coercedType.getSubModules == null)
"coercedTo: coercedType must not have submodules (it’s a ${
coercedType.description})";
mkOptionType rec {
name = "coercedTo";
description = "${finalType.description} or ${coercedType.description} convertible to it";
Expand Down
5 changes: 2 additions & 3 deletions pkgs/applications/misc/lilyterm/default.nix
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
{ stdenv, fetchurl, fetchFromGitHub
{ stdenv, lib, fetchurl, fetchFromGitHub
, pkgconfig
, autoconf, automake, intltool, gettext
, gtk, vte

# "stable" or "git"
, flavour ? "stable"
}:

assert flavour == "stable" || flavour == "git";
assert lib.assertOneOf "flavour" flavour [ "stable" "git" ];

let
stuff =
Expand Down
3 changes: 2 additions & 1 deletion pkgs/applications/science/math/ripser/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@

with stdenv.lib;

assert elem fileFormat ["lowerTriangularCsv" "upperTriangularCsv" "dipha"];
assert assertOneOf "fileFormat" fileFormat
["lowerTriangularCsv" "upperTriangularCsv" "dipha"];
assert useGoogleHashmap -> sparsehash != null;

let
Expand Down