Revert "Merge "Add file_accessible method" from Benny"

This reverts commit dac264b, reversing
changes made to 85f7e61.

c640d8f contained changed to a public
switch, on which external projects depend.

Fixes scylladb#576
Message-Id: <20190114235444.7979-1-duarte@scylladb.com>

apps/iotune/iotune.cc

@@ -29,6 +29,7 @@
 #include <cmath>
 #include <sys/vfs.h>
 #include <sys/sysmacros.h>
+#include <boost/filesystem.hpp>
 #include <boost/range/irange.hpp>
 #include <boost/program_options.hpp>
 #include <boost/iterator/counting_iterator.hpp>
@@ -51,7 +52,7 @@
 
 using namespace seastar;
 using namespace std::chrono_literals;
-namespace fs = seastar::compat::filesystem;
+namespace fs = std::experimental::filesystem;
 
 logger iotune_logger("iotune");
 

configure.py

@@ -100,8 +100,8 @@ def dialect_supported(dialect, compiler='g++'):
 arg_parser.add_argument('--without-tests', dest='exclude_tests', action='store_true', help='Do not build tests by default')
 arg_parser.add_argument('--without-apps', dest='exclude_apps', action='store_true', help='Do not build applications by default')
 arg_parser.add_argument('--without-demos', dest='exclude_demos', action='store_true', help='Do not build demonstrations by default')
-arg_parser.add_argument('--use-std-optional-variant-stringview-filesystem', dest='cpp17_goodies', action='store', type=int, default=0,
-                        help='Use C++17 std types for optional, variant, string_view, and filesystem. Requires C++17 dialect and GCC >= 8.1.1-5')
+arg_parser.add_argument('--use-std-optional-variant-stringview', dest='cpp17_goodies', action='store', type=int, default=0,
+                        help='Use C++17 std types for optional, variant, and string_view. Requires C++17 dialect and GCC >= 8.1.1-5')
 arg_parser.add_argument('--prefix', dest='install_prefix', default='/usr/local', help='Root installation path of Seastar files')
 args = arg_parser.parse_args()
 
@@ -157,7 +157,7 @@ def configure_mode(mode):
         tr(args.alloc_failure_injector, 'ALLOC_FAILURE_INJECTOR'),
         tr(args.exception_workaround, 'EXCEPTION_SCALABILITY_WORKAROUND', value_when_none='yes'),
         tr(args.allocator_page_size, 'ALLOCATOR_PAGE_SIZE'),
-        tr(args.cpp17_goodies, 'STD_OPTIONAL_VARIANT_STRINGVIEW_FILESYSTEM'),
+        tr(args.cpp17_goodies, 'STD_OPTIONAL_VARIANT_STRINGVIEW'),
     ]
 
     # Generate a new build by pointing to the source directory.

include/seastar/core/reactor.hh

@@ -451,21 +451,6 @@ inline open_flags operator&(open_flags a, open_flags b) {
     return open_flags(std::underlying_type_t<open_flags>(a) & std::underlying_type_t<open_flags>(b));
 }
 
