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

syn 2.0 and MSRV #3113

Closed
alex opened this issue Apr 21, 2023 · 19 comments · Fixed by #3239
Closed

syn 2.0 and MSRV #3113

alex opened this issue Apr 21, 2023 · 19 comments · Fixed by #3239

Comments

@alex
Copy link
Contributor

alex commented Apr 21, 2023

👋

Right now, for pyca/cryptography, we're in an awkward space where half our dependencies use syn 2.0, and half use 1.0 (including pyo3). This is unfortunate from a compilation time perspective. The reason not to upgrade, and presumably why pyo3 hasn't, is that syn 2.0 raises their MSRV to 1.56.

I wanted to start a conversation on at what point it would make sense to raise pyo3's MSRV, and thus be able to use syn 2.0?

For pyca/cryptography, our upcoming release will raise our own MSRV from 1.48 to 1.56. You can see some of the data/reasoning behind this here: pyca/cryptography#7172, pyca/cryptography#8195, and pyca/cryptography#8587

@adamreichold
Copy link
Member

I was expecting us to bump our MSRV to 1.63 when Debian Bookworm becomes stable in "mid 2023" as per https://github.com/PyO3/pyo3/blob/main/Contributing.md#python-and-rust-version-support-policy

@adamreichold
Copy link
Member

Alpine 3.17 has 1.64 but RHEL 9 seems to carry 1.62, so we would actually end up with 1.62 instead of 1.63 following that policy.

@alex
Copy link
Contributor Author

alex commented Apr 21, 2023

Based on our experience with /metrics for pyca/cryptography, I would advocate for a slightly smaller jump.

Based on our metrics, our plan is to do 1.56.0 as MSRV for our next release, and then 1.60 as our expected (subject to change) MSRV.

@adamreichold
Copy link
Member

Out of curiosity and to understand if our policy needs to be amended: Do your metrics include any indication of what users in the 1.56 to 1.61 bracket are running on or how they installed Rust?

@alex
Copy link
Contributor Author

alex commented Apr 21, 2023

We use metrics from PyPI, so we hvae rustc version, OS, and other metdata. Here's the script I used to compute the data for us:

import datetime
import decimal

from packaging.version import Version

import pandas


MIN_RUSTC_VERSION = 48
MAX_RUSTC_VERSION = 68


rustc_results = pandas.io.gbq.read_gbq("""
SELECT
  details.rustc_version,
  COUNT(*) AS download_count,
  ROUND(100 * COUNT(*) / (SUM(COUNT(*)) OVER ()), 2) AS download_percent
FROM
  `bigquery-public-data.pypi.file_downloads`
WHERE
  file.PROJECT = 'cryptography'
  AND file.type = 'sdist'
  AND (file.version LIKE '39.%')
  AND DATE(timestamp) > DATE_SUB(CURRENT_DATE(), INTERVAL 30 DAY)
GROUP BY
  details.rustc_version
ORDER BY
  download_count DESC
LIMIT
  100
""", project_id="blog-193401").to_dict("records")

for d in rustc_results:
    d["download_count"] = decimal.Decimal(d["download_count"])
total = sum(d["download_count"] for d in rustc_results)
with_rustc = sum(d["download_count"] for d in rustc_results if d["rustc_version"])
print(f"As of: {datetime.datetime.utcnow()}")
print(f"Percent with rustc_version: {round(100 * with_rustc / total, 3)}%")
print(f"Total with rustc_version: {with_rustc}")

for minor_version in range(MIN_RUSTC_VERSION, MAX_RUSTC_VERSION + 1):
    target_version = Version(f"1.{minor_version}.0")
    downloads = sum(
        round(100 * d["download_count"] / with_rustc, 1)
        for d in rustc_results
        if d["rustc_version"]
        and Version(d["rustc_version"].strip()) >= target_version
    )
    print(f"{target_version}: {downloads}%") 

