Remove vespalib::rmdir and vespalib::mkdir

3 files changed, 23 insertions, 178 deletions

diff --git a/vespalib/src/tests/io/fileutil/fileutiltest.cpp b/vespalib/src/tests/io/fileutil/fileutiltest.cpp
index 51cf2055d33..4eb700fd4ed 100644
--- a/vespalib/src/tests/io/fileutil/fileutiltest.cpp
+++ b/vespalib/src/tests/io/fileutil/fileutiltest.cpp
@@ -53,7 +53,7 @@ TEST("require that vespalib::File::open works")
     }
         // Opening file in non-existing subdir should fail.
     try{
-        rmdir("mydir", true); // Just in case
+        std::filesystem::remove_all(std::filesystem::path("mydir")); // Just in case
         File f("mydir/myfile");
         f.open(File::CREATE);
         TEST_FATAL("Opening non-existing file for reading should fail.");
@@ -156,10 +156,10 @@ TEST("require that vespalib::File::isOpen works")
 TEST("require that vespalib::File::stat works")
 {
     unlink("myfile");
-    rmdir("mydir", true);
+    std::filesystem::remove_all(std::filesystem::path("mydir"));
     EXPECT_EQUAL(false, fileExists("myfile"));
     EXPECT_EQUAL(false, fileExists("mydir"));
-    mkdir("mydir");
+    std::filesystem::create_directory(std::filesystem::path("mydir"));
     FileInfo::UP info = stat("myfile");
     ASSERT_TRUE(info.get() == 0);
     File f("myfile");
@@ -206,83 +206,11 @@ TEST("require that vespalib::File::resize works")
     EXPECT_EQUAL(std::string("foo"), std::string(&vec[0], 3));
 }
 
-TEST("require that vespalib::mkdir and vespalib::rmdir works")
-{
-    rmdir("mydir", true);
-    ASSERT_TRUE(!fileExists("mydir"));
-        // Cannot create recursive without recursive option
-    try{
-        mkdir("mydir/otherdir", false);
-        TEST_FATAL("Should not work without recursive option set");
-    } catch (IoException& e) {
-        //std::cerr << e.what() << "\n";
-        EXPECT_EQUAL(IoException::NOT_FOUND, e.getType());
-    }
-        // Works with recursive option
-    {
-        ASSERT_TRUE(mkdir("mydir/otherdir"));
-        ASSERT_TRUE(!mkdir("mydir/otherdir"));
-            // Test chdir / getCurrentDirectory
-        chdir("mydir");
-        std::string currDir = getCurrentDirectory();
-        std::string::size_type pos = currDir.rfind('/');
-        ASSERT_TRUE(pos != std::string::npos);
-        EXPECT_EQUAL("mydir", currDir.substr(pos + 1));
-        EXPECT_EQUAL('/', currDir[0]);
-        chdir("..");
-        currDir = getCurrentDirectory();
-        pos = currDir.rfind('/');
-        EXPECT_EQUAL("fileutil", currDir.substr(pos + 1));
-    }
-        // rmdir fails with content
-    try{
-        ASSERT_TRUE(mkdir("mydir/otherdir/evenmorestuff"));
-        rmdir("mydir");
-        TEST_FATAL("Should not work without recursive option set");
-    } catch (std::filesystem::filesystem_error& e) {
-        //std::cerr << e.what() << "\n";
-        EXPECT_EQUAL(make_error_code(std::errc::directory_not_empty), e.code());
-    }
-        // Works with recursive option
-    {
-        ASSERT_TRUE(rmdir("mydir", true));
-        ASSERT_TRUE(!fileExists("mydir"));
-        ASSERT_TRUE(!rmdir("mydir", true));
-    }
-
-    // mkdir works when a path component is a symlink which points to
-    // another directory and the final path component does not exist.
-    {
-        rmdir("mydir", true);
-        rmdir("otherdir", true);
-        unlink("linkeddir"); // symlink if exists
-        mkdir("otherdir/stuff");
-        symlink("otherdir", "linkeddir");
-        // Should now be able to resolve through symlink and create dir
-        // at the appropriate (linked) location.
-        ASSERT_TRUE(mkdir("linkeddir/stuff/fluff"));
-    }
-
-    // mkdir works when the final path component is a symlink which points
-    // to another directory (causing OS mkdir to fail since something already
-    // exists at the location).
-    {
-        rmdir("mydir", true);
-        rmdir("otherdir", true);
-        unlink("linkeddir"); // symlink if exists
-        mkdir("otherdir/stuff");
-        symlink("otherdir", "linkeddir");
-        // Should now be able to resolve through symlink and create dir
-        // at the appropriate (linked) location.
-        ASSERT_FALSE(mkdir("linkeddir"));
-    }
-}
-
 TEST("require that vespalib::unlink works")
 {
         // Fails on directory
     try{
-        mkdir("mydir");
+        std::filesystem::create_directory(std::filesystem::path("mydir"));
         unlink("mydir");
         TEST_FATAL("Should work on directories.");
     } catch (IoException& e) {
@@ -309,7 +237,7 @@ TEST("require that vespalib::unlink works")
 
 TEST("require that vespalib::rename works")
 {
-    rmdir("mydir", true);
+    std::filesystem::remove_all(std::filesystem::path("mydir"));
     File f("myfile");
     f.open(File::CREATE | File::TRUNC);
     f.write("Hello World!\n", 13, 0);
@@ -353,7 +281,7 @@ TEST("require that vespalib::rename works")
     }
         // Overwriting directory fails (does not put inside dir)
     try{
-        mkdir("mydir");
+        std::filesystem::create_directory(std::filesystem::path("mydir"));
         f.open(File::CREATE | File::TRUNC);
         f.write("Bah\n", 4, 0);
         f.close();
@@ -365,7 +293,7 @@ TEST("require that vespalib::rename works")
         // Moving directory works
     {
         ASSERT_TRUE(isDirectory("mydir"));
-        rmdir("myotherdir", true);
+        std::filesystem::remove_all(std::filesystem::path("myotherdir"));
         ASSERT_TRUE(rename("mydir", "myotherdir"));
         ASSERT_TRUE(isDirectory("myotherdir"));
         ASSERT_TRUE(!isDirectory("mydir"));
@@ -389,7 +317,7 @@ TEST("require that vespalib::rename works")
 
 TEST("require that vespalib::copy works")
 {
-    rmdir("mydir", true);
+    std::filesystem::remove_all(std::filesystem::path("mydir"));
     File f("myfile");
     f.open(File::CREATE | File::TRUNC);
 
@@ -416,7 +344,7 @@ TEST("require that vespalib::copy works")
     }
         // Fails if target is directory
     try{
-        mkdir("mydir");
+        std::filesystem::create_directory(std::filesystem::path("mydir"));
         copy("myfile", "mydir");
         TEST_FATAL("Should fail trying to overwrite directory");
     } catch (IoException& e) {
@@ -425,7 +353,7 @@ TEST("require that vespalib::copy works")
     }
         // Fails if source is directory
     try{
-        mkdir("mydir");
+        std::filesystem::create_directory(std::filesystem::path("mydir"));
         copy("mydir", "myfile");
         TEST_FATAL("Should fail trying to copy directory");
     } catch (IoException& e) {
@@ -479,8 +407,8 @@ TEST("require that vespalib::symlink works")
 {
     // Target exists
     {
-        rmdir("mydir", true);
-        mkdir("mydir");
+        std::filesystem::remove_all(std::filesystem::path("mydir"));
+        std::filesystem::create_directory(std::filesystem::path("mydir"));
 
         File f("mydir/myfile");
         f.open(File::CREATE | File::TRUNC);
@@ -500,9 +428,9 @@ TEST("require that vespalib::symlink works")
 
     // POSIX symlink() fails
     {
-        rmdir("mydir", true);
-        mkdir("mydir/a", true);
-        mkdir("mydir/b");
+        std::filesystem::remove_all(std::filesystem::path("mydir"));
+        std::filesystem::create_directories(std::filesystem::path("mydir/a"));
+        std::filesystem::create_directory(std::filesystem::path("mydir/b"));
         try {
             // Link already exists
             symlink("a", "mydir/b");
@@ -513,8 +441,8 @@ TEST("require that vespalib::symlink works")
     }
 
     {
-        rmdir("mydir", true);
-        mkdir("mydir");
+        std::filesystem::remove_all(std::filesystem::path("mydir"));
+        std::filesystem::create_directory(std::filesystem::path("mydir"));
 
         File f("mydir/myfile");
         f.open(File::CREATE | File::TRUNC);
@@ -586,8 +514,8 @@ TEST("require that vespalib::dirname works")
 TEST("require that vespalib::getOpenErrorString works")
 {
     stringref dirName = "mydir";
-    rmdir(dirName, true);
-    mkdir(dirName, false);
+    std::filesystem::remove_all(std::filesystem::path(dirName));
+    std::filesystem::create_directory(std::filesystem::path(dirName));
     {
         File foo("mydir/foo");
         foo.open(File::CREATE);
@@ -605,7 +533,7 @@ TEST("require that vespalib::getOpenErrorString works")
     std::cerr << "getOpenErrorString(1, \"notFound\") is " << err2 <<
         ", normalized to " << normErr2 << std::endl;
     EXPECT_EQUAL(expErr2, normErr2);
-    rmdir(dirName, true);
+    std::filesystem::remove_all(std::filesystem::path(dirName));
 }
 
 } // vespalib
diff --git a/vespalib/src/vespa/vespalib/io/fileutil.cpp b/vespalib/src/vespa/vespalib/io/fileutil.cpp
index 2d28f07e600..ff39e56f000 100644
--- a/vespalib/src/vespa/vespalib/io/fileutil.cpp
+++ b/vespalib/src/vespa/vespalib/io/fileutil.cpp
@@ -151,7 +151,7 @@ namespace {
             auto pos = filename.rfind('/');
             if (pos != string::npos) {
                 string path(filename.substr(0, pos));
-                mkdir(path);
+                std::filesystem::create_directories(std::filesystem::path(path));
                 LOG(spam, "open(%s, %d): Retrying open after creating parent "
                           "directories.", filename.c_str(), flags);
                 fd = ::open(filename.c_str(), flags, 0644);
@@ -448,50 +448,6 @@ getCurrentDirectory()
     throw IoException(ost.str(), IoException::getErrorType(errno), VESPA_STRLOC);
 }
 
-bool
-mkdir(const string & directory, bool recursive)
-{
-    if (::mkdir(directory.c_str(), 0777) == 0) {
-        LOG(debug, "mkdir(%s): Created directory", directory.c_str());
-        return true;
-    }
-    if (recursive && errno == ENOENT) {
-        auto slashpos = directory.rfind('/');
-        if (slashpos != string::npos) {
-            /* Recursively make superdirs.*/
-            string superdir = directory.substr(0, slashpos);
-            mkdir(superdir, recursive);
-            if (::mkdir(directory.c_str(), 0777) == 0) {
-                LOG(debug, "mkdir(%s): Created directory recursively", directory.c_str());
-                return true;
-            }
-        }
-    }
-    if (errno == EEXIST) {
-        // Use stat rather than lstat since we don't really care if the path
-        // component is a symbolic link as long as it points to a directory.
-        FileInfo::UP info(vespalib::stat(directory));
-        if (info.get() != 0 && info->_directory) {
-            LOG(debug, "mkdir(%s): Directory existed", directory.c_str());
-            return false;
-        } else if (info.get() != 0) {
-            asciistream ost;
-            ost << "mkdir(" << directory << (recursive ? ", recursive" : "")
-                << "): Failed.";
-            if (info->_plainfile) {
-                ost << " A plain file already exist.";
-            } else {
-                ost << " A file of some sort already exist.";
-            }
-            throw IoException(ost.str(), IoException::ILLEGAL_PATH, VESPA_STRLOC);
-        }
-    }
-    asciistream ost;
-    ost << "mkdir(" << directory << (recursive ? ", recursive" : "")
-        << "): Failed, errno(" << errno << "): " << safeStrerror(errno);
-    throw IoException(ost.str(), IoException::getErrorType(errno), VESPA_STRLOC);
-}
-
 void
 symlink(const string & oldPath, const string & newPath)
 {
@@ -532,17 +488,6 @@ chdir(const string & directory)
     LOG(debug, "chdir(%s): Working directory changed.", directory.c_str());
 }
 
-bool
-rmdir(const string & directory, bool recursive)
-{
-    std::filesystem::path path(directory);
-    if (recursive) {
-        return std::filesystem::remove_all(path) > 0;
-    } else {
-        return std::filesystem::remove(path);
-    }
-}
-
 FileInfo::UP
 stat(const string & path)
 {
@@ -593,7 +538,7 @@ rename(const string & frompath, const string & topath,
                 string::size_type pos = topath.rfind('/');
                 if (pos != string::npos) {
                     string path(topath.substr(0, pos));
-                    vespalib::mkdir(path);
+                    std::filesystem::create_directories(std::filesystem::path(path));
                     LOG(debug, "rename(%s, %s): Created target directory. Calling recursively.",
                         frompath.c_str(), topath.c_str());
                     return rename(frompath, topath, copyDeleteBetweenFilesystems, false);
diff --git a/vespalib/src/vespa/vespalib/io/fileutil.h b/vespalib/src/vespa/vespalib/io/fileutil.h
index 3c9f50c37d5..acbfe14020a 100644
--- a/vespalib/src/vespa/vespalib/io/fileutil.h
+++ b/vespalib/src/vespa/vespalib/io/fileutil.h
@@ -216,20 +216,6 @@ public:
 extern vespalib::string getCurrentDirectory();
 
 /**
- * Creates a directory.
- *
- * @param directory   The directory to create.
- * @param recursive   If set, create all parent directories needed if missing.
- * @throw IoException If we failed create the directory.
- *
- * @return True if it did not exist, false if it did.
- *
- * This function is deprecated. Use std::filesystem::create_directory or std::filesystem::create_directories
- * instead.
- */
-extern bool mkdir(const vespalib::string & directory, bool recursive = true);
-
-/**
  * Change working directory.
  *
  * @param directory   The directory to change to.
@@ -238,21 +224,6 @@ extern bool mkdir(const vespalib::string & directory, bool recursive = true);
 extern void chdir(const vespalib::string & directory);
 
 /**
- * Remove a directory.
- *
- * @param directory   The directory name.
- * @param recursive   If set, remove all content of the directory to. If not
- *                    set, fail if the directory is not empty.
- * @throw std::filesystem::filesystem_error If we failed to remove the directory.
- *
- * @return True if directory existed, false if not.
- *
- * This function is deprecated. Use std::filesystem::remove or std::filesystem::remove_all
- * instead.
- */
-extern bool rmdir(const vespalib::string & directory, bool recursive = false);
-
-/**
  * Stat a file.
  *
  * @throw  IoException If we failed to stat the file.
@@ -376,6 +347,7 @@ extern bool unlink(const vespalib::string & filename);
  * should be created if it's missing, or not.
  *
  * @throw IoException If we failed to rename the file.
+ * @throw std::filesystem::filesystem_error If we failed to create a target directory
  * @return True if file was renamed, false if frompath did not exist.
  */
 extern bool rename(const vespalib::string & frompath,