Skip to content

Bump googletest for compatibility with clang-cl#13161

Merged
mattklein123 merged 4 commits intoenvoyproxy:masterfrom
greenhouse-org:update-googletest
Sep 21, 2020
Merged

Bump googletest for compatibility with clang-cl#13161
mattklein123 merged 4 commits intoenvoyproxy:masterfrom
greenhouse-org:update-googletest

Conversation

@sunjayBhatia
Copy link
Member

@sunjayBhatia sunjayBhatia commented Sep 17, 2020

Commit Message: Bump googletest

For compatibility with Windows clang-cl
Additional Description: Submitting this independently to determine that this revision of googletest does not introduce breakage on any toolchain. See issue: google/googletest#2490. Step to address #11974
Risk Level: Low
Testing: Compiles with Windows clang-cl
Docs Changes: N/A
Release Notes: N/A

@wrowe @sunjayBhatia
Bump googletest for compatibility with clang-cl
7aac48f 
- Submitting this independently to determine that this revision of
  googletest does not introduce breakage on any toolchain

- See issue: google/googletest#2490

Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com>
Co-authored-by: William A Rowe Jr <wrowe@vmware.com>
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Signed-off-by: William A Rowe Jr <wrowe@vmware.com>
@repokitteh-read-only
Copy link

repokitteh-read-only bot commented Sep 17, 2020

CC @envoyproxy/dependency-watchers: FYI only for changes made to (bazel/repository_locations\.bzl)|(api/bazel/repository_locations\.bzl)|(.*/requirements\.txt).

🐱

Caused by: #13161 was opened by sunjayBhatia.

see: more, trace.

@sunjayBhatia
Copy link
Member Author

sunjayBhatia commented Sep 17, 2020

cc @wrowe

@wrowe
Copy link
Contributor

wrowe commented Sep 17, 2020

@htuch This is the googletest bump required for clang-cl on Windows. Let's see how it pans out on all existing CI pipelines, then please expedite merging to master on success.

@sunjayBhatia
Fix compilation
58fe966 
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
htuch
htuch reviewed Sep 18, 2020
View reviewed changes
bazel/repository_locations.bzl
sha256 = "9dc9157a9a1551ec7a7e43daea9a694a0bb5fb8bec81235d8a1e6ef64c716dcb",
strip_prefix = "googletest-release-{version}",
urls = ["https://github.com/google/googletest/archive/release-{version}.tar.gz"],
# 2020-09-10
Copy link
Member

@htuch htuch Sep 18, 2020

Choose a reason for hiding this comment

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

Thanks for the update. Please include an explanation as per policy, see https://github.com/envoyproxy/envoy/blob/master/DEPENDENCY_POLICY.md

Versions should prefer release versions over master branch GitHub SHA tarballs. A comment is necessary if the latter is used. This comment should contain the reason that a non-release version is being used and the YYYY-MM-DD when the last update was performed.

Copy link
Member

@mattklein123 mattklein123 Sep 18, 2020

Choose a reason for hiding this comment

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

@wrowe ping can you add the comment?

/wait

Copy link
Contributor

@wrowe wrowe Sep 20, 2020

Choose a reason for hiding this comment

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

Indeed, I had misunderstood the exception r.e. CPE's and test-only elements of the build, but hope you find the new message appropriate. Not really certain if there will be a 1.11 "release" given the general google philosophy of consuming the dev branch, and with respect to our test architecture, any of their bleed efforts to identify defects should be picked up fairly frequently (although probably not daily.)

@wrowe wrowe requested a review from htuch September 18, 2020 21:45
@wrowe
Copy link
Contributor

wrowe commented Sep 18, 2020

As this is a blocking issue for other PR's we would ask @envoyproxy/senior-maintainers if anyone has cycles to give this a green light and pull it?

@mattklein123 mattklein123 self-assigned this Sep 18, 2020
@wrowe
Merge remote-tracking branch 'origin/master' into update-googletest
c736033 
Signed-off-by: William A Rowe Jr <wrowe@vmware.com>
@wrowe
Clarify reason for electing a non-release point per DEPENDENCY_POLICY
6f2421d 
Signed-off-by: William A Rowe Jr <wrowe@vmware.com>
@wrowe
Copy link
Contributor

wrowe commented Sep 20, 2020

/retest

@repokitteh-read-only
Copy link

repokitteh-read-only bot commented Sep 20, 2020

Retrying Azure Pipelines, to retry CircleCI checks, use /retest-circle.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13161 (comment) was created by @wrowe.

see: more, trace.

mattklein123
mattklein123 approved these changes Sep 21, 2020
View reviewed changes
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!

@mattklein123 mattklein123 merged commit 25fe01c into envoyproxy:master Sep 21, 2020
@wrowe wrowe deleted the update-googletest branch September 21, 2020 13:39
stonigrl
stonigrl reviewed Aug 29, 2022
View reviewed changes
Copy link

@stonigrl stonigrl left a comment

Choose a reason for hiding this comment

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

🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

@htuch htuch Awaiting requested review from htuch

+2 more reviewers

@wrowe wrowe wrowe left review comments

@stonigrl stonigrl stonigrl left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@mattklein123 mattklein123

@htuch htuch

Labels

area/build

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sunjayBhatia @wrowe @mattklein123 @htuch @stonigrl