Replace data-encoding with base64

[serprex]

data-encoding 2.5.0 raises their MSRV to 1.70,
meanwhile base64 has 0.21.3 reduced MSRV to 1.48 in order to be compatible with Debian Bullseye

ia0/data-encoding#77
marshallpierce/rust-base64#239
marshallpierce/rust-base64#246

This also attempts to reduce dependencies for crates depending on both hyper & tungstenite-rs where headers/postgres-protocol/rustls-pemfile use base64 0.21

[Dushistov]

I was just about to make the same pull request. The reqwest crate has the dependency for base64. And two base64 crates in dependency tree is look as overkill.
May be it is time to reconsider decision made in #330 ?

[daniel-abramov]

Maybe you're right. Maybe we should give it a try, in the worst case (if there are strong reasons not to use it), we can always get back to the data-encoding. From what I can currently see, base64 is not that more complex to use than data-encoding. I personally don't have a strong opinion. @sdroege, given the reasoning above, would you agree that base64 would be a better fit than data-encoding for now? (if so, I would be glad to merge this PR and release a new version)

[sdroege]

Personally I'm not going to touch anything related to the base64 crate but that's your decision to make :)

If the only problem is the MSRV then I would get in touch with the maintainer first, but also managing a low MSRV is practically impossible nowadays with the lack of support for it in cargo so... 🤷‍♂️ That data-encoding has raised the MSRV is actually only because cargo doesn't support handling MSRV, from what I can see. proc-macro2 has raised its MSRV but data-encoding does not use any newer versions of it and would still work fine with the older version, just that cargo will automatically select the latest version unless you have a carefully manually managed Cargo.lock. IMHO the change in data-encoding to raise the MSRV is wrong and misguided and instead this should be raised (again) with the cargo maintainers.

That reqwest is using base64 would also make it a solution to switch reqwest over to data-encoding if that's the only concern.

[sdroege]

I've added a comment about that to the data-encoding PR.

[sdroege]

Also, of course, in extension, tungstenite works fine with data-encoding < 2.5.0 too and the only reason why there are complaints now is because cargo doesn't handle MSRV in any meaningful way.

This is not even a problem in the initially mentioned case because Debian bullseye will not ship the latest proc-macro2 version or data-encoding >= 2.5.0 anyway, so you have to do updates yourself to get into the situation where a newer Rust toolchain is required and then you can also update your toolchain the same way.

[Dushistov]

This is optional dependency. So if some users prefer to use data-encoding, while others base64,
may be we can keep both cases, and to call panic! at compile time, if user enable them at the same time.

[serprex]

This is optional dependency. So if some users prefer to use data-encoding, while others base64, may be we can keep both cases, and to call panic! at compile time, if user enable them at the same time.

features are additive since different dependencies could enable both features, if both are enabled one should be preferred over the other

[sdroege]

data-encoding will likely have a lower MSRV with the next release, basically the same as the current release (1.48): ia0/data-encoding#85

That also shows how you can actually test in the CI that your crate still compiles with that Rust version and a manually maintained Cargo.lock against that version.

edit: note that data-encoding also has no dependencies at all, so the updated proc-macro2 MSRV does not affect tungstenite. That only comes in via the macros from the data-encoding-macro crate which tungstenite does not even depend on indirectly.

[Dushistov]

That reqwest is using base64 would also make it a solution to switch reqwest over to data-encoding if that's the only concern.

The reqwest is only example. The whole rust http stack should be switch to data-encoding:
rustls, cookie, axum, tower-http, headers, actix-http, jsonwebtoken.

[sdroege]

Related to the cargo issue about not considering MSRV there's a pre-RFC now for implementing that: https://internals.rust-lang.org/t/pre-rfc-msrv-aware-resolver/19871/1

[daniel-abramov]

Nice! @sdroege, thanks for elaborated answers and kicking off the whole "revert MSRV change" in data-encoding 👍

So just to summarize things: folks that are not happy with the current dependency on data-encoding, do I get you right, that the only remaining concern with data-encoding is that base64 is used by "many popular Rust crates" and so depending on tungstenite (that uses data-encoding) and some other popular crate (that most likely uses base64) would mean that both crates will be downloaded/built and that's something that you feel is suboptimal? Or are there any other problems that I missed?

[Dushistov]

@daniel-abramov , yes the main concern is one more useless dependency. A useless dependency, not in a bad way, but because exactly the same functionality is already among of the dependencies.

One more dependency is may be looks like is not big deal, but with big enough dependency graph,
each one new dependency is huge pain in the ass, for watching, updating, auditing and so on.

So I think it is time to reconsider #330 , it is not true that "base64 requires quite a bit more code", as demonstrated this PR,
but it is true that base64 with almost 90% probability exists among dependency tree of big enough project.

[davemilter]

Not sure what is going on. But if run cargo tree --all-features you can see that tungstenite-rs depend on base64 via rustls-native-certs. So why use "one more additional crate" for base64?

Not to mention that other web related crates depend on base64: cookie, tower-http, axum, reqwest, ureq, jsonwebtoken and so on.

May be the problem is just in "This branch has conflicts that must be resolved", and after conflicts were resolved the branch would be merged?

[serprex]

@davemilter it isn't a technical issue, not using base64 is rooted in disagreeing with maintainer's rude handling of a request to maintain simple api: marshallpierce/rust-base64#213

Maybe an effort can be made to get consensus in ecosystem to use data-encoding, or create a hyperium/base64 fork optimized for base64 specifically as seen on web

@seanmonstar does this seem practical to you? I can create necessary PRs. But would want to know which direction you think is best, even if that direction should be maintaining this split