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.

Sign up for GitHub

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

Jump to bottom

bazel: update to v5.0.0 #19706

Merged
merged 10 commits into from
Jan 31, 2022
Merged

bazel: update to v5.0.0 #19706

merged 10 commits into from
Jan 31, 2022

Conversation

kfaseela
Copy link
Contributor

#18718

Depends on envoyproxy/envoy-build-tools#157

Signed-off-by: Faseela K faseela.k@est.tech

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Jan 26, 2022
@repokitteh-read-only
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @wrowe

🐱

Caused by: #19706 was opened by kfaseela.

see: more, trace.

@phlax phlax marked this pull request as draft January 27, 2022 20:19
moderation
moderation reviewed Jan 27, 2022
View reviewed changes
.bazelrc Show resolved Hide resolved
@phlax
Copy link
Member

phlax commented Jan 27, 2022 •
edited
Loading

just tracking through trying to figure the couple of errors

one is the same error that @keith hit previously #18718 (comment)

the other is a problem with it not finding ld in the gcc test - a bit beyond my understanding of how the toolchains work, but i think the issue is the change here envoyproxy/envoy-build-tools@8e4e382#diff-7d042474e6c30aa209b372e1391823e65a962c03318c945d049e8d40fc272e1dR115 - not sure tho

@phlax
Copy link
Member

phlax commented Jan 28, 2022

im not certain - but i think that envoyproxy/envoy-build-tools#161 can fix the ld problem

@phlax
Copy link
Member

phlax commented Jan 28, 2022

@htuch @adisuissa @keith it looks like the only remaining blocker to upgrading to bazel 5 is the pgv issue

Not sure exactly why but it cant find the compiled protobuf files

/pgv_test: error while loading shared libraries: libexternal_Senvoy_Uapi_Senvoy_Sconfig_Sbootstrap_Sv3_Slibpkg_Ucc_Uproto.so: cannot open shared object file: No such file or directory
================================================================================

any clues on this one ?

@kfaseela
WIP bazel: update to v5.0.0
3576135 
Signed-off-by: Faseela K <faseela.k@est.tech>
@kfaseela
Update envoy-build-tools dependency
ac0a910 
Signed-off-by: Faseela K <faseela.k@est.tech>
@kfaseela
Remove envoy_build_tools.patch
1f19b3e 
Signed-off-by: Faseela K <faseela.k@est.tech>
@kfaseela
Fix java runtime version flag
fbd51b5 
Signed-off-by: Faseela K <faseela.k@est.tech>
@kfaseela
Fix java_runtime_version
e35e270 
Signed-off-by: Faseela K <faseela.k@est.tech>
@kfaseela
Fix gcc test
4894e3b 
Signed-off-by: Faseela K <faseela.k@est.tech>
@kfaseela
Remove outdated java flags
2353f22 
Signed-off-by: Faseela K <faseela.k@est.tech>
@phlax
Copy link
Member

phlax commented Jan 28, 2022

re the pgv/api errors, and on the basis that the fail seems only to occur in CI, im guessing that the it must be RBE/toolchain related

at the same time the fail was happening before we upgraded the toolchains, so it seems to be something related to how bazel 5 loads external deps (for RBE)

@keith
Copy link
Member

keith commented Jan 28, 2022

Can we try disabling remote execution for this CI job / action specifically, I bet this is this issue: bazelbuild/bazel#13819

@keith
Copy link
Member

keith commented Jan 28, 2022

For just this test we could add tags = ["no-remote-exec"] and see if any other tests failed the same way

@phlax
Copy link
Member

phlax commented Jan 28, 2022

..., I bet this is this issue:

the issue looks correct in terms of symptoms, but we werent having this problem before bazel 5 and that ticket has been around a while, and refers to 4.1, so im wondering what changed that made it no longer work

@keith
Copy link
Member

keith commented Jan 28, 2022

Yea good question, there are some open fixes for it though, so hopefully we won't have to live with any workaround for long

@kfaseela
Workaround attempt for test failures
76f434d 
Signed-off-by: Faseela K <faseela.k@est.tech>
@kfaseela
Re-enable precheck
dc6bde7 
Signed-off-by: Faseela K <faseela.k@est.tech>
@kfaseela kfaseela changed the title WIP bazel: update to v5.0.0 bazel: update to v5.0.0 Jan 29, 2022
@kfaseela kfaseela marked this pull request as ready for review January 29, 2022 10:01
phlax
phlax reviewed Jan 29, 2022
View reviewed changes
WORKSPACE Outdated Show resolved Hide resolved
keith
keith previously approved these changes Jan 29, 2022
View reviewed changes
@kfaseela
Incorporate review comments
d082087 
Signed-off-by: Faseela K <faseela.k@est.tech>
phlax
phlax approved these changes Jan 30, 2022
View reviewed changes
Copy link
Member

@phlax phlax left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm, thanks @kfaseela and @ALL

ill leave to @wrowe as he is the assigned maintainer to land, thanks again!

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Jan 30, 2022
wrowe
wrowe approved these changes Jan 31, 2022
View reviewed changes
Copy link
Contributor

@wrowe wrowe left a comment

Choose a reason for hiding this comment

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

LGTM. Landing it. We have the entire week to sort out any collateral dmg.

@wrowe wrowe merged commit bbcd487 into envoyproxy:main Jan 31, 2022
@keith
Copy link
Member

keith commented Jan 31, 2022

Great work, thanks for doing this!

This was referenced Jan 31, 2022
Closed
Closed
joshperry pushed a commit to joshperry/envoy that referenced this pull request Feb 13, 2022
@kfaseela
bazel: update to v5.0.0 (envoyproxy#19706)
5da4749 
* bazel: update to v5.0.0
* Updated envoy-build-tools dependency
* Removed envoy_build_tools.patch
* Fixed java runtime version flag
* Fixed gcc test
* Remove outdated java flags
* Workaround attempt for test failures
* Incorporate review comments

Signed-off-by: Faseela K <faseela.k@est.tech>
Signed-off-by: Josh Perry <josh.perry@mx.com>
phlax pushed a commit to phlax/envoy that referenced this pull request Nov 15, 2022
@kfaseela @phlax
bazel: update to v5.0.0 (envoyproxy#19706)
66e3fde 
* bazel: update to v5.0.0
* Updated envoy-build-tools dependency
* Removed envoy_build_tools.patch
* Fixed java runtime version flag
* Fixed gcc test
* Remove outdated java flags
* Workaround attempt for test failures
* Incorporate review comments

Signed-off-by: Faseela K <faseela.k@est.tech>
Signed-off-by: Ryan Northey <ryan@synca.io>
phlax pushed a commit that referenced this pull request Nov 18, 2022
@kfaseela @phlax
bazel: update to v5.0.0 (#19706)
c1183a7 
* bazel: update to v5.0.0
* Updated envoy-build-tools dependency
* Removed envoy_build_tools.patch
* Fixed java runtime version flag
* Fixed gcc test
* Remove outdated java flags
* Workaround attempt for test failures
* Incorporate review comments

Signed-off-by: Faseela K <faseela.k@est.tech>
Signed-off-by: Ryan Northey <ryan@synca.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moderation moderation moderation left review comments

@phlax phlax phlax approved these changes

@wrowe wrowe wrowe approved these changes

@keith keith keith left review comments

Assignees

@wrowe wrowe

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kfaseela @phlax @keith @wrowe @moderation