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

Bump googletest for compatibility with clang-cl #13161

Merged
merged 4 commits into from
Sep 21, 2020
Merged

Bump googletest for compatibility with clang-cl #13161

merged 4 commits into from
Sep 21, 2020

Conversation

sunjayBhatia
Copy link
Member

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

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

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

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

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

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

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

@wrowe wrowe wrowe left review comments

@stonigrl stonigrl stonigrl left review comments

@mattklein123 mattklein123 mattklein123 approved these changes

@htuch htuch Awaiting requested review from htuch

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