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

fix(openssl): Update to 0.9.x #51

Merged
merged 1 commit into from
Jan 30, 2017
Merged

Conversation

martell
Copy link
Contributor

@martell martell commented Jan 26, 2017

Hey so I tried to use this on MacOS and ran into the typical rust openssl include error.
This was fixed in openssl 0.9.4.
I figured updating yup-oauth2 would help others that run into a similar issue.

This also updates hyper to 0.10.x because it uses openssl
@dermesser dermesser merged commit fe0a094 into dermesser:master Jan 30, 2017
dermesser added a commit that referenced this pull request Jan 30, 2017
@dermesser
Copy link
Owner

I just reverted the fix again :( It turns out that some other dependencies still depend on openssl 0.7, and if we try and link both 0.7 and 0.9, the build fails:

error: native library `openssl` is being linked to by more than one version of the same package, but it can only be linked once; try updating or pinning your dependencies to ensure that this package only shows up once

  openssl-sys v0.9.6

  openssl-sys v0.7.17

funnily enough I couldn't reproduce it locally yet...

@martell
Copy link
Contributor Author

martell commented Jan 30, 2017

Ahh yeah I noticed that on the build bots.
That has to do with the Cargo.lock file.
Running cargo update and committing the lock update should fix that.
I didn't want to include that in my PR for readability.

@dermesser
Copy link
Owner

Let me check again, because I did update the lockfile in db9824.

@dermesser
Copy link
Owner

aah, it's the tests.

@dermesser
Copy link
Owner

also, your patch fails a test. I'm fully reverting it now.

I have a pull request to yup-hyper-mock which will probably solve the openssl issue.

@dermesser
Copy link
Owner

when that pull request has been merged, please try reapplying your patch. And please run the tests.

Sorry for the mess, classic dependency hell.

@dermesser
Copy link
Owner

(this was the test failure: http://pastebin.com/raw/1v5MfDpq)

@Byron
Copy link
Collaborator

Byron commented Jan 31, 2017

@dermesser v2.0 of yup-hyper-mock was just published - let's hope that you can now proceed.

dermesser added a commit that referenced this pull request Jan 31, 2017
dermesser added a commit that referenced this pull request Jan 31, 2017
fix(deps): Fix openssl dependency chaos and test failure (#51)
dermesser added a commit that referenced this pull request Feb 2, 2017
This should get rid of openssl-sys 0.7 once and for all (#51).
@dermesser
Copy link
Owner

38fd851 fixes the last remains of this PR, especially the part where it changed RSA to HMAC, leading to 400 errors when requesting tokens. :-(

@martell
Copy link
Contributor Author

martell commented Feb 6, 2017

Thanks for catching that @dermesser.
I started running into that today myself and found your fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants