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

4 files changed, 104 insertions, 11 deletions

diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc
index 0696b5a8a5..6237fb43dd 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -424,6 +424,14 @@ blaze_exit_code::ExitCode OptionProcessor::ParseStartupOptions(string *error) {
 // 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 @@ void OptionProcessor::AddRcfileArgsAndOptions(bool batch, const string& cwd) {
 
     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 38021d518a..76b182af4f 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 @@ public class BlazeCommandDispatcher {
       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 b900612b28..b35c7f10d4 100644
--- a/src/test/shell/integration/BUILD
+++ b/src/test/shell/integration/BUILD
@@ -31,6 +31,13 @@ sh_test(
     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 0000000000..e34914175d
--- /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"