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

valentina: downgrade qt to 5.12 #122211

Merged
merged 1 commit into from May 19, 2021
Merged

valentina: downgrade qt to 5.12 #122211

merged 1 commit into from May 19, 2021

Conversation

jnetod
Copy link
Contributor

@jnetod jnetod commented May 8, 2021

Motivation for this change

ZHF: #122042

Support for late Qt versions doesn't landed in a stable release yet:

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@jonringer
Copy link
Contributor

cc @NixOS/qt-kde

I would like to avoid having people have multiple versions of qt on their system if possible.

@samueldr
Copy link
Member

samueldr commented May 8, 2021

Given this seems to be "specialist" software, it's probably much less of an issue than if it was a popular often-used tool. Though it would be nice if we can avoid having multiple Qts.

@doronbehar
Copy link
Contributor

Support for late Qt versions doesn't landed in a stable release yet:

* [gitlab.com/smart-pattern/valentina/-/commit/3b728f10dae114ea76f97b5e75a775a9364bfb52](https://gitlab.com/smart-pattern/valentina/-/commit/3b728f10dae114ea76f97b5e75a775a9364bfb52)

* [gitlab.com/smart-pattern/valentina/-/commit/6864ddc1af63074b811a7c54fb9d37d0b141fa02](https://gitlab.com/smart-pattern/valentina/-/commit/6864ddc1af63074b811a7c54fb9d37d0b141fa02)

* [gitlab.com/smart-pattern/valentina/-/commit/6af9d104cdc9c9a2a008a0b5fca63d907c16a8f1](https://gitlab.com/smart-pattern/valentina/-/commit/6af9d104cdc9c9a2a008a0b5fca63d907c16a8f1)

How about adding these patches to the package and make it use libsForQt5?

@jnetod
Copy link
Contributor Author

jnetod commented May 10, 2021

@doronbehar I've tried to cherry-pick those commits, no luck; too much changes in between, those commits alone doesn't help.

@jonringer
Copy link
Contributor

well, having a bloated closure is "better" than no closure, I'm okay with merging it; and when upstream allows for qt>=5.15 then we can update it.

@doronbehar
Copy link
Contributor

doronbehar commented May 10, 2021

@doronbehar I've tried to cherry-pick those commits, no luck; too much changes in between, those commits alone doesn't help.

The upstream commits you referenced should be merged into their latest release - For instance https://gitlab.com/smart-pattern/valentina/-/blob/v0.7.46/src/libs/vlayout/vlayoutdef.h includes the QPainterPath. I'm rather positive you should be able to update the package with qt5.15, the current version in nixpkgs is 0.6.1 from 2 years ago.

@jnetod
Copy link
Contributor Author

jnetod commented May 10, 2021

v0.7.x is for test builds. Should we maintain both versions, newer only or stable only?

Currently, we have two different versions: stable and test builds. I propose to change the schema. Each version with even number like 0.6.0, 0.8.0, 1.0.0 and so on will be stable release. Each with odd number 0.7.0, 0.9.0, 1.1.0 will be a version for testing. Instead of publishing a test build each time we will publish a new release for the test version. At the end of the test cycle, a new stable version will be released. For example, 0.7.0 will be published as 0.8.0. Testing will be continued in 0.9.0.

https://gitlab.com/smart-pattern/valentina/-/issues/5

@doronbehar
Copy link
Contributor

v0.7.x is for test builds. Should we maintain both versions, newer only or stable only?

👍 for noticing this. That's a good question and I don't have a strong opinion, there's not even a strong consensus among other repos https://repology.org/project/valentina/versions .

cc @jfrankenau maintainer for an opinion. I think that if the update doesn't introduce too many changes we should update it to what upstream considers as unstable yet.

@@ -26582,7 +26582,7 @@ in

utox = callPackage ../applications/networking/instant-messengers/utox { };

valentina = libsForQt514.callPackage ../applications/misc/valentina { };
valentina = libsForQt512.callPackage ../applications/misc/valentina { };
Copy link
Contributor

Choose a reason for hiding this comment

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

In anycase, if the PR will be merged as is, please add:

Suggested change
valentina = libsForQt512.callPackage ../applications/misc/valentina { };
# Version 0.8.x or above should support qt5.15
valentina = libsForQt512.callPackage ../applications/misc/valentina { };

@jonringer
Copy link
Contributor

If the update works with little issues, I would prefer that route.

Unfortunate that upstream is significantly far behind qt development.

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

I would rather have a working application with closure bloat, than a failing one..

https://github.com/NixOS/nixpkgs/pull/122211

1 package built:
valentina

Hopefully we can update this to 515 once upstream supports it.

@jonringer jonringer merged commit 3165afc into NixOS:master May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants