diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc index 8493b5ec39b495..b9665e06730fee 100644 --- a/src/main/cpp/blaze.cc +++ b/src/main/cpp/blaze.cc @@ -998,6 +998,7 @@ static DurationMillis ExtractData(const string &self_path, result == blaze_util::kRenameDirectoryFailureNotEmpty) { // If renaming fails because the directory already exists and is not // empty, then we assume another good installation snuck in before us. + blaze_util::RemoveRecursively(tmp_install); break; } else { // Otherwise the install directory may still be scanned by the antivirus @@ -1013,6 +1014,7 @@ static DurationMillis ExtractData(const string &self_path, // Give up renaming after 120 failed attempts / 2 minutes. if (attempts == 120) { + blaze_util::RemoveRecursively(tmp_install); BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) << "install base directory '" << tmp_install << "' could not be renamed into place: " << GetLastErrorString(); diff --git a/src/main/cpp/util/file_platform.h b/src/main/cpp/util/file_platform.h index bf462fd7bc8430..a918dcea7f0e1d 100644 --- a/src/main/cpp/util/file_platform.h +++ b/src/main/cpp/util/file_platform.h @@ -192,6 +192,11 @@ void SyncFile(const Path &path); bool MakeDirectories(const std::string &path, unsigned int mode); bool MakeDirectories(const Path &path, unsigned int mode); +// 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. +bool RemoveRecursively(const std::string &path); + // Returns the current working directory. // The path is platform-specific (e.g. Windows path of Windows) and absolute. std::string GetCwd(); diff --git a/src/main/cpp/util/file_posix.cc b/src/main/cpp/util/file_posix.cc index c225f9ca90d6a5..84559b976dc58e 100644 --- a/src/main/cpp/util/file_posix.cc +++ b/src/main/cpp/util/file_posix.cc @@ -124,6 +124,45 @@ static bool MakeDirectories(const string &path, mode_t mode, bool childmost) { return stat_succeeded; } +static bool RemoveDirRecursively(const std::string &path) { + DIR *dir; + if ((dir = opendir(path.c_str())) == NULL) { + return false; + } + + struct dirent *ent; + while ((ent = readdir(dir)) != NULL) { + if (!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, "..")) { + continue; + } + + if (!RemoveRecursively(blaze_util::JoinPath(path, ent->d_name))) { + closedir(dir); + return false; + } + } + + if (closedir(dir) != 0) { + return false; + } + + return rmdir(path.c_str()) == 0; +} + +bool RemoveRecursively(const std::string &path) { + struct stat stat_buf; + if (lstat(path.c_str(), &stat_buf) == -1) { + // Non-existent is good enough. + return errno == ENOENT; + } + + if (S_ISDIR(stat_buf.st_mode) && !S_ISLNK(stat_buf.st_mode)) { + return RemoveDirRecursively(path); + } else { + return UnlinkPath(path); + } +} + class PosixPipe : public IPipe { public: PosixPipe(int recv_socket, int send_socket) diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc index 5dc1c5aa6dbda9..475884997c48e8 100644 --- a/src/main/cpp/util/file_windows.cc +++ b/src/main/cpp/util/file_windows.cc @@ -646,6 +646,11 @@ bool MakeDirectories(const Path& path, unsigned int mode) { return MakeDirectoriesW(path.AsNativePath(), mode); } +bool RemoveRecursively(const string &path) { + // TODO(bazel-team): Implement this. + return false; +} + static inline void ToLowerW(WCHAR* p) { while (*p) { *p++ = towlower(*p); diff --git a/src/test/cpp/util/file_posix_test.cc b/src/test/cpp/util/file_posix_test.cc index 75026af6b310e1..be5864f48b63f4 100644 --- a/src/test/cpp/util/file_posix_test.cc +++ b/src/test/cpp/util/file_posix_test.cc @@ -322,4 +322,67 @@ TEST(FilePosixTest, ForEachDirectoryEntry) { rmdir(root.c_str()); } +TEST(FileTest, TestRemoveRecursively) { + const char* tempdir_cstr = getenv("TEST_TMPDIR"); + ASSERT_NE(tempdir_cstr, nullptr); + string tempdir(tempdir_cstr); + ASSERT_TRUE(PathExists(tempdir)); + + string non_existent_dir(JoinPath(tempdir, "test_rmr_non_existent")); + EXPECT_TRUE(RemoveRecursively(non_existent_dir)); + EXPECT_FALSE(PathExists(non_existent_dir)); + + string empty_dir(JoinPath(tempdir, "test_rmr_empty_dir")); + EXPECT_TRUE(MakeDirectories(empty_dir, 0700)); + EXPECT_TRUE(RemoveRecursively(empty_dir)); + EXPECT_FALSE(PathExists(empty_dir)); + + string dir_with_content(JoinPath(tempdir, "test_rmr_dir_w_content")); + EXPECT_TRUE(MakeDirectories(dir_with_content, 0700)); + EXPECT_TRUE(WriteFile("junkdata", 8, JoinPath(dir_with_content, "file"))); + EXPECT_TRUE(MakeDirectories(JoinPath(dir_with_content, "dir"), 0700)); + EXPECT_TRUE(RemoveRecursively(dir_with_content)); + EXPECT_FALSE(PathExists(dir_with_content)); + + string regular_file(JoinPath(tempdir, "test_rmr_regular_file")); + EXPECT_TRUE(WriteFile("junkdata", 8, regular_file)); + EXPECT_TRUE(RemoveRecursively(regular_file)); + EXPECT_FALSE(PathExists(regular_file)); + + string unwritable_dir(JoinPath(tempdir, "test_rmr_unwritable")); + EXPECT_TRUE(MakeDirectories(unwritable_dir, 0700)); + EXPECT_TRUE(WriteFile("junkdata", 8, JoinPath(unwritable_dir, "file"))); + ASSERT_EQ(0, chmod(unwritable_dir.c_str(), 0500)); + EXPECT_FALSE(RemoveRecursively(unwritable_dir)); + + string symlink_target_dir(JoinPath(tempdir, "test_rmr_symlink_target_dir")); + EXPECT_TRUE(MakeDirectories(symlink_target_dir, 0700)); + string symlink_target_dir_file(JoinPath(symlink_target_dir, "file")); + EXPECT_TRUE(WriteFile("junkdata", 8, symlink_target_dir_file)); + string symlink_dir(JoinPath(tempdir, "test_rmr_symlink_dir")); + EXPECT_EQ(0, symlink(symlink_target_dir.c_str(), symlink_dir.c_str())); + EXPECT_TRUE(RemoveRecursively(symlink_dir)); + EXPECT_FALSE(PathExists(symlink_dir)); + EXPECT_TRUE(PathExists(symlink_target_dir)); + EXPECT_TRUE(PathExists(symlink_target_dir_file)); + + string dir_with_symlinks(JoinPath(tempdir, "test_rmr_dir_w_symlinks")); + EXPECT_TRUE(MakeDirectories(dir_with_symlinks, 0700)); + string file_symlink_target(JoinPath(tempdir, "test_rmr_dir_w_symlinks_file")); + EXPECT_TRUE(WriteFile("junkdata", 8, file_symlink_target)); + EXPECT_EQ(0, symlink(file_symlink_target.c_str(), + JoinPath(dir_with_symlinks, "file").c_str())); + string dir_symlink_target(JoinPath(tempdir, "test_rmr_dir_w_symlinks_dir")); + EXPECT_TRUE(MakeDirectories(dir_symlink_target, 0700)); + string dir_symlink_target_file(JoinPath(dir_symlink_target, "file")); + EXPECT_TRUE(WriteFile("junkdata", 8, dir_symlink_target_file)); + EXPECT_EQ(0, symlink(dir_symlink_target.c_str(), + JoinPath(dir_with_symlinks, "dir").c_str())); + EXPECT_TRUE(RemoveRecursively(dir_with_symlinks)); + EXPECT_FALSE(PathExists(dir_with_symlinks)); + EXPECT_TRUE(PathExists(dir_symlink_target)); + EXPECT_TRUE(PathExists(dir_symlink_target_file)); + EXPECT_TRUE(PathExists(file_symlink_target)); +} + } // namespace blaze_util diff --git a/src/test/shell/integration/client_test.sh b/src/test/shell/integration/client_test.sh index febb2d29256778..b0330621589ff8 100755 --- a/src/test/shell/integration/client_test.sh +++ b/src/test/shell/integration/client_test.sh @@ -153,6 +153,20 @@ function test_nonwritable_output_base() { expect_log "FATAL.* Output base directory '/' must be readable and writable." } +function test_install_base_races_dont_leave_temp_files() { + declare -a client_pids + for i in {1..3}; do + bazel --install_base="$TEST_TMPDIR/race/install" \ + --output_base="$TEST_TMPDIR/out$i" info install_base & + client_pids+=($!) + done + for pid in "${client_pids[@]}"; do + wait $pid + done + # Expect "install" to be the only thing in the "race" directory. + assert_equals "install" "$(ls "$TEST_TMPDIR/race/")" +} + function test_no_arguments() { bazel >&$TEST_log || fail "Expected zero exit" expect_log "Usage: b\\(laze\\|azel\\)"