diff options
author | Klaus Aehlig <aehlig@google.com> | 2016-04-20 15:13:51 +0000 |
---|---|---|
committer | Damien Martin-Guillerez <dmarting@google.com> | 2016-04-21 10:57:42 +0000 |
commit | 3c9a2265ea043c52c81951907fbc626b8c03add4 (patch) | |
tree | 8b164b31b2b8d7bd13538bb8eed4581cc6abcc51 | |
parent | 6d26ceba17d6a9577d2c1fa3ff4f8a4516a40ec2 (diff) |
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.
This is a roll-forward of commit 044adedc70de040475443e52eb1a3c692159790e
--
MOS_MIGRATED_REVID=120338148
-rw-r--r-- | src/main/cpp/option_processor.cc | 16 | ||||
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java | 7 | ||||
-rw-r--r-- | src/test/shell/integration/BUILD | 7 | ||||
-rwxr-xr-x | src/test/shell/integration/rc_options_test.sh | 82 |
4 files changed, 102 insertions, 10 deletions
diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc index 0696b5a8a5..3e2f662212 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]; @@ -442,17 +450,11 @@ 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()) + ":" + "--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) { command_arguments_.push_back("--ignore_client_env"); 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 707aabe259..5f288d7487 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 @@ -533,9 +533,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 38f489d122..66c68dd434 100644 --- a/src/test/shell/integration/BUILD +++ b/src/test/shell/integration/BUILD @@ -39,6 +39,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" |