Skip to content

bazel: add .bazelrc.#4771

Merged
htuch merged 5 commits intoenvoyproxy:masterfrom
PiotrSikora:bazelrc
Oct 19, 2018
Merged

bazel: add .bazelrc.#4771
htuch merged 5 commits intoenvoyproxy:masterfrom
PiotrSikora:bazelrc

Conversation

@PiotrSikora
Copy link
Copy Markdown
Contributor

This is future-proofing, since %workspace%/tools/bazel.rc won't be
automatically processed starting with Bazel 0.19 (October 2018).

Risk Level: Low
Testing: bazel build --config=clang-asan //source/... with 0.17, 0.18 and 0.19
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Piotr Sikora piotrsikora@google.com

This is future-proofing, since %workspace%/tools/bazel.rc won't be
automatically processed starting with Bazel 0.19 (October 2018).

Risk Level: Low
Testing: bazel build --config=clang-asan //source/... with 0.17, 0.18 and 0.19
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@htuch htuch self-assigned this Oct 17, 2018
@htuch
Copy link
Copy Markdown
Member

htuch commented Oct 17, 2018

@PiotrSikora Thanks. What is the implication for consuming projects when this happens, e.g. envoy-filter-example? I feel lots of folks will break.

@lizan
Copy link
Copy Markdown
Member

lizan commented Oct 17, 2018

@htuch How? bazel doesn't propagate tools/bazel.rc to consuming projects anyway.

@PiotrSikora
Copy link
Copy Markdown
Contributor Author

@htuch the implication (of change in Bazel, not this PR) is that tools/bazel.rc will be no longer processed, so projects will silently lose their defaults, and builds with --config= will break.

envoy-filter-example will need to add an extra link for .bazelrc (see: envoyproxy/envoy-filter-example#66), other projects will need to move/update their tools/bazel.rc files to .bazelrc.

@htuch
Copy link
Copy Markdown
Member

htuch commented Oct 18, 2018

Yeah, I think that's it. @lizan all these projects that have been symlinking or the like into our tools will need to change. Maybe we can maintain the ability to symlink via tools/bazel.rc in our repo with a relative symlink?

@mattklein123
Copy link
Copy Markdown
Member

Yeah, I think that's it. @lizan all these projects that have been symlinking or the like into our tools will need to change. Maybe we can maintain the ability to symlink via tools/bazel.rc in our repo with a relative symlink?

+1, I know it's not what Bazel recommends but this seems like the path of least breakage.

@lizan
Copy link
Copy Markdown
Member

lizan commented Oct 18, 2018

@htuch Ah ok that makes sense. Should we only keep this for a while and eventually remove tools/bazel.rc?

@PiotrSikora
Copy link
Copy Markdown
Contributor Author

Yeah, I think that's it. @lizan all these projects that have been symlinking or the like into our tools will need to change. Maybe we can maintain the ability to symlink via tools/bazel.rc in our repo with a relative symlink?

Except that tools/bazel.rc in the consuming projects won't be processed in the first place, rendering any workarounds in the envoy repo useless.

Each consuming project needs to add .bazelrc, in one form or another.

@htuch
Copy link
Copy Markdown
Member

htuch commented Oct 18, 2018

@PiotrSikora Ack, but in the interim this would avoid breakage. Maybe it's better to just tear the bandaid now though, since we don't have an official deprecation policy. for things like this.

@htuch
Copy link
Copy Markdown
Member

htuch commented Oct 18, 2018

@junr03 @ccaraman I know Lyft is doing internal symlink magic today for the bazelrc. As an example of an Envoy consumer, how would you like us to best handle this change? Would an envoy-announce@ e-mail be best? Should we merge with a tools/bazel.rc --> ../.bazelrc symlink and give you a deprecation period? Just merge it and clean up after the fact as it's trivial?

@PiotrSikora
Copy link
Copy Markdown
Contributor Author

@htuch tools/bazel.rc in this PR already imports %workspace%/.bazelrc using Bazel's import feature.

This breaks symlinking, since consuming projects will read their own tools/bazel.rc that links to <envoy>/tools/bazel.rc which imports %workspace%/.bazelrc (i.e. .bazelrc in their own workspace, which doesn't exist, and not <envoy>/.bazelrc), but that can be easily fixed by keeping tools/bazel.rc as the source of truth and importing it from .bazelrc instead (basically, what I had before 74bfdf9).

I can revert to that state, giving some extra time to the consuming projects, but they'll need to add .bazelrc sooner rather than later.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PiotrSikora Thanks.. can you also send an e-mail out to envoy-dev/envoy-users and also update envoy-filter-example? Appreciated!

@htuch htuch merged commit dba4622 into envoyproxy:master Oct 19, 2018
@moderation
Copy link
Copy Markdown
Contributor

@PiotrSikora is it expected that I see the following warning after this PR?

WARNING: Duplicate rc file: ~/Library/envoyproxy/envoy/tools/bazel.rc is read multiple times, most recently imported from ~/Library/envoyproxy/envoy/.bazelrc
WARNING: Processed legacy workspace file ~/Library/envoyproxy/envoy/tools/bazel.rc. This file will not be processed in the next release of Bazel. Please read https://github.com/bazelbuild/bazel/issues/6319 for further information, including how to upgrade.

@PiotrSikora
Copy link
Copy Markdown
Contributor Author

@moderation yes, it's a WARNING in Bazel 0.18, since it reads both old (tools/bazel.rc) and new (.bazelrc) locations, it's harmless, but unfortunately there is no way around that, as far as I know.

@PiotrSikora
Copy link
Copy Markdown
Contributor Author

@htuch sure, any preference on sending email to envoy-dev@, envoy-users@ or both?

@htuch
Copy link
Copy Markdown
Member

htuch commented Oct 19, 2018

@PiotrSikora send to both I think, since envoy-users@ might not be doing much Envoy dev but still have some inhouse filters..

soya3129 pushed a commit to soya3129/envoy that referenced this pull request Oct 19, 2018
This is future-proofing, since %workspace%/tools/bazel.rc won't be
automatically processed starting with Bazel 0.19 (October 2018).

Risk Level: Low
Testing: bazel build --config=clang-asan //source/... with 0.17, 0.18 and 0.19
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

Signed-off-by: Yang Song <yasong@yasong00.cam.corp.google.com>
yuval-k added a commit to solo-io/envoy-gloo that referenced this pull request Dec 6, 2018
Add .bazelrc.

See envoyproxy/envoy#4771

Co-Authored-By: yuval-k <yuval.kohavi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants