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

grpc_java_repositories: fix com_google_protobuf_javalite sha256. #5456

Merged
merged 1 commit into from Mar 11, 2019
Merged

grpc_java_repositories: fix com_google_protobuf_javalite sha256. #5456

merged 1 commit into from Mar 11, 2019

Conversation

Ubehebe
Copy link
Contributor

@Ubehebe Ubehebe commented Mar 9, 2019

curl -sL https://github.com/google/protobuf/archive/384989534b2246d413dbcd750744faab2607b516.zip | shasum -a 256
gives 79d102c61e2a479a0b7e5fc167bcfaa4832a0c6aad4a75fa7da0480564931bcc.

@Ubehebe
Copy link
Contributor Author

Ubehebe commented Mar 9, 2019

This appears to have been broken since eaca734 (early February), which bumped the javalite version but suspiciously only changed the last digit of the sha. I'm not sure why this went unnoticed; bazel query rdeps shows 14 targets in this repo depend on it.

carl-mastrangelo
carl-mastrangelo approved these changes Mar 9, 2019
View reviewed changes
Copy link
Contributor

@carl-mastrangelo carl-mastrangelo left a comment

Choose a reason for hiding this comment

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

LGTM

@carl-mastrangelo
Copy link
Contributor

@ejona86 PTAL

@ejona86
Copy link
Member

ejona86 commented Mar 11, 2019

Hmmm... that means we don't actually have a reference to it anywhere. I added a trash java_lite_proto_library and built and it detected the mismatch. protobuf-lite does have a dependency on it, but only for android builds...

It seems there should be some sort of addition to prevent regressions in the future or it should be removed, but it's not obvious what form it should take. Whatever.

@ejona86 ejona86 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Mar 11, 2019
@grpc-kokoro grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Mar 11, 2019
@ejona86
Copy link
Member

ejona86 commented Mar 11, 2019

(FYI: a common way that I figure out the new SHA is to modify the old sha and rebuilt. The build will fail and tell me the sha of what it downloaded. I then copy-paste it into the build file. That's why the last digit changed; I had expected the build to fail.)

@ejona86 ejona86 merged commit 3c24dc6 into grpc:master Mar 11, 2019
@ejona86
Copy link
Member

ejona86 commented Mar 11, 2019

@Ubehebe, thank you!

@Ubehebe
Copy link
Contributor Author

Ubehebe commented Mar 11, 2019

@ejona86 no problem. FYI, the reason I noticed this was because I use ibazel for local development. ibazel build ... was broken on this bad sha, even though nothing in my repo depends on the javalite artifact (and bazel build ... worked). So ibazel must be doing some extra analysis.

ivucica added a commit to ivucica/rules_proto that referenced this pull request Apr 7, 2019
@ivucica
Switch to newer io_grpc_grpc_java.
f7d545c 
This is required to pull in the newer com_google_protobuf_javalite.

Pristine v1.19.0 is not usable due to the incorrect SHA256 checksum;
see grpc/grpc-java#5456.
@zhangkun83 zhangkun83 added the TODO:backport PR needs to be backported. Removed after backport complete label Apr 10, 2019
@zhangkun83
Copy link
Contributor

This needs to be backported to 1.19.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 9, 2019
@ejona86 ejona86 removed the TODO:backport PR needs to be backported. Removed after backport complete label Nov 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ejona86 ejona86 ejona86 approved these changes

@carl-mastrangelo carl-mastrangelo carl-mastrangelo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Ubehebe @carl-mastrangelo @ejona86 @zhangkun83 @grpc-kokoro