diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index d0fcfd194..db4237130 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1568,23 +1568,50 @@ static RegisterPrimOp primop_pathExists({ .fun = prim_pathExists, }); +// Ideally, all trailing slashes should have been removed, but it's been like this for +// almost a decade as of writing. Changing it will affect reproducibility. +static std::string_view legacyBaseNameOf(std::string_view path) +{ + if (path.empty()) + return ""; + + auto last = path.size() - 1; + if (path[last] == '/' && last > 0) + last -= 1; + + auto pos = path.rfind('/', last); + if (pos == path.npos) + pos = 0; + else + pos += 1; + + return path.substr(pos, last - pos + 1); +} + /* Return the base name of the given string, i.e., everything following the last slash. */ static void prim_baseNameOf(EvalState & state, const PosIdx pos, Value * * args, Value & v) { NixStringContext context; - v.mkString(baseNameOf(*state.coerceToString(pos, *args[0], context, + v.mkString(legacyBaseNameOf(*state.coerceToString(pos, *args[0], context, "while evaluating the first argument passed to builtins.baseNameOf", false, false)), context); } static RegisterPrimOp primop_baseNameOf({ .name = "baseNameOf", - .args = {"s"}, + .args = {"x"}, .doc = R"( - Return the *base name* of the string *s*, that is, everything - following the final slash in the string. This is similar to the GNU - `basename` command. + Return the *base name* of either a [path value](@docroot@/language/values.md#type-path) *x* or a string *x*, depending on which type is passed, and according to the following rules. + + For a path value, the *base name* is considered to be the part of the path after the last directory separator, including any file extensions. + This is the simple case, as path values don't have trailing slashes. + + When the argument is a string, a more involved logic applies. If the string ends with a `/`, only this one final slash is removed. + + After this, the *base name* is returned as previously described, assuming `/` as the directory separator. (Note that evaluation must be platform independent.) + + This is somewhat similar to the [GNU `basename`](https://www.gnu.org/software/coreutils/manual/html_node/basename-invocation.html) command, but GNU `basename` will strip any number of trailing slashes. )", .fun = prim_baseNameOf, }); diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 9f81ee452..c0e268e9d 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -128,7 +128,7 @@ std::string_view baseNameOf(std::string_view path) return ""; auto last = path.size() - 1; - if (path[last] == '/' && last > 0) + while (last > 0 && path[last] == '/') last -= 1; auto pos = path.rfind('/', last); diff --git a/tests/functional/lang/eval-okay-baseNameOf.exp b/tests/functional/lang/eval-okay-baseNameOf.exp new file mode 100644 index 000000000..52c33a57c --- /dev/null +++ b/tests/functional/lang/eval-okay-baseNameOf.exp @@ -0,0 +1 @@ +"ok" diff --git a/tests/functional/lang/eval-okay-baseNameOf.nix b/tests/functional/lang/eval-okay-baseNameOf.nix new file mode 100644 index 000000000..a7afdd896 --- /dev/null +++ b/tests/functional/lang/eval-okay-baseNameOf.nix @@ -0,0 +1,32 @@ +assert baseNameOf "" == ""; +assert baseNameOf "." == "."; +assert baseNameOf ".." == ".."; +assert baseNameOf "a" == "a"; +assert baseNameOf "a." == "a."; +assert baseNameOf "a.." == "a.."; +assert baseNameOf "a.b" == "a.b"; +assert baseNameOf "a.b." == "a.b."; +assert baseNameOf "a.b.." == "a.b.."; +assert baseNameOf "a/" == "a"; +assert baseNameOf "a/." == "."; +assert baseNameOf "a/.." == ".."; +assert baseNameOf "a/b" == "b"; +assert baseNameOf "a/b." == "b."; +assert baseNameOf "a/b.." == "b.."; +assert baseNameOf "a/b/c" == "c"; +assert baseNameOf "a/b/c." == "c."; +assert baseNameOf "a/b/c.." == "c.."; +assert baseNameOf "a/b/c/d" == "d"; +assert baseNameOf "a/b/c/d." == "d."; +assert baseNameOf "a\\b" == "a\\b"; +assert baseNameOf "C:a" == "C:a"; +assert baseNameOf "a//b" == "b"; + +# It's been like this for close to a decade. We ought to commit to it. +# https://github.com/NixOS/nix/pull/582#issuecomment-121014450 +assert baseNameOf "a//" == ""; + +assert baseNameOf ./foo == "foo"; +assert baseNameOf ./foo/bar == "bar"; + +"ok" diff --git a/tests/unit/libutil/tests.cc b/tests/unit/libutil/tests.cc index 4406fd184..d7e9edf0a 100644 --- a/tests/unit/libutil/tests.cc +++ b/tests/unit/libutil/tests.cc @@ -151,6 +151,16 @@ namespace nix { ASSERT_EQ(p1, "dir"); } + TEST(baseNameOf, trailingSlashes) { + auto p1 = baseNameOf("/dir//"); + ASSERT_EQ(p1, "dir"); + } + + TEST(baseNameOf, absoluteNothingSlashNothing) { + auto p1 = baseNameOf("//"); + ASSERT_EQ(p1, ""); + } + /* ---------------------------------------------------------------------------- * isInDir * --------------------------------------------------------------------------*/