Automated rollback of commit e9a908560133770614eca89ef64681bdf3f04b3e.
*** Reason for rollback ***
TAP has detected 2 or more targets failed to build at https://github.com/bazelbuild/bazel/commit/e9a908560133770614eca89ef64681bdf3f04b3e.
TO ROLLFORWARD (without additional approval): Use[]
To see all broken targets visit []
To prevent noise from flakes, TAP double-checked the following target fails to build:
[]
but used to build fine:
[]
Questions? Comments? See the URL:[]
*** Original change description ***
Bazel client: fix reporting of ignored RC files
Compute the difference between the read RC files
and the ignored ones based on their canonical
paths, not on the textual value of the path.
This aids for proper reporting of ignored RC
files, while reporting them with a path familiar
to the user (e.g. paths the user passed with flags
rather than the potentially different canonical
version of the same path).
Also: respect $HOME on Windows wh...
***
PiperOrigin-RevId: 225790787
diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc
index b35af9b..d8ec359 100644
--- a/src/main/cpp/blaze_util.cc
+++ b/src/main/cpp/blaze_util.cc
@@ -181,8 +181,6 @@
}
}
-bool IsRunningWithinTest() { return !GetEnv("TEST_TMPDIR").empty(); }
-
void WithEnvVars::SetEnvVars(const map<string, EnvVarValue>& vars) {
for (const auto& var : vars) {
switch (var.second.action) {
diff --git a/src/main/cpp/blaze_util.h b/src/main/cpp/blaze_util.h
index b0b5538..b9f5a51 100644
--- a/src/main/cpp/blaze_util.h
+++ b/src/main/cpp/blaze_util.h
@@ -104,10 +104,6 @@
// Revisit once client logging is fixed (b/32939567).
void SetDebugLog(bool enabled);
-// Returns true if this Bazel instance is running inside of a Bazel test.
-// This method observes the TEST_TMPDIR envvar.
-bool IsRunningWithinTest();
-
// What WithEnvVar should do with an environment variable
enum EnvVarAction { UNSET, SET };
diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc
index 1483dfb..3319faf 100644
--- a/src/main/cpp/blaze_util_windows.cc
+++ b/src/main/cpp/blaze_util_windows.cc
@@ -406,12 +406,6 @@
}
string GetHomeDir() {
- if (IsRunningWithinTest()) {
- // Bazel is running inside of a test. Respect $HOME that the test setup has
- // set instead of using the actual home directory of the current user.
- return GetEnv("HOME");
- }
-
PWSTR wpath;
// Look up the user's home directory. The default value of "FOLDERID_Profile"
// is the same as %USERPROFILE%, but it does not require the envvar to be set.
diff --git a/src/main/cpp/option_processor-internal.h b/src/main/cpp/option_processor-internal.h
index e1b9e7d..b25db9c 100644
--- a/src/main/cpp/option_processor-internal.h
+++ b/src/main/cpp/option_processor-internal.h
@@ -26,9 +26,7 @@
namespace internal {
// Returns the deduped set of bazelrc paths (with respect to its canonical form)
-// preserving the original order.
-// All paths in the result were verified to exist (otherwise their canonical
-// form couldn't have been computed). The paths that cannot be resolved are
+// preserving the original order. The paths that cannot be resolved are
// omitted.
std::vector<std::string> DedupeBlazercPaths(
const std::vector<std::string>& paths);
diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc
index 181afd2..1298ea1 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -185,22 +185,18 @@
internal::FindRcAlongsideBinary(cwd, path_to_binary);
candidate_bazelrc_paths = {workspace_rc, binary_rc, system_bazelrc_path};
}
+ const std::vector<std::string> deduped_blazerc_paths =
+ internal::DedupeBlazercPaths(candidate_bazelrc_paths);
+ std::set<std::string> old_rc_paths(deduped_blazerc_paths.begin(),
+ deduped_blazerc_paths.end());
string user_bazelrc_path = internal::FindLegacyUserBazelrc(
SearchUnaryOption(startup_args, "--bazelrc"), workspace);
if (!user_bazelrc_path.empty()) {
- candidate_bazelrc_paths.push_back(user_bazelrc_path);
+ old_rc_paths.insert(user_bazelrc_path);
}
- // DedupeBlazercPaths returns paths whose canonical path could be computed,
- // therefore these paths must exist.
- const std::vector<std::string> deduped_existing_blazerc_paths =
- internal::DedupeBlazercPaths(candidate_bazelrc_paths);
- return std::set<std::string>(deduped_existing_blazerc_paths.begin(),
- deduped_existing_blazerc_paths.end());
+ return old_rc_paths;
}
-// Deduplicates the given paths based on their canonical form.
-// Computes the canonical form using blaze_util::MakeCanonical.
-// Returns the unique paths in their original form (not the canonical one).
std::vector<std::string> DedupeBlazercPaths(
const std::vector<std::string>& paths) {
std::set<std::string> canonical_paths;
@@ -295,20 +291,6 @@
}
}
-std::vector<std::string> GetLostFiles(
- const std::set<std::string>& old_files,
- const std::set<std::string>& read_files_canon) {
- std::vector<std::string> result;
- for (const auto& old : old_files) {
- std::string old_canon = blaze_util::MakeCanonical(old.c_str());
- if (!old_canon.empty() &&
- read_files_canon.find(old_canon) == read_files_canon.end()) {
- result.push_back(old);
- }
- }
- return result;
-}
-
} // namespace internal
// TODO(#4502) Consider simplifying result_rc_files to a vector of RcFiles, no
@@ -384,7 +366,7 @@
// that don't point to real files.
rc_files = internal::DedupeBlazercPaths(rc_files);
- std::set<std::string> read_files_canonical_paths;
+ std::set<std::string> read_files;
// Parse these potential files, in priority order;
for (const std::string& top_level_bazelrc_path : rc_files) {
std::unique_ptr<RcFile> parsed_rc;
@@ -395,10 +377,9 @@
}
// Check that none of the rc files loaded this time are duplicate.
- const std::deque<std::string>& sources =
- parsed_rc->canonical_source_paths();
- internal::WarnAboutDuplicateRcFiles(read_files_canonical_paths, sources);
- read_files_canonical_paths.insert(sources.begin(), sources.end());
+ const std::deque<std::string>& sources = parsed_rc->sources();
+ internal::WarnAboutDuplicateRcFiles(read_files, sources);
+ read_files.insert(sources.begin(), sources.end());
result_rc_files->push_back(std::move(parsed_rc));
}
@@ -413,8 +394,16 @@
workspace_layout, workspace, cwd, cmd_line->path_to_binary,
cmd_line->startup_args, internal::FindSystemWideRc(system_bazelrc_path_));
- std::vector<std::string> lost_files =
- internal::GetLostFiles(old_files, read_files_canonical_paths);
+ // std::vector<std::string> old_files = internal::GetOldRcPathsInOrder(
+ // workspace_layout, workspace, cwd, cmd_line->path_to_binary,
+ // cmd_line->startup_args);
+ //
+ // std::sort(old_files.begin(), old_files.end());
+ std::vector<std::string> lost_files(old_files.size());
+ std::vector<std::string>::iterator end_iter = std::set_difference(
+ old_files.begin(), old_files.end(), read_files.begin(), read_files.end(),
+ lost_files.begin());
+ lost_files.resize(end_iter - lost_files.begin());
if (!lost_files.empty()) {
std::string joined_lost_rcs;
blaze_util::JoinStrings(lost_files, '\n', &joined_lost_rcs);
@@ -636,7 +625,7 @@
int cur_index = 1;
std::map<std::string, int> rcfile_indexes;
for (const auto& blazerc : blazercs) {
- for (const std::string& source_path : blazerc->canonical_source_paths()) {
+ for (const std::string& source_path : blazerc->sources()) {
// Deduplicate the rc_source list because the same file might be included
// from multiple places.
if (rcfile_indexes.find(source_path) != rcfile_indexes.end()) continue;
diff --git a/src/main/cpp/rc_file.cc b/src/main/cpp/rc_file.cc
index 1a0f842..e356ca6 100644
--- a/src/main/cpp/rc_file.cc
+++ b/src/main/cpp/rc_file.cc
@@ -63,10 +63,10 @@
const std::string canonical_filename =
blaze_util::MakeCanonical(filename.c_str());
- canonical_rcfile_paths_.push_back(canonical_filename);
- // Keep a pointer to the canonical_filename string in canonical_rcfile_paths_
- // for the RcOptions.
- string* filename_ptr = &canonical_rcfile_paths_.back();
+ rcfile_paths_.push_back(canonical_filename);
+ // Keep a pointer to the canonical_filename string in rcfile_paths_ for the
+ // RcOptions.
+ string* filename_ptr = &rcfile_paths_.back();
// A '\' at the end of a line continues the line.
blaze_util::Replace("\\\r\n", "", &contents);
diff --git a/src/main/cpp/rc_file.h b/src/main/cpp/rc_file.h
index 5f1eee7..0d462dc 100644
--- a/src/main/cpp/rc_file.h
+++ b/src/main/cpp/rc_file.h
@@ -42,9 +42,7 @@
std::string workspace, ParseError* error, std::string* error_text);
// Returns all relevant rc sources for this file (including itself).
- const std::deque<std::string>& canonical_source_paths() const {
- return canonical_rcfile_paths_;
- }
+ const std::deque<std::string>& sources() const { return rcfile_paths_; }
// Command -> all options for that command (in order of appearance).
using OptionMap = std::unordered_map<std::string, std::vector<RcOption>>;
@@ -70,12 +68,9 @@
const std::string workspace_;
// Full closure of rcfile paths imported from this file (including itself).
- // These are all canonical paths, created with blaze_util::MakeCanonical.
- // This also means all of these paths should exist.
- //
// The RcOption structs point to the strings in here so they need to be stored
// in a container that offers stable pointers, like a deque (and not vector).
- std::deque<std::string> canonical_rcfile_paths_;
+ std::deque<std::string> rcfile_paths_;
// All options parsed from the file.
OptionMap options_;
};
diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc
index c343db9..1267384 100644
--- a/src/main/cpp/startup_options.cc
+++ b/src/main/cpp/startup_options.cc
@@ -98,7 +98,8 @@
idle_server_tasks(true),
original_startup_options_(std::vector<RcStartupFlag>()),
unlimit_coredumps(false) {
- if (blaze::IsRunningWithinTest()) {
+ bool testing = !blaze::GetEnv("TEST_TMPDIR").empty();
+ if (testing) {
output_root = blaze_util::MakeAbsolute(blaze::GetEnv("TEST_TMPDIR"));
max_idle_secs = 15;
BAZEL_LOG(USER) << "$TEST_TMPDIR defined: output root default is '"
diff --git a/src/test/cpp/rc_file_test.cc b/src/test/cpp/rc_file_test.cc
index 25baaaa..01a4d5a 100644
--- a/src/test/cpp/rc_file_test.cc
+++ b/src/test/cpp/rc_file_test.cc
@@ -173,10 +173,8 @@
ASSERT_EQ(2, parsed_rcs.size());
const std::deque<std::string> expected_system_rc_que = {system_rc};
const std::deque<std::string> expected_workspace_rc_que = {workspace_rc};
- EXPECT_EQ(expected_system_rc_que,
- parsed_rcs[0].get()->canonical_source_paths());
- EXPECT_EQ(expected_workspace_rc_que,
- parsed_rcs[1].get()->canonical_source_paths());
+ EXPECT_EQ(expected_system_rc_que, parsed_rcs[0].get()->sources());
+ EXPECT_EQ(expected_workspace_rc_que, parsed_rcs[1].get()->sources());
}
TEST_F(GetRcFileTest, GetRcFilesRespectsNoSystemRc) {
@@ -197,8 +195,7 @@
ASSERT_EQ(1, parsed_rcs.size());
const std::deque<std::string> expected_workspace_rc_que = {workspace_rc};
- EXPECT_EQ(expected_workspace_rc_que,
- parsed_rcs[0].get()->canonical_source_paths());
+ EXPECT_EQ(expected_workspace_rc_que, parsed_rcs[0].get()->sources());
}
TEST_F(GetRcFileTest, GetRcFilesRespectsNoWorkspaceRc) {
@@ -219,8 +216,7 @@
ASSERT_EQ(1, parsed_rcs.size());
const std::deque<std::string> expected_system_rc_que = {system_rc};
- EXPECT_EQ(expected_system_rc_que,
- parsed_rcs[0].get()->canonical_source_paths());
+ EXPECT_EQ(expected_system_rc_que, parsed_rcs[0].get()->sources());
}
TEST_F(GetRcFileTest, GetRcFilesRespectsNoWorkspaceRcAndNoSystemCombined) {
@@ -321,9 +317,8 @@
// Because of the variety of path representations in windows, this
// equality test does not attempt to check the entire path.
ASSERT_EQ(1, parsed_rcs.size());
- ASSERT_EQ(1, parsed_rcs[0].get()->canonical_source_paths().size());
- EXPECT_THAT(parsed_rcs[0].get()->canonical_source_paths().front(),
- HasSubstr("mybazelrc"));
+ ASSERT_EQ(1, parsed_rcs[0].get()->sources().size());
+ EXPECT_THAT(parsed_rcs[0].get()->sources().front(), HasSubstr("mybazelrc"));
}
TEST_F(GetRcFileTest, GetRcFilesAcceptsNullCommandLineRc) {
@@ -343,7 +338,7 @@
// but it does technically count as a file
ASSERT_EQ(1, parsed_rcs.size());
const std::deque<std::string> expected_rc_que = {kNullDevice};
- EXPECT_EQ(expected_rc_que, parsed_rcs[0].get()->canonical_source_paths());
+ EXPECT_EQ(expected_rc_que, parsed_rcs[0].get()->sources());
}
class ParseOptionsTest : public RcFileTest {
