Conversation
github-actions
bot
added
6.topic: qt/kde
|
Whether we want to recommend taking |
ofborg
bot
added
10.rebuild-darwin: 0
There was a problem hiding this comment.
But where should they place the new packages in this kind?
There was a problem hiding this comment.
In the old structure, at least for now.
There was a problem hiding this comment.
Is it possible to use syntax like this:
{
# in all-packages.nix
a = callPackage ./... { }; # not needed in by-name though
a-qt5 = callPackage ./... { qt6 = qt5; };
}Or if we can't make the two Qt sets consistent:
{ lib
, something
, qt6
, libsForQt5
, qtVersion? "6"
, ...
}:
something.makePackage {
...
buildInputs = [
...
] ++ lib.optionals (qtVersion == "6") [
...
] ++ lib.optionals (qtVersion == "5") [
...
];
}There was a problem hiding this comment.
Theoretically possible, but the sets would never be consistent, because there are major API changes between 5 and 6.
There was a problem hiding this comment.
In the old structure, at least for now.
As far as I'm concerned, we shouldn't depend on the old structure from now on. If it belongs to a package set or scope or has special dependencies, there may be exceptions, but there are too many applications that support both qt5 and qt6. For packages already in Nixpkgs, we should also make treewide changes after the automatic migration is completed, so that they can be migrated to the by-name directory structure.
There was a problem hiding this comment.
Are there really that many?
It seems that not many of them already support qt5 and qt6 in nixpkgs, but there may be many that "should support" but don't.
And do we really need to shove them all into by-name?
Maybe ask @infinisil. The RFC and the guide don't say that it must be placed in by-name, but perhaps we should still try to minimize this.
There was a problem hiding this comment.
I generally don't think we should be having two versions of every single package that can be built with both Qt versions. It's fine to use whatever is preferred upstream, and multiple versions can be added if there's actual demand for them and the maintenance costs aren't too much.
There was a problem hiding this comment.
For top-level attrs, I'd rather discourage these custom callPackages entirely, as all they're really doing is masking which package set the expression args come from.
There was a problem hiding this comment.
That's what the by-name structure already achieves.
There was a problem hiding this comment.
using by-name doesn't prevent a custom callPackage though
(edit: after checking again, it seems the intention was to only permit the regular callPackage, so other callPackages slipping through might be a CI bug.)
There was a problem hiding this comment.
Please also provide an example for Qt5.
There was a problem hiding this comment.
Do we really need one? It feels pretty obvious to me.
There was a problem hiding this comment.
For top-level attrs, I'd rather discourage these custom callPackages entirely, as all they're really doing is masking which package set the expression args come from.
|
Rewrote a bunch of it, feels better now. |
|
@NixOS/documentation-reviewers |
Make examples don't require a custom callPackage, remove note on multiversioning, reword a few things. Fixes NixOS#287015
| ::: | ||
|
|
||
| ## Locating runtime dependencies {#qt-runtime-dependencies} | ||
| ## Packages supporting multiple Qt versions {#qt-versions} |
There was a problem hiding this comment.
The header ids are here so that documentation links don't become invalid. We can manually add anchors for previous ids:
| ## Packages supporting multiple Qt versions {#qt-versions} | |
| []{#qt-runtime-dependencies} | |
| ## Packages supporting multiple Qt versions {#qt-versions} |
There was a problem hiding this comment.
This is just github diffs being shitty, the next header still has that anchor.
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/how-to-deal-with-two-versions-of-a-package-in-by-name/40841/10 |
5 participants
Make examples don't require a custom callPackage, remove note on multiversioning, reword a few things.
Fixes #287015
Description of changes
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.