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

Allow pre-/post-releases in pythonRuntimeDepsCheckHook #301698

Open
genevieve-me opened this issue Apr 5, 2024 · 4 comments
Open

Allow pre-/post-releases in pythonRuntimeDepsCheckHook #301698

genevieve-me opened this issue Apr 5, 2024 · 4 comments
Labels
0.kind: bug Something is broken

Comments

@genevieve-me
Copy link

Describe the bug

In its current implementation, python-runtime-deps-check-hook.py instantiates a Requirement from packaging.requirements, which has a SpecifierSet with prereleases=False. https://packaging.pypa.io/en/stable/requirements.html

Because of this, several Python packages within nixpkgs cause this hook to fail. One example is psycopg.optional-dependencies.pool. Upstream keeps the version at "x.y.dev1" except on release tags, but psycopg-pool is built from the release tag of psycopg.

In practice, that means that building any python package with any ~= or >= version specifiers for psycopg-pool, or any other python library with a version in nixpkgs like x.y.a2, n.m.beta, etc., will fail. (I'm not sure why the packaging package behaves like this, since it seems obvious that 3.0.0.dev1 >= 2.0 is true, so I might raise an issue there).

Unfortunately, you can't override the version specifier set of a Requirement, so the hook implementation would need to be slightly refactored to use a SpecifierSet with the parameter prereleases=True: https://packaging.pypa.io/en/stable/specifiers.html.

I'm not sure if there's a better way to approach this, or if it makes more sense to generally try to avoid prerelease versions in nixpkgs, but this is currently breaking a build for me so I wanted to raise it. Thanks for your attention!

This issue was previously mentioned by @mweinelt at #270457 (comment), but the implementation was kept and the package in question was updated to a non-prerelease version.


Add a 👍 reaction to issues you find important.

@genevieve-me genevieve-me added the 0.kind: bug Something is broken label Apr 5, 2024
@genevieve-me
Copy link
Author

pypa/packaging#790

@genevieve-me genevieve-me changed the title Allow prereleases in pythonRuntimeDepsCheckHook Allow pre-/post-releases in pythonRuntimeDepsCheckHook Apr 5, 2024
@mweinelt
Copy link
Member

mweinelt commented Apr 5, 2024

I'm pretty sure this behavior is in place, so that pre-releases need an explicit opt in, and won't be automatically selected as the latest and greatest version. Many packages in fact don't want to upgrade to pre-release versions.

Packages that do can usually opt in by specifying the pre-release version in the requirement specifier. That should make the comparison work.

@genevieve-me
Copy link
Author

OK, not getting an alpha release by default does make sense for upstream behavior. However, I tried specifying pre-releases and still find the results a little odd, eg Requirement("foo>0.1.dev1").specifier.contains("1.0.dev1") is False.

Getting back to nixpkgs, is there a policy about packaging developmental releases? My naive reaction is that they will often fail pythonRuntimeDepsCheckHook for most packages consuming them as a dependency, so it would be nice to avoid that, but I'm sure there is a lot of context I'm missing.

For example, the exact python library that bit me was psycopg-pool, and I don't face the issue with a small patch to nixpkgs to build it from the pool-3.2.1 tag instead of the same tag as its parent psycopg. Would such a PR be welcome?

mweinelt added a commit to mweinelt/nixpkgs that referenced this issue Oct 28, 2024
Updating to prereleases should be possible, but making this an option is
difficult, given that for packages with many consumers you would have to
set it in each consumer.

We thoroughly test the package set, so allowing prereleases unconditionally
shouldn't be too bad.

Closes: NixOS#301698
mweinelt added a commit to mweinelt/nixpkgs that referenced this issue Oct 29, 2024
Updating to prereleases should be possible, but making this an option is
difficult, given that for packages with many consumers you would have to
set it in each consumer.

We thoroughly test the package set, so allowing prereleases unconditionally
shouldn't be too bad.

Closes: NixOS#301698
mweinelt added a commit to mweinelt/nixpkgs that referenced this issue Oct 29, 2024
Updating to prereleases should be possible, but making this an option is
difficult, given that for packages with many consumers you would have to
set it in each consumer.

We thoroughly test the package set, so allowing prereleases unconditionally
shouldn't be too bad.

Closes: NixOS#301698
mweinelt added a commit to mweinelt/nixpkgs that referenced this issue Oct 30, 2024
Updating to prereleases should be possible, but making this an option is
difficult, given that for packages with many consumers you would have to
set it in each consumer.

We thoroughly test the package set, so allowing prereleases unconditionally
shouldn't be too bad.

Closes: NixOS#301698
@mweinelt
Copy link
Member

Getting back to nixpkgs, is there a policy about packaging developmental releases? My naive reaction is that they will often fail pythonRuntimeDepsCheckHook for most packages consuming them as a dependency, so it would be nice to avoid that, but I'm sure there is a lot of context I'm missing.

There is no policy against packaging pre-releases, and it can even be helpful to ease the transition between Python releases. In practice, we're not seeing much of these kinds of conflicts.

For example, the exact python library that bit me was psycopg-pool, and I don't face the issue with a small patch to nixpkgs to build it from the pool-3.2.1 tag instead of the same tag as its parent psycopg. Would such a PR be welcome?

A tagged version like that should have no pre-release tag. For sqlalchemy we just stripped it, as that was what they did themselves in their CI when building releases.

Either way, we will be allowing prereleases after the NixOS 24.11 branch-off to make the Python 3.13 transition bearable.

mweinelt added a commit that referenced this issue Nov 8, 2024
Updating to prereleases should be possible, but making this an option is
difficult, given that for packages with many consumers you would have to
set it in each consumer.

We thoroughly test the package set, so allowing prereleases unconditionally
shouldn't be too bad.

Closes: #301698
mweinelt added a commit that referenced this issue Nov 8, 2024
Updating to prereleases should be possible, but making this an option is
difficult, given that for packages with many consumers you would have to
set it in each consumer.

We thoroughly test the package set, so allowing prereleases unconditionally
shouldn't be too bad.

Closes: #301698
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug Something is broken
Projects
None yet
Development

No branches or pull requests

2 participants