Unwind the __fixed_rust business

[davidben]

This gives the unsuffixed names the correct signatures, and deprecates the suffixed names in favor of the now fixed unsuffixed ones. The suffixed names now forward to the unsuffixed ones.

(This isn't a semver-compatible change. But @alex tells me you all might be bumping that soon, so I'm uploading this to add to the queue. Also uploading to run it through CI because I'm sure I've gotten one of the myriad of configurations wrong. 😄)

[sfackler]

We'll probably be bumping the openssl crate soon, but openssl-sys may be staying as-is unfortunately. Cargo prevents multiple semver-incompatible copies of -sys crates from existing in a dependency tree, so going from openssl-sys 0.9 to 0.10 is hugely disruptive to the ecosystem :(

It has been a while since I've checked in on the state of things though, so there may be ways of cleaning this up anyway.

[alex]

@sfackler I thought we wanted to do it for foreign-type, is that only in openssl?

[davidben]

The motivation here was that #1945 left out doing anything about https://github.com/sfackler/rust-openssl/blob/master/openssl/src/ssl/bio.rs#L193. Making BIO and BIO_METHOD opaque isn't as high priority for me as the others, but still you should be using the actual APIs for this.

The problem is we'd otherwise have to add this __fixed_rust mess to bssl-sys, and I'm not excited about bssl-sys having to replicate the old mistakes here. There's a few places where rust-openssl has cfg(boringssl) branches, but that's also pretty tedious.

Particularly for a low-level crate like openssl-sys, getting in a state where you can never fix bugs sounds unfortunate.

[davidben]

Not just low-level, but also an FFI. Rust has numerous deficiencies around FFI, so it's very likely the correct ways to do things will shift over time. (E.g. there's an issue where Rust and C's incompatible empty slice representations mean basically every FFI crate is unsound, including this one. I still need to write that up.)

[davidben]

(Looks like I got some business wrong around unsafe with some of the functions. Will stare at that later and fix.)

[sfackler]

Yep, foreign_types is only used in the openssl crate.

One option we could take is to just make the change on 0.9. It'll break people that use an old openssl-sys against a new openssl, but that's probably preferrable to the major bump mess?

[alex]

Is there anything we can do quantify this -- e.g., scan the top N crates on crates.io that depend directly on openssl-sys and see if any of them have an older -sys but a new openssl?

…

On Sat, Jun 3, 2023 at 3:00 PM Steven Fackler ***@***.***> wrote: Yep, foreign_types is only used in the openssl crate. One option we *could* take is to just make the change on 0.9. It'll break people that use an old openssl-sys against a new openssl, but that's probably preferrable to the major bump mess? — Reply to this email directly, view it on GitHub <#1946 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBFN4URDZ4WWON3NPJDXJOCTRANCNFSM6AAAAAAYZMOJKA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- All that is necessary for evil to succeed is for good people to do nothing.

[davidben]

(Looks like I got some business wrong around unsafe with some of the functions. Will stare at that later and fix.)

Resolved.

[alex]

So, I've attempted to build a bunch of ecosystem crates with -Z minimal-versions to see what happens if you play with openssl-sys and openssl versions independently. My conclusion is that basically everyone's minimal versions are already broken. rust-native-tls is an example where I can't even figure out what the correct minimums are.

rust-lang/git2-rs#960 and alexcrichton/curl-rust#503 are examples of PRs fixing this.

I want to try to make the compatibility matrix explicit here:

	Old openssl	New openssl
Old openssl-sys	n/a[0]	Broken[1]
New openssl-sys	Hard[2]	Fine[3]

0. Who cares :-)
1. New openssl will try to use the un-suffixed names with the new signature, which old openssl-sys doesn't support. But this one is rendered impossible because openssl can just set the minimum openssl-sys version.
2. This one depends how old the openssl is. If it's just a little old, you're calling the __fixed_rust versions, which are maintained, so you're fine. If it's really old then you're assuming you can call the un-suffixed with their current signature, which will break.
3. Obviously :-)

So all of this really hinges on: What version of openssl started using the suffixed names, and not the unsuffixed ones? And does anyone still use that version of openssl?

Did I get my analysis right? Does anyone know what version that was? I can go look at the mininums for widely used crates + download data from there.

[alex]

I believe 1ce53a6 is the commit that introduced the distinction, meaning https://github.com/sfackler/rust-openssl/releases/tag/openssl-v0.10.39 is the first release to contain it.

[alex]

Ok, I spent some time with the crates.io, it looks like for all of 2023, 92% of downloads of openssl are for versions >= 0.10.39. Looking at just June 2023 (i.e., about a week), 93.3% of downloads of openssl are for versions >= 0.10.39.

Methodology

If you're curious how I did this:

1. Download all the data from https://crates.io/data-access
2. Convert the CSVs to parquet with https://github.com/alex/csv-to-parquet (nb, currently hardcoded for TSVs, so requires editing the code)
3. Get the crate_id for openssl from crates.parquet - 231
4. Get the version_id for 0.10.39 from versions.parquet - 542827
5. Compute total downloads in the period: select sum(download_count) from (select v.id, v.num, sum(vd.downloads::int) as download_count from 'version_downloads.parquet' vd inner join 'versions.parquet' v ON vd.version_id = v.id WHERE v.crate_id = 231 AND vd.date LIKE '2023-%' group by v.id, v.num order by v.id::int asc);
6. Compute downloads of versions >= 0.10.39: select sum(download_count) from (select v.id, v.num, sum(vd.downloads::int) as download_count from 'version_downloads.parquet' vd inner join 'versions.parquet' v ON vd.version_id = v.id WHERE v.crate_id = 231 AND vd.date LIKE '2023-%' and v.id::int >= 542827 group by v.id, v.num order by v.id::int asc);
7. Do division

[alex]

I've now got myself a little data pipeline for computing these stats on demand. Here's the percent of openssl downloads that are >= 0.10.39 by month:

2023-04 | 92.72%
2023-05 | 93.58%
2023-06 | 94.26%

[alex]

We're now up to 97.59% of downloads of openssl being >= 0.10.39 in the last month.