reqwest 0.10 and http 0.2 about oauth2-rs

[ramosbugs]

if possible, I'd like to switch entirely to http 0.2, as long as that works with both reqwest versions. I would have preferred to switch to reqwest 0.10 entirely as well, but it doesn't maintain backward compatibility with futures 0.1.

[ramosbugs]

I tried upgrading to reqwest 0.10 and http 0.2 in 81e2a87, but that breaks compatibility with rustc < 1.39 because http 0.2 depends on bytes 0.5, which uses newer language features not supported by older compiler versions.

for now, I think I'm going to leave the external HTTP client interface as http 0.1, and have the reqwest 0.10 client convert to http 0.2 types internally

[ramosbugs]

e.g.:

error: `core::slice::<impl [T]>::len` is not yet stable as a const fn
   --> /Users/daramos/.cargo/registry/src/github.com-1ecc6299db9ec823/bytes-0.5.3/src/bytes.rs:121:18
    |
121 |             len: bytes.len(),
    |                  ^^^^^^^^^^^

[dthul]

Seems like a reasonable approach. I have some crude version of that http type conversion code in my own project (since I already use reqwest 0.10 with this crate through a custom async client implementation). I unfortunately won't have time in January to make that into anything mergeable though.

[gzp79]

Just from curiosity. As it is a major version change, why not update the minimal required rust version as well ?
Here is a branch where I made a few a minor changes and it depends on reqwest 0.10 and http 0.2:
https://github.com/gzp-crey/oauth2-rs/tree/http_hack
(The only change was the use of redirect::Policy instead of RedirectPolicy)

[dthul]

(The only change was the use of redirect::Policy instead of RedirectPolicy)

If this crate wants to keep support for reqwest 0.9 (which I guess is the goal at least for the near future), then there are a few more changes necessary. Try to compile with the reqwest-09 feature flag and you will see that there will be some http version mismatches.

[gzp79]

Ok, thanks, I see :(.

[gzp79]

I've created a new PR where http versiosn and reqwest version are tied together.
Do you think if this is a good compromise as projects targeting reqwest 0.10 are also using http 0.2 ?

[ramosbugs]

I've created a new PR where http versiosn and reqwest version are tied together.
Do you think if this is a good compromise as projects targeting reqwest 0.10 are also using http 0.2 ?

Unfortunately, I don't think this approach meets the requirements since http is still needed even without reqwest (see #82 (comment)).

[gzp79]

And what about a solution where http version is also a feature ?
The default is http 0.1, reqwest 0.9 (and later at some point http 0.2, future 0.3, reqwest 0.10)
Pro: it's almost done only the feature in the toml file have to be modified.
Contra: there are feature sets those won't compile. Like http 0.1with reqwest 0.10.
I don't know if such dependency/restriction can be expressed through the features.

Converting between http versions on the external interface does not seem a good solution. Especially if it is the 0.1 version. For new projects and updates we are forced to use two version of the same crate and perform conversions to and back.

[ramosbugs]

any time a crate re-exports symbols from some other crate, there are going to be version issues across major releases of the depended-upon crate. some users will want the newer version, and some will want the older version. #81 should make it much easier to use whichever http version this crate uses, since it's now exported explicitly.

the feature flags for this crate are already fairly complex, and adding two more to toggle between http versions would be adding too much additional complexity. I wanted to upgrade the external interface to 2.0, but that would require everyone to be on rustc 1.39 or newer, which would probably cause issues for some other set of users.

for 3.x, let's leave the external interface as http 1.0 with an explicit re-export, and for the next major version (4.x), I'll probably migrate the interface to http 2.0.

[gzp79]

Ok, than I have to wait for your fix and there is no "quick fix". :(
(Sry for urging the fix, but this is the last dependency blocking my PR in another project to use actix 2.0 cleanly from crates.io)

[ramosbugs]

which fix are you waiting on? reqwest 0.10 support?

[gzp79]

yes, to have reqwest 0.10

[gzp79]

thanks a lot