Skip to content

bazel: Update googleurl#18249

Merged
mattklein123 merged 5 commits intoenvoyproxy:mainfrom
keith:with-android
Sep 30, 2021
Merged

bazel: Update googleurl#18249
mattklein123 merged 5 commits intoenvoyproxy:mainfrom
keith:with-android

Conversation

@keith
Copy link
Member

@keith keith commented Sep 24, 2021

This was reverted previously due to an issue with older android NDK versions, the last commit works around that issue until bazel supports newer versions.

Signed-off-by: Keith Smiley keithbsmiley@gmail.com

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Sep 24, 2021
@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).

🐱

Caused by: #18249 was opened by keith.

see: more, trace.

@keith
Copy link
Member Author

keith commented Sep 24, 2021

@mattklein123 @wrowe @buildbreaker this is the Android only part of the fix (targeting Matt's revert branch) I think with this we should be unblocked to land these

buildbreaker
buildbreaker previously approved these changes Sep 24, 2021
Copy link

@buildbreaker buildbreaker left a comment

Choose a reason for hiding this comment

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

Yep! This was what I was going for

@keith
Copy link
Member Author

keith commented Sep 24, 2021

Testing on envoy-mobile CI envoyproxy/envoy-mobile#1830

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Awesome, thanks a ton for tracking this down.

I closed my other PR so can you just bump the dep at the same time and we can review/merge it all at once?

/wait

@buildbreaker
Copy link

@mattklein123 This PR was intended to merge into your PR 😬

@keith keith changed the base branch from gurl_update to main September 24, 2021 02:43
@keith keith dismissed buildbreaker’s stale review September 24, 2021 02:43

The base branch was changed.

@keith keith changed the title bazel: Fix googleurl on Android bazel: Update googleurl Sep 24, 2021
@keith keith requested a review from mattklein123 September 24, 2021 02:44
mattklein123
mattklein123 previously approved these changes Sep 24, 2021
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@moderation
Copy link
Contributor

Hhhmmm. The error is the incorrect date for rules_go

github_release:  GitHubRelease(organization='bazelbuild', project='rules_go', version='v0.27.0', tagged=True)
*WARNING* io_bazel_rules_go has a newer release than v0.27.0@<2021-03-18 15:02:42>: v0.28.0@<2021-07-06 23:21:45>
io_bazel_rules_go has a GitHub release date 2021-03-18
An error occurred while processing ./bazel/repository_locations.bzl, please verify the correctness of the metadata: Mismatch with metadata release date of 2021-03-17

mattklein123 and others added 5 commits September 29, 2021 11:46
…nvoyproxy#17958)"

This reverts commit 338a42c.

Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks!

@keith
Copy link
Member Author

keith commented Sep 30, 2021

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #18249 (comment) was created by @keith.

see: more, trace.

@mattklein123
Copy link
Member

Coverage seems like a flake to me.

@mattklein123
Copy link
Member

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #18249 (comment) was created by @mattklein123.

see: more, trace.

@mattklein123
Copy link
Member

Somehow line coverage for lua seems to be persistently failing on this PR which I really don't understand. Can you take a quick look at the line diffs from coverage to see if anything pops up? You can see the coverage report in CI and here is the main report: https://storage.googleapis.com/envoy-postsubmit/main/coverage/index.html. cc @lizan for any ideas on this.

@keith
Copy link
Member Author

keith commented Sep 30, 2021

image

I imagine the tests have some async behavior that causes this to be hit or not, doesn't appear related to this change

@mattklein123
Copy link
Member

Yup OK I'm fine force merging. We can sort out coverage on main. Thank you!

@mattklein123 mattklein123 merged commit e9ee985 into envoyproxy:main Sep 30, 2021
@keith keith deleted the with-android branch September 30, 2021 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deps Approval required for changes to Envoy's external dependencies

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants