-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
digikam: modernize, update, and use Qt 6; libsForQt5.libqtav: drop #329470
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
60b1c87
digikam: reformat with `nixfmt-rfc-style`
emilazy c363e27
digikam: modernize
emilazy 282f344
digikam: drop libqtav dependency
emilazy 94a39c9
digikam: 8.3.0 -> 8.4.0
emilazy 5ca9433
digikam: use Qt 6
emilazy fee2b5c
libsForQt5.libqtav: drop
emilazy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| diff --git a/CMakeLists.txt b/CMakeLists.txt | ||
| index 43636fa9b9...e8da76c480 100644 | ||
| --- a/CMakeLists.txt | ||
| +++ b/CMakeLists.txt | ||
| @@ -208,10 +208,6 @@ | ||
| # For CI runners to run tests, the following custom target serves to perform the download automatically. | ||
| # If the directory "test-data" has already been created, the target becomes a "no-op". | ||
| # | ||
| - add_custom_command(OUTPUT ${CMAKE_SOURCE_DIR}/test-data | ||
| - COMMENT "Checkout unit-test data repository. Please wait..." | ||
| - COMMAND git | ||
| - ARGS clone https://invent.kde.org/graphics/digikam-test-data.git ${CMAKE_SOURCE_DIR}/test-data) | ||
| add_custom_target(test-data ALL DEPENDS ${CMAKE_SOURCE_DIR}/test-data) | ||
|
|
||
| endif() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,149 +1,202 @@ | ||
| { stdenv, config, lib, fetchurl, cmake, doxygen, extra-cmake-modules, wrapGAppsHook3 | ||
|
|
||
| # For `digitaglinktree` | ||
| , perl, sqlite | ||
|
|
||
| , libsForQt5 | ||
|
|
||
| , bison | ||
| , boost | ||
| , eigen | ||
| , exiv2 | ||
| , ffmpeg_4 | ||
| , flex | ||
| , graphviz | ||
| , imagemagick | ||
| , lcms2 | ||
| , lensfun | ||
| , libgphoto2 | ||
| , liblqr1 | ||
| , libusb1 | ||
| , libheif | ||
| , libGL | ||
| , libGLU | ||
| , opencv | ||
| , pcre | ||
| , x265 | ||
| , jasper | ||
|
|
||
| , bash | ||
| # For panorama and focus stacking | ||
| , enblend-enfuse | ||
| , hugin | ||
| , gnumake | ||
|
|
||
| , cudaSupport ? config.cudaSupport | ||
| , cudaPackages ? {} | ||
| { | ||
| stdenv, | ||
| config, | ||
| lib, | ||
| fetchFromGitLab, | ||
| fetchgit, | ||
|
|
||
| cmake, | ||
| ninja, | ||
| extra-cmake-modules, | ||
| flex, | ||
| bison, | ||
| wrapGAppsHook3, | ||
|
|
||
| opencv, | ||
| libtiff, | ||
| libpng, | ||
| libjpeg, | ||
| libheif, | ||
| libjxl, | ||
| boost, | ||
| lcms2, | ||
| expat, | ||
| exiv2, | ||
| libxml2, | ||
| libxslt, | ||
| ffmpeg, | ||
| jasper, | ||
| eigen, | ||
| lensfun, | ||
| liblqr1, | ||
| libgphoto2, | ||
| libusb1, | ||
| imagemagick, | ||
| x265, | ||
| libGLX, | ||
| libGLU, | ||
|
|
||
| kdePackages, | ||
|
|
||
| # For `digitaglinktree` | ||
| perl, | ||
| sqlite, | ||
|
|
||
| runtimeShell, | ||
| # For panorama and focus stacking | ||
| enblend-enfuse, | ||
| hugin, | ||
| gnumake, | ||
| }: | ||
|
|
||
| stdenv.mkDerivation rec { | ||
| pname = "digikam"; | ||
| version = "8.3.0"; | ||
|
|
||
| src = fetchurl { | ||
| url = "mirror://kde/stable/${pname}/${version}/digiKam-${version}-1.tar.xz"; | ||
| hash = "sha256-BbFF/38vIAX6IbxXnBUqsjyBkbZ4/ylEyPBAbWud5tg="; | ||
| let | ||
| testData = fetchgit { | ||
| url = "https://invent.kde.org/graphics/digikam-test-data.git"; | ||
| rev = "d02dd20b23cc279792325a0f03d21688547a7a59"; | ||
| fetchLFS = true; | ||
| hash = "sha256-SvsmcniDRorwu9x9OLtHD9ftgquyoE5Kl8qDgqi1XdQ="; | ||
| }; | ||
| in | ||
|
|
||
| stdenv.mkDerivation (finalAttrs: { | ||
| pname = "digikam"; | ||
| version = "8.4.0"; | ||
|
|
||
| src = fetchFromGitLab { | ||
| domain = "invent.kde.org"; | ||
| owner = "graphics"; | ||
| repo = "digikam"; | ||
| rev = "v${finalAttrs.version}"; | ||
| hash = "sha256-GJYlxJkvFEXppVk0yC9ojszylfAGt3eBMAjNUu60XDY="; | ||
| }; | ||
|
|
||
| strictDeps = true; | ||
| patches = [ ./disable-tests-download.patch ]; | ||
|
|
||
| depsBuildBuild = [ cmake ]; | ||
| strictDeps = true; | ||
|
|
||
| nativeBuildInputs = [ | ||
| cmake | ||
| doxygen | ||
| ninja | ||
| extra-cmake-modules | ||
| libsForQt5.kdoctools | ||
| libsForQt5.wrapQtAppsHook | ||
| flex | ||
| bison | ||
| kdePackages.wrapQtAppsHook | ||
| wrapGAppsHook3 | ||
| ] ++ lib.optionals cudaSupport (with cudaPackages; [ | ||
| cuda_nvcc | ||
| ]); | ||
| ]; | ||
|
|
||
| # Based on <https://www.digikam.org/api/index.html#externaldeps>, | ||
| # but it doesn’t have everything, so you also have to check the | ||
| # CMake files… | ||
| # | ||
| # We list non‐Qt dependencies first to override Qt’s propagated | ||
| # build inputs. | ||
|
|
||
| buildInputs = [ | ||
| bison | ||
| opencv | ||
| libtiff | ||
| libpng | ||
| # TODO: Figure out how on earth to get it to pick up libjpeg8 for | ||
| # lossy DNG support. | ||
| libjpeg | ||
| libheif | ||
| libjxl | ||
| boost | ||
| eigen | ||
| exiv2 | ||
| ffmpeg_4 | ||
| flex | ||
| graphviz | ||
| imagemagick | ||
| lcms2 | ||
| expat | ||
| exiv2 | ||
| libxml2 | ||
| libxslt | ||
| # Qt WebEngine uses and propagates FFmpeg, and if it’s a | ||
| # different version it causes linker warnings. | ||
| #ffmpeg | ||
| jasper | ||
| eigen | ||
| lensfun | ||
| libgphoto2 | ||
| libheif | ||
| liblqr1 | ||
| libgphoto2 | ||
| libusb1 | ||
| libGL | ||
| libGLU | ||
| opencv | ||
| pcre | ||
| imagemagick | ||
| x265 | ||
| jasper | ||
| ] ++ (with libsForQt5; [ | ||
| libkipi | ||
| libksane | ||
| libqtav | ||
|
|
||
| qtbase | ||
| qtxmlpatterns | ||
| qtsvg | ||
| qtwebengine | ||
| qtnetworkauth | ||
|
|
||
| akonadi-contacts | ||
| kcalendarcore | ||
| kconfigwidgets | ||
| kcoreaddons | ||
| kfilemetadata | ||
| knotifications | ||
| knotifyconfig | ||
| ktextwidgets | ||
| kwidgetsaddons | ||
| kxmlgui | ||
|
|
||
| breeze-icons | ||
| marble | ||
| oxygen | ||
| threadweaver | ||
| ]) ++ lib.optionals cudaSupport (with cudaPackages; [ | ||
| cuda_cudart | ||
| ]); | ||
| libGLX | ||
| libGLU | ||
|
|
||
| kdePackages.qtbase | ||
| kdePackages.qtnetworkauth | ||
| kdePackages.qtscxml | ||
| kdePackages.qtsvg | ||
| kdePackages.qtwebengine | ||
| kdePackages.qt5compat | ||
| kdePackages.qtmultimedia | ||
|
|
||
| kdePackages.kconfig | ||
| kdePackages.kxmlgui | ||
| kdePackages.ki18n | ||
| kdePackages.kwindowsystem | ||
| kdePackages.kservice | ||
| kdePackages.solid | ||
| kdePackages.kcoreaddons | ||
| kdePackages.knotifyconfig | ||
| kdePackages.knotifications | ||
| kdePackages.threadweaver | ||
| kdePackages.kiconthemes | ||
| kdePackages.kfilemetadata | ||
| kdePackages.kcalendarcore | ||
| kdePackages.kio | ||
| kdePackages.sonnet | ||
| # libksane and akonadi-contacts do not yet work when building for | ||
| # Qt 6. | ||
| ]; | ||
|
|
||
| checkInputs = [ kdePackages.qtdeclarative ]; | ||
|
|
||
| postConfigure = lib.optionalString finalAttrs.doCheck '' | ||
| ln -s ${testData} $cmakeDir/test-data | ||
| ''; | ||
|
|
||
| postPatch = '' | ||
| substituteInPlace \ | ||
| core/dplugins/bqm/custom/userscript/userscript.cpp \ | ||
| core/utilities/import/backend/cameracontroller.cpp \ | ||
| --replace-fail \"/bin/bash\" \"${lib.getExe bash}\" | ||
| --replace-fail '"/bin/bash"' ${lib.escapeShellArg "\"${runtimeShell}\""} | ||
| ''; | ||
|
|
||
| cmakeFlags = [ | ||
| "-DENABLE_MYSQLSUPPORT=1" | ||
| "-DENABLE_INTERNALMYSQL=1" | ||
| "-DENABLE_MEDIAPLAYER=1" | ||
| "-DENABLE_QWEBENGINE=on" | ||
| "-DENABLE_APPSTYLES=on" | ||
| "-DCMAKE_CXX_FLAGS=-I${libsForQt5.libksane}/include/KF5" # fix `#include <ksane_version.h>` | ||
| (lib.cmakeBool "BUILD_WITH_QT6" true) | ||
| (lib.cmakeBool "ENABLE_KFILEMETADATASUPPORT" true) | ||
| #(lib.cmakeBool "ENABLE_AKONADICONTACTSUPPORT" true) | ||
| (lib.cmakeBool "ENABLE_MEDIAPLAYER" true) | ||
| (lib.cmakeBool "ENABLE_APPSTYLES" true) | ||
| ]; | ||
|
|
||
| # Tests segfault for some reason… | ||
| # TODO: Get them working. | ||
| doCheck = false; | ||
|
|
||
| dontWrapGApps = true; | ||
|
|
||
| preFixup = '' | ||
| qtWrapperArgs+=("''${gappsWrapperArgs[@]}") | ||
| qtWrapperArgs+=(--prefix PATH : ${lib.makeBinPath [ gnumake hugin enblend-enfuse ]}) | ||
| qtWrapperArgs+=(--suffix DK_PLUGIN_PATH : ${placeholder "out"}/${libsForQt5.qtbase.qtPluginPrefix}/${pname}) | ||
| qtWrapperArgs+=(--prefix PATH : ${ | ||
| lib.makeBinPath [ | ||
| gnumake | ||
| hugin | ||
| enblend-enfuse | ||
| ] | ||
| }) | ||
| qtWrapperArgs+=(--suffix DK_PLUGIN_PATH : ${placeholder "out"}/${kdePackages.qtbase.qtPluginPrefix}/digikam) | ||
| substituteInPlace $out/bin/digitaglinktree \ | ||
| --replace "/usr/bin/perl" "${perl}/bin/perl" \ | ||
| --replace "/usr/bin/sqlite3" "${sqlite}/bin/sqlite3" | ||
| ''; | ||
|
|
||
| meta = with lib; { | ||
| description = "Photo Management Program"; | ||
| license = licenses.gpl2; | ||
| homepage = "https://www.digikam.org"; | ||
| maintainers = with maintainers; [ ]; | ||
| platforms = platforms.linux; | ||
| meta = { | ||
| description = "Photo management application"; | ||
| homepage = "https://www.digikam.org/"; | ||
| changelog = "${finalAttrs.src.meta.homepage}-/blob/master/project/NEWS.${finalAttrs.version}"; | ||
| sourceProvenance = [ lib.sourceTypes.fromSource ]; | ||
| license = lib.licenses.gpl2Plus; | ||
| maintainers = [ ]; | ||
| platforms = lib.platforms.linux; | ||
| mainProgram = "digikam"; | ||
| }; | ||
| } | ||
| }) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://nixos.org/manual/nixpkgs/unstable/#sec-meta-sourceProvenance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
sourceProvenancelist 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‐definedsourceProvenanceis intended to convey something exhaustive, likelicenseis, 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The manual doesn’t say that you shouldn’t define
sourceProvenancelike 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
sourceProvenanceand 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
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 alicense(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 specifiedlicense = 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
fromSourceat 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.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.