Skip to content

Conversation

@panicbit
Copy link

@panicbit panicbit commented May 23, 2016

This change is Reviewable

Ok(Firebase {
url: Arc::new(url),
})
Firebase::from_url(url)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like this refactor 👍

@illegalprime
Copy link
Collaborator

Hi @panicbit thanks for the PR!
I will take a longer look at it soon, can you tell me why you thought to switch to hyper?

@panicbit
Copy link
Author

panicbit commented Jun 1, 2016

I decided to switch to hyper to get rid of the dependency on curl, though, it turns out to probably not be worth it. A lot of the Request/Url caching got lost in the process 😞 . I should probably rebase and single out the non-hyper changes.

@illegalprime
Copy link
Collaborator

It is because curl is not compatible with Windows? I remember some trouble with that.
Anyway will take a look soon, I totally forgot why Arc had to be there, but I'm glad to see it gone.

@panicbit
Copy link
Author

panicbit commented Jun 1, 2016

I'm not using Windows primarily but AFAIK setting up native dependencies is a pain.

@illegalprime
Copy link
Collaborator

Oh so we shouldn't depend on libcurl? That makes sense.

@panicbit
Copy link
Author

panicbit commented Jun 1, 2016

I'm not saying that you shouldn't, but I guess it would be nice to gradually move towards pure Rust crates. Hyper sadly still has a dependency on openssl.

@illegalprime
Copy link
Collaborator

You're right that pure rust crates are ideal, I'll think about it and get back to you.

let opts = vec![ (AUTH, auth_token) ];
url.set_query_from_pairs(opts.into_iter());
let mut url = try!(Url::parse(&url));
url.query_pairs_mut().append_pair(AUTH, auth_token).finish();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is good too!

@phrohdoh
Copy link

I cannot build this.

tmba:rust-firebase thill $ cargo build
    Updating registry `https://github.com/rust-lang/crates.io-index`
 Downloading hyper v0.9.10
 Downloading language-tags v0.2.2
 Downloading httparse v1.1.2
 Downloading cookie v0.2.5
 Downloading typeable v0.1.2
 Downloading openssl-verify v0.1.0
 Downloading num_cpus v0.2.13
 Downloading mime v0.2.2
 Downloading traitobject v0.0.1
 Downloading openssl v0.7.14
 Downloading solicit v0.4.4
 Downloading unicase v1.4.0
 Downloading openssl-sys-extras v0.7.14
 Downloading openssl-sys v0.7.17
 Downloading hpack v0.2.0
 Downloading rustc_version v0.1.7
 Downloading semver v0.1.20
   Compiling log v0.3.6
   Compiling lazy_static v0.2.1
   Compiling bitflags v0.7.0
   Compiling libc v0.2.16
   Compiling httparse v1.1.2
   Compiling gcc v0.3.35
   Compiling semver v0.1.20
   Compiling unicode-normalization v0.1.2
   Compiling num_cpus v0.2.13
   Compiling mime v0.2.2
   Compiling pkg-config v0.3.8
   Compiling openssl v0.7.14
   Compiling language-tags v0.2.2
   Compiling hpack v0.2.0
   Compiling rustc_version v0.1.7
   Compiling openssl-sys v0.7.17
   Compiling unicase v1.4.0
   Compiling winapi-build v0.1.1
   Compiling traitobject v0.0.1
   Compiling solicit v0.4.4
Build failed, waiting for other jobs to finish...
error: failed to run custom build command for `openssl v0.7.14`
process didn't exit successfully: `/Users/thill/projects/play/rust/rust-firebase/target/debug/build/openssl-5464f8f6e728c35a/build-script-build` (exit code: 101)
--- stdout
TARGET = Some("x86_64-apple-darwin")
OPT_LEVEL = Some("0")
PROFILE = Some("debug")
TARGET = Some("x86_64-apple-darwin")
debug=true opt-level=0
HOST = Some("x86_64-apple-darwin")
TARGET = Some("x86_64-apple-darwin")
TARGET = Some("x86_64-apple-darwin")
HOST = Some("x86_64-apple-darwin")
CC_x86_64-apple-darwin = None
CC_x86_64_apple_darwin = None
HOST_CC = None
CC = None
HOST = Some("x86_64-apple-darwin")
TARGET = Some("x86_64-apple-darwin")
HOST = Some("x86_64-apple-darwin")
CFLAGS_x86_64-apple-darwin = None
CFLAGS_x86_64_apple_darwin = None
HOST_CFLAGS = None
CFLAGS = None
running: "cc" "-O0" "-ffunction-sections" "-fdata-sections" "-g" "-m64" "-fPIC" "-o" "/Users/thill/projects/play/rust/rust-firebase/target/debug/build/openssl-5464f8f6e728c35a/out/src/c_helpers.o" "-c" "src/c_helpers.c"
cargo:warning=src/c_helpers.c:1:10: fatal error: 'openssl/ssl.h' file not found
cargo:warning=#include <openssl/ssl.h>
cargo:warning=         ^
cargo:warning=1 error generated.
ExitStatus(ExitStatus(256))


command did not execute successfully, got: exit code: 1



--- stderr
thread 'main' panicked at 'explicit panic', /Users/thill/.cargo/registry/src/github.com-1ecc6299db9ec823/gcc-0.3.35/src/lib.rs:897
stack backtrace:
   1:        0x10270c969 - std::sys::backtrace::tracing::imp::write::h29f5fdb9fc0a7395
   2:        0x102712c70 - std::panicking::default_hook::_{{closure}}::h2cc84f0378700526
   3:        0x102711f94 - std::panicking::default_hook::hbbe7fa36a995aca0
   4:        0x102712582 - std::panicking::rust_panic_with_hook::h105c3d42fcd2fb5e
   5:        0x1026e2807 - std::panicking::begin_panic::h5c54e97e5cda1cc4
   6:        0x1026fd824 - gcc::fail::h61b641a0af002bb7
   7:        0x1026fd45e - gcc::run::hbeab7a383c7357cf
   8:        0x1026f6f42 - gcc::Config::compile_object::he1280a23d2ea58fb
   9:        0x1026f68b9 - gcc::Config::compile_objects::hdb20fd8be1b6199c
  10:        0x1026f5e90 - gcc::Config::compile::h8371c79ea2fe64ff
  11:        0x1026dad62 - build_script_build::main::he5071e1b00fcfe0e
  12:        0x10271223c - std::panicking::try::call::h5df3ac2979db3c90
  13:        0x10271325a - __rust_maybe_catch_panic
  14:        0x102711a4d - std::rt::lang_start::hfe9ab243c60ffb9b
  15:        0x1026daf29 - main

@nelsonjchen
Copy link

For my own crate, I've switched to using reqwest. That may not be applicable here but I did it to use its default of using native-tls. That's the kind of stuff that'll knock off the OpenSSL error on the Mac. I don't recall that really existing at the start of this PR or the post above.

@illegalprime illegalprime self-requested a review March 30, 2017 18:45
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.

4 participants