Test and document builtins.baseNameOf, improve internal baseNameOf()

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,
 });

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);

tests/functional/lang/eval-okay-baseNameOf.exp

@@ -0,0 +1 @@
+"ok"

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"

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
      * --------------------------------------------------------------------------*/