-// Access flags for files/directories
-enum class access_flags {
-    exists = F_OK,
-    read = R_OK,
-    write = W_OK,
-    execute = X_OK,
-
-    // alias for directory access
-    lookup = execute,
-};
-
-inline access_flags operator|(access_flags a, access_flags b) {
-    return access_flags(std::underlying_type_t<access_flags>(a) | std::underlying_type_t<access_flags>(b));
-}
-
 class reactor_backend;
 
 class io_queue {
@@ -915,10 +900,7 @@ public:
     future<> touch_directory(sstring name);
     future<compat::optional<directory_entry_type>>  file_type(sstring name);
     future<uint64_t> file_size(sstring pathname);
-    future<bool> file_accessible(sstring pathname, access_flags flags);
-    future<bool> file_exists(sstring pathname) {
-        return file_accessible(pathname, access_flags::exists);
-    }
+    future<bool> file_exists(sstring pathname);
     future<fs_type> file_system_at(sstring pathname);
     future<struct statvfs> statvfs(sstring pathname);
     future<> remove_file(sstring pathname);

include/seastar/core/seastar.hh

@@ -61,7 +61,6 @@ enum class transport;
 class file;
 class file_open_options;
 enum class open_flags;
-enum class access_flags;
 enum class fs_type;
 
 // Networking API
@@ -264,22 +263,6 @@ future<> rename_file(sstring old_name, sstring new_name);
 /// \param name name of the file to return the size
 future<uint64_t> file_size(sstring name);
 
-/// Check file access.
-///
-/// \param name name of the file to check
-/// \param flags bit pattern containing type of access to check (read/write/execute or exists).
-///
-/// If only access_flags::exists is queried, returns true if the file exists, or false otherwise.
-/// Throws a compat::filesystem::filesystem_error exception if any error other than ENOENT is encountered.
-///
-/// If any of the access_flags (read/write/execute) is set, returns true if the file exists and is
-/// accessible with the requested flags, or false if the file exists and is not accessible
-/// as queried.
-/// Throws a compat::filesystem::filesystem_error exception if any error other than EACCES is encountered.
-/// Note that if any path component leading to the file is not searchable, the file is considered inaccessible
-/// with the requested mode and false will be returned.
-future<bool> file_accessible(sstring name, access_flags flags);
-
 /// check if a file exists.
 ///
 /// \param name name of the file to check

include/seastar/util/read_first_line.hh

@@ -1,8 +1,8 @@
-#include <seastar/util/std-compat.hh>
+#include <experimental/filesystem>
 #include <seastar/core/sstring.hh>
 
 namespace seastar {
 
-sstring read_first_line(compat::filesystem::path sys_file);
+sstring read_first_line(std::experimental::filesystem::path sys_file);
 
 }

include/seastar/util/std-compat.hh

@@ -21,16 +21,14 @@
 
 #pragma once
 
-#ifdef SEASTAR_USE_STD_OPTIONAL_VARIANT_STRINGVIEW_FILESYSTEM
+#ifdef SEASTAR_USE_STD_OPTIONAL_VARIANT_STRINGVIEW
 #include <optional>
 #include <string_view>
 #include <variant>
-#include <filesystem>
 #else
 #include <experimental/optional>
 #include <experimental/string_view>
 #include <boost/variant.hpp>
-#include <experimental/filesystem>
 #endif
 
 namespace seastar {
@@ -39,9 +37,7 @@ namespace seastar {
 
 namespace compat {
 
-#ifdef SEASTAR_USE_STD_OPTIONAL_VARIANT_STRINGVIEW_FILESYSTEM
-
-namespace filesystem = std::filesystem;
+#ifdef SEASTAR_USE_STD_OPTIONAL_VARIANT_STRINGVIEW
 
 template <typename T>
 using optional = std::optional<T>;
@@ -118,8 +114,6 @@ constexpr const U* get_if(const variant<Types...>* v) {
 
 #else
 
-namespace filesystem = std::experimental::filesystem;
-
 template <typename T>
 using optional = std::experimental::optional<T>;
 

include/seastar/util/variant_utils.hh

@@ -105,7 +105,7 @@ template <typename Variant, typename... Args>
 inline auto visit(Variant&& variant, Args&&... args)
 {
     static_assert(sizeof...(Args) > 0, "At least one lambda must be provided for visitation");
-#ifdef SEASTAR_USE_STD_OPTIONAL_VARIANT_STRINGVIEW_FILESYSTEM
+#ifdef SEASTAR_USE_STD_OPTIONAL_VARIANT_STRINGVIEW
     return std::visit(
 #else
     return boost::apply_visitor(
@@ -114,7 +114,7 @@ inline auto visit(Variant&& variant, Args&&... args)
         variant);
 }
 
-#ifdef SEASTAR_USE_STD_OPTIONAL_VARIANT_STRINGVIEW_FILESYSTEM
+#ifdef SEASTAR_USE_STD_OPTIONAL_VARIANT_STRINGVIEW
 
 namespace internal {
 template<typename... Args>

src/core/reactor.cc

@@ -53,6 +53,7 @@
 #include <sys/eventfd.h>
 #include <sys/poll.h>
 #include <boost/lexical_cast.hpp>
+#include <boost/filesystem.hpp>
 #include <boost/thread/barrier.hpp>
 #include <boost/algorithm/string/classification.hpp>
 #include <boost/algorithm/string/split.hpp>
@@ -65,6 +66,7 @@
 #include <boost/range/adaptor/map.hpp>
 #include <boost/version.hpp>
 #include <atomic>
+#include <experimental/filesystem>
 #include <dirent.h>
 #include <linux/types.h> // for xfs, below
 #include <sys/ioctl.h>
@@ -191,7 +193,7 @@ void task_histogram_add_task(const task& t) {
 }
 
 using namespace std::chrono_literals;
-namespace fs = seastar::compat::filesystem;
+namespace fs = std::experimental::filesystem;
 
 using namespace net;
 
@@ -250,33 +252,25 @@ struct syscall_result {
     int error;
     syscall_result(T result, int error) : result{std::move(result)}, error{error} {
     }
-    void throw_if_error() const {
+    void throw_if_error() {
         if (long(result) == -1) {
             throw std::system_error(ec());
         }
     }
 
-    void throw_fs_exception(const sstring& reason, const fs::path& path) const {
-        throw fs::filesystem_error(reason, path, ec());
-    }
-
-    void throw_fs_exception(const sstring& reason, const fs::path& path1, const fs::path& path2) const {
-        throw fs::filesystem_error(reason, path1, path2, ec());
-    }
-
-    void throw_fs_exception_if_error(const sstring& reason, const sstring& path) const {
+    void throw_fs_exception_if_error(sstring reason, sstring path) {
         if (long(result) == -1) {
-            throw_fs_exception(reason, fs::path(path));
+            throw fs::filesystem_error(reason, fs::path(path), ec());
         }
     }
 
-    void throw_fs_exception_if_error(const sstring& reason, const sstring& path1, const sstring& path2) const {
+    void throw_fs_exception_if_error(sstring reason, sstring path1, sstring path2) {
         if (long(result) == -1) {
-            throw_fs_exception(reason, fs::path(path1), fs::path(path2));
+            throw fs::filesystem_error(reason, fs::path(path1), fs::path(path2), ec());
         }
     }
 protected:
-    std::error_code ec() const {
+    std::error_code ec() {
         return std::error_code(error, std::system_category());
     }
 };
@@ -2649,20 +2643,16 @@ reactor::file_size(sstring pathname) {
 }
 
 future<bool>
-reactor::file_accessible(sstring pathname, access_flags flags) {
-    return _thread_pool.submit<syscall_result<int>>([pathname, flags] {
-        auto aflags = std::underlying_type_t<access_flags>(flags);
-        auto ret = ::access(pathname.c_str(), aflags);
-        return wrap_syscall(ret);
-    }).then([this, pathname, flags] (syscall_result<int> sr) {
-        if (sr.result < 0) {
-            if ((sr.error == ENOENT && flags == access_flags::exists) ||
-                (sr.error == EACCES && flags != access_flags::exists)) {
-                return make_ready_future<bool>(false);
-            }
-            sr.throw_fs_exception("access failed", fs::path(pathname));
+reactor::file_exists(sstring pathname) {
+    return _thread_pool.submit<syscall_result_extra<struct stat>>([pathname] {
+        struct stat st;
+        auto ret = stat(pathname.c_str(), &st);
+        return wrap_syscall(ret, st);
+    }).then([pathname] (syscall_result_extra<struct stat> sr) {
+        if (sr.result < 0 && sr.error == ENOENT) {
+            return make_ready_future<bool>(false);
         }
-
+        sr.throw_fs_exception_if_error("stat failed", pathname);
         return make_ready_future<bool>(true);
     });
 }
@@ -5256,10 +5246,6 @@ future<uint64_t> file_size(sstring name) {
     return engine().file_size(name);
 }
 
-future<bool> file_accessible(sstring name, access_flags flags) {
-    return engine().file_accessible(name, flags);
-}
-
 future<bool> file_exists(sstring name) {
     return engine().file_exists(name);
 }
@@ -5293,7 +5279,7 @@ void reactor::add_high_priority_task(std::unique_ptr<task>&& t) {
 static
 bool
 virtualized() {
-    return fs::exists("/sys/hypervisor/type");
+    return boost::filesystem::exists("/sys/hypervisor/type");
 }
 
 std::chrono::nanoseconds

src/util/read_first_line.cc

@@ -3,7 +3,7 @@
 
 namespace seastar {
 
-sstring read_first_line(compat::filesystem::path sys_file) {
+sstring read_first_line(std::experimental::filesystem::path sys_file) {
     auto file = file_desc::open(sys_file.string(), O_RDONLY | O_CLOEXEC);
     sstring buf;
     size_t n = 0;

tests/unit/file_io_test.cc

@@ -43,39 +43,6 @@ SEASTAR_TEST_CASE(open_flags_test) {
     return make_ready_future<>();
 }
 
-SEASTAR_TEST_CASE(access_flags_test) {
-    access_flags flags = access_flags::read | access_flags::write  | access_flags::execute;
-    BOOST_REQUIRE(std::underlying_type_t<open_flags>(flags) ==
-                  (std::underlying_type_t<open_flags>(access_flags::read) |
-                   std::underlying_type_t<open_flags>(access_flags::write) |
-                   std::underlying_type_t<open_flags>(access_flags::execute)));
-    return make_ready_future<>();
-}
-
-SEASTAR_TEST_CASE(file_exists_test) {
-    return seastar::async([] {
-        sstring filename = "testfile.tmp";
-        auto f = open_file_dma(filename, open_flags::rw | open_flags::create).get0();
-        f.close().get();
-        auto exists = file_exists(filename).get0();
-        BOOST_REQUIRE(exists);
-        remove_file(filename).get();
-        exists = file_exists(filename).get0();
-        BOOST_REQUIRE(!exists);
-    });
-}
-
-SEASTAR_TEST_CASE(file_access_test) {
-    return seastar::async([] {
-        sstring filename = "testfile.tmp";
-        auto f = open_file_dma(filename, open_flags::rw | open_flags::create).get0();
-        f.close().get();
-        auto is_accessible = file_accessible(filename, access_flags::read | access_flags::write).get0();
-        BOOST_REQUIRE(is_accessible);
-        remove_file(filename).get();
-    });
-}
-
 struct file_test {
     file_test(file&& f) : f(std::move(f)) {}
     file f;