Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move file operations to <filesystem> where possible #113

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion dist-clang.files
Original file line number Diff line number Diff line change
Expand Up @@ -2202,6 +2202,7 @@ src/base/file/data.cc
src/base/file/data.h
src/base/file/epoll_linux.cc
src/base/file/epoll_linux.h
src/base/file/file.cc
src/base/file/file.h
src/base/file/file_posix.cc
src/base/file/file_test.cc
Expand All @@ -2215,7 +2216,7 @@ src/base/file/pipe.h
src/base/file/pipe_linux.cc
src/base/file/pipe_mac.cc
src/base/file_utils.h
src/base/file_utils_posix.cc
src/base/file_utils.cc
src/base/file_utils_test.cc
src/base/future.h
src/base/future_test.cc
Expand Down Expand Up @@ -2283,7 +2284,10 @@ src/client/clean_command.hh
src/client/command.cc
src/client/command.hh
src/client/command_test.cc
src/client/configuration.cc
src/client/configuration.hh
src/client/configuration.proto
src/client/configuration_test.cc
src/client/driver_command.cc
src/client/driver_command.hh
src/daemon/absorber.cc
Expand Down Expand Up @@ -8735,6 +8739,7 @@ src/third_party/snappy/exported/snappy_unittest.cc
src/third_party/snappy/linux/snappy-stubs-public.h
src/third_party/snappy/mac/snappy-stubs-public.h
src/third_party/snappy/win/snappy-stubs-public.h
tools/clang/main.cc
tools/args_parser/BUILD.gn
tools/args_parser/main.cc
tools/get_report.py
3 changes: 2 additions & 1 deletion src/base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ shared_library("base") {
"file/data.h",
"file/epoll_linux.cc",
"file/epoll_linux.h",
"file/file.cc",
"file/file.h",
"file/file_posix.cc",
"file/handle_posix.cc",
Expand All @@ -42,7 +43,7 @@ shared_library("base") {
"file/pipe_linux.cc",
"file/pipe_mac.cc",
"file_utils.h",
"file_utils_posix.cc",
"file_utils.cc",
"future.h",
"locked_list.h",
"locked_queue.h",
Expand Down
6 changes: 6 additions & 0 deletions src/base/aliases.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
#include STL(unordered_set)
#include STL(vector)

#include STL_EXPERIMENTAL(filesystem)

namespace dist_clang {

namespace base {
Expand Down Expand Up @@ -67,6 +69,10 @@ using Mutex = std::mutex;
template <class U, class V = U>
using Pair = std::pair<U, V>;

using Path = std::experimental::filesystem::path;

using Perms = std::experimental::filesystem::perms;

using Seconds = std::chrono::seconds;

template <class T>
Expand Down
39 changes: 30 additions & 9 deletions src/base/c_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,31 @@
#include <stdlib.h>
#include <string.h>

#include STL_EXPERIMENTAL(filesystem)
#include STL(system_error)

namespace dist_clang {
namespace base {

inline bool ChangeCurrentDir(Immutable path, String* error) {
if (chdir(path.c_str()) == -1) {
GetLastError(error);
inline bool ChangeCurrentDir(const Path& path, String* error) {
std::error_code ec;
std::experimental::filesystem::current_path(path, ec);
if (ec) {
if (error) {
*error = ec.message();
}
return false;
}
return true;
}

inline Immutable GetCurrentDir(String* error) {
UniquePtr<char[]> buf(new char[PATH_MAX]);
if (!getcwd(buf.get(), PATH_MAX)) {
GetLastError(error);
return Immutable();
inline Path GetCurrentDir(String* error) {
std::error_code ec;
const auto& current_dir = std::experimental::filesystem::current_path(ec);
if (ec && error) {
*error = ec.message();
}
return buf;
return current_dir;
}

inline Literal GetEnv(Literal env_name, Literal default_env) {
Expand All @@ -52,5 +59,19 @@ inline void GetLastError(String* error) {
}
}

inline bool SetPermissions(const Path& path, const Perms permissions,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const Perms& ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should fit the machine word, but I'll make it a const ref no difference for me.

String* error) {
std::error_code ec;
std::experimental::filesystem::permissions(path.c_str(), permissions, ec);
if (ec) {
if (error) {
*error = ec.message();
}
return false;
}
return true;
}


} // namespace base
} // namespace dist_clang
11 changes: 6 additions & 5 deletions src/base/c_utils_forward.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,17 @@
namespace dist_clang {
namespace base {

bool ChangeCurrentDir(Immutable path, String* error = nullptr);
String CreateTempFile(String* error = nullptr);
String CreateTempFile(const char suffix[], String* error = nullptr);
Immutable GetCurrentDir(String* error = nullptr);
bool ChangeCurrentDir(const Path& path, String* error = nullptr);
Path CreateTempFile(String* error = nullptr);
Path CreateTempFile(const char suffix[], String* error = nullptr);
Path GetCurrentDir(String* error = nullptr);
Literal GetEnv(Literal env_name, Literal default_env = Literal::empty);
Literal GetHomeDir(String* error = nullptr);
void GetLastError(String* error);
bool GetSelfPath(String& result, String* error = nullptr);
Literal SetEnv(Literal env_name, const String& value, String* error = nullptr);
bool SetPermissions(const String& path, int mask, String* error = nullptr);
bool SetPermissions(const Path& path, const Perms permissions,
String* error = nullptr);

} // namespace base
} // namespace dist_clang
21 changes: 6 additions & 15 deletions src/base/c_utils_posix.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ inline Literal SetEnv(Literal env_name, const String& value, String* error) {
return old_value;
}

inline String CreateTempFile(String* error) {
inline Path CreateTempFile(String* error) {
char buf[] = "/tmp/clangd-XXXXXX.files";
#if defined(OS_LINUX)
const int fd = mkostemps(buf, 6, O_CLOEXEC);
Expand All @@ -35,13 +35,13 @@ inline String CreateTempFile(String* error) {
#endif
if (fd == -1) {
GetLastError(error);
return String();
return Path();
}
close(fd);
return String(buf);
return Path(buf);
}

inline String CreateTempFile(const char suffix[], String* error) {
inline Path CreateTempFile(const char suffix[], String* error) {
const char prefix[] = "/tmp/clangd-XXXXXX";
const size_t prefix_size = sizeof(prefix) - 1;
UniquePtr<char[]> buf(new char[prefix_size + strlen(suffix) + 1]);
Expand All @@ -59,11 +59,10 @@ inline String CreateTempFile(const char suffix[], String* error) {
#endif
if (fd == -1) {
GetLastError(error);
return String();
return Path();
}
close(fd);
const auto result = String(buf.get());
return result;
return Path(buf.get());
}

inline Literal GetHomeDir(String* error) {
Expand Down Expand Up @@ -104,13 +103,5 @@ inline bool GetSelfPath(String& result, String* error) {
return true;
}

inline bool SetPermissions(const String& path, int mask, String* error) {
if (chmod(path.c_str(), mask) == -1) {
GetLastError(error);
return false;
}
return true;
}

} // namespace base
} // namespace dist_clang
1 change: 1 addition & 0 deletions src/base/const_string.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ namespace base {

class Literal {
public:
inline const char* c_str() const { return str_; }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is it used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In ReplaceTildeInPath from file_cache.cc.
GetHomeDir() returns a Literal thats why I should use c_str() to create a Path form it.

inline operator const char*() const { return str_; }
inline size_t size() const { return strlen(str_); }
inline bool operator==(const Literal& other) const {
Expand Down
150 changes: 150 additions & 0 deletions src/base/file/file.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
#include <base/file/file.h>

#include <base/c_utils.h>
#include <base/const_string.h>
#include <base/string_utils.h>

#include STL(cstdio)
#include STL_EXPERIMENTAL(filesystem)
#include STL(system_error)

namespace dist_clang {

namespace {

bool GetStatus(const Path& path,
std::experimental::filesystem::file_status* status,
String* error) {
CHECK(status);
std::error_code ec;
*status = std::experimental::filesystem::status(path, ec);
if (ec) {
if (error) {
*error = ec.message();
}
return false;
}
return true;
}

} // anonymous namespace

namespace base {

// static
bool File::IsFile(const Path& path, String* error) {
std::experimental::filesystem::file_status status;
if (!GetStatus(path, &status, error)) {
return false;
}
return !std::experimental::filesystem::is_directory(status);
}

bool File::Hash(Immutable* output, const List<Literal>& skip_list,
String* error) {
DCHECK(IsValid());

Immutable tmp_output;
if (!Read(&tmp_output, error)) {
return false;
}

for (const char* skip : skip_list) {
if (tmp_output.find(skip) != String::npos) {
if (error) {
error->assign("Skip-list hit: " + String(skip));
}
return false;
}
}

output->assign(base::Hexify(tmp_output.Hash()));
return true;
}

// static
ui64 File::Size(const Path& path, String* error) {
std::error_code ec;
const auto file_size = std::experimental::filesystem::file_size(path, ec);
if (ec) {
if (error) {
*error = ec.message();
}
return 0;
}
return static_cast<ui64>(file_size);
}

// static
bool File::Read(const Path& path, Immutable* output, String* error) {
File file(path);

if (!file.IsValid()) {
file.GetCreationError(error);
return false;
}

return file.Read(output, error);
}

// static
bool File::Exists(const Path& path, String* error) {
std::experimental::filesystem::file_status status;
if (!GetStatus(path, &status, error)) {
return false;
}
return std::experimental::filesystem::exists(status) &&
!std::experimental::filesystem::is_directory(status);
}

// static
bool File::Hash(const Path& path, Immutable* output,
const List<Literal>& skip_list, String* error) {
File file(path);

if (!file.IsValid()) {
file.GetCreationError(error);
return false;
}

return file.Hash(output, skip_list, error);
}

// static
bool File::Copy(const Path& src_path, const Path& dst_path, String* error) {
File src(src_path);

if (!src.IsValid()) {
src.GetCreationError(error);
return false;
}

return src.CopyInto(dst_path, error);
}

// static
bool File::Move(const Path& src, const Path& dst, String* error) {
std::error_code ec;
std::experimental::filesystem::rename(src, dst, ec);
if (ec) {
if (error) {
*error = ec.message();
}
return false;
}
return true;
}

// static
bool File::Delete(const Path& path, String* error) {
if (std::remove(path.c_str())) {
GetLastError(error);
return false;
}

return true;
}

} // namespace base

} // namespace dist_clang
Loading