Support multiple --bazelrc on command line by wisechengyi · Pull Request #12740 · bazelbuild/bazel

wisechengyi · 2020-12-21T20:08:57Z

Address #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.

bierbaum

Hi Yi, I left some style nits to get this change in line with what is going on in this codebase.

You also might want to add coverage in src/test/cpp/startup_options_test.cc.

wisechengyi · 2020-12-22T19:05:46Z

You also might want to add coverage in src/test/cpp/startup_options_test.cc.

In terms of bazelrcs chaining together, it was already supported, but just not from command line. Hence the scope of this change merely adds the bazelrcs passed via CLI to the final list of bazelrcs that are going to be parsed. I don't see anything specific about CLI related tests in startup_options_test.cc. Happy to add more if @michajlo thinks it is necessary and can kindly offer some guidance.

aiuto · 2020-12-23T19:37:35Z

cc: @juliexxia
This should have been discussed as an issue first so we could discuss the merits. It's debatable if we want to add any new complexity to arg parsing, since a few of us have recently noticed that it is a mess in many ways and is ripe for a complete rebuilding.

wisechengyi · 2021-03-02T00:49:59Z

Hi @michajlo I have made the changes as discussed. Please kindly give it another go when you get a chance. Thanks!

wisechengyi · 2021-03-09T16:28:22Z

Comment addressed. Thanks!

michajlo · 2021-03-15T18:53:33Z

I had overlooked that there's some documentation that needs to be updated. Can you update https://github.com/bazelbuild/bazel/blob/master/site/docs/guide.md and https://github.com/bazelbuild/bazel/blob/master/site/docs/user-manual.html, and do a quick skim to see if there's anything else we missed?

wisechengyi · 2021-03-15T22:25:11Z

Hi @michajlo, happy to do that. However logistically are we going to run into issues like #12194 where we cannot directly update doc from github PR? If so, would you like me to send a separate PR, and then you can modify the internal repo accordingly?

michajlo · 2021-03-15T22:41:11Z

I think this instance is fine to change.

wisechengyi

Doc updated. https://github.com/bazelbuild/bazel/blob/master/site/docs/user-manual.html should be still accurate on --bazelrc

wisechengyi · 2021-03-23T14:08:20Z

Thank you @michajlo !

Address #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 #12740. PiperOrigin-RevId: 364407234

wisechengyi added 4 commits December 21, 2020 11:46

nary

a89e22a

more

bda828c

no print

e8b8931

no dedup

50d2b04

google-cla Bot added the cla: yes label Dec 21, 2020

wisechengyi changed the title ~~Multi rc~~ Support multiple --bazelrc on command line Dec 21, 2020

minor

13ae538

wisechengyi commented Dec 21, 2020

View reviewed changes

Comment thread src/main/cpp/option_processor.cc Outdated

wisechengyi added 5 commits December 21, 2020 15:38

cmt

421de4f

initial tests

2f08fa8

more

b52aa55

rm unused

6b8b397

rename

d38eca0

wisechengyi marked this pull request as ready for review December 22, 2020 02:54

wisechengyi mentioned this pull request Dec 22, 2020

Add support for specifying --bazelrc multiple times #7489

Closed

meisterT requested a review from michajlo December 22, 2020 07:08

bierbaum reviewed Dec 22, 2020

View reviewed changes

Comment thread src/main/cpp/option_processor.cc Outdated

Comment thread src/main/cpp/option_processor.cc Outdated

Comment thread src/main/cpp/option_processor.cc Outdated

wisechengyi added 3 commits December 22, 2020 10:26

size_type

fd7540b

fmt

17e8b19

one more size_t

e1b2a07

eed3si9n reviewed Dec 22, 2020

View reviewed changes

Comment thread src/main/cpp/option_processor.cc Outdated

auto

3aa941e

noxiouz reviewed Dec 22, 2020

View reviewed changes

Comment thread src/main/cpp/blaze_util.cc Outdated

Comment thread src/main/cpp/blaze_util.cc Outdated

noxiouz reviewed Dec 22, 2020

View reviewed changes

Comment thread src/main/cpp/blaze_util.cc Outdated

wisechengyi added 4 commits December 22, 2020 13:18

potential index out of bound

d72e7bc

nullptr

2888227

use vector<string>

dda34c2

algo

b39c124

Merge branch 'master' into multi_rc

a2a7142

michajlo reviewed Mar 4, 2021

View reviewed changes

Comment thread src/main/cpp/blaze_util.cc Outdated

Comment thread src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java Outdated

Comment thread src/test/shell/integration/startup_options_test.sh

wisechengyi added 3 commits March 5, 2021 12:32

impl

658a9bd

help msg

23cdaf7

add integ test

2a5ef01

wisechengyi force-pushed the multi_rc branch from 2e2670c to 2a5ef01 Compare March 9, 2021 01:01

strcmp

cf7f538

michajlo reviewed Mar 10, 2021

View reviewed changes

Comment thread src/test/cpp/blaze_util_test.cc Outdated

Comment thread src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java Outdated

wisechengyi force-pushed the multi_rc branch 2 times, most recently from 318d34c to 34786aa Compare March 11, 2021 17:25

help msg tweaks and fix format (retrigger ci x2)

f8f5b4b

wisechengyi force-pushed the multi_rc branch from 34786aa to f8f5b4b Compare March 11, 2021 17:26

michajlo reviewed Mar 11, 2021

View reviewed changes

Comment thread src/main/java/com/google/devtools/build/lib/bazel/BazelStartupOptionsModule.java Outdated

grammar

af82fc1

michajlo reviewed Mar 11, 2021

View reviewed changes

Comment thread src/test/shell/integration/startup_options_test.sh Outdated

use info

c5350b6

doc update

c8d093a

wisechengyi requested review from floriographygoth, gregestren and jin as code owners March 18, 2021 21:00

wisechengyi commented Mar 18, 2021

View reviewed changes

Comment thread site/docs/guide.md

bazel-io closed this in 61da1d2 Mar 22, 2021

wisechengyi deleted the multi_rc branch March 23, 2021 14:08

Conversation

wisechengyi commented Dec 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Note

Result

Uh oh!

Uh oh!

bierbaum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wisechengyi commented Dec 22, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aiuto commented Dec 23, 2020

Uh oh!

wisechengyi commented Mar 2, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wisechengyi commented Mar 9, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michajlo commented Mar 15, 2021

Uh oh!

wisechengyi commented Mar 15, 2021

Uh oh!

michajlo commented Mar 15, 2021

Uh oh!

wisechengyi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wisechengyi commented Mar 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

wisechengyi commented Dec 21, 2020 •

edited

Loading