Commit 0ace3834 authored by Silvan Mosberger's avatar Silvan Mosberger
Browse files

lib.fileset: Make error messages more uniform 

Just minor changes like:
- Always using "X is a Y, but it should be Z"
- "X is a path that does not exist" rather than "X does not exist"
- Always using multi-line strings for errors
- Always quoting string-like values and not quoting path-like values
  - But do quote filesystem roots. Even though they're paths, they might
    be very small, good to have quotes to know the start/end
- Capitalise the first word
- Distinguish root vs filesystem root more
parent 2556605a

lib/fileset/default.nix

+20 −18
Original line number Diff line number Diff line
@@ -154,7 +154,7 @@ in { 
    if ! isPath root then

      if isStringLike root then

        throw ''

          lib.fileset.toSource: `root` ("${toString root}") is a string-like value, but it should be a path instead.

          lib.fileset.toSource: `root` (${toString root}) is a string-like value, but it should be a path instead.

              Paths in strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.''

      else

        throw ''
@@ -163,13 +163,13 @@ in { 
    # See also ../path/README.md

    else if ! fileset._internalIsEmptyWithoutBase && rootFilesystemRoot != filesetFilesystemRoot then

      throw ''

        lib.fileset.toSource: Filesystem roots are not the same for `fileset` and `root` ("${toString root}"):

            `root`: root "${toString rootFilesystemRoot}"

            `fileset`: root "${toString filesetFilesystemRoot}"

            Different roots are not supported.''

        lib.fileset.toSource: Filesystem roots are not the same for `fileset` and `root` (${toString root}):

            `root`: Filesystem root is "${toString rootFilesystemRoot}"

            `fileset`: Filesystem root is "${toString filesetFilesystemRoot}"

            Different filesystem roots are not supported.''

    else if ! pathExists root then

      throw ''

        lib.fileset.toSource: `root` (${toString root}) does not exist.''

        lib.fileset.toSource: `root` (${toString root}) is a path that does not exist.''

    else if pathType root != "directory" then

      throw ''

        lib.fileset.toSource: `root` (${toString root}) is a file, but it should be a directory instead. Potential solutions:
@@ -221,11 +221,11 @@ in { 
    _unionMany

      (_coerceMany "lib.fileset.union" [

        {

          context = "first argument";

          context = "First argument";

          value = fileset1;

        }

        {

          context = "second argument";

          context = "Second argument";

          value = fileset2;

        }

      ]);
@@ -267,12 +267,13 @@ in { 
    # which get [implicitly coerced to file sets](#sec-fileset-path-coercion).

    filesets:

    if ! isList filesets then

      throw "lib.fileset.unions: Expected argument to be a list, but got a ${typeOf filesets}."

      throw ''

        lib.fileset.unions: Argument is of type ${typeOf filesets}, but it should be a list instead.''

    else

      pipe filesets [

        # Annotate the elements with context, used by _coerceMany for better errors

        (imap0 (i: el: {

          context = "element ${toString i}";

          context = "Element ${toString i}";

          value = el;

        }))

        (_coerceMany "lib.fileset.unions")
@@ -323,10 +324,11 @@ in { 
    # The file set to filter based on the predicate function

    fileset:

    if ! isFunction predicate then

      throw "lib.fileset.fileFilter: Expected the first argument to be a function, but it's a ${typeOf predicate} instead."

      throw ''

        lib.fileset.fileFilter: First argument is of type ${typeOf predicate}, but it should be a function.''

    else

      _fileFilter predicate

        (_coerce "lib.fileset.fileFilter: second argument" fileset);

        (_coerce "lib.fileset.fileFilter: Second argument" fileset);



  /*

    The file set containing all files that are in both of two given file sets.
@@ -354,11 +356,11 @@ in { 
    let

      filesets = _coerceMany "lib.fileset.intersection" [

        {

          context = "first argument";

          context = "First argument";

          value = fileset1;

        }

        {

          context = "second argument";

          context = "Second argument";

          value = fileset2;

        }

      ];
@@ -406,11 +408,11 @@ in { 
    let

      filesets = _coerceMany "lib.fileset.difference" [

        {

          context = "first argument (positive set)";

          context = "First argument (positive set)";

          value = positive;

        }

        {

          context = "second argument (negative set)";

          context = "Second argument (negative set)";

          value = negative;

        }

      ];
@@ -454,7 +456,7 @@ in { 
    let

      # "fileset" would be a better name, but that would clash with the argument name,

      # and we cannot change that because of https://github.com/nix-community/nixdoc/issues/76

      actualFileset = _coerce "lib.fileset.trace: argument" fileset;

      actualFileset = _coerce "lib.fileset.trace: Argument" fileset;

    in

    seq

      (_printFileset actualFileset)
@@ -501,7 +503,7 @@ in { 
    let

      # "fileset" would be a better name, but that would clash with the argument name,

      # and we cannot change that because of https://github.com/nix-community/nixdoc/issues/76

      actualFileset = _coerce "lib.fileset.traceVal: argument" fileset;

      actualFileset = _coerce "lib.fileset.traceVal: Argument" fileset;

    in

    seq

      (_printFileset actualFileset)

lib/fileset/internal.nix

+4 −4
Original line number Diff line number Diff line
@@ -179,7 +179,7 @@ rec { 
          ${context} is of type ${typeOf value}, but it should be a file set or a path instead.''

    else if ! pathExists value then

      throw ''

        ${context} (${toString value}) does not exist.''

        ${context} (${toString value}) is a path that does not exist.''

    else

      _singleton value;
@@ -208,9 +208,9 @@ rec { 
    if firstWithBase != null && differentIndex != null then

      throw ''

        ${functionContext}: Filesystem roots are not the same:

            ${(head list).context}: root "${toString firstBaseRoot}"

            ${(elemAt list differentIndex).context}: root "${toString (elemAt filesets differentIndex)._internalBaseRoot}"

            Different roots are not supported.''

            ${(head list).context}: Filesystem root is "${toString firstBaseRoot}"

            ${(elemAt list differentIndex).context}: Filesystem root is "${toString (elemAt filesets differentIndex)._internalBaseRoot}"

            Different filesystem roots are not supported.''

    else

      filesets;

lib/fileset/tests.sh

+23 −23
Original line number Diff line number Diff line
@@ -318,7 +318,7 @@ checkFileset() { 
#### Error messages #####



# Absolute paths in strings cannot be passed as `root`

expectFailure 'toSource { root = "/nix/store/foobar"; fileset = ./.; }' 'lib.fileset.toSource: `root` \("/nix/store/foobar"\) is a string-like value, but it should be a path instead.

expectFailure 'toSource { root = "/nix/store/foobar"; fileset = ./.; }' 'lib.fileset.toSource: `root` \(/nix/store/foobar\) is a string-like value, but it should be a path instead.

\s*Paths in strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.'



# Only paths are accepted as `root`
@@ -328,14 +328,14 @@ expectFailure 'toSource { root = 10; fileset = ./.; }' 'lib.fileset.toSource: `r 
mkdir -p {foo,bar}/mock-root

expectFailure 'with ((import <nixpkgs/lib>).extend (import <nixpkgs/lib/fileset/mock-splitRoot.nix>)).fileset;

  toSource { root = ./foo/mock-root; fileset = ./bar/mock-root; }

' 'lib.fileset.toSource: Filesystem roots are not the same for `fileset` and `root` \("'"$work"'/foo/mock-root"\):

\s*`root`: root "'"$work"'/foo/mock-root"

\s*`fileset`: root "'"$work"'/bar/mock-root"

\s*Different roots are not supported.'

' 'lib.fileset.toSource: Filesystem roots are not the same for `fileset` and `root` \('"$work"'/foo/mock-root\):

\s*`root`: Filesystem root is "'"$work"'/foo/mock-root"

\s*`fileset`: Filesystem root is "'"$work"'/bar/mock-root"

\s*Different filesystem roots are not supported.'

rm -rf -- *



# `root` needs to exist

expectFailure 'toSource { root = ./a; fileset = ./.; }' 'lib.fileset.toSource: `root` \('"$work"'/a\) does not exist.'

expectFailure 'toSource { root = ./a; fileset = ./.; }' 'lib.fileset.toSource: `root` \('"$work"'/a\) is a path that does not exist.'



# `root` needs to be a file

touch a
@@ -367,7 +367,7 @@ expectFailure 'toSource { root = ./.; fileset = "/some/path"; }' 'lib.fileset.to 
\s*Paths represented as strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.'



# Path coercion errors for non-existent paths

expectFailure 'toSource { root = ./.; fileset = ./a; }' 'lib.fileset.toSource: `fileset` \('"$work"'/a\) does not exist.'

expectFailure 'toSource { root = ./.; fileset = ./a; }' 'lib.fileset.toSource: `fileset` \('"$work"'/a\) is a path that does not exist.'



# File sets cannot be evaluated directly

expectFailure 'union ./. ./.' 'lib.fileset: Directly evaluating a file set is not supported.
@@ -490,26 +490,26 @@ mkdir -p {foo,bar}/mock-root 
expectFailure 'with ((import <nixpkgs/lib>).extend (import <nixpkgs/lib/fileset/mock-splitRoot.nix>)).fileset;

  toSource { root = ./.; fileset = union ./foo/mock-root ./bar/mock-root; }

' 'lib.fileset.union: Filesystem roots are not the same:

\s*first argument: root "'"$work"'/foo/mock-root"

\s*second argument: root "'"$work"'/bar/mock-root"

\s*Different roots are not supported.'

\s*First argument: Filesystem root is "'"$work"'/foo/mock-root"

\s*Second argument: Filesystem root is "'"$work"'/bar/mock-root"

\s*Different filesystem roots are not supported.'



expectFailure 'with ((import <nixpkgs/lib>).extend (import <nixpkgs/lib/fileset/mock-splitRoot.nix>)).fileset;

  toSource { root = ./.; fileset = unions [ ./foo/mock-root ./bar/mock-root ]; }

' 'lib.fileset.unions: Filesystem roots are not the same:

\s*element 0: root "'"$work"'/foo/mock-root"

\s*element 1: root "'"$work"'/bar/mock-root"

\s*Different roots are not supported.'

\s*Element 0: Filesystem root is "'"$work"'/foo/mock-root"

\s*Element 1: Filesystem root is "'"$work"'/bar/mock-root"

\s*Different filesystem roots are not supported.'

rm -rf -- *



# Coercion errors show the correct context

expectFailure 'toSource { root = ./.; fileset = union ./a ./.; }' 'lib.fileset.union: first argument \('"$work"'/a\) does not exist.'

expectFailure 'toSource { root = ./.; fileset = union ./. ./b; }' 'lib.fileset.union: second argument \('"$work"'/b\) does not exist.'

expectFailure 'toSource { root = ./.; fileset = unions [ ./a ./. ]; }' 'lib.fileset.unions: element 0 \('"$work"'/a\) does not exist.'

expectFailure 'toSource { root = ./.; fileset = unions [ ./. ./b ]; }' 'lib.fileset.unions: element 1 \('"$work"'/b\) does not exist.'

expectFailure 'toSource { root = ./.; fileset = union ./a ./.; }' 'lib.fileset.union: First argument \('"$work"'/a\) is a path that does not exist.'

expectFailure 'toSource { root = ./.; fileset = union ./. ./b; }' 'lib.fileset.union: Second argument \('"$work"'/b\) is a path that does not exist.'

expectFailure 'toSource { root = ./.; fileset = unions [ ./a ./. ]; }' 'lib.fileset.unions: Element 0 \('"$work"'/a\) is a path that does not exist.'

expectFailure 'toSource { root = ./.; fileset = unions [ ./. ./b ]; }' 'lib.fileset.unions: Element 1 \('"$work"'/b\) is a path that does not exist.'



# unions needs a list

expectFailure 'toSource { root = ./.; fileset = unions null; }' 'lib.fileset.unions: Expected argument to be a list, but got a null.'

expectFailure 'toSource { root = ./.; fileset = unions null; }' 'lib.fileset.unions: Argument is of type null, but it should be a list instead.'



# The tree of later arguments should not be evaluated if a former argument already includes all files

tree=()
@@ -603,14 +603,14 @@ mkdir -p {foo,bar}/mock-root 
expectFailure 'with ((import <nixpkgs/lib>).extend (import <nixpkgs/lib/fileset/mock-splitRoot.nix>)).fileset;

  toSource { root = ./.; fileset = intersection ./foo/mock-root ./bar/mock-root; }

' 'lib.fileset.intersection: Filesystem roots are not the same:

\s*first argument: root "'"$work"'/foo/mock-root"

\s*second argument: root "'"$work"'/bar/mock-root"

\s*Different roots are not supported.'

\s*First argument: Filesystem root is "'"$work"'/foo/mock-root"

\s*Second argument: Filesystem root is "'"$work"'/bar/mock-root"

\s*Different filesystem roots are not supported.'

rm -rf -- *



# Coercion errors show the correct context

expectFailure 'toSource { root = ./.; fileset = intersection ./a ./.; }' 'lib.fileset.intersection: first argument \('"$work"'/a\) does not exist.'

expectFailure 'toSource { root = ./.; fileset = intersection ./. ./b; }' 'lib.fileset.intersection: second argument \('"$work"'/b\) does not exist.'

expectFailure 'toSource { root = ./.; fileset = intersection ./a ./.; }' 'lib.fileset.intersection: First argument \('"$work"'/a\) is a path that does not exist.'

expectFailure 'toSource { root = ./.; fileset = intersection ./. ./b; }' 'lib.fileset.intersection: Second argument \('"$work"'/b\) is a path that does not exist.'



# The tree of later arguments should not be evaluated if a former argument already excludes all files

tree=(