Client: convert IFileTime to use Path
Benefits of a Path abstraction instead of raw
strings:
- type safety
- no need to convert paths on Windows all the time
- no need to check if paths are absolute
PiperOrigin-RevId: 266312309
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index a13acbd..fd2bb50 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -913,6 +913,8 @@
}
static void MoveFiles(const string &embedded_binaries) {
+ blaze_util::Path embedded_binaries_(embedded_binaries);
+
// Set the timestamps of the extracted files to the future and make sure (or
// at least as sure as we can...) that the files we have written are actually
// on the disk.
@@ -923,9 +925,9 @@
blaze_util::GetAllFilesUnder(embedded_binaries, &extracted_files);
std::unique_ptr<blaze_util::IFileMtime> mtime(blaze_util::CreateFileMtime());
- set<string> synced_directories;
- for (const auto &it : extracted_files) {
- const char *extracted_path = it.c_str();
+ set<blaze_util::Path> synced_directories;
+ for (const auto &f : extracted_files) {
+ blaze_util::Path it(f);
// Set the time to a distantly futuristic value so we can observe tampering.
// Note that keeping a static, deterministic timestamp, such as the default
@@ -935,14 +937,15 @@
// changed. This is essential for the correctness of actions that use
// embedded binaries as artifacts.
if (!mtime->SetToDistantFuture(it)) {
+ string err = GetLastErrorString();
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
- << "failed to set timestamp on '" << extracted_path
- << "': " << GetLastErrorString();
+ << "failed to set timestamp on '" << it.AsPrintablePath()
+ << "': " << err;
}
blaze_util::SyncFile(it);
- string directory = blaze_util::Dirname(extracted_path);
+ blaze_util::Path directory = it.GetParent();
// Now walk up until embedded_binaries and sync every directory in between.
// synced_directories is used to avoid syncing the same directory twice.
@@ -950,16 +953,16 @@
// conditions are not strictly needed, but it makes this loop more robust,
// because otherwise, if due to some glitch, directory was not under
// embedded_binaries, it would get into an infinite loop.
- while (directory != embedded_binaries &&
- synced_directories.count(directory) == 0 && !directory.empty() &&
+ while (directory != embedded_binaries_ &&
+ synced_directories.count(directory) == 0 && !directory.IsEmpty() &&
!blaze_util::IsRootDirectory(directory)) {
blaze_util::SyncFile(directory);
synced_directories.insert(directory);
- directory = blaze_util::Dirname(directory);
+ directory = directory.GetParent();
}
}
- blaze_util::SyncFile(embedded_binaries);
+ blaze_util::SyncFile(embedded_binaries_);
}
@@ -980,8 +983,7 @@
// Work in a temp dir to avoid races.
string tmp_install = startup_options.install_base + ".tmp." +
blaze::GetProcessIdAsString();
- string tmp_binaries =
- blaze_util::JoinPath(tmp_install, "_embedded_binaries");
+ string tmp_binaries = GetEmbeddedBinariesRoot(tmp_install);
ExtractArchiveOrDie(
self_path,
startup_options.product_name,
@@ -1030,13 +1032,14 @@
std::unique_ptr<blaze_util::IFileMtime> mtime(
blaze_util::CreateFileMtime());
- string real_install_dir = blaze_util::JoinPath(
- startup_options.install_base, "_embedded_binaries");
+ blaze_util::Path real_install_dir =
+ blaze_util::Path(startup_options.install_base)
+ .GetRelative("_embedded_binaries");
for (const auto &it : archive_contents) {
- string path = blaze_util::JoinPath(real_install_dir, it);
+ blaze_util::Path path = real_install_dir.GetRelative(it);
if (!mtime->IsUntampered(path)) {
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
- << "corrupt installation: file '" << path
+ << "corrupt installation: file '" << path.AsPrintablePath()
<< "' is missing or modified. Please remove '"
<< startup_options.install_base << "' and try again.";
}
@@ -1202,10 +1205,11 @@
// find install bases that haven't been used for a long time
std::unique_ptr<blaze_util::IFileMtime> mtime(
blaze_util::CreateFileMtime());
- if (!mtime->SetToNow(startup_options.install_base)) {
+ if (!mtime->SetToNow(blaze_util::Path(startup_options.install_base))) {
+ string err = GetLastErrorString();
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
<< "failed to set timestamp on '" << startup_options.install_base
- << "': " << GetLastErrorString();
+ << "': " << err;
}
}
}
diff --git a/src/main/cpp/util/file_platform.h b/src/main/cpp/util/file_platform.h
index aa48029..bf462fd 100644
--- a/src/main/cpp/util/file_platform.h
+++ b/src/main/cpp/util/file_platform.h
@@ -42,17 +42,17 @@
// TODO(laszlocsomor): move this function, and with it the whole IFileMtime
// class into blaze_util_<platform>.cc, because it is Bazel-specific logic,
// not generic file-handling logic.
- virtual bool IsUntampered(const std::string &path) = 0;
+ virtual bool IsUntampered(const Path &path) = 0;
// Sets the mtime of file under `path` to the current time.
// Returns true if the mtime was changed successfully.
- virtual bool SetToNow(const std::string &path) = 0;
+ virtual bool SetToNow(const Path &path) = 0;
// Sets the mtime of file under `path` to the distant future.
// "Distant future" should be on the order of some years into the future, like
// a decade.
// Returns true if the mtime was changed successfully.
- virtual bool SetToDistantFuture(const std::string &path) = 0;
+ virtual bool SetToDistantFuture(const Path &path) = 0;
};
// Creates a platform-specific implementation of `IFileMtime`.
@@ -184,6 +184,7 @@
// Calls fsync() on the file (or directory) specified in 'file_path'.
// pdie() if syncing fails.
void SyncFile(const std::string& path);
+void SyncFile(const Path &path);
// mkdir -p path. All newly created directories use the given mode.
// `mode` should be an octal permission mask, e.g. 0755.
diff --git a/src/main/cpp/util/file_posix.cc b/src/main/cpp/util/file_posix.cc
index 57c3fbe..c225f9c 100644
--- a/src/main/cpp/util/file_posix.cc
+++ b/src/main/cpp/util/file_posix.cc
@@ -358,15 +358,17 @@
close(fd);
}
+void SyncFile(const Path &path) { SyncFile(path.AsNativePath()); }
+
class PosixFileMtime : public IFileMtime {
public:
PosixFileMtime()
: near_future_(GetFuture(9)),
distant_future_({GetFuture(10), GetFuture(10)}) {}
- bool IsUntampered(const string &path) override;
- bool SetToNow(const string &path) override;
- bool SetToDistantFuture(const string &path) override;
+ bool IsUntampered(const Path &path) override;
+ bool SetToNow(const Path &path) override;
+ bool SetToDistantFuture(const Path &path) override;
private:
// 9 years in the future.
@@ -374,14 +376,14 @@
// 10 years in the future.
const struct utimbuf distant_future_;
- static bool Set(const string &path, const struct utimbuf &mtime);
+ static bool Set(const Path &path, const struct utimbuf &mtime);
static time_t GetNow();
static time_t GetFuture(unsigned int years);
};
-bool PosixFileMtime::IsUntampered(const string &path) {
+bool PosixFileMtime::IsUntampered(const Path &path) {
struct stat buf;
- if (stat(path.c_str(), &buf)) {
+ if (stat(path.AsNativePath().c_str(), &buf)) {
return false;
}
@@ -393,18 +395,18 @@
return S_ISDIR(buf.st_mode) || (buf.st_mtime > near_future_);
}
-bool PosixFileMtime::SetToNow(const string &path) {
+bool PosixFileMtime::SetToNow(const Path &path) {
time_t now(GetNow());
struct utimbuf times = {now, now};
return Set(path, times);
}
-bool PosixFileMtime::SetToDistantFuture(const string &path) {
+bool PosixFileMtime::SetToDistantFuture(const Path &path) {
return Set(path, distant_future_);
}
-bool PosixFileMtime::Set(const string &path, const struct utimbuf &mtime) {
- return utime(path.c_str(), &mtime) == 0;
+bool PosixFileMtime::Set(const Path &path, const struct utimbuf &mtime) {
+ return utime(path.AsNativePath().c_str(), &mtime) == 0;
}
time_t PosixFileMtime::GetNow() {
diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc
index faf146d..e14434e 100644
--- a/src/main/cpp/util/file_windows.cc
+++ b/src/main/cpp/util/file_windows.cc
@@ -111,9 +111,9 @@
WindowsFileMtime()
: near_future_(GetFuture(9)), distant_future_(GetFuture(10)) {}
- bool IsUntampered(const string& path) override;
- bool SetToNow(const string& path) override;
- bool SetToDistantFuture(const string& path) override;
+ bool IsUntampered(const Path& path) override;
+ bool SetToNow(const Path& path) override;
+ bool SetToDistantFuture(const Path& path) override;
private:
// 9 years in the future.
@@ -123,32 +123,24 @@
static FILETIME GetNow();
static FILETIME GetFuture(WORD years);
- static bool Set(const string& path, FILETIME time);
+ static bool Set(const Path& path, FILETIME time);
};
-bool WindowsFileMtime::IsUntampered(const string& path) {
- if (path.empty() || IsDevNull(path.c_str())) {
+bool WindowsFileMtime::IsUntampered(const Path& path) {
+ if (path.IsEmpty() || path.IsNull()) {
return false;
}
- wstring wpath;
- string error;
- if (!AsAbsoluteWindowsPath(path, &wpath, &error)) {
- BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
- << "WindowsFileMtime::IsUntampered(" << path
- << "): AsAbsoluteWindowsPath failed: " << error;
- }
-
// Get attributes, to check if the file exists. (It may still be a dangling
// junction.)
- DWORD attrs = GetFileAttributesW(wpath.c_str());
+ DWORD attrs = GetFileAttributesW(path.AsNativePath().c_str());
if (attrs == INVALID_FILE_ATTRIBUTES) {
return false;
}
bool is_directory = attrs & FILE_ATTRIBUTE_DIRECTORY;
AutoHandle handle(CreateFileW(
- /* lpFileName */ wpath.c_str(),
+ /* lpFileName */ path.AsNativePath().c_str(),
/* dwDesiredAccess */ GENERIC_READ,
/* dwShareMode */ FILE_SHARE_READ,
/* lpSecurityAttributes */ NULL,
@@ -181,35 +173,23 @@
}
}
-bool WindowsFileMtime::SetToNow(const string& path) {
+bool WindowsFileMtime::SetToNow(const Path& path) {
return Set(path, GetNow());
}
-bool WindowsFileMtime::SetToDistantFuture(const string& path) {
+bool WindowsFileMtime::SetToDistantFuture(const Path& path) {
return Set(path, distant_future_);
}
-bool WindowsFileMtime::Set(const string& path, FILETIME time) {
- if (path.empty()) {
- return false;
- }
- wstring wpath;
- string error;
- if (!AsAbsoluteWindowsPath(path, &wpath, &error)) {
- BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
- << "WindowsFileMtime::Set(" << path
- << "): AsAbsoluteWindowsPath failed: " << error;
- return false;
- }
-
+bool WindowsFileMtime::Set(const Path& path, FILETIME time) {
AutoHandle handle(::CreateFileW(
- /* lpFileName */ wpath.c_str(),
+ /* lpFileName */ path.AsNativePath().c_str(),
/* dwDesiredAccess */ FILE_WRITE_ATTRIBUTES,
/* dwShareMode */ FILE_SHARE_READ,
/* lpSecurityAttributes */ NULL,
/* dwCreationDisposition */ OPEN_EXISTING,
/* dwFlagsAndAttributes */
- IsDirectoryW(wpath)
+ IsDirectoryW(path.AsNativePath())
? (FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS)
: FILE_ATTRIBUTE_NORMAL,
/* hTemplateFile */ NULL));
@@ -610,6 +590,8 @@
// fsync always fails on Cygwin with "Permission denied" for some reason.
}
+void SyncFile(const Path& path) {}
+
bool MakeDirectoriesW(const wstring& path, unsigned int mode) {
if (path.empty()) {
return false;
diff --git a/src/main/cpp/util/path_platform.h b/src/main/cpp/util/path_platform.h
index bc23192..bfa448b 100644
--- a/src/main/cpp/util/path_platform.h
+++ b/src/main/cpp/util/path_platform.h
@@ -27,13 +27,14 @@
explicit Path(const std::string &path);
bool operator==(const Path &o) const { return path_ == o.path_; }
bool operator!=(const Path &o) const { return path_ != o.path_; }
+ bool operator<(const Path &o) const { return path_ < o.path_; }
bool IsEmpty() const { return path_.empty(); }
bool IsNull() const;
bool Contains(const char c) const;
bool Contains(const std::string &s) const;
Path GetRelative(const std::string &r) const;
- // Returns the canonical form (like realpath(2)) of this path.
+ // Returns the canonical form (like realpath(1)) of this path.
// All symlinks in the path are resolved.
// If canonicalization fails, returns an empty Path.
Path Canonicalize() const;
@@ -95,6 +96,7 @@
// Returns true if `path` is the root directory or a Windows drive root.
bool IsRootDirectory(const std::string &path);
+bool IsRootDirectory(const Path &path);
// Returns true if `path` is absolute.
bool IsAbsolute(const std::string &path);
diff --git a/src/main/cpp/util/path_posix.cc b/src/main/cpp/util/path_posix.cc
index 9c44d0e..a5be228 100644
--- a/src/main/cpp/util/path_posix.cc
+++ b/src/main/cpp/util/path_posix.cc
@@ -56,6 +56,10 @@
return path.size() == 1 && path[0] == '/';
}
+bool IsRootDirectory(const Path &path) {
+ return IsRootDirectory(path.AsNativePath());
+}
+
bool IsAbsolute(const std::string &path) {
return !path.empty() && path[0] == '/';
}
diff --git a/src/main/cpp/util/path_windows.cc b/src/main/cpp/util/path_windows.cc
index 7ebf2a8..a9f9780 100644
--- a/src/main/cpp/util/path_windows.cc
+++ b/src/main/cpp/util/path_windows.cc
@@ -449,6 +449,10 @@
return IsRootOrAbsolute(path, true);
}
+bool IsRootDirectory(const Path& path) {
+ return IsRootOrAbsolute(path.AsNativePath(), true);
+}
+
bool IsAbsolute(const std::string& path) {
return IsRootOrAbsolute(path, false);
}
diff --git a/src/test/cpp/util/file_test.cc b/src/test/cpp/util/file_test.cc
index eeb1e2c..178ff48 100644
--- a/src/test/cpp/util/file_test.cc
+++ b/src/test/cpp/util/file_test.cc
@@ -145,14 +145,14 @@
const char* tempdir_cstr = getenv("TEST_TMPDIR");
ASSERT_NE(tempdir_cstr, nullptr);
ASSERT_NE(tempdir_cstr[0], 0);
- string tempdir(tempdir_cstr);
+ Path tempdir(tempdir_cstr);
std::unique_ptr<IFileMtime> mtime(CreateFileMtime());
// Assert that a directory is always untampered with. (We do
// not care about directories' mtimes.)
ASSERT_TRUE(mtime->IsUntampered(tempdir));
// Create a new file, assert its mtime is not in the future.
- string file(JoinPath(tempdir, "foo.txt"));
+ Path file = tempdir.GetRelative("foo.txt");
ASSERT_TRUE(WriteFile("hello", 5, file));
ASSERT_FALSE(mtime->IsUntampered(file));
// Set the file's mtime to the future, assert that it's so.
diff --git a/src/test/cpp/util/file_windows_test.cc b/src/test/cpp/util/file_windows_test.cc
index a86d664..83ae4f6 100644
--- a/src/test/cpp/util/file_windows_test.cc
+++ b/src/test/cpp/util/file_windows_test.cc
@@ -320,24 +320,23 @@
const char* tempdir_cstr = getenv("TEST_TMPDIR");
ASSERT_NE(tempdir_cstr, nullptr);
ASSERT_NE(tempdir_cstr[0], 0);
- string tempdir(tempdir_cstr);
+ Path tempdir(tempdir_cstr);
- string target(JoinPath(tempdir, "target" TOSTRING(__LINE__)));
- wstring wtarget;
- EXPECT_TRUE(AsWindowsPath(target, &wtarget, nullptr));
- EXPECT_TRUE(CreateDirectoryW(wtarget.c_str(), NULL));
+ Path target = tempdir.GetRelative("target" TOSTRING(__LINE__));
+ EXPECT_TRUE(CreateDirectoryW(target.AsNativePath().c_str(), NULL));
std::unique_ptr<IFileMtime> mtime(CreateFileMtime());
// Assert that a directory is always a good embedded binary. (We do not care
// about directories' mtimes.)
ASSERT_TRUE(mtime.get()->IsUntampered(target));
// Assert that junctions whose target exists are "good" embedded binaries.
- string sym(JoinPath(tempdir, "junc" TOSTRING(__LINE__)));
- CREATE_JUNCTION(sym, target);
+ Path sym = tempdir.GetRelative("junc" TOSTRING(__LINE__));
+ EXPECT_EQ(CreateJunction(sym.AsNativePath(), target.AsNativePath(), nullptr),
+ CreateJunctionResult::kSuccess);
ASSERT_TRUE(mtime.get()->IsUntampered(sym));
// Assert that checking fails for non-existent directories and dangling
// junctions.
- EXPECT_TRUE(RemoveDirectoryW(wtarget.c_str()));
+ EXPECT_TRUE(RemoveDirectoryW(target.AsNativePath().c_str()));
ASSERT_FALSE(mtime.get()->IsUntampered(target));
ASSERT_FALSE(mtime.get()->IsUntampered(sym));
}
