Cleanup abspath to have python-like semantics

Author: spanezz

wobble/sys-test.cc

@@ -8,6 +8,7 @@
 #include <cstdlib>
 #include <set>
 #include <unistd.h>
+#include <thread>
 
 using namespace std;
 using namespace wobble::sys;
@@ -43,13 +44,52 @@ add_method("isdir", []() {
     wassert(actual(isdir("testdir")).istrue());
 });
 
+#pragma GCC diagnostic pop
+
 add_method("abspath", []() {
-    std::string cwd = std::filesystem::current_path().string();
-    wassert(actual(abspath(".")) == cwd + "/");
-    wassert(actual(abspath("foo")) == cwd + "/foo");
-    wassert(actual(abspath("foo/")) == cwd + "/foo/");
+    auto cwd = std::filesystem::current_path();
+    wassert(actual(abspath(".")) == cwd);
+    wassert(actual(abspath("foo")) == cwd / "foo");
+    wassert(actual(abspath("foo/")) == cwd / "foo/");
+});
+
+add_method("abspath_concurrency", []() {
+    // See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118733
+    std::filesystem::path workdir("testdir");
+    rmtree_ifexists(workdir);
+    std::filesystem::create_directory(workdir);
+
+    auto expected = std::filesystem::absolute(workdir) / "dir";
+
+    auto testpath = workdir / "dir";
+    bool done = false;
+
+    auto glitch = [&done, &testpath] {
+        while (! done)
+        {
+            std::filesystem::remove(testpath);
+            std::filesystem::create_directory(testpath);
+        }
+    };
+
+    std::thread glitcher(glitch);
+
+    int failed_iteration = -1;
+    std::string failed_message;
+    for (unsigned iteration = 0; iteration < 1000; ++iteration)
+        try {
+            wassert(actual(abspath(testpath)) == expected);
+        } catch (std::exception& e) {
+            failed_iteration = iteration;
+            failed_message = e.what();
+            break;
+        }
+    done = true;
+    glitcher.join();
+
+    if (failed_iteration != -1)
+        wfail_test("abspath failed at iteration " + std::to_string(failed_iteration) + ": " + failed_message);
 });
-#pragma GCC diagnostic pop
 
 add_method("timestamp", []() {
     using namespace wobble;

wobble/sys.cc

@@ -219,9 +219,57 @@ mode_t umask(mode_t mask)
     return ::umask(mask);
 }
 
-std::string abspath(const std::string& pathname)
+std::filesystem::path abspath(const char* file)
 {
-    return std::filesystem::absolute(pathname).lexically_normal().native();
+    return abspath(std::filesystem::path(file));
+}
+std::string abspath(const std::string& path)
+{
+    return abspath(std::filesystem::path(path)).native();
+}
+
+std::filesystem::path abspath(const std::filesystem::path& path)
+{
+    // weakly_canonical is defined as "the result of calling canonical() with a
+    // path argument composed of the leading elements of p that exist (as
+    // determined by status(p) or status(p, ec)), if any, followed by the
+    // elements of p that do not exist."
+    //
+    // This means that if no lead components of the path exist then the
+    // resulting path is not made absolute, and we need to work around that.
+    if (!path.is_absolute())
+        return abspath(std::filesystem::current_path() / path);
+
+    // This is further and needlessly complicated because we need to work
+    // around https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118733
+    unsigned retry = 0;
+    while (true)
+    {
+        std::error_code code;
+        auto result = std::filesystem::weakly_canonical(path, code);
+        if (!code)
+        {
+            // fprintf(stderr, "%s: ok in %u tries\n", path.c_str(), retry+1);
+            return result;
+        }
+
+        if (code == std::errc::no_such_file_or_directory)
+        {
+            ++retry;
+            if (retry > 50)
+                throw std::system_error(code);
+        }
+        else
+            throw std::system_error(code);
+    }
+
+    // Alternative implementation that however may not work on all platforms
+    // since, formally, "[std::filesystem::absolute] Implementations are
+    // encouraged to not consider p not existing to be an error", but they do
+    // not mandate it, and if they did, they might still be affected by the
+    // undefined behaviour outlined in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118733
+    //
+    // return std::filesystem::absolute(path).lexically_normal();
 }
 
 

wobble/sys.h

@@ -133,7 +133,9 @@ void chroot(const std::filesystem::path& dir);
 mode_t umask(mode_t mask);
 
 /// Get the absolute path of a file
-[[deprecated("Use std::filesystem::canonical")]] std::string abspath(const std::string& pathname);
+std::filesystem::path abspath(const std::filesystem::path& path);
+[[deprecated("Use abspath(const std::filesystem::path&)")]] std::string abspath(const std::string& pathname);
+std::filesystem::path abspath(const char* path);
 
 /**
  * Wraps a mmapped memory area, unmapping it on destruction.