Skip to content

Commit

Permalink
Support multiple --bazelrc on command line
Browse files Browse the repository at this point in the history
Address bazelbuild#7489

### Motivation

Multiple --bazelrc on CLI would be useful for us for various reasons:

Mostly importantly, we will have very long argument lists to pass to bazel query, so they have to be in a bazelrc file. `import/try import` in bazelrc would still work but very awkwardly. For example, if there are multiple bazelrc files to import, say `A` and `B`,

import `A` needs to be added into `$WORKSPACE/.bazelrc`
import `B` needs to be added into `A`
meaning the former bazelrc file needs to know what comes next.

Therefore allowing multiple --bazelrc would greatly improve the ergonomics, so the caller can create and append the new bazelrc without modifying the previous rc files.

### Note

Options passed on CLI will still overwrite any options specified in any bazelrcs.

### Result
`bazel --bazelrc=x.rc --bazelrc=y.rc ...` now works.
```
  --bazelrc (a string; default: see description)
    The location of the user .bazelrc file containing default values of Bazel
    options. This option can also be chained together.
    E.g. `--bazelrc=x.rc --bazelrc=y.rc` so options in both RCs will be read.
    Note: `--bazelrc x.rc y.rc` is illegal, and each bazelrc file needs to be
    accompanied by --bazelrc flag before it.
    If unspecified, Bazel uses the first .bazelrc file it finds in the
    following two locations: the workspace directory, then the user's home
    directory. Use /dev/null to disable the search for a user rc file, e.g. in
    release builds.
```

Closes bazelbuild#12740.

PiperOrigin-RevId: 364407234
  • Loading branch information
wisechengyi authored and copybara-github committed Mar 22, 2021
1 parent 29e9369 commit 61da1d2
Show file tree
Hide file tree
Showing 7 changed files with 263 additions and 15 deletions.
13 changes: 11 additions & 2 deletions site/docs/guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -776,8 +776,17 @@ before the command (`build`, `test`, etc).
4. **The user-specified RC file**, if specified with
<code>--bazelrc=<var>file</var></code>

This flag is optional. However, if the flag is specified, then the file must
exist.
This flag is optional but can also be specified multiple times.

`/dev/null` indicates that all further `--bazelrc`s will be ignored, which
is useful to disable the search for a user rc file, e.g. in release builds.

