Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for specifying --bazelrc multiple times #7489

Closed
burkpojken opened this issue Feb 21, 2019 · 22 comments
Closed

Add support for specifying --bazelrc multiple times #7489

burkpojken opened this issue Feb 21, 2019 · 22 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: feature request
Milestone

Comments

@burkpojken
Copy link

burkpojken commented Feb 21, 2019

Description of the problem / feature request:

Today it is not possible to add multiple --bazelrc options to the command line.

Feature requests: what underlying problem are you trying to solve with this feature?

We have some default options in a common bazelrc file that is used in many workspaces since the options are needed to build for our platform.
Today we need to add those options in all user .bazelrc WORKSPACE files.
If it would be possible to add multiple --bazelrc options on the command line we could have one rc file for the common parameters and still be able to use other bazelrc files for the user projects.

What operating system are you running Bazel on?

linux

What's the output of bazel info release?

0.20.0

Have you found anything relevant by searching the web?

No

Any other information, logs, or outputs that you want to share?

@jin jin added untriaged team-Bazel General Bazel product/strategy issues labels Feb 21, 2019
@burkpojken
Copy link
Author

Are there any objections to this?
We can try to implement it and make a pull request.
Today you do not even get an error or warning if you specify multiple --bazelrc

@dslomov dslomov added team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website and removed team-Bazel General Bazel product/strategy issues labels Jul 19, 2019
@philwo philwo added P1 I'll work on this now. (Assignee required) type: feature request labels Jul 29, 2019
@philwo philwo removed the untriaged label Jul 29, 2019
@laszlocsomor
Copy link
Contributor

Bazel should at least warn that it ignores --bazelrc flags. I'll add that warning.

I opt for a warning instead of an error, because adding an error would be an incompatible change while adding a warning isn't.

(A few months ago the Bazel team decided to adhere to our own incompatible change policy. Adding a new error is an incompatible change. Per policy I had to add an --incompatible_... flag that turns the warning into an error, then flip the flag in the next incompatible release. With our planned 3 month long stability windows, this means 3 months to roll out the flag by default, which seems too long for such a small change as an error message.)

@laszlocsomor
Copy link
Contributor

@burkpojken : Did you know bazelrc files can include other bazelrc files? Could everyone's user rc file, or every project's project-rc file include the common rc file?

@burkpojken
Copy link
Author

HI!
I am aware of that you can use include but you cannot use @ to point out another workspace e.g.
We have some flags added from a bazelrc file in our "platform" workspace that then are needed in our "application" workspaces. I really think that it would be useful to be able to use more than one --bazelrc files. Why do you think that would be a bad thing?

@laszlocsomor
Copy link
Contributor

Why do you think that would be a bad thing?

I never said it was. I don't know why Bazel supports just one user-rc file.

Current behavior: multiple --bazelrc flags allowed, first one used rest are ignored, no warning.

I'm looking at adding a warning, but it takes longer than I expected because the code is complex.
I'm not looking at adding an error, because that's an incompatible change.

@laszlocsomor laszlocsomor reopened this Aug 8, 2019
@laszlocsomor
Copy link
Contributor

Do you mean loading .bazelrc files from repositories declared in the WORKSPACE? I don't think we can support that. .bazelrc parsing is one of the first things Bazel does right after startup, way before even looking at the WORKSPACE.

@egechir
Copy link
Contributor

egechir commented Sep 30, 2019

Hello @laszlocsomor ,

We are starting to work on a feature that requires this behavior and we are from the same workgroup as @burkpojken . We understand the need for this behavior and would like to answer the following question.

Do you mean loading .bazelrc files from repositories declared in the WORKSPACE? I don't think we can support that. .bazelrc parsing is one of the first things Bazel does right after startup, way before even looking at the WORKSPACE.

The intention of this feature request is not to specify multiple .bazelrc files in terms of WORKSPACE, rather it is to specify via the path to the .bazelrc file on the bazel build command.
For example, bazel build --bazelrc=/repo/platform/file1 --bazelrc=/repo/application/file2 .
In this case, the resulting configuration should include options from both the /repo/platform/file1 and /repo/platform/file2. If there is a conflict in the value of an option between those files, then the values from the second file should be taken by bazel.

As you have stated previously, the current behavior is that the first file is used and the rest is ignored. I believe this is the behavior we want to change. We hope that it won't be a completely new behavior because today it is possible for options placed in different .bazelrc files to override each other.

Do you think this is a desirable feature for Bazel upstream ?
If so, would you reopen this issue so that we can continue the discussions ?

@laszlocsomor
Copy link
Contributor

Hi @egechir ,

Do you mean loading .bazelrc files from repositories declared in the WORKSPACE? I don't think we can support that. .bazelrc parsing is one of the first things Bazel does right after startup, way before even looking at the WORKSPACE.

The intention of this feature request is not to specify multiple .bazelrc files in terms of WORKSPACE, rather it is to specify via the path to the .bazelrc file on the bazel build command.

Thanks for clarifying. That makes sense to me, but it seems to be different than @burkpojken's request:

I am aware of that you can use include but you cannot use @ to point out another workspace

If I understand correctly, this is about referencing workspaces either in bazelrc files or in --bazelrc flags, which is infeasible with the current design.

Do you think this is a desirable feature for Bazel upstream ?

Hard question.

I understand your use case and I think it'd be a nice feature. But I don't think it's desirable for upstream. It would be if many people requested the feature and implementing it benefited many, but that doesn't seem to be the case.

There's another reason I'm hesitant to consider implementing or upstreaming this feature: I suspect certain Google-internal logging infrastructure to depend on --bazelrc having a single value, so upstreaming the change would (if my hunch is right) require changing that infrastructure, which I wouldn't do unless there's internal demand, which there seems to be none.

The workaround is to have a single bazelrc file and import other bazelrc files. I understand it's not as flexible as multiple --bazelrc flags, but it is available and it works.

If so, would you reopen this issue so that we can continue the discussions ?

Sure. If you wanted to implement this feature, I'd happily review the PR and run it against internal tests and tell you of any complications.

@egechir
Copy link
Contributor

egechir commented Oct 1, 2019

Thanks for the reply @laszlocsomor .
We will make an estimate of a possible implementation and will get back to you if we intend to make a pull request. So, the issue need not be reopened now.

@laszlocsomor
Copy link
Contributor

Thanks! (And sorry I didn't reopen when I said so, forgot it like I never wrote it..)

@emlrsua
Copy link

emlrsua commented Oct 8, 2019

I have a question. I'm working with egechir on this issue. I've tried changing the "bazelrc" definition in BazelStartupOptionsModule.java so that it has "allowMultiple = true" and so that the field is now a "List". I'm wondering where in the code it will use this data so that I can make it accept and pass on a List rather than a single String. Do you have any advice there?

@laszlocsomor
Copy link
Contributor

@emlrsua : It's not used from Java. The flag is only defined in Java to attach documentation to it:

defaultValue = "null", // NOTE: purely decorative, rc files are read by the client.

The actual flag handling is in C++, here:

if ((*value = GetUnaryOption(arg, next_arg, "--bazelrc")) != nullptr) {

@wisechengyi
Copy link
Contributor

wisechengyi commented Dec 7, 2020

Do you think this is a desirable feature for Bazel upstream ?

Hard question.
I understand your use case and I think it'd be a nice feature. But I don't think it's desirable for upstream. It would be if many people requested the feature and implementing it benefited many ..

Hi @laszlocsomor,

Please kindly reopen the issue if you think the case is valid, and redirect if you are no longer active on bazel.

I'd like to raise this again on behalf of Twitter, multiple --bazelrc on CLI would be useful for us for various reasons:

  1. Mostly importantly, we will have very long argument lists to pass to bazel query, so they have to be in a bazelrc file. From the info above, 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,

    1. import A needs to be added into$WORKSPACE/.bazelrc
    2. import B needs to be added into A

    meaning the former bazelrc file needs to know what comes next.

  2. Allowing multiple --bazelrc would be similar to how we configure Pants run in CI today, because CI needs to pass different configs so we use --pants-config-files=a,b,c,d where a,b,c,d are different config files and the latter overwrites the former, so the concept of --bazelrc would translate directly.

The reality is, per https://docs.bazel.build/versions/master/guide.html#bazelrc-the-bazel-configuration-file, multiple bazelrcs are allowed in some order of WORKSPACE bazelrc, homedir bazelrc, etc, so I think it would make sense to allow it on CLI as well.

@laszlocsomor laszlocsomor removed their assignment Dec 8, 2020
@laszlocsomor
Copy link
Contributor

@meisterT could you please re-route

@meisterT meisterT added untriaged and removed P1 I'll work on this now. (Assignee required) labels Dec 8, 2020
@meisterT meisterT reopened this Dec 8, 2020
@ulrfa
Copy link
Contributor

ulrfa commented Dec 8, 2020

Thanks for reopening @meisterT. This would be useful also for Ericsson, for the same reasons mentioned by @wisechengyi.

@philwo
Copy link
Member

philwo commented Dec 8, 2020

I'm in favor of supporting this and can't think of good reasons not to. 🤔

The concerns that Laszlo had earlier about backend systems (e.g. logging, monitoring, ...) assuming that --bazelrc is a singular option might be annoying if true, I agree with that. But as you all said, Bazel already supports multiple sources of bazelrcs already anyway, so the concept of having multiple files at least isn't new.

Would someone like to contribute a PR? My team would be happy to review and merge.

@philwo philwo added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Dec 8, 2020
@wisechengyi
Copy link
Contributor

Happy to take a stab at this. Hoping to have something out by EOY, but if anyone wants to jump in prior, please do by all means.

@wisechengyi
Copy link
Contributor

Review out at #12740

bazel-io pushed a commit that referenced this issue Mar 22, 2021
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
philwo pushed a commit that referenced this issue Apr 19, 2021
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
@aiuto aiuto added this to the flags cleanup milestone Dec 11, 2021
@aranguyen
Copy link
Contributor

aranguyen commented Feb 17, 2022

Hello @wisechengyi , I'm working on a clean up effort for flags parsing. While reviewing the behavior of the flag --blazerc=/dev/null introduced in this pr #12740, I find that the current behavior a bit surprising. Currently,

 bazel --bazelrc=<file-a> --bazelrc=/dev/null --bazelrc=<file-b> build ...

Bazel would ignore file-b instead of file-a. Considering our flag evaluation order, I would expect --blazerc=/dev/null to void content in file-a and Bazel would only consider file-b. Plus, the flag --bazelrc=/dev/null in Bazel's world currently only affect user-defined rc-files and the command above could be reduced to the following if users do not want file-b to be considered

 bazel --bazelrc=<file-a>  build ...

I'm curious to know how is this being used for your specific use case. And if we were to make the change as described above, would it break you?

@wisechengyi
Copy link
Contributor

wisechengyi commented Feb 17, 2022

Hi @aranguyen,

This was intentional at the time of the PR to be backward compatible with the status before this change.

https://docs.bazel.build/versions/main/guide.html

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

We (twitter) aren't using --bazelrc=/dev/null internally, so it wouldn't break us. IIUC your proposal would basically reverse the behavior, so other stakeholders could be affected.

Are you by any chance speaking with @michajlo as he/she has provided feedbacks in this specific area in #12740?

@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Jun 13, 2023
@github-actions
Copy link

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: feature request
Projects
None yet
Development

No branches or pull requests