From 99bc90e3019cd79de82b41481fe1cc2ef4900caa Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 14 Mar 2023 23:28:48 +0100 Subject: [PATCH 1/3] lib.strings: Deprecate path prefix/suffix/infix arguments lib.{hasPrefix,hasInfix,hasSuffix} would otherwise return an always-false result, which can be very unexpected: nix-repl> lib.strings.hasPrefix ./lib ./lib/meta.nix false --- lib/strings.nix | 43 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/lib/strings.nix b/lib/strings.nix index 68d930950662..963d937947f2 100644 --- a/lib/strings.nix +++ b/lib/strings.nix @@ -2,7 +2,9 @@ { lib }: let -inherit (builtins) length; + inherit (builtins) length; + + inherit (lib.trivial) warnIf; in @@ -238,7 +240,17 @@ rec { # Prefix to check for pref: # Input string - str: substring 0 (stringLength pref) str == pref; + str: + # Before 23.05, paths would be copied to the store before converting them + # to strings and comparing. This was surprising and confusing. + warnIf + (isPath pref) + '' + lib.strings.hasPrefix: The first argument (${toString pref}) is a path value, but only strings are supported. + There is almost certainly a bug in the calling code, since this function always returns `false` in such a case. + This function also copies the path to the Nix store, which may not be what you want. + This behavior is deprecated and will throw an error in the future.'' + (substring 0 (stringLength pref) str == pref); /* Determine whether a string has given suffix. @@ -258,8 +270,20 @@ rec { let lenContent = stringLength content; lenSuffix = stringLength suffix; - in lenContent >= lenSuffix && - substring (lenContent - lenSuffix) lenContent content == suffix; + in + # Before 23.05, paths would be copied to the store before converting them + # to strings and comparing. This was surprising and confusing. + warnIf + (isPath suffix) + '' + lib.strings.hasSuffix: The first argument (${toString suffix}) is a path value, but only strings are supported. + There is almost certainly a bug in the calling code, since this function always returns `false` in such a case. + This function also copies the path to the Nix store, which may not be what you want. + This behavior is deprecated and will throw an error in the future.'' + ( + lenContent >= lenSuffix + && substring (lenContent - lenSuffix) lenContent content == suffix + ); /* Determine whether a string contains the given infix @@ -276,7 +300,16 @@ rec { => false */ hasInfix = infix: content: - builtins.match ".*${escapeRegex infix}.*" "${content}" != null; + # Before 23.05, paths would be copied to the store before converting them + # to strings and comparing. This was surprising and confusing. + warnIf + (isPath infix) + '' + lib.strings.hasInfix: The first argument (${toString infix}) is a path value, but only strings are supported. + There is almost certainly a bug in the calling code, since this function always returns `false` in such a case. + This function also copies the path to the Nix store, which may not be what you want. + This behavior is deprecated and will throw an error in the future.'' + (builtins.match ".*${escapeRegex infix}.*" "${content}" != null); /* Convert a string to a list of characters (i.e. singleton strings). This allows you to, e.g., map a function over each character. However, From 5e8b9de7285497e4ef749eb9c55916dd26fecb88 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 14 Mar 2023 23:29:08 +0100 Subject: [PATCH 2/3] lib.strings.normalizePath: Deprecate for path values There's no need to call this function on path data types, and it's confusing with the new lib.path library functions --- lib/strings.nix | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/strings.nix b/lib/strings.nix index 963d937947f2..be4e8ece5936 100644 --- a/lib/strings.nix +++ b/lib/strings.nix @@ -207,7 +207,20 @@ rec { normalizePath "/a//b///c/" => "/a/b/c/" */ - normalizePath = s: (builtins.foldl' (x: y: if y == "/" && hasSuffix "/" x then x else x+y) "" (stringToCharacters s)); + normalizePath = s: + warnIf + (isPath s) + '' + lib.strings.normalizePath: The argument (${toString s}) is a path value, but only strings are supported. + Path values are always normalised in Nix, so there's no need to call this function on them. + This function also copies the path to the Nix store and returns the store path, the same as "''${path}" will, which may not be what you want. + This behavior is deprecated and will throw an error in the future.'' + ( + builtins.foldl' + (x: y: if y == "/" && hasSuffix "/" x then x else x+y) + "" + (stringToCharacters s) + ); /* Depending on the boolean `cond', return either the given string or the empty string. Useful to concatenate against a bigger string. From 61012f6daf61e2cca664c333453a8b868908dc97 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 15 Mar 2023 19:49:33 +0100 Subject: [PATCH 3/3] lib.strings.remove{Prefix,Suffix}: Deprecate for path prefix/suffix arguments See also parent commits --- lib/strings.nix | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/lib/strings.nix b/lib/strings.nix index be4e8ece5936..46a62522e1f4 100644 --- a/lib/strings.nix +++ b/lib/strings.nix @@ -601,14 +601,23 @@ rec { prefix: # Input string str: - let + # Before 23.05, paths would be copied to the store before converting them + # to strings and comparing. This was surprising and confusing. + warnIf + (isPath prefix) + '' + lib.strings.removePrefix: The first argument (${toString prefix}) is a path value, but only strings are supported. + There is almost certainly a bug in the calling code, since this function never removes any prefix in such a case. + This function also copies the path to the Nix store, which may not be what you want. + This behavior is deprecated and will throw an error in the future.'' + (let preLen = stringLength prefix; sLen = stringLength str; in - if hasPrefix prefix str then + if substring 0 preLen str == prefix then substring preLen (sLen - preLen) str else - str; + str); /* Return a string without the specified suffix, if the suffix matches. @@ -625,14 +634,23 @@ rec { suffix: # Input string str: - let + # Before 23.05, paths would be copied to the store before converting them + # to strings and comparing. This was surprising and confusing. + warnIf + (isPath suffix) + '' + lib.strings.removeSuffix: The first argument (${toString suffix}) is a path value, but only strings are supported. + There is almost certainly a bug in the calling code, since this function never removes any suffix in such a case. + This function also copies the path to the Nix store, which may not be what you want. + This behavior is deprecated and will throw an error in the future.'' + (let sufLen = stringLength suffix; sLen = stringLength str; in if sufLen <= sLen && suffix == substring (sLen - sufLen) sufLen str then substring 0 (sLen - sufLen) str else - str; + str); /* Return true if string v1 denotes a version older than v2.