(Note that the BigQuery auth might be broken ATM? It stopped working a few weeks ago, so I don't have more recent metrics)

Here's recent data on what OS+rustc combos are most frequently in the "has 1.56 but doesn't have 1.60" bucket:

Screenshot 2023-04-21 at 10 47 30

@adamreichold
Copy link
Member

Interesting, so mostly outdated Alpine versions. So glad everyone keeps their container image builds fresh as daisies...

@alex
Copy link
Contributor Author

alex commented Apr 21, 2023

It likely reflects that musllinux wheel support requires a newer pip, so those users also have an older pip.

@davidhewitt
Copy link
Member

Sorry for the slow reply here on my part.

I think the aging MSRV in PyO3 is probably slowing us down more than continuing to support Python 3.7 is. I also had the impression that Python 3.7 was still a significant proportion of downloads, e.g. about a third of recent cryptography downloads according to https://pypistats.org/packages/cryptography

I was considering if we should propose keeping 3.7 support until the end of the year or even longer, and bumping MSRV sooner (maybe even as part of 0.19). @alex I'd be interested to hear whether you think that would be beneficial for cryptography.

@davidhewitt
Copy link
Member

To avoid leaving Debian users behind, I think we should at least wait with any MSRV increase until Debian Bookworm releases, and then we just need a decision whether we want to go straight to 1.62 or have a period of support with 1.56 first.

@mejrs
Copy link
Member

mejrs commented May 4, 2023

Is there something in between 1.56 and 1.62 that we want? The main things that we want are below that, I think:

  • const generics (1.51)
  • addr_of (1.51)
  • #![doc = include_str!("README.md")] syntax (1.54)
  • syn 2.0 (1.56)

@adamreichold
Copy link
Member

I think the aging MSRV in PyO3 is probably slowing us down more than continuing to support Python 3.7 is.

Do you have anything specific in mind w.r.t. slowing us do? Keeping our set-minimal-package-versions Nox session working?

Otherwise, I am not sure it is really slowing us down. It does prevent certain simplifications and prevents some API improvements, but we have workarounds for example for the points @mejrs lists in place. Is there some place where lack of language (or standard library) features is really blocking us? Compared to say plain lack of time on our part?

(Having to build both syn 1.0 and 2.0 in a single project is not nice, but it also not the end of the world IMHO.)

@adamreichold
Copy link
Member

Is there something in between 1.56 and 1.62 that we want? The main things that we want are below that, I think:

I also think we should start with 1.56 and only bump to 1.62 if we actually have a situation where we want some later feature.

@mejrs
Copy link
Member

mejrs commented May 4, 2023

I do think the 1.48 msrv is slowing/limiting us. We currently use a build script and conditional compilation to work around these, those have a bunch of downsides:

  • they make the code harder to reason about
  • require annoying workarounds (by-value array iteration for example)
  • confusing for users when they read our docs but they're on a really old version

@adamreichold
Copy link
Member

While I mostly agree with the assessment, it is the conclusion I am sceptical about: All of this applies to code bases that are frequently modified or see sweeping changes. I think that is unfortunately not the case for PyO3 at the moment. And I also do not think that removing those costs would change this meaningfully.

Then again, we should not have even potential road blocks for no reason. But in this case, e.g. the Debian-oriented MSRV policy really is a feature in itself which currently costs us very little because the workarounds while ugly have been written and are not frequently modified.

@mejrs
Copy link
Member

mejrs commented May 4, 2023

I have written code using manuallydrop and transmutes to get around not being able to iterate by-value over an array. I'd love to delete that code.

@alex
Copy link
Contributor Author

alex commented May 4, 2023

@davidhewitt from our perspective, keeping Python 3.7 for a while (as you note, it's still widely used, we're only just removing 3.6 now) and raising the MSRV to 1.56 is optimal for us. This keeps support for things that are widely used, but let's us remove our usage of syn 1.0 (and it sounds like it allows some cleanups internal to pyo3 as well).

If there's ways we can be helpful on these fronts, we're happy to. The period during which we supported 3.6, but pyo3 didn't, was pretty painful. Going forward we'd rather help out upstream if you're amenable.

@davidhewitt
Copy link
Member

Is there something in between 1.56 and 1.62 that we want?

A couple of Rust releases contain some potential improvements:

  • 1.61:
    • Use of function pointers in const fn (this allows us to drop cases where we've used newtypes purely to wrap ffi function pointer typedefs in macro code).
    • whole-archive link modifier might let us support static embedding of Python interpreters
    • generics in const fn might be useful in future feature work
  • 1.58:
    • Identifiers in format strings would let us simplify documentation and internal code

That said, I think we're all converging on a 1.56 version bump once Debian bookworm releases, and then we can revisit later.

If there's ways we can be helpful on these fronts, we're happy to. The period during which we supported 3.6, but pyo3 didn't, was pretty painful. Going forward we'd rather help out upstream if you're amenable.

The two main drawbacks to supporting older Python versions in my opinion are:

  • It prevents us from making full use of newer Python features. For example PEP 587 embedded interpreter initialization will give us power to create an API which could significantly improve upon prepare_freethreaded_python(), however we can't realistically use PEP 587 by default on 3.8+ without risking behavioural differences on 3.7.
  • CI - it's significantly less bad since we switched to bors, so we'll have to see what it's like after Replace bors-ng by self-hosted instance or switch to GH merge queues #3132

The code in pyo3-ffi is pretty static once support for a Python version is added, so it's not really an issue in that library at all whether to keep or remove Python 3.6 support.

I think for now we're not feeling an immediate need to drop 3.7, however if we did, I think we'd be open to accepting bugfixes to the last PyO3 version which supported 3.7.

@alex are there particular pain points which you hit? I recall the PyPy support, not sure what else you encountered.

@alex
Copy link
Contributor Author

alex commented May 9, 2023

During the period where we were pinned to an older pyo3 version there were two pain points:

  1. The PyPy issue where, had y'all not been willing to do the backport, we would have had to choose between 3.6 and PyPy support.
  2. Not being able to benefit from upstream improvements (which probably, at the margin, was a disincentive for us to send PRs upstream)

@alex
Copy link
Contributor Author

alex commented May 31, 2023

FYI, I managed to fix our Rust versions infra. Here's the numbers for the last 30 days, for version 40 of cryptography (MSRV of 1.48).

For each version, the % is the % of downloads, with Rust version info, that have a rustc >= to that version.

As of: 2023-05-29 00:03:32.388608
Percent with rustc_version: 5.869%
Total with rustc_version: 49511
1.48.0: 99.4%
1.49.0: 95.0%
1.50.0: 95.0%
1.51.0: 95.0%
1.52.0: 95.0%
1.53.0: 94.9%
1.54.0: 94.9%
1.55.0: 94.9%
1.56.0: 94.9%
1.57.0: 94.4%
1.58.0: 94.4%
1.59.0: 94.1%
1.60.0: 94.0%
1.61.0: 85.3%
1.62.0: 85.1%
1.63.0: 84.1%
1.64.0: 82.5%
1.65.0: 81.6%
1.66.0: 77.1%
1.67.0: 75.3%
1.68.0: 75.0%
1.69.0: 74.3%

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 a pull request may close this issue.

4 participants