digikam: modernize, update, and use Qt 6; libsForQt5.libqtav: drop#329470
digikam: modernize, update, and use Qt 6; libsForQt5.libqtav: drop#329470emilazy merged 6 commits intoNixOS:masterfrom
Conversation
This isn’t being used any more; on Qt 5 they use QtAVPlayer and on Qt 6 they use the Qt Multimedia module.
Includes fixes for FFmpeg 7.
|
it's for strictDeps. |
pkgs/by-name/di/digikam/package.nix
Outdated
There was a problem hiding this comment.
| sourceProvenance = [ lib.sourceTypes.fromSource ]; |
https://nixos.org/manual/nixpkgs/unstable/#sec-meta-sourceProvenance
There was a problem hiding this comment.
Hmm, I see that this is technically a semantic no‐op according to the manual, but it’s intended to be a forward‐looking change: defining a sourceProvenance list conveys something different to using the default, because it implies a best-effort attempt at exhaustively outlining the provenance; hopefully, in the future, we can change the semantics so that an explicitly‐defined sourceProvenance is intended to convey something exhaustive, like license is, and at that point there would be a desire to annotate from‐source packages like this which I’m trying to get ahead of. Especially since there are definitely non‐source packages out there with no annotation.
There was a problem hiding this comment.
I'd prefer to get consensus before explicitly contradicting the manual.
And then the manual should be updated in that case.
There was a problem hiding this comment.
The manual doesn’t say that you shouldn’t define sourceProvenance like this. It just says that it currently doesn’t mean anything more than the empty string. But to humans, it clearly does, I think.
Do you mind if I ask on Matrix for opinions about this? If people are strongly against having this in there then I can remove it, but I do suspect we’re going to hopefully end up in a world where we expect everything to define sourceProvenance and I’d rather document it now than for there to be one more package that will need inspection and annotation later.
There was a problem hiding this comment.
If a package contains elements that are not built from the original source by a nixpkgs derivation, the meta.sourceProvenance attribute should be a list containing one or more value from lib.sourceTypes defined in nixpkgs/lib/source-types.nix.
If we're expecting people to document this even for packages built from source, this bit needs to be changed.
There was a problem hiding this comment.
I wouldn’t say expected; I just don’t think it’s harmful in any way, and provides some benefit. To quote @toonn on Matrix:
I think it can only be helpful, not harmful. Don't even think the manual has to change, it says "if binary then SHOULD..." that says nothing about "if not binary," so it leaves it open to whatever the package maintainer feels like.
Unless it's a lie of course, then it could be harmful.
As an analogy, consider licensing – we assume the best by default, so license = lib.licenses.free; is technically redundant, but it’s clearly more valuable to have that declaration than not, even if it has no strict semantic effect: it makes it clear that someone has thought about what licence the package is under and thinks it qualifies as FOSS. In the future, we may want to be more strict, requiring packages to list a license (perhaps with a treewide sweep for ones that don’t), and not assuming that packages with no licence declared are FOSS (as it has been wrong in the past); in that case, we’d be grateful for people who specified license = lib.licenses.free;. In my view, the exact same situation applies here, and I don’t think that’s in contradiction to what the manual says at all.
There was a problem hiding this comment.
To put it another way: If “from source” is assumed by default, and omission from the list is not a statement of absence, then why have fromSource at all, as it never conveys any additional information per the semantics given in the manual? Because it’s still providing more information that can be useful to readers now and policy in the future.
There was a problem hiding this comment.
If a package contains elements that are not built from the original source by a nixpkgs derivation, the meta.sourceProvenance attribute should be a list containing one or more value from lib.sourceTypes defined in nixpkgs/lib/source-types.nix.
If we're expecting people to document this even for packages built from source, this bit needs to be changed.
That reads, to me, like it's just explaining how you'd define things that are not built from source and that it may be multiple source types. Not that you shouldn't use it when it is built from source.
But why? Is it because cmake generates makefiles for the build itself to use? |
|
Yeah, I don’t quite understand since |
|
If it works without it, might as well remove it, it's almost a completely different derivation at this point. |
|
It does build for me, at least. I thought maybe it would only cause an issue for cross‐compilation or something. |
|
Well just for fun, I'll see if cross-compilation works. Could've been some artifact of qt5 packaging at the time, I really don't recall. |
Abandoned upstream, only works with old versions of FFmpeg, and the only reverse dependency (digiKam) hadn’t actually been using it in quite a while.
ofborg
bot
added
8.has: clean-up
emilazy
force-pushed
the
push-omxsrkkkvymx
branch
from
56b4c43 to
fee2b5c
Compare
|
@eclairevoyant Any luck with the cross‐compile? |
|
No, qt cross in general is broken, so feel free to merge as is. I haven't tested runtime myself either fyi |
|
I already ran it myself and exercised a bunch of functionality, so hopefully this should be good to go. The main thing I can see biting us is the temporary removal of scanner support, but hopefully that’ll hit the Qt 6 branch soon. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/frustrations-about-splicing/49607/23 |
5 participants
Description of changes
As part of my quest to get rid of FFmpeg 4, I noticed that digiKam was depending on an unmaintained library called QtAV. As it turns out, it wasn‘t actually using the library at all, but I gave it a bump to prepare it for FFmpeg 7 and updated the whole thing to Qt 6 for good measure too. There are a couple of features still missing from the Qt 6 version, but some new ones that are exclusive to it as well.
I‘ve tested various parts of the application‘s functionality and it seems to work fine.
@eclairevoyant I noticed you added
cmaketodepsBuildBuildeven though it was already innativeBuildInputs. Can you help me understand if that was necessary and if I should keep it here?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.