Move tmpdir creation logic into platform-specific files
Makes the logic testable, removes a hard to follow ifdef, and lets us get rid
of some redundant logic.
PiperOrigin-RevId: 303382036
diff --git a/src/main/cpp/archive_utils.cc b/src/main/cpp/archive_utils.cc
index aae4957..ab6445f 100644
--- a/src/main/cpp/archive_utils.cc
+++ b/src/main/cpp/archive_utils.cc
@@ -114,10 +114,10 @@
if (dumper == nullptr) {
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) << error;
}
- if (!blaze_util::MakeDirectories(output_dir, 0777)) {
+
+ if (!blaze_util::PathExists(output_dir)) {
BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR)
- << "couldn't create '" << output_dir
- << "': " << blaze_util::GetLastErrorString();
+ << "Archive output directory didn't exist: " << output_dir;
}
BAZEL_LOG(USER) << "Extracting " << product_name << " installation...";
diff --git a/src/main/cpp/archive_utils.h b/src/main/cpp/archive_utils.h
index 9410781..f970080 100644
--- a/src/main/cpp/archive_utils.h
+++ b/src/main/cpp/archive_utils.h
@@ -27,6 +27,7 @@
std::string *install_md5);
// Extracts the embedded data files in `archive_path` into `output_dir`.
+// It's expected that `output_dir` already exists and that it's a directory.
// Fails if `expected_install_md5` doesn't match that contained in the archive,
// as this could indicate that the contents has unexpectedly changed.
void ExtractArchiveOrDie(const std::string &archive_path,
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index 63007b6..4b0b5fb 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -965,29 +965,7 @@
if (!blaze_util::PathExists(install_base)) {
uint64_t st = GetMillisecondsMonotonic();
// Work in a temp dir to avoid races.
-#if defined(_WIN32)
- string tmp_install = install_base + ".tmp." + blaze::GetProcessIdAsString();
-#else
- // On Linux, we can't use the PID as a unique identifier, because Bazel
- // might run in a PID namespace and then all Bazel clients have the same
- // PID, so we use mkdtemp instead.
- if (!blaze_util::MakeDirectories(blaze_util::Dirname(install_base), 0777)) {
- BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR)
- << "couldn't create '" << blaze_util::Dirname(install_base)
- << "': " << blaze_util::GetLastErrorString();
- }
- std::string tmp_install(install_base + ".tmp.XXXXXX");
- if (mkdtemp(&tmp_install[0]) == nullptr) {
- std::string err = GetLastErrorString();
- BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
- << "could not create temporary directory to extract install base"
- << " (" << err << ")";
- }
- // There's no better way to get the current umask than to set and reset it.
- const mode_t um = umask(0);
- umask(um);
- chmod(tmp_install.c_str(), 0777 & ~um);
-#endif
+ string tmp_install = blaze_util::CreateTempDir(install_base + ".tmp.");
ExtractArchiveOrDie(self_path, startup_options.product_name,
expected_install_md5, tmp_install);
BlessFiles(tmp_install);
diff --git a/src/main/cpp/util/file_platform.h b/src/main/cpp/util/file_platform.h
index a918dce..4e8e254 100644
--- a/src/main/cpp/util/file_platform.h
+++ b/src/main/cpp/util/file_platform.h
@@ -192,6 +192,10 @@
bool MakeDirectories(const std::string &path, unsigned int mode);
bool MakeDirectories(const Path &path, unsigned int mode);
+// Creates a directory starting with prefix for temporary usage. The directory
+// name is guaranteed to be at least unique to this process.
+std::string CreateTempDir(const std::string &prefix);
+
// Removes the specified path or directory, and in the latter case, all of its
// contents. Returns true iff the path doesn't exists when the method completes
// (including if the path didn't exist to begin with). Does not follow symlinks.
diff --git a/src/main/cpp/util/file_posix.cc b/src/main/cpp/util/file_posix.cc
index 84559b9..4a636e8 100644
--- a/src/main/cpp/util/file_posix.cc
+++ b/src/main/cpp/util/file_posix.cc
@@ -124,6 +124,33 @@
return stat_succeeded;
}
+
+string CreateTempDir(const std::string &prefix) {
+ std::string parent = Dirname(prefix);
+ // Need parent to exist first.
+ // TODO(b/150220877): Avoid trying to recreate a dir that already exists.
+ if (!blaze_util::MakeDirectories(parent, 0777)) {
+ BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR)
+ << "couldn't create '" << parent << "': "
+ << blaze_util::GetLastErrorString();
+ }
+
+ std::string result(prefix + "XXXXXX");
+ if (mkdtemp(&result[0]) == nullptr) {
+ std::string err = GetLastErrorString();
+ BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
+ << "could not create temporary directory to extract install base"
+ << " (" << err << ")";
+ }
+
+ // There's no better way to get the current umask than to set and reset it.
+ const mode_t um = umask(0);
+ umask(um);
+ chmod(result.c_str(), 0777 & ~um);
+
+ return result;
+}
+
static bool RemoveDirRecursively(const std::string &path) {
DIR *dir;
if ((dir = opendir(path.c_str())) == NULL) {
diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc
index e25dab4..533ccfb 100644
--- a/src/main/cpp/util/file_windows.cc
+++ b/src/main/cpp/util/file_windows.cc
@@ -646,6 +646,15 @@
return MakeDirectoriesW(path.AsNativePath(), mode);
}
+string CreateTempDir(const std::string &prefix) {
+ string result = prefix + blaze_util::ToString(GetCurrentProcessId());
+ if (!blaze_util::MakeDirectories(result, 0777)) {
+ BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR)
+ << "couldn't create '" << result
+ << "': " << blaze_util::GetLastErrorString();
+ }
+ return result;
+}
static bool RemoveContents(wstring path) {
static const wstring kDot(L".");
diff --git a/src/test/cpp/util/file_test.cc b/src/test/cpp/util/file_test.cc
index 869a719..e67622b 100644
--- a/src/test/cpp/util/file_test.cc
+++ b/src/test/cpp/util/file_test.cc
@@ -175,6 +175,28 @@
ASSERT_FALSE(mtime->IsUntampered(file));
}
+TEST(FileTest, TestCreateTempDir) {
+ const char* tempdir_cstr = getenv("TEST_TMPDIR");
+ EXPECT_NE(tempdir_cstr, nullptr);
+ EXPECT_NE(tempdir_cstr[0], 0);
+ string tempdir(tempdir_cstr);
+ string tmpdir(tempdir_cstr);
+
+ string prefix_in_existing_dir(JoinPath(tempdir, "foo."));
+ string result_in_existing_dir(CreateTempDir(prefix_in_existing_dir));
+ ASSERT_NE(result_in_existing_dir, prefix_in_existing_dir);
+ ASSERT_EQ(0, result_in_existing_dir.find(prefix_in_existing_dir));
+ EXPECT_TRUE(PathExists(result_in_existing_dir));
+
+ string base_dir(JoinPath(tempdir, "doesntexistyet"));
+ ASSERT_FALSE(PathExists(base_dir));
+ string prefix_in_new_dir(JoinPath(base_dir, "foo."));
+ string result_in_new_dir(CreateTempDir(prefix_in_new_dir));
+ ASSERT_NE(result_in_new_dir, prefix_in_new_dir);
+ ASSERT_EQ(0, result_in_new_dir.find(prefix_in_new_dir));
+ EXPECT_TRUE(PathExists(result_in_new_dir));
+}
+
TEST(FileTest, TestRenameDirectory) {
const char* tempdir_cstr = getenv("TEST_TMPDIR");
EXPECT_NE(tempdir_cstr, nullptr);
