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.

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

cloud oauth_client: update for OpenSSL 1.1.0 compatibility #24666

Merged
merged 1 commit into from Jan 7, 2019

Conversation

perfinion
Copy link
Member

EVP_MD_CTX_cleanup was removed in OpenSSL 1.1.0. There is a call to
EVP_MD_CTX_destroy right after and _destroy will call _cleanup if
required. EVP_MD_CTX_destroy exists in OpenSSL 1.0, 1.1 and BoringSSL so
removing the call to _cleanup works everywhere.

Signed-off-by: Jason Zaman jason@perfinion.com

@perfinion perfinion added the kokoro:force-run Tests on submitted change label Jan 2, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jan 2, 2019
@Harshini-Gadige Harshini-Gadige self-assigned this Jan 2, 2019
@Harshini-Gadige Harshini-Gadige added awaiting review Pull request awaiting review and removed awaiting review Pull request awaiting review labels Jan 2, 2019
@rthadur rthadur added the size:XS CL Change Size: Extra Small label Jan 2, 2019
@perfinion
Copy link
Member Author

Related Gentoo bug: https://bugs.gentoo.org/673968

@Harshini-Gadige Harshini-Gadige added the awaiting review Pull request awaiting review label Jan 3, 2019
@Harshini-Gadige Harshini-Gadige added this to Assigned Reviewer in PR Queue via automation Jan 3, 2019
@penpornk penpornk added the kokoro:force-run Tests on submitted change label Jan 3, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jan 3, 2019
PR Queue automation moved this from Assigned Reviewer to Reviewer Requested Changes Jan 3, 2019
Copy link
Member

@penpornk penpornk left a comment

Choose a reason for hiding this comment

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

Thank you for the fix!
Could you please add comments near the EVP_MD_CTX_destroy calls that EVP_MD_CTX_destroy is renamed to EVP_MD_CTX_free in OpenSSL 1.1.0 but the old name is still retained as macro; and maybe add a link to the change notes? Just for reference.

@penpornk penpornk added kokoro:force-run Tests on submitted change and removed awaiting review Pull request awaiting review labels Jan 4, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jan 4, 2019
EVP_MD_CTX_cleanup was removed in OpenSSL 1.1.0. There is a call to
EVP_MD_CTX_destroy right after and _destroy will call _cleanup if
required. EVP_MD_CTX_destroy exists in OpenSSL 1.0, 1.1 and BoringSSL so
removing the call to _cleanup works everywhere.

Signed-off-by: Jason Zaman <jason@perfinion.com>
@perfinion
Copy link
Member Author

@penpornk Good idea, I pushed an update with the comment and a note to not change it till support is dropped for 1.0.x.
Distros will take a while to fully move over to OpenSSL 1.1 so that part is in no hurry.

PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Jan 5, 2019
Copy link
Member

@penpornk penpornk left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you very much again for the PR! :)

@penpornk penpornk added the kokoro:force-run Tests on submitted change label Jan 5, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jan 5, 2019
@perfinion perfinion added the kokoro:force-run Tests on submitted change label Jan 6, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jan 6, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jan 7, 2019
@Harshini-Gadige Harshini-Gadige added ready to pull PR ready for merge process and removed awaiting testing (then merge) labels Jan 7, 2019
@tensorflow-copybara tensorflow-copybara merged commit 5633ba3 into tensorflow:master Jan 7, 2019
PR Queue automation moved this from Approved by Reviewer to Merged Jan 7, 2019
tensorflow-copybara pushed a commit that referenced this pull request Jan 7, 2019
@perfinion perfinion deleted the openssl11 branch January 8, 2019 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes ready to pull PR ready for merge process size:XS CL Change Size: Extra Small
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

7 participants