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

Switch from curl to hyper, refactor JSON handling #67

Closed
wants to merge 1 commit into from

Conversation

killercup
Copy link
Owner

I had some time this morning, so I finally replaced curl with hyper and along the way got rid of a lot of HTTP/JSON handling. Might also help with #55.

@Seeker14491, @iopq, could you test this on Windows?

@Seeker14491
Copy link

I've run into two issues while attempting to install this. The first is that when I tried to install via the 'cargo install' command, I get the message

multiple packages with binaries found: cargo-edit, cargo-list-empty-test-fixture, cargo-list-test-fixture, cargo-rm-test-fixture

I don't know how to proceed from here, so I've instead tried cloning the branch and ran cargo build --release, but I get a build error compiling openssl.

@killercup
Copy link
Owner Author

I can see what the first issue is about (and just renaming the files should do it), but I have no idea how to address the OpenSSL issue. Cannot open include file: 'openssl/ssl.h' looks like a problem locating OpenSSL's C library, which I assume it should be provided by openssl-sys?

I vaguely remember having similar problems on a Mac when installing some Ruby gem but the solution was to install OpenSSL manually (using brew and with brew link --force openssl IIRC).

@cnd
Copy link

cnd commented Jan 18, 2016

failed to run custom build command for openssl v0.7.4

I think it could be problem of openssl cargo script

@killercup
Copy link
Owner Author

Thanks for you quick reply, @Heather. I have no way of verifying that as I have no Windows machine here right now. Could you maybe investigate further?

@cnd
Copy link

cnd commented Jan 18, 2016

there is discussion about it sfackler/rust-openssl#143 and ways to manually get ssl provided, yes I will try

@cnd
Copy link

cnd commented Jan 20, 2016

@killercup with some reason I end with strange linking issue, it wants static linking even if I didn't specified env variable for static library
LINK : fatal error LNK1181: cannot open input file 'ssl32.lib' but I don't even have statlic library seems like

@cnd
Copy link

cnd commented Jan 20, 2016

okay, renamed ssleay32.lib to ssl32.lib and libeay32.lib to eay32.lib ... (those steps really weak)

and it works now!

@killercup
Copy link
Owner Author

Wow, thanks for investigating, Heather! That sounds pretty crazy. Is there
a logic to this, and could this be fixed upstream?

(A solution for cargo-edit might be to just offer correct compiled
executables but it's not something I really want to maintain.)
Heather notifications@github.com schrieb am Mi., 20. Jan. 2016 um 08:51:

okay, renamed ssleay32.lib to ssl32.lib and libeay32.lib to eay32.lib ...
(those steps really weak)

and it works now!


Reply to this email directly or view it on GitHub
#67 (comment).

@cnd
Copy link

cnd commented Jan 20, 2016

@killercup I've added notes about my experience here: sfackler/rust-openssl#337

@homu
Copy link
Contributor

homu commented Feb 3, 2016

☔ The latest upstream changes (presumably #71) made this pull request unmergeable. Please resolve the merge conflicts.

@caulagi
Copy link
Contributor

caulagi commented Mar 3, 2016

Given that there are problems building curl also on Windows, would it make sense to merge this to master? I checked the branch on a Mac and it worked fine for me.

@killercup
Copy link
Owner Author

Thanks for getting this PR back on my radar, @caulagi.

Are there any news on building hyper on Windows? If not, given that

  • curl and hyper work equally well on Linux and Mac (correct me if I'm wrong),
  • and cargo itself uses curl,
  • and we want to integrate cargo-edit into cargo eventually,

the only reason of merging this is that the code is cleaner. This is not enough for me right now.

@caulagi
Copy link
Contributor

caulagi commented Mar 3, 2016

@killercup thanks, seems valid reasons for me. I don't have access to a Windows machine, unfortunately.

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

Successfully merging this pull request may close these issues.

5 participants