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_options_test.cc b/src/test/cpp/rc_options_test.cc
index 41684a5..daaba7f 100644
--- a/src/test/cpp/rc_options_test.cc
+++ b/src/test/cpp/rc_options_test.cc
@@ -88,8 +88,6 @@
}
};
-// Effectively empty file tests
-
TEST_F(RcOptionsTest, Empty) {
WriteRc("empty.bazelrc",
"");
@@ -127,8 +125,6 @@
no_expected_args);
}
-// Single command tests - testing tokenization and accumulation of arguments.
-
TEST_F(RcOptionsTest, SingleStartupArg) {
WriteRc("startup_foo.bazelrc",
"startup foo");
@@ -249,8 +245,6 @@
}});
}
-// Testing which commands different args belong to.
-
TEST_F(RcOptionsTest, MultipleCommands) {
WriteRc("multiple_commands_intermixed.bazelrc",
"startup foo\n"
@@ -263,8 +257,6 @@
{{"startup", {"foo", "bar", "baz"}}, {"build", {"aaa", "bbb", "ccc"}}});
}
-// Successful import tests
-
TEST_F(RcOptionsTest, SimpleImportFoo) {
WriteRc("startup_foo.bazelrc",
"startup foo");
@@ -297,9 +289,36 @@
{{"startup", {"bar", "foo"}}});
}
-// Consider making this an error, or at least a warning - most likely, import
-// diamonds like this are unintended, and they might lead to surprising doubled
-// values for allow_multiple options.
+TEST_F(RcOptionsTest, SimpleTryImportFoo) {
+ WriteRc("startup_foo.bazelrc", "startup foo");
+ WriteRc("import_simple.bazelrc",
+ "try-import %workspace%/startup_foo.bazelrc");
+ SuccessfullyParseRcWithExpectedArgs("import_simple.bazelrc",
+ {{"startup", {"foo"}}});
+}
+
+TEST_F(RcOptionsTest, ImportTryFooThenAddBar) {
+ WriteRc("startup_foo.bazelrc", "startup foo");
+ WriteRc("import_foo_then_bar.bazelrc",
+ "try-import %workspace%/startup_foo.bazelrc\n"
+ "startup bar");
+ SuccessfullyParseRcWithExpectedArgs("import_foo_then_bar.bazelrc",
+ {{"startup", {"foo", "bar"}}});
+}
+
+TEST_F(RcOptionsTest, StartupBarThenTryImportFoo) {
+ WriteRc("startup_foo.bazelrc", "startup foo");
+ WriteRc("bar_then_import_foo.bazelrc",
+ "startup bar\n"
+ "try-import %workspace%/startup_foo.bazelrc");
+ SuccessfullyParseRcWithExpectedArgs("bar_then_import_foo.bazelrc",
+ {{"startup", {"bar", "foo"}}});
+}
+
+// Most likely, import diamonds like this are unintended, and they might lead
+// to surprising doubled values for allow_multiple options. This causes a
+// warning in option_processor, which checks for duplicates across multiple rc
+// files.
TEST_F(RcOptionsTest, ImportDiamond) {
WriteRc("startup_foo.bazelrc",
"startup foo");
@@ -317,7 +336,6 @@
{{"startup", {"foo", "bar", "bar", "foo"}}});
}
-// Testing failure modes
TEST_F(RcOptionsTest, ImportCycleFails) {
WriteRc("import_cycle_1.bazelrc",
@@ -392,6 +410,14 @@
ASSERT_EQ(error_text, "Unexpected error reading .blazerc file 'somefile'");
}
+TEST_F(RcOptionsTest, TryImportedFileDoesNotExist) {
+ WriteRc("try_import_fake_file.bazelrc", "try-import somefile");
+
+ unordered_map<string, vector<string>> no_expected_args;
+ SuccessfullyParseRcWithExpectedArgs("try_import_fake_file.bazelrc",
+ no_expected_args);
+}
+
TEST_F(RcOptionsTest, ImportHasTooManyArgs) {
WriteRc("bad_import.bazelrc",
"import somefile bar");
@@ -407,6 +433,20 @@
"in your source checkout/WORKSPACE\\?\\)"));
}
+TEST_F(RcOptionsTest, TryImportHasTooManyArgs) {
+ WriteRc("bad_import.bazelrc", "try-import somefile bar");
+
+ RcFile::ParseError error;
+ string error_text;
+ std::unique_ptr<RcFile> rc = Parse("bad_import.bazelrc", &error, &error_text);
+ EXPECT_EQ(error, RcFile::ParseError::INVALID_FORMAT);
+ ASSERT_THAT(
+ error_text,
+ MatchesRegex("Invalid import declaration in .blazerc file "
+ "'.*bad_import.bazelrc': 'try-import somefile bar' \\(are "
+ "you in your source checkout/WORKSPACE\\?\\)"));
+}
+
// TODO(b/34811299) The tests below identify ways that '\' used as a line
// continuation is broken. This is on top of user-reported cases where an
// unintentional '\' made the command on the following line show up as
