Make client-provided options an rc source
The client provides information about whether the terminal is a tty, and
which width the output should be formatted for. Passing this information
as explicit command-line arguments has the disadvantage that it overrides
any setting in configuration files. While usually there is no one-size-fits-all
value for terminal width, it doesn't make sense either to have an option
where the user cannot set a default. Fix this by providing the client options
as least imported rc-source.
--
Change-Id: Iad9eddbb3ff1777f4b423053e21aeac9fd7c466f
Reviewed-on: https://bazel-review.googlesource.com/#/c/3092
MOS_MIGRATED_REVID=117833645
diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc
index 0696b5a..6237fb4 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -424,6 +424,14 @@
// the command and the arguments. NB: Keep the options added here in sync with
// BlazeCommandDispatcher.INTERNAL_COMMAND_OPTIONS!
void OptionProcessor::AddRcfileArgsAndOptions(bool batch, const string& cwd) {
+ // Provide terminal options as coming from the least important rc file.
+ command_arguments_.push_back("--rc_source=client");
+ command_arguments_.push_back("--default_override=0:common=--isatty=" +
+ ToString(IsStandardTerminal()));
+ command_arguments_.push_back(
+ "--default_override=0:common=--terminal_columns=" +
+ ToString(GetTerminalColumns()));
+
// Push the options mapping .blazerc numbers to filenames.
for (int i_blazerc = 0; i_blazerc < blazercs_.size(); i_blazerc++) {
const RcFile* blazerc = blazercs_[i_blazerc];
@@ -441,17 +449,12 @@
for (int ii = 0; ii < it->second.size(); ii++) {
const RcOption& rcoption = it->second[ii];
- command_arguments_.push_back(
- "--default_override=" + ToString(rcoption.rcfile_index()) + ":"
- + it->first + "=" + rcoption.option());
+ command_arguments_.push_back("--default_override=" +
+ ToString(rcoption.rcfile_index() + 1) + ":" +
+ it->first + "=" + rcoption.option());
}
}
- // Splice the terminal options.
- command_arguments_.push_back(
- "--isatty=" + ToString(IsStandardTerminal()));
- command_arguments_.push_back(
- "--terminal_columns=" + ToString(GetTerminalColumns()));
// Pass the client environment to the server in server mode.
if (batch) {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
index 38021d5..76b182a 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
@@ -477,9 +477,10 @@
throws OptionsParsingException {
if (!rcfileOptions.isEmpty()) {
String inherited = commandToParse.equals(originalCommand) ? "" : "Inherited ";
- rcfileNotes.add("Reading options for '" + originalCommand +
- "' from " + rcfile + ":\n" +
- " " + inherited + "'" + commandToParse + "' options: "
+ String source = rcfile.equals("client") ? "Options provided by the client"
+ : "Reading options for '" + originalCommand + "' from " + rcfile;
+ rcfileNotes.add(source + ":\n"
+ + " " + inherited + "'" + commandToParse + "' options: "
+ Joiner.on(' ').join(rcfileOptions));
optionsParser.parse(OptionPriority.RC_FILE, rcfile, rcfileOptions);
}
diff --git a/src/test/shell/integration/BUILD b/src/test/shell/integration/BUILD
index b900612..b35c7f1 100644
--- a/src/test/shell/integration/BUILD
+++ b/src/test/shell/integration/BUILD
@@ -31,6 +31,13 @@
data = [":test-deps"],
)
+sh_test(
+ name = "rc_options_test",
+ size = "medium",
+ srcs = ["rc_options_test.sh"],
+ data = [":test-deps"],
+)
+
test_suite(
name = "all_tests",
visibility = ["//visibility:public"],
diff --git a/src/test/shell/integration/rc_options_test.sh b/src/test/shell/integration/rc_options_test.sh
new file mode 100755
index 0000000..e349141
--- /dev/null
+++ b/src/test/shell/integration/rc_options_test.sh
@@ -0,0 +1,82 @@
+#!/bin/bash
+#
+# Copyright 2016 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# An end-to-end test for Bazel's option handling
+
+# Load test environment
+source $(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/testenv.sh \
+ || { echo "testenv.sh not found!" >&2; exit 1; }
+
+create_and_cd_client
+put_bazel_on_path
+write_default_bazelrc
+
+#### SETUP #############################################################
+
+set -e
+
+function set_up() {
+ mkdir -p pkg
+ # have test with a long name, to be able to test line breaking in the output
+ cat > pkg/xxxxxxxxxxxxxxxxxxxxxxxxxtrue.sh <<EOF
+#!/bin/sh
+exit 0
+EOF
+ chmod 755 pkg/xxxxxxxxxxxxxxxxxxxxxxxxxtrue.sh
+ cat > pkg/BUILD <<EOF
+sh_test(
+ name = "xxxxxxxxxxxxxxxxxxxxxxxxxtrue",
+ srcs = ["xxxxxxxxxxxxxxxxxxxxxxxxxtrue.sh"],
+)
+EOF
+}
+
+#### TESTS #############################################################
+
+function test_terminal_columns_honored() {
+ setup_bazelrc
+ cat >>$TEST_TMPDIR/bazelrc <<EOF
+build --terminal_columns=6
+EOF
+ bazel test --curses=yes --color=yes pkg:xxxxxxxxxxxxxxxxxxxxxxxxxtrue \
+ 2>$TEST_log || fail "bazel test failed"
+ # the lines are wrapped to 6 characters
+ expect_log '^xxxx'
+ expect_not_log '^xxxxxxx'
+}
+
+function test_options_override() {
+ setup_bazelrc
+ cat >>$TEST_TMPDIR/bazelrc <<EOF
+build --terminal_columns=6
+EOF
+ bazel test --curses=yes --color=yes --terminal_columns=10 \
+ pkg:xxxxxxxxxxxxxxxxxxxxxxxxxtrue 2>$TEST_log || fail "bazel test failed"
+ # the lines are wrapped to 10 characters
+ expect_log '^xxxxxxxx'
+ expect_not_log '^xxxxxxxxxxx'
+}
+
+function test_terminal_columns_default() {
+ setup_bazelrc
+ bazel test --curses=yes --color=yes pkg:xxxxxxxxxxxxxxxxxxxxxxxxxtrue \
+ 2>$TEST_log || fail "bazel test failed"
+ # default terminal columns should be large enough to not wrap
+ # this short example
+ expect_not_log '^xxx'
+}
+
+run_suite "Integration tests for rc options handling"
