From 99bc90e3019cd79de82b41481fe1cc2ef4900caa Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 14 Mar 2023 23:28:48 +0100 Subject: [PATCH] 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,