From 2d28e0cd109dc9f04d2ab0d8ac90395feb2acb3e Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 8 Nov 2023 23:28:25 +0100 Subject: [PATCH 1/4] lib.fileset: Remove nixdoc workaround This problem was fixed in https://github.com/nix-community/nixdoc/pull/81 which is included in version 2.5.1, which is now used in Nixpkgs --- lib/fileset/default.nix | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/fileset/default.nix b/lib/fileset/default.nix index 4a97633b4a89..547809cf0634 100644 --- a/lib/fileset/default.nix +++ b/lib/fileset/default.nix @@ -122,11 +122,10 @@ in { Paths in [strings](https://nixos.org/manual/nix/stable/language/values.html#type-string), including Nix store paths, cannot be passed as `root`. `root` has to be a directory. - -:::{.note} -Changing `root` only affects the directory structure of the resulting store path, it does not change which files are added to the store. -The only way to change which files get added to the store is by changing the `fileset` attribute. -::: + :::{.note} + Changing `root` only affects the directory structure of the resulting store path, it does not change which files are added to the store. + The only way to change which files get added to the store is by changing the `fileset` attribute. + ::: */ root, /* @@ -135,10 +134,9 @@ The only way to change which files get added to the store is by changing the `fi This argument can also be a path, which gets [implicitly coerced to a file set](#sec-fileset-path-coercion). - -:::{.note} -If a directory does not recursively contain any file, it is omitted from the store path contents. -::: + :::{.note} + If a directory does not recursively contain any file, it is omitted from the store path contents. + ::: */ fileset, From 2556605a356c59a7269f601f56e9271ed3f487d1 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 8 Nov 2023 23:32:53 +0100 Subject: [PATCH 2/4] lib.fileset: Remove "somewhat limited" from the docs It's not very limited anymore :) --- doc/functions/fileset.section.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/doc/functions/fileset.section.md b/doc/functions/fileset.section.md index 08b9ba9eaedc..c42337feaba4 100644 --- a/doc/functions/fileset.section.md +++ b/doc/functions/fileset.section.md @@ -6,11 +6,8 @@ The [`lib.fileset`](#sec-functions-library-fileset) library allows you to work w A file set is a mathematical set of local files that can be added to the Nix store for use in Nix derivations. File sets are easy and safe to use, providing obvious and composable semantics with good error messages to prevent mistakes. -These sections apply to the entire library. See the [function reference](#sec-functions-library-fileset) for function-specific documentation. -The file set library is currently somewhat limited but is being expanded to include more functions over time. - ## Implicit coercion from paths to file sets {#sec-fileset-path-coercion} All functions accepting file sets as arguments can also accept [paths](https://nixos.org/manual/nix/stable/language/values.html#type-path) as arguments. From 0ace383438b84b0db04b4103d0b2d73400294058 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 8 Nov 2023 23:59:04 +0100 Subject: [PATCH 3/4] 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 --- lib/fileset/default.nix | 38 +++++++++++++++++---------------- lib/fileset/internal.nix | 8 +++---- lib/fileset/tests.sh | 46 ++++++++++++++++++++-------------------- 3 files changed, 47 insertions(+), 45 deletions(-) diff --git a/lib/fileset/default.nix b/lib/fileset/default.nix index 547809cf0634..fe7b304ba698 100644 --- a/lib/fileset/default.nix +++ b/lib/fileset/default.nix @@ -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) diff --git a/lib/fileset/internal.nix b/lib/fileset/internal.nix index b919a5de3eef..853115df9f61 100644 --- a/lib/fileset/internal.nix +++ b/lib/fileset/internal.nix @@ -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; diff --git a/lib/fileset/tests.sh b/lib/fileset/tests.sh index 2df0727bde38..c1c67800f5e2 100755 --- a/lib/fileset/tests.sh +++ b/lib/fileset/tests.sh @@ -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 ).extend (import )).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 ).extend (import )).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 ).extend (import )).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 ).extend (import )).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=( From 9cbd394aa036442e0ccd0459908b96a5a60b2a9e Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 9 Nov 2023 00:02:43 +0100 Subject: [PATCH 4/4] lib.fileset: Remove unused bindings Thanks nixd! --- lib/fileset/internal.nix | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/fileset/internal.nix b/lib/fileset/internal.nix index 853115df9f61..d55c84a395e7 100644 --- a/lib/fileset/internal.nix +++ b/lib/fileset/internal.nix @@ -7,7 +7,6 @@ let isString pathExists readDir - seq split trace typeOf @@ -17,7 +16,6 @@ let attrNames attrValues mapAttrs - setAttrByPath zipAttrsWith ; @@ -28,7 +26,6 @@ let inherit (lib.lists) all commonPrefix - drop elemAt filter findFirst