Rollforward of https://github.com/bazelbuild/bazel/commit/2d3adfb93c023f9752f00d4bf3440ee9869f64f2: Add a warning if the same rc file is read multiple times.
Prerequisite for fixing #5765, to minimize the risk that multiple rc files import the same file, which is slightly more likely with try-import.
Added extra test for double-load rcs imported using different syntax, fixing an issue where paths were not canonicalized for reliable comparison.
RELNOTES: None.
PiperOrigin-RevId: 210140725
diff --git a/src/test/cpp/rc_file_test.cc b/src/test/cpp/rc_file_test.cc
index 615bf42..be9ac06 100644
--- a/src/test/cpp/rc_file_test.cc
+++ b/src/test/cpp/rc_file_test.cc
@@ -28,11 +28,12 @@
#include "googletest/include/gtest/gtest.h"
namespace blaze {
+using ::testing::ContainsRegex;
using ::testing::HasSubstr;
using ::testing::MatchesRegex;
#if defined(_WIN32) || defined(__CYGWIN__)
-constexpr const char* kNullDevice = "nul";
+constexpr const char* kNullDevice = "NUL";
#else // Assume POSIX if not Windows.
constexpr const char* kNullDevice = "/dev/null";
#endif
@@ -110,7 +111,7 @@
blaze_util::ConvertPath(blaze_util::JoinPath(cwd_, "bazel.bazelrc"));
if (blaze_util::WriteFile(contents, system_rc_path, 0755)) {
- *rcfile_path = system_rc_path;
+ *rcfile_path = blaze_util::MakeCanonical(system_rc_path.c_str());
return true;
}
return false;
@@ -121,7 +122,7 @@
const std::string workspace_user_rc_path =
blaze_util::JoinPath(workspace_, ".bazelrc");
if (blaze_util::WriteFile(contents, workspace_user_rc_path, 0755)) {
- *rcfile_path = workspace_user_rc_path;
+ *rcfile_path = blaze_util::MakeCanonical(workspace_user_rc_path.c_str());
return true;
}
return false;
@@ -154,6 +155,22 @@
return false;
}
+ void ParseOptionsAndCheckOutput(
+ const std::vector<std::string>& args,
+ const blaze_exit_code::ExitCode expected_exit_code,
+ const std::string& expected_error_regex,
+ const std::string& expected_output_regex) {
+ std::string error;
+ testing::internal::CaptureStderr();
+ const blaze_exit_code::ExitCode exit_code =
+ option_processor_->ParseOptions(args, workspace_, cwd_, &error);
+ const std::string output = testing::internal::GetCapturedStderr();
+
+ ASSERT_EQ(expected_exit_code, exit_code) << error;
+ ASSERT_THAT(error, ContainsRegex(expected_error_regex));
+ ASSERT_THAT(output, ContainsRegex(expected_output_regex));
+ }
+
const std::string workspace_;
std::string cwd_;
const std::string binary_dir_;
@@ -530,10 +547,7 @@
const std::vector<std::string> args = {"bazel", "--noworkspace_rc",
"--workspace_rc", "build"};
- std::string error;
- ASSERT_EQ(blaze_exit_code::SUCCESS,
- option_processor_->ParseOptions(args, workspace_, cwd_, &error))
- << error;
+ ParseOptionsAndCheckOutput(args, blaze_exit_code::SUCCESS, "", "");
EXPECT_EQ(123, option_processor_->GetParsedStartupOptions()->max_idle_secs);
@@ -555,10 +569,7 @@
"startup --max_idle_secs=42\nstartup --io_nice_level=6", &workspace_rc));
const std::vector<std::string> args = {binary_path_, "build"};
- std::string error;
- ASSERT_EQ(blaze_exit_code::SUCCESS,
- option_processor_->ParseOptions(args, workspace_, cwd_, &error))
- << error;
+ ParseOptionsAndCheckOutput(args, blaze_exit_code::SUCCESS, "", "");
EXPECT_EQ(42, option_processor_->GetParsedStartupOptions()->max_idle_secs);
EXPECT_EQ(6, option_processor_->GetParsedStartupOptions()->io_nice_level);
@@ -591,10 +602,7 @@
const std::vector<std::string> args = {
"bazel", "--bazelrc=" + cmdline_rc_path, "build"};
- std::string error;
- ASSERT_EQ(blaze_exit_code::SUCCESS,
- option_processor_->ParseOptions(args, workspace_, cwd_, &error))
- << error;
+ ParseOptionsAndCheckOutput(args, blaze_exit_code::SUCCESS, "", "");
EXPECT_EQ(123, option_processor_->GetParsedStartupOptions()->max_idle_secs);
EXPECT_EQ(6, option_processor_->GetParsedStartupOptions()->io_nice_level);
@@ -631,10 +639,7 @@
&workspace_rc));
const std::vector<std::string> args = {"bazel", "build"};
- std::string error;
- ASSERT_EQ(blaze_exit_code::SUCCESS,
- option_processor_->ParseOptions(args, workspace_, cwd_, &error))
- << error;
+ ParseOptionsAndCheckOutput(args, blaze_exit_code::SUCCESS, "", "");
EXPECT_EQ(123, option_processor_->GetParsedStartupOptions()->max_idle_secs);
EXPECT_EQ(6, option_processor_->GetParsedStartupOptions()->io_nice_level);
@@ -655,4 +660,201 @@
"--io_nice_level=6\n"));
}
+TEST_F(ParseOptionsTest, BazelRcImportFailsForMissingFile) {
+ const std::string missing_imported_rc_path =
+ blaze_util::JoinPath(workspace_, "myimportedbazelrc");
+ std::string workspace_rc;
+ ASSERT_TRUE(SetUpWorkspaceRcFile("import " + missing_imported_rc_path,
+ &workspace_rc));
+
+ const std::vector<std::string> args = {"bazel", "build"};
+ ParseOptionsAndCheckOutput(
+ args, blaze_exit_code::INTERNAL_ERROR,
+ "Unexpected error reading .blazerc file '.*myimportedbazelrc'", "");
+}
+
+TEST_F(ParseOptionsTest, DoubleImportsCauseAWarning) {
+ const std::string imported_rc_path =
+ blaze_util::JoinPath(workspace_, "myimportedbazelrc");
+ ASSERT_TRUE(
+ blaze_util::MakeDirectories(blaze_util::Dirname(imported_rc_path), 0755));
+ ASSERT_TRUE(blaze_util::WriteFile("", imported_rc_path, 0755));
+
+ // Import the custom location twice.
+ std::string workspace_rc;
+ ASSERT_TRUE(SetUpWorkspaceRcFile(
+ "import " + imported_rc_path + "\n"
+ "import " + imported_rc_path + "\n",
+ &workspace_rc));
+
+ const std::vector<std::string> args = {"bazel", "build"};
+ ParseOptionsAndCheckOutput(
+ args, blaze_exit_code::SUCCESS, "",
+ "WARNING: Duplicate rc file: .*myimportedbazelrc is imported multiple "
+ "times from .*workspace.*bazelrc\n");
+}
+
+TEST_F(ParseOptionsTest, DoubleImportWithWorkspaceRelativeSyntaxCauseAWarning) {
+ const std::string imported_rc_path =
+ blaze_util::JoinPath(workspace_, "myimportedbazelrc");
+ ASSERT_TRUE(
+ blaze_util::MakeDirectories(blaze_util::Dirname(imported_rc_path), 0755));
+ ASSERT_TRUE(blaze_util::WriteFile("", imported_rc_path, 0755));
+
+ // Import the custom location twice.
+ std::string workspace_rc;
+ ASSERT_TRUE(SetUpWorkspaceRcFile(
+ "import " + imported_rc_path + "\n"
+ "import %workspace%/myimportedbazelrc\n",
+ &workspace_rc));
+
+ const std::vector<std::string> args = {"bazel", "build"};
+ ParseOptionsAndCheckOutput(
+ args, blaze_exit_code::SUCCESS, "",
+ "WARNING: Duplicate rc file: .*myimportedbazelrc is imported multiple "
+ "times from .*workspace.*bazelrc\n");
+}
+
+TEST_F(ParseOptionsTest, DoubleImportWithExcessPathSyntaxCauseAWarning) {
+ const std::string imported_rc_path =
+ blaze_util::JoinPath(workspace_, "myimportedbazelrc");
+ ASSERT_TRUE(
+ blaze_util::MakeDirectories(blaze_util::Dirname(imported_rc_path), 0755));
+ ASSERT_TRUE(blaze_util::WriteFile("", imported_rc_path, 0755));
+
+ // Import the custom location twice.
+ std::string workspace_rc;
+ ASSERT_TRUE(SetUpWorkspaceRcFile(
+ "import " + imported_rc_path + "\n"
+ "import %workspace%///.//myimportedbazelrc\n",
+ &workspace_rc));
+
+ const std::vector<std::string> args = {"bazel", "build"};
+ ParseOptionsAndCheckOutput(
+ args, blaze_exit_code::SUCCESS, "",
+ "WARNING: Duplicate rc file: .*myimportedbazelrc is imported multiple "
+ "times from .*workspace.*bazelrc\n");
+}
+
+// The following tests unix-path semantics.
+#if !defined(_WIN32) && !defined(__CYGWIN__)
+TEST_F(ParseOptionsTest,
+ DoubleImportWithEnclosingDirectorySyntaxCauseAWarning) {
+ const std::string imported_rc_path =
+ blaze_util::JoinPath(workspace_, "myimportedbazelrc");
+ ASSERT_TRUE(
+ blaze_util::MakeDirectories(blaze_util::Dirname(imported_rc_path), 0755));
+ ASSERT_TRUE(blaze_util::WriteFile("", imported_rc_path, 0755));
+
+ ASSERT_TRUE(blaze_util::MakeDirectories(
+ blaze_util::JoinPath(workspace_, "extra"), 0755));
+
+ // Import the custom location twice.
+ std::string workspace_rc;
+ ASSERT_TRUE(SetUpWorkspaceRcFile(
+ "import " + imported_rc_path + "\n"
+ "import %workspace%/extra/../myimportedbazelrc\n",
+ &workspace_rc));
+
+ const std::vector<std::string> args = {"bazel", "build"};
+ ParseOptionsAndCheckOutput(
+ args, blaze_exit_code::SUCCESS, "",
+ "WARNING: Duplicate rc file: .*myimportedbazelrc is imported multiple "
+ "times from .*workspace.*bazelrc\n");
+}
+#endif // !defined(_WIN32) && !defined(__CYGWIN__)
+
+TEST_F(ParseOptionsTest, DeepDoubleImportCausesAWarning) {
+ const std::string dual_imported_rc_path =
+ blaze_util::JoinPath(workspace_, "dual_imported.bazelrc");
+ ASSERT_TRUE(blaze_util::MakeDirectories(
+ blaze_util::Dirname(dual_imported_rc_path), 0755));
+ ASSERT_TRUE(blaze_util::WriteFile("", dual_imported_rc_path, 0755));
+
+ const std::string intermediate_import_1 =
+ blaze_util::JoinPath(workspace_, "intermediate_import_1");
+ ASSERT_TRUE(blaze_util::MakeDirectories(
+ blaze_util::Dirname(intermediate_import_1), 0755));
+ ASSERT_TRUE(blaze_util::WriteFile("import " + dual_imported_rc_path,
+ intermediate_import_1, 0755));
+
+ const std::string intermediate_import_2 =
+ blaze_util::JoinPath(workspace_, "intermediate_import_2");
+ ASSERT_TRUE(blaze_util::MakeDirectories(
+ blaze_util::Dirname(intermediate_import_2), 0755));
+ ASSERT_TRUE(blaze_util::WriteFile("import " + dual_imported_rc_path,
+ intermediate_import_2, 0755));
+
+ // Import the custom location twice.
+ std::string workspace_rc;
+ ASSERT_TRUE(SetUpWorkspaceRcFile(
+ "import " + intermediate_import_1 + "\n"
+ "import " + intermediate_import_2 + "\n",
+ &workspace_rc));
+
+ const std::vector<std::string> args = {"bazel", "build"};
+ ParseOptionsAndCheckOutput(args, blaze_exit_code::SUCCESS, "",
+ "WARNING: Duplicate rc file: "
+ ".*dual_imported.bazelrc is imported multiple "
+ "times from .*workspace.*bazelrc\n");
+}
+
+
+TEST_F(ParseOptionsTest, ImportingAFileAndPassingItInCausesAWarning) {
+ const std::string imported_rc_path =
+ blaze_util::JoinPath(workspace_, "myimportedbazelrc");
+ ASSERT_TRUE(
+ blaze_util::MakeDirectories(blaze_util::Dirname(imported_rc_path), 0755));
+ ASSERT_TRUE(blaze_util::WriteFile("", imported_rc_path, 0755));
+
+ // Import the custom location, and pass it in by flag.
+ std::string workspace_rc;
+ ASSERT_TRUE(
+ SetUpWorkspaceRcFile("import " + imported_rc_path, &workspace_rc));
+
+ const std::vector<std::string> args = {
+ "bazel", "--bazelrc=" + imported_rc_path, "build"};
+ ParseOptionsAndCheckOutput(
+ args, blaze_exit_code::SUCCESS, "",
+ "WARNING: Duplicate rc file: .*myimportedbazelrc is read multiple times, "
+ "it is a standard rc file location but must have been unnecessarilly "
+ "imported earlier.\n");
+}
+
+// TODO(b/112908763): Somehow, in the following tests, we end with a relative
+// path written in the import line on Windows. Figure out what's going on and
+// reinstate these tests
+#if !defined(_WIN32) && !defined(__CYGWIN__)
+TEST_F(ParseOptionsTest, ImportingAPreviouslyLoadedStandardRcCausesAWarning) {
+ std::string system_rc;
+ ASSERT_TRUE(SetUpSystemRcFile("", &system_rc));
+
+ // Import the system_rc extraneously.
+ std::string workspace_rc;
+ ASSERT_TRUE(SetUpWorkspaceRcFile("import " + system_rc, &workspace_rc));
+
+ const std::vector<std::string> args = {"bazel", "build"};
+ ParseOptionsAndCheckOutput(
+ args, blaze_exit_code::SUCCESS, "",
+ "WARNING: Duplicate rc file: .*bazel.bazelrc is read multiple "
+ "times, most recently imported from .*workspace.*bazelrc\n");
+}
+
+TEST_F(ParseOptionsTest, ImportingStandardRcBeforeItIsLoadedCausesAWarning) {
+ std::string workspace_rc;
+ ASSERT_TRUE(SetUpWorkspaceRcFile("", &workspace_rc));
+
+ // Import the workspace_rc extraneously.
+ std::string system_rc;
+ ASSERT_TRUE(SetUpSystemRcFile("import " + workspace_rc, &system_rc));
+
+ const std::vector<std::string> args = {"bazel", "build"};
+ ParseOptionsAndCheckOutput(
+ args, blaze_exit_code::SUCCESS, "",
+ "WARNING: Duplicate rc file: .*workspace.*bazelrc is read multiple "
+ "times, it is a standard rc file location but must have been "
+ "unnecessarilly imported earlier.\n");
+}
+#endif // !defined(_WIN32) && !defined(__CYGWIN__)
+
} // namespace blaze
diff --git a/src/test/cpp/rc_options_test.cc b/src/test/cpp/rc_options_test.cc
index 58251af..41684a5 100644
--- a/src/test/cpp/rc_options_test.cc
+++ b/src/test/cpp/rc_options_test.cc
@@ -22,12 +22,14 @@
#include "src/main/cpp/util/path.h"
#include "src/main/cpp/util/strings.h"
#include "src/main/cpp/workspace_layout.h"
+#include "googlemock/include/gmock/gmock.h"
#include "googletest/include/gtest/gtest.h"
namespace blaze {
using std::string;
using std::unordered_map;
using std::vector;
+using ::testing::MatchesRegex;
class RcOptionsTest : public ::testing::Test {
protected:
@@ -328,13 +330,12 @@
std::unique_ptr<RcFile> rc =
Parse("import_cycle_1.bazelrc", &error, &error_text);
EXPECT_EQ(error, RcFile::ParseError::IMPORT_LOOP);
- string expected_error;
- blaze_util::StringPrintf(
- &expected_error, "Import loop detected:\n %s\n %s\n %s\n",
- blaze_util::JoinPath(test_file_dir_, "import_cycle_1.bazelrc").c_str(),
- blaze_util::JoinPath(test_file_dir_, "import_cycle_2.bazelrc").c_str(),
- blaze_util::JoinPath(test_file_dir_, "import_cycle_1.bazelrc").c_str());
- ASSERT_EQ(error_text, expected_error);
+ ASSERT_THAT(
+ error_text,
+ MatchesRegex("Import loop detected:\n"
+ " .*import_cycle_1.bazelrc\n"
+ " .*import_cycle_2.bazelrc\n"
+ " .*import_cycle_1.bazelrc\n"));
}
TEST_F(RcOptionsTest, LongImportCycleFails) {
@@ -356,18 +357,16 @@
std::unique_ptr<RcFile> rc =
Parse("chain_to_cycle_1.bazelrc", &error, &error_text);
EXPECT_EQ(error, RcFile::ParseError::IMPORT_LOOP);
- string expected_error;
- blaze_util::StringPrintf(
- &expected_error,
- "Import loop detected:\n %s\n %s\n %s\n %s\n %s\n %s\n %s\n",
- blaze_util::JoinPath(test_file_dir_, "chain_to_cycle_1.bazelrc").c_str(),
- blaze_util::JoinPath(test_file_dir_, "chain_to_cycle_2.bazelrc").c_str(),
- blaze_util::JoinPath(test_file_dir_, "chain_to_cycle_3.bazelrc").c_str(),
- blaze_util::JoinPath(test_file_dir_, "chain_to_cycle_4.bazelrc").c_str(),
- blaze_util::JoinPath(test_file_dir_, "import_cycle_1.bazelrc").c_str(),
- blaze_util::JoinPath(test_file_dir_, "import_cycle_2.bazelrc").c_str(),
- blaze_util::JoinPath(test_file_dir_, "import_cycle_1.bazelrc").c_str());
- ASSERT_EQ(error_text, expected_error);
+ ASSERT_THAT(
+ error_text,
+ MatchesRegex("Import loop detected:\n"
+ " .*chain_to_cycle_1.bazelrc\n"
+ " .*chain_to_cycle_2.bazelrc\n"
+ " .*chain_to_cycle_3.bazelrc\n"
+ " .*chain_to_cycle_4.bazelrc\n"
+ " .*import_cycle_1.bazelrc\n"
+ " .*import_cycle_2.bazelrc\n"
+ " .*import_cycle_1.bazelrc\n"));
}
TEST_F(RcOptionsTest, FileDoesNotExist) {
@@ -375,11 +374,10 @@
string error_text;
std::unique_ptr<RcFile> rc = Parse("not_a_file.bazelrc", &error, &error_text);
EXPECT_EQ(error, RcFile::ParseError::UNREADABLE_FILE);
- string expected_error;
- blaze_util::StringPrintf(
- &expected_error, "Unexpected error reading .blazerc file '%s'",
- blaze_util::JoinPath(test_file_dir_, "not_a_file.bazelrc").c_str());
- ASSERT_EQ(error_text, expected_error);
+ ASSERT_THAT(
+ error_text,
+ MatchesRegex(
+ "Unexpected error reading .blazerc file '.*not_a_file.bazelrc'"));
}
TEST_F(RcOptionsTest, ImportedFileDoesNotExist) {
@@ -402,14 +400,11 @@
string error_text;
std::unique_ptr<RcFile> rc = Parse("bad_import.bazelrc", &error, &error_text);
EXPECT_EQ(error, RcFile::ParseError::INVALID_FORMAT);
-
- string expected_error;
- blaze_util::StringPrintf(
- &expected_error,
- "Invalid import declaration in .blazerc file '%s': "
- "'import somefile bar' (are you in your source checkout/WORKSPACE?)",
- blaze_util::JoinPath(test_file_dir_, "bad_import.bazelrc").c_str());
- ASSERT_EQ(error_text, expected_error);
+ ASSERT_THAT(
+ error_text,
+ MatchesRegex("Invalid import declaration in .blazerc file "
+ "'.*bad_import.bazelrc': 'import somefile bar' \\(are you "
+ "in your source checkout/WORKSPACE\\?\\)"));
}
// TODO(b/34811299) The tests below identify ways that '\' used as a line