For example:
```
--bazelrc=x.rc --bazelrc=y.rc --bazelrc=/dev/null --bazelrc=z.rc
```
1. `x.rc` and `y.rc` are read.
2. `z.rc` is ignored due to the prior `/dev/null`.
In addition to this optional configuration file, Bazel looks for a global rc
file. For more details, see the [global bazelrc section](#global_bazelrc).
Expand Down
37 changes: 37 additions & 0 deletions src/main/cpp/blaze_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include <algorithm>
#include <cassert>
#include <iostream>

Expand All @@ -35,6 +37,7 @@
namespace blaze {

using std::map;
using std::min;
using std::string;
using std::vector;

Expand Down Expand Up @@ -74,6 +77,40 @@ bool GetNullaryOption(const char *arg, const char *key) {
return true;
}

std::vector<std::string> GetAllUnaryOptionValues(
const vector<string>& args, const char* key,
const char* ignore_after_value) {
vector<std::string> values;
for (vector<string>::size_type i = 0; i < args.size(); ++i) {
if (args[i] == "--") {
// "--" means all remaining args aren't options
return values;
}

const char* next_arg = args[std::min(i + 1, args.size() - 1)].c_str();
const char* result = GetUnaryOption(args[i].c_str(), next_arg, key);
if (result != nullptr) {
// 'key' was found and 'result' has its value.
values.push_back(result);

if (ignore_after_value != nullptr &&
strcmp(result, ignore_after_value) == 0) {
break;
}
}

// This is a pointer comparison, so equality means that the result must be
// from the next arg instead of happening to match the value from
// "--key=<value>" string, in which case we need to advance the index to
// skip the next arg for later iterations.
if (result == next_arg) {
i++;
}
}

return values;
}

const char* SearchUnaryOption(const vector<string>& args,
const char *key, bool warn_if_dupe) {
if (args.empty()) {
Expand Down
9 changes: 9 additions & 0 deletions src/main/cpp/blaze_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ bool GetNullaryOption(const char *arg, const char *key);
const char* SearchUnaryOption(const std::vector<std::string>& args,
const char* key, bool warn_if_dupe);

// Searches for 'key' in 'args' using GetUnaryOption. Arguments found after '--'
// are omitted from the search.
// If 'ignore_after_value' is not nullptr, all values matching the 'key'
// following 'ignore_after_value' will be ignored. Returns the values of the
// 'key' flag iff it occurs in args. Returns empty vector otherwise.
std::vector<std::string> GetAllUnaryOptionValues(
const std::vector<std::string>& args, const char* key,
const char* ignore_after_value = nullptr);

// Searches for '--flag_name' and '--noflag_name' in 'args' using
// GetNullaryOption. Arguments found after '--' are omitted from the search.
// Returns true if '--flag_name' is a flag in args and '--noflag_name' does not
Expand Down
32 changes: 22 additions & 10 deletions src/main/cpp/option_processor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,24 @@ std::set<std::string> GetOldRcPaths(
internal::FindRcAlongsideBinary(cwd, path_to_binary);
candidate_bazelrc_paths = {workspace_rc, binary_rc, system_bazelrc_path};
}
string user_bazelrc_path = internal::FindLegacyUserBazelrc(
SearchUnaryOption(startup_args, "--bazelrc", /* warn_if_dupe */ true),
workspace);
if (!user_bazelrc_path.empty()) {
candidate_bazelrc_paths.push_back(user_bazelrc_path);
vector<std::string> cmd_line_rc_files =
GetAllUnaryOptionValues(startup_args, "--bazelrc", "/dev/null");
// Divide the cases where the vector is empty vs not, as
// `FindLegacyUserBazelrc` has a case for rc_file to be a nullptr.
if (cmd_line_rc_files.empty()) {
string user_bazelrc_path =
internal::FindLegacyUserBazelrc(nullptr, workspace);
if (!user_bazelrc_path.empty()) {
candidate_bazelrc_paths.push_back(user_bazelrc_path);
}
} else {
for (auto& rc_file : cmd_line_rc_files) {
string user_bazelrc_path =
internal::FindLegacyUserBazelrc(rc_file.c_str(), workspace);
if (!user_bazelrc_path.empty()) {
candidate_bazelrc_paths.push_back(user_bazelrc_path);
}
}
}
// DedupeBlazercPaths returns paths whose canonical path could be computed,
// therefore these paths must exist.
Expand Down Expand Up @@ -370,11 +383,10 @@ blaze_exit_code::ExitCode OptionProcessor::GetRcFiles(

// Get the command-line provided rc, passed as --bazelrc or nothing if the
// flag is absent.
const char* cmd_line_rc_file =
SearchUnaryOption(cmd_line->startup_args, "--bazelrc",
/* warn_if_dupe */ true);
if (cmd_line_rc_file != nullptr) {
string absolute_cmd_line_rc = blaze::AbsolutePathFromFlag(cmd_line_rc_file);
vector<std::string> cmd_line_rc_files =
GetAllUnaryOptionValues(cmd_line->startup_args, "--bazelrc", "/dev/null");
for (auto& rc_file : cmd_line_rc_files) {
string absolute_cmd_line_rc = blaze::AbsolutePathFromFlag(rc_file);
// Unlike the previous 3 paths, where we ignore it if the file does not
// exist or is unreadable, since this path is explicitly passed, this is an
// error. Check this condition here.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,19 @@ public static final class Options extends OptionsBase {
valueHelp = "<path>",
help =
"The location of the user .bazelrc file containing default values of "
+ "Bazel options. If unspecified, Bazel uses the first .bazelrc file it finds in "
+ "Bazel options. "
+ "/dev/null indicates that all further `--bazelrc`s will be ignored, "
+ "which is useful to disable the search for a user rc file, "
+ "e.g. in release builds.\n"
+ "This option can also be specified multiple times.\n"
+ "E.g. with "
+ "`--bazelrc=x.rc --bazelrc=y.rc --bazelrc=/dev/null --bazelrc=z.rc`,\n"
+ " 1) x.rc and y.rc are read.\n"
+ " 2) z.rc is ignored due to the prior /dev/null.\n"
+ "If unspecified, Bazel uses the first .bazelrc file it finds in "
+ "the following two locations: the workspace directory, then the user's home "
+ "directory. Use /dev/null to disable the search for a user rc file, e.g. in "
+ "release builds.")
+ "directory.\n"
+ "Note: command line options will always supersede any option in bazelrc.")
public String blazerc;

// TODO(b/36168162): Remove this after the transition period is ower. This now only serves to
Expand Down
136 changes: 136 additions & 0 deletions src/test/cpp/blaze_util_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -234,4 +234,140 @@ TEST_F(BlazeUtilTest, TestSearchUnaryCommandOptionWarnsAboutDuplicates) {
}
}

void assert_equal_vector_char_pointer(std::vector<std::string> expected,
std::vector<std::string> actual) {
ASSERT_EQ(actual.size(), expected.size())
<< "Vectors expected and actual are of unequal length";

for (int i = 0; i < actual.size(); ++i) {
ASSERT_EQ(actual[i], expected[i])
<< "Vectors expected and actual differ at index " << i;
}
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryForEmpty) {
assert_equal_vector_char_pointer(
{}, GetAllUnaryOptionValues({"bazel", "build", ":target"}, ""));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryFlagNotPresent) {
assert_equal_vector_char_pointer(
{}, GetAllUnaryOptionValues({"bazel", "build", ":target"}, "--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionWithEquals) {
assert_equal_vector_char_pointer(
{"value"}, GetAllUnaryOptionValues(
{"bazel", "--flag=value", "build", ":target"}, "--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionWithEquals2) {
assert_equal_vector_char_pointer(
{"value1", "value2"},
GetAllUnaryOptionValues(
{"bazel", "--flag=value1", "--flag=value2", "build", ":target"},
"--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionWithRepeatingFlag) {
assert_equal_vector_char_pointer(
{"--flag"}, GetAllUnaryOptionValues({"bazel", "--flag", "--flag",
"value1", "build", ":target"},
"--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionWithRepeatingFlagOptions) {
assert_equal_vector_char_pointer(
{"--flag"}, GetAllUnaryOptionValues(
{"bazel", "--flag", "--flag", "value1"}, "--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionValuesWithEquals) {
assert_equal_vector_char_pointer(
{"--flag", "value1"},
GetAllUnaryOptionValues({"bazel", "--flag=--flag", "--flag", "value1"},
"--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionWithEquals3) {
assert_equal_vector_char_pointer(
{"value1", "value2", "value3"},
GetAllUnaryOptionValues({"bazel", "--flag=value1", "--flag=value2",
"--flag=value3", "build", ":target"},
"--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionWithoutEquals) {
assert_equal_vector_char_pointer(
{"value"},
GetAllUnaryOptionValues({"bazel", "--flag", "value", "build", ":target"},
"--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryOptionWithoutEquals2) {
assert_equal_vector_char_pointer(
{"value1", "value2"},
GetAllUnaryOptionValues(
{"bazel", "--flag", "value1", "--flag", "value2", "build", ":target"},
"--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithEquals) {
assert_equal_vector_char_pointer(
{"value"},
GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag", "value"},
"--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithEquals2) {
assert_equal_vector_char_pointer(
{"value1", "value2"},
GetAllUnaryOptionValues(
{"bazel", "build", ":target", "--flag", "value1", "--flag", "value2"},
"--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithoutEquals) {
assert_equal_vector_char_pointer(
{"value"}, GetAllUnaryOptionValues(
{"bazel", "build", ":target", "--flag=value"}, "--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithoutEquals2) {
assert_equal_vector_char_pointer(
{"value1", "value2"},
GetAllUnaryOptionValues(
{"bazel", "build", ":target", "--flag=value1", "--flag=value2"},
"--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnarySkipsAfterDashDashWithEquals) {
assert_equal_vector_char_pointer(
{},
GetAllUnaryOptionValues(
{"bazel", "build", ":target", "--", "--flag", "value"}, "--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnarySkipsAfterDashDashWithoutEquals) {
assert_equal_vector_char_pointer(
{}, GetAllUnaryOptionValues(
{"bazel", "build", ":target", "--", "--flag=value"}, "--flag"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithIgnoreAfter) {
assert_equal_vector_char_pointer(
{"value1", "/dev/null"},
GetAllUnaryOptionValues({"bazel", "build", ":target", "--flag", "value1",
"--flag", "/dev/null", "--flag", "value3"},
"--flag", "/dev/null"));
}

TEST_F(BlazeUtilTest, TestSearchAllUnaryCommandOptionWithIgnoreAfterDevNull) {
assert_equal_vector_char_pointer(
{"/dev/null"}, GetAllUnaryOptionValues(
{"bazel", "build", ":target", "--flag", "/dev/null",
"--flag", "value2", "--flag", "value3"},
"--flag", "/dev/null"));
}

} // namespace blaze
36 changes: 36 additions & 0 deletions src/test/shell/integration/startup_options_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,40 @@ function test_autodetect_server_javabase() {
bazel --noautodetect_server_javabase version &> $TEST_log || fail "Should pass"
}

# Below are the regression tests for Issue #7489
function test_multiple_bazelrc_later_overwrites_earlier() {
# Help message only visible with --help_verbosity=medium
help_message_in_description="--${PRODUCT_NAME}rc (a string; default: see description)"

echo "help --help_verbosity=short" > 1.rc
echo "help --help_verbosity=medium" > 2.rc
bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" help startup_options &> $TEST_log || fail "Should pass"
expect_log "$help_message_in_description"

echo "help --help_verbosity=medium" > 1.rc
echo "help --help_verbosity=short" > 2.rc
bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" help startup_options &> $TEST_log || fail "Should pass"
expect_not_log "$help_message_in_description"
}

function test_multiple_bazelrc_set_different_options() {
echo "common --verbose_failures" > 1.rc
echo "common --test_output=all" > 2.rc
bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" info --announce_rc &> $TEST_log || fail "Should pass"
expect_log "Inherited 'common' options: --verbose_failures"
expect_log "Inherited 'common' options: --test_output=all"
}

function test_bazelrc_after_devnull_ignored() {
echo "common --verbose_failures" > 1.rc
echo "common --test_output=all" > 2.rc
echo "common --definitely_invalid_config" > 3.rc

bazel "--${PRODUCT_NAME}rc=1.rc" "--${PRODUCT_NAME}rc=2.rc" "--${PRODUCT_NAME}rc=/dev/null" \
"--${PRODUCT_NAME}rc=3.rc" info --announce_rc &> $TEST_log || fail "Should pass"
expect_log "Inherited 'common' options: --verbose_failures"
expect_log "Inherited 'common' options: --test_output=all"
expect_not_log "--definitely_invalid_config"
}

run_suite "${PRODUCT_NAME} startup options test"

0 comments on commit 61da1d2

Please sign in to comment.