Skip to content

fplll: update to 5.5.0, python3-fpylll: update to 0.6.2.#53702

Merged
ahesford merged 2 commits intovoid-linux:masterfrom
tornaria:fplll
Jan 3, 2025
Merged

fplll: update to 5.5.0, python3-fpylll: update to 0.6.2.#53702
ahesford merged 2 commits intovoid-linux:masterfrom
tornaria:fplll

Conversation

@tornaria
Copy link
Contributor

  • fplll: update to 5.5.0.
  • python3-fpylll: update to 0.6.2.

Testing the changes

Comment on lines +20 to +23
post_patch() {
vsed -i -e s/"cysignals<1.12.0"/"cysignals"/ pyproject.toml
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally prefer that vsed be reserved for patches that depend on runtime conditions like per-architecture flags. I suggest making this change as a proper patch. Alternatively, just pass --skip-dependency-check in make_build_args. I've given up the fight in python3-scipy and a few other packages that abuse these version restrictions. It's not worth the hassle.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind this here at all, but let me mention one reason to do it this way: this is a very semantic change (replace a dependency on "cysignals<1.12.0" by a dependency on "cysignals") and easy to create. While a patch is very fragile due to the 3-line context, meaning if other dependencies change, or the order, etc, the patch needs to be edited, etc.

I know about --skip-dependency-check, which would work here but it seems too big a hammer. Maybe I still want to be warned about other dependencies that upstream adds in the future.

Moreover, policy now is that CI fails when we build a python package with missing dependencies. E.g. I had to disable debugpy dependency for ipykernel: https://github.com/void-linux/void-packages/blob/master/srcpkgs/python3-ipython_ipykernel/patches/disable-debugpy.patch this would be simpler as a vsed, in the sense that this patch will probably have to be adjusted every time. And I don't think --skip-dependency-check would work for that.

Anyway, for the current package, it's not a big deal since it updates slowly. And I didn't update cysignals in the end since 1.12 breaks other stuff (i.e. sagemath).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and I pushed a new version without the vsed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, disabling checks is a big hammer, but I've received such pushback from upstreams when I complain about how they abuse these version restrictions that I've decided to swing it.

Fragility of the patch is a feature. Changes will be immediately obvious and spur contributors to consider whether ignoring upper bounds is appropriate when the dependency list changes. Patching with vsed can subtly fail when changes to the dependency result in a failure to match. In CI, this is a hard error, but local testers might overlook the default behavior of simply warning when vsed makes no changes.

@ahesford ahesford merged commit 4981f83 into void-linux:master Jan 3, 2025
@tornaria tornaria deleted the fplll branch January 3, 2025 02:47
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 this pull request may close these issues.

2 participants