Allow optional-imports for rc files.
`try-import` will not cause an error if the file does not exist. We warn for imports that lead to duplicate rc files being read, but we can't remove the duplicate without messing up ordering semantics.
Users should be wary if they import the same file from two different rc files, as duplicate options can be surprising in some cases, notably if the options are passed on to another tool which does not accept duplicates. A warning will be printed, but warnings are easy to ignore.
Fixes #5765
RELNOTES: None.
PiperOrigin-RevId: 211869620
diff --git a/src/test/cpp/rc_file_test.cc b/src/test/cpp/rc_file_test.cc
index be9ac06..816535d 100644
--- a/src/test/cpp/rc_file_test.cc
+++ b/src/test/cpp/rc_file_test.cc
@@ -155,22 +155,6 @@
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_;
@@ -370,7 +354,24 @@
EXPECT_EQ(expected_rc_que, parsed_rcs[0].get()->sources());
}
-using ParseOptionsTest = RcFileTest;
+class ParseOptionsTest : public RcFileTest {
+ protected:
+ 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));
+ }
+};
TEST_F(ParseOptionsTest, IgnoreAllRcFilesIgnoresAllMasterAndUserRcFiles) {
// Put fake options in different expected rc files, to check that none of them
@@ -621,46 +622,167 @@
"--max_idle_secs=123\n"));
}
-TEST_F(ParseOptionsTest, BazelRcImportsMaintainsFlagOrdering) {
- // Override one of the master bazelrc's flags in the custom bazelrc.
- 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(
- "startup --max_idle_secs=123\nstartup --io_nice_level=4",
- imported_rc_path, 0755));
+class BlazercImportTest : public ParseOptionsTest {
+ protected:
+ void TestBazelRcImportsMaintainsFlagOrdering(const std::string& import_type) {
+ // Override one of the master bazelrc's flags in the custom bazelrc.
+ 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(
+ "startup --max_idle_secs=123\n"
+ "startup --io_nice_level=4",
+ imported_rc_path, 0755));
- // Add startup flags the imported bazelrc.
- std::string workspace_rc;
- ASSERT_TRUE(SetUpWorkspaceRcFile("startup --max_idle_secs=42\nimport " +
- imported_rc_path +
- "\nstartup --io_nice_level=6",
- &workspace_rc));
+ // Add startup flags the imported bazelrc.
+ std::string workspace_rc;
+ ASSERT_TRUE(SetUpWorkspaceRcFile(
+ "startup --max_idle_secs=42\n" +
+ import_type + " " + imported_rc_path + "\n"
+ "startup --io_nice_level=6",
+ &workspace_rc));
- const std::vector<std::string> args = {"bazel", "build"};
- ParseOptionsAndCheckOutput(args, blaze_exit_code::SUCCESS, "", "");
+ const std::vector<std::string> args = {"bazel", "build"};
+ ParseOptionsAndCheckOutput(args, blaze_exit_code::SUCCESS, "", "");
- EXPECT_EQ(123, option_processor_->GetParsedStartupOptions()->max_idle_secs);
- EXPECT_EQ(6, option_processor_->GetParsedStartupOptions()->io_nice_level);
+ EXPECT_EQ(123, option_processor_->GetParsedStartupOptions()->max_idle_secs);
+ EXPECT_EQ(6, option_processor_->GetParsedStartupOptions()->io_nice_level);
- // Check that the options are reported in the correct order in the provenance
- // message, the imported file between the two master flags
- testing::internal::CaptureStderr();
- option_processor_->PrintStartupOptionsProvenanceMessage();
- const std::string output = testing::internal::GetCapturedStderr();
+ // Check that the options are reported in the correct order in the
+ // provenance message, the imported file between the two master flags
+ testing::internal::CaptureStderr();
+ option_processor_->PrintStartupOptionsProvenanceMessage();
+ const std::string output = testing::internal::GetCapturedStderr();
- EXPECT_THAT(
- output,
- MatchesRegex("INFO: Reading 'startup' options from .*workspace.*bazelrc: "
- "--max_idle_secs=42\n"
- "INFO: Reading 'startup' options from .*myimportedbazelrc: "
- "--max_idle_secs=123 --io_nice_level=4\n"
- "INFO: Reading 'startup' options from .*workspace.*bazelrc: "
- "--io_nice_level=6\n"));
-}
+ EXPECT_THAT(
+ output,
+ MatchesRegex(
+ "INFO: Reading 'startup' options from .*workspace.*bazelrc: "
+ "--max_idle_secs=42\n"
+ "INFO: Reading 'startup' options from .*myimportedbazelrc: "
+ "--max_idle_secs=123 --io_nice_level=4\n"
+ "INFO: Reading 'startup' options from .*workspace.*bazelrc: "
+ "--io_nice_level=6\n"));
+ }
-TEST_F(ParseOptionsTest, BazelRcImportFailsForMissingFile) {
+ void TestThatDoubleImportsCauseAWarning(const std::string& import_type) {
+ const std::string imported_rc_path =
+ blaze_util::JoinPath(workspace_, "myimportedbazelrc");
+ ASSERT_TRUE(blaze_util::WriteFile("", imported_rc_path, 0755));
+
+ // Import the custom location twice.
+ std::string workspace_rc;
+ ASSERT_TRUE(SetUpWorkspaceRcFile(
+ import_type + " " + imported_rc_path + "\n" +
+ import_type + " " + 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");
+ }
+
+ void TestThatDoubleImportWithWorkspaceRelativeSyntaxCauseAWarning(
+ const std::string& import_type) {
+ const std::string imported_rc_path =
+ blaze_util::JoinPath(workspace_, "myimportedbazelrc");
+ ASSERT_TRUE(blaze_util::WriteFile("", imported_rc_path, 0755));
+
+ // Import the custom location twice.
+ std::string workspace_rc;
+ ASSERT_TRUE(
+ SetUpWorkspaceRcFile(
+ import_type + " " + imported_rc_path + "\n" +
+ import_type + " %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");
+ }
+
+ void TestThatDoubleImportWithExcessPathSyntaxCauseAWarning(
+ const std::string& import_type) {
+ const std::string imported_rc_path =
+ blaze_util::JoinPath(workspace_, "myimportedbazelrc");
+ ASSERT_TRUE(blaze_util::WriteFile("", imported_rc_path, 0755));
+
+ // Import the custom location twice.
+ std::string workspace_rc;
+ ASSERT_TRUE(
+ SetUpWorkspaceRcFile(
+ import_type + " " + imported_rc_path + "\n" +
+ import_type + " %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");
+ }
+
+ void TestThatDeepDoubleImportCausesAWarning(const std::string& import_type) {
+ const std::string dual_imported_rc_path =
+ blaze_util::JoinPath(workspace_, "dual_imported.bazelrc");
+ 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::WriteFile(
+ import_type + " " + 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::WriteFile(
+ import_type + " " + dual_imported_rc_path,
+ intermediate_import_2, 0755));
+
+ // Import the custom location twice.
+ std::string workspace_rc;
+ ASSERT_TRUE(SetUpWorkspaceRcFile(
+ import_type + " " + intermediate_import_1 + "\n" +
+ import_type + " " + 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");
+ }
+
+ void TestThatImportingAFileAndPassingItInCausesAWarning(
+ const std::string& import_type) {
+ const std::string imported_rc_path =
+ blaze_util::JoinPath(workspace_, "myimportedbazelrc");
+ 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_type + " " + 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");
+ }
+};
+
+TEST_F(BlazercImportTest, BazelRcImportFailsForMissingFile) {
const std::string missing_imported_rc_path =
blaze_util::JoinPath(workspace_, "myimportedbazelrc");
std::string workspace_rc;
@@ -673,77 +795,76 @@
"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.
+TEST_F(BlazercImportTest, BazelRcTryImportDoesNotFailForMissingFile) {
+ const std::string missing_imported_rc_path =
+ blaze_util::JoinPath(workspace_, "tryimported.bazelrc");
std::string workspace_rc;
- ASSERT_TRUE(SetUpWorkspaceRcFile(
- "import " + imported_rc_path + "\n"
- "import " + imported_rc_path + "\n",
- &workspace_rc));
+ ASSERT_TRUE(SetUpWorkspaceRcFile("try-import " + missing_imported_rc_path,
+ &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");
+ ParseOptionsAndCheckOutput(args, blaze_exit_code::SUCCESS, "", "");
}
-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.
+// rc_file does not differentiate between non-existent and unreadable files. We
+// don't necessarily want try-import to ignore unreadable files, but this test
+// exists to make sure we don't change the behavior by accident. Any change that
+// makes existent but unreadable files a failure with try-import should inform
+// users.
+TEST_F(BlazercImportTest, BazelRcTryImportDoesNotFailForUnreadableFile) {
+ const std::string unreadable_rc_path =
+ blaze_util::JoinPath(workspace_, "tryimported.bazelrc");
+ ASSERT_TRUE(blaze_util::WriteFile("startup --max_idle_secs=123",
+ unreadable_rc_path, 222));
std::string workspace_rc;
- ASSERT_TRUE(SetUpWorkspaceRcFile(
- "import " + imported_rc_path + "\n"
- "import %workspace%/myimportedbazelrc\n",
- &workspace_rc));
+ ASSERT_TRUE(
+ SetUpWorkspaceRcFile("try-import " + unreadable_rc_path, &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");
+ ParseOptionsAndCheckOutput(args, blaze_exit_code::SUCCESS, "", "");
}
-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));
+TEST_F(BlazercImportTest, BazelRcImportsMaintainsFlagOrdering) {
+ TestBazelRcImportsMaintainsFlagOrdering("import");
+}
- 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(BlazercImportTest, BazelRcTryImportsMaintainsFlagOrdering) {
+ TestBazelRcImportsMaintainsFlagOrdering("try-import");
+}
+
+TEST_F(BlazercImportTest, DoubleImportsCauseAWarning) {
+ TestThatDoubleImportsCauseAWarning("import");
+}
+
+TEST_F(BlazercImportTest, DoubleTryImportsCauseAWarning) {
+ TestThatDoubleImportsCauseAWarning("try-import");
+}
+
+TEST_F(BlazercImportTest,
+ DoubleImportWithWorkspaceRelativeSyntaxCauseAWarning) {
+ TestThatDoubleImportWithWorkspaceRelativeSyntaxCauseAWarning("import");
+}
+
+TEST_F(BlazercImportTest,
+ DoubleTryImportWithWorkspaceRelativeSyntaxCauseAWarning) {
+ TestThatDoubleImportWithWorkspaceRelativeSyntaxCauseAWarning("try-import");
+}
+
+TEST_F(BlazercImportTest, DoubleImportWithExcessPathSyntaxCauseAWarning) {
+ TestThatDoubleImportWithExcessPathSyntaxCauseAWarning("import");
+}
+
+TEST_F(BlazercImportTest, DoubleTryImportWithExcessPathSyntaxCauseAWarning) {
+ TestThatDoubleImportWithExcessPathSyntaxCauseAWarning("try-import");
}
// The following tests unix-path semantics.
#if !defined(_WIN32) && !defined(__CYGWIN__)
-TEST_F(ParseOptionsTest,
+TEST_F(BlazercImportTest,
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(
@@ -764,61 +885,20 @@
}
#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(BlazercImportTest, DeepDoubleImportCausesAWarning) {
+ TestThatDeepDoubleImportCausesAWarning("import");
}
+TEST_F(BlazercImportTest, DeepDoubleTryImportCausesAWarning) {
+ TestThatDeepDoubleImportCausesAWarning("try-import");
+}
-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));
+TEST_F(BlazercImportTest, ImportingAFileAndPassingItInCausesAWarning) {
+ TestThatImportingAFileAndPassingItInCausesAWarning("import");
+}
- // 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");
+TEST_F(BlazercImportTest, TryImportingAFileAndPassingItInCausesAWarning) {
+ TestThatImportingAFileAndPassingItInCausesAWarning("try-import");
}
// TODO(b/112908763): Somehow, in the following tests, we end with a relative
