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

[python3-yaml] update to 5.4.1.1. Contributes to JB#58954 #1

Merged
merged 2 commits into from Oct 17, 2022

Conversation

nephros
Copy link

@nephros nephros commented Oct 11, 2022

There are three possible update candidates for this package:

The fork this PR originates from has branches implementing a bump for each of those. 5.4 is proposed here as a middle ground between conservative bump and security fixes gained.

For testing packages are available for each of these: https://build.merproject.org/project/show/home:nephros:sailfishos:python

The main motivation for this PR is the ability to bump the 'yq' tool (in chum) to a more recent version - which requires pyyaml >= 3.5.1.
Due to the "don't ship packages which replace system packages" agreement on Chum, we can't bump pyyaml there.

The only user of this package I am aware of is the spectacle tool, which works fine with 5.4.x, but there might be others to test.

@nephros
Copy link
Author

nephros commented Oct 11, 2022

The current .spec packages documentation as well.

If requested, I can remove those from the package (or put in a subpackage) after the version has been decided.

@vigejolla
Copy link
Member

To me updating to 5.4.1.1 makes most sense. I created an item for this in our internal bug tracker, so perhaps you could add "JB#58954" to the commit message?

As for the documentation, your suggestion to separate it to a subpackage makes sense, but let's do that in a separate PR.

@nephros nephros changed the title [python3-yaml] update to 5.4.1.1 [python3-yaml] update to 5.4.1.1. Contributes to JB#58954 Oct 12, 2022
nephros pushed a commit to nephros/python3-yaml that referenced this pull request Oct 13, 2022
@Thaodan
Copy link

Thaodan commented Oct 14, 2022

Please also add 'BuildRequires pkgconfig(yaml-0.1)' so it picks up libyaml properly instead of using internal yaml implementation.

@nephros
Copy link
Author

nephros commented Oct 17, 2022

dependency added and squashed

@nephros
Copy link
Author

nephros commented Oct 17, 2022

rpmlint suggests to switch to BuildArch: noarch.

Is it right? I'm never sure with python packages.

@vigejolla
Copy link
Member

dependency added and squashed

I kind of wish Thaodan hadn't asked you to add the dependency, as it's a bit out of scope here. The old version wasn't using libyaml either. But anyway, since you already started moving towards that direction... Just adding pkgconfig(yaml-0.1) isn't enough. You need python3dist(cython) as well.

@vigejolla
Copy link
Member

rpmlint suggests to switch to BuildArch: noarch.

Is it right? I'm never sure with python packages.

Well, technically speaking, for the current package, rpmlint speaks the truth. However, once you get it to use libyaml, it will contain architecture-specific library, so after that noarch will no longer do. But I suppose rpmlint will also stop complaining.

@vigejolla
Copy link
Member

According to my tests, it seems to work. However, I would still like those commits cleaned up a bit before merging. i.e. both BuildRequirements added in a single commit, with a nice commit message with a changelog entry. e.g. [python3-yaml] Use libyaml instead of internal implementation

@vigejolla vigejolla merged commit 8bf5fe5 into sailfishos:master Oct 17, 2022
@vigejolla
Copy link
Member

Thank you!

@nephros nephros deleted the update-5.4.1.1 branch November 4, 2022 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants