Bug 2254709

Copr build:
https://copr.fedorainfracloud.org/coprs/build/6759814
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2254709-shotcut/fedora-rawhide-x86_64/06759814-shotcut/fedora-review/review.txt

Found issues:

- Unversioned so-files directly in %_libdir.
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages

Please know that there can be false-positives.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

built new package, please can someone review it.

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/shotcut.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/shotcut-24.02.29-1.fc40.src.rpm

%changelog
* Wed Apr 10 2024 Martin Gansser <martinkg> - 24.02.29-1
- Update to 24.02.29

> Requires:       mlt-freeworld >= 7.6.0

This package doesn't exist in Fedora, and seems to have been retired in rpmfusion. We _do_ have mlt in Fedora though: https://src.fedoraproject.org/rpms/mlt

built new package.

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/shotcut.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/shotcut-24.04.13-1.fc40.src.rpm

%changelog
* Mon Apr 22 2024 Martin Gansser <martinkg> - 24.04.13-1
- Update to 24.04.13
- Remove mlt-freeworld

Hello,

Sorry for the noise, but as shotcut is not available in bugzilla as a 
Fedora component, I post this issue on this review request.

When installing shotcut-24.04.13-1.fc40.x86_64.rpm, dnf downgrades a lot
of Qt6 packages to 6.6.x version and all the dependent packages.

Then when upgrading all the packages through the command: 

dnf --refresh upgrade

I hit this problem:

package shotcut-24.04.13-1.fc40.x86_64 from @System requires libQt6Charts.so.6(Qt_6.6_PRIVATE_API)(64bit), but none of the providers can be installed
  - cannot install both qt6-qtcharts-6.7.0-1.fc40.x86_64 from updates and qt6-qtcharts-6.6.2-1.fc40.x86_64 from @System
  - cannot install both qt6-qtcharts-6.7.0-1.fc40.x86_64 from updates and qt6-qtcharts-6.6.2-1.fc40.x86_64 from fedora

There seems that shotcut packages need to be rebuild against Qt6 
version 6.7.x or that the version dependency needs to be relaxed.

Rebuilding shotcut packages for f40 from SRPM provided in comment #4 with 
mock succeed. Packages can be installed and dnf upgrades Qt6 dependencies
to version 6.7.x and all the other dependencies of Qt 6.7.x.

Hope this will help to get shotcut in f40. Any comment are welcome.

Cordially,


-- 
NVieville

built new package.

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/shotcut.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/shotcut-24.06.02-1.fc40.src.rpm

%changelog
* Mon Jun 10 2024 Martin Gansser <martinkg> - 24.06.02-1
- Update to 24.06.02

Created attachment 2036931 [details]
The .spec file difference from Copr build 6759814 to 7585836

Copr build:
https://copr.fedorainfracloud.org/coprs/build/7585836
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2254709-shotcut/fedora-rawhide-x86_64/07585836-shotcut/fedora-review/review.txt

Found issues:

- Unversioned so-files directly in %_libdir.
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages

Please know that there can be false-positives.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Taking review. Quick notes:

1. %{_libdir}/libCuteLogger.so
   A shared library without SONAME in %{_libdir} should be moved out of linker search path,
   e.g. to %{_libdir}/%{name} if it's not supposed to be available for consumption by other
   packages. You might need to apply
   https://docs.fedoraproject.org/en-US/packaging-guidelines/#_downstream_so_name_versioning
   otherwise.
   This is highlighted by rpmlint, too:
shotcut.x86_64: E: invalid-soname /usr/lib64/libCuteLogger.so libCuteLogger.so

2. Requires:       ffmpeg
   must be changed to e.g.
   Requires:       /usr/bin/ffmpeg

3. Patch0:         Force_X.patch
   Fedora is Wayland by default, so any deviations should be explained. Is there an upstream
   bug report?

4. It might be worth using a macro for org.%{name}.Shotcut string, which gets repeated
   multiple times in the spec.

5. There are multiple duplicate files:
shotcut.x86_64: E: files-duplicated-waste 779412
shotcut.x86_64: W: files-duplicate /usr/share/shotcut/qml/filters/bigsh0t_rect_to_eq/icon.webp /usr/share/shotcut/qml/filters/bigsh0t_eq_to_rect/icon.webp
shotcut.x86_64: W: files-duplicate /usr/share/shotcut/qml/filters/select0r/icon.webp /usr/share/shotcut/qml/filters/bluescreen0r/icon.webp:/usr/share/shotcut/qml/filters/mask_chromakey/icon.webp
shotcut.x86_64: W: files-duplicate /usr/share/shotcut/qml/filters/blur_lowpass/icon.webp /usr/share/shotcut/qml/filters/blur_exponential/icon.webp:/usr/share/shotcut/qml/filters/blur_gaussian/icon.webp
shotcut.x86_64: W: files-duplicate /usr/share/shotcut/qml/filters/gpstext/vui.qml /usr/share/shotcut/qml/filters/dynamictext/vui.qml
shotcut.x86_64: W: files-duplicate /usr/share/shotcut/qml/filters/fadein_movit/icon.webp /usr/share/shotcut/qml/filters/fadein_brightness/icon.webp
shotcut.x86_64: W: files-duplicate /usr/share/shotcut/qml/filters/fadeout_movit/icon.webp /usr/share/shotcut/qml/filters/fadeout_brightness/icon.webp
shotcut.x86_64: W: files-duplicate /usr/share/shotcut/qml/filters/lenscorrection/icon.webp /usr/share/shotcut/qml/filters/fisheye/icon.webp
shotcut.x86_64: W: files-duplicate /usr/share/shotcut/qml/filters/vaguedenoiser/icon.webp /usr/share/shotcut/qml/filters/fspp/icon.webp:/usr/share/shotcut/qml/filters/hqdn3d/icon.webp:/usr/share/shotcut/qml/filters/noise_fast/icon.webp:/usr/share/shotcut/qml/filters/smartblur/icon.webp
shotcut.x86_64: W: files-duplicate /usr/share/shotcut/qml/filters/vibrance/icon.webp /usr/share/shotcut/qml/filters/hue_lightness_saturation/icon.webp
shotcut.x86_64: W: files-duplicate /usr/share/shotcut/qml/filters/spectrum/vui_spectrum.qml /usr/share/shotcut/qml/filters/lightshow/vui.qml
shotcut.x86_64: W: files-duplicate /usr/share/shotcut/qml/filters/mask_shape/icon.webp /usr/share/shotcut/qml/filters/mask_alphaspot/icon.webp

Consider hardlinking them to save space.

6. # The entire source code is GPLv3+ except mvcp/ which is LGPLv2+
   License:        GPL-3.0-or-later AND LGPL-2.1-or-later

Licensing needs an update:
* mvcp is no longer there
* CuteLogger is LGPL 2.1 only (source code doesn't mention "or any later version") and I'm not sure if it can be combined with GPL 3.0 as-is.
./CuteLogger/include/AbstractAppender.h: LGPL-2.1
./CuteLogger/include/AbstractStringAppender.h: LGPL-2.1
./CuteLogger/include/ConsoleAppender.h: LGPL-2.1
./CuteLogger/include/FileAppender.h: LGPL-2.1
./CuteLogger/include/Logger.h: LGPL-2.1
./CuteLogger/include/OutputDebugAppender.h: LGPL-2.1
./CuteLogger/src/AbstractAppender.cpp: LGPL-2.1
./CuteLogger/src/AbstractStringAppender.cpp: LGPL-2.1
./CuteLogger/src/ConsoleAppender.cpp: LGPL-2.1
./CuteLogger/src/FileAppender.cpp: LGPL-2.1
./CuteLogger/src/Logger.cpp: LGPL-2.1
./CuteLogger/src/OutputDebugAppender.cpp: LGPL-2.1
* spatialmedia is Apache-licensed:
./src/spatialmedia/box.cpp: Apache-2.0
./src/spatialmedia/box.h: Apache-2.0
./src/spatialmedia/constants.h: Apache-2.0
./src/spatialmedia/container.cpp: Apache-2.0
./src/spatialmedia/container.h: Apache-2.0
./src/spatialmedia/mpeg4_container.cpp: Apache-2.0
./src/spatialmedia/mpeg4_container.h: Apache-2.0
./src/spatialmedia/sa3d.cpp: Apache-2.0
./src/spatialmedia/sa3d.h: Apache-2.0
./src/spatialmedia/spatialmedia.cpp: Apache-2.0
./src/spatialmedia/spatialmedia.h: Apache-2.0
* There's a bunch of bundled JavaScript, which is MIT licensed:
./doc/html/clipboard.js: MIT
./doc/html/dynsections.js: MIT
./doc/html/jquery.js: MIT
./doc/html/menu.js: MIT
./doc/html/menudata.js: MIT
./doc/html/resize.js: MIT

(In reply to Dominik 'Rathann' Mierzejewski from comment #9)
[...]
> 6. # The entire source code is GPLv3+ except mvcp/ which is LGPLv2+
>    License:        GPL-3.0-or-later AND LGPL-2.1-or-later
> 
> Licensing needs an update:
> * mvcp is no longer there
> * CuteLogger is LGPL 2.1 only (source code doesn't mention "or any later
> version") and I'm not sure if it can be combined with GPL 3.0 as-is.

gnu.org says it can:
https://www.gnu.org/licenses/gpl-faq.html.en#compat-matrix-footnote-7 so it seems fine.

(In reply to Dominik 'Rathann' Mierzejewski from comment #9)
> Taking review. Quick notes:

> 3. Patch0:         Force_X.patch
>    Fedora is Wayland by default, so any deviations should be explained. Is
> there an upstream
>    bug report?

I have removed the Force_X.patch for now, as I have not noticed any problems with wayland so far.

> 
> 6. # The entire source code is GPLv3+ except mvcp/ which is LGPLv2+
>    License:        GPL-3.0-or-later AND LGPL-2.1-or-later
> 
I don't know if I'm right about the comments on the licence.

built new package.

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/shotcut.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/shotcut-24.06.02-2.fc40.src.rpm

%changelog
* Wed Jun 12 2024 Martin Gansser <martinkg> - 24.06.02-2
- Move libCuteLogger.so out of linker search path
- Changed RR ffmpeg to RR /usr/bin/ffmpeg
- Use macro %%{org_name_shotcut}
- Licensing update
- hardlink duplicate files

Created attachment 2037082 [details]
The .spec file difference from Copr build 7585836 to 7608493

Copr build:
https://copr.fedorainfracloud.org/coprs/build/7608493
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2254709-shotcut/fedora-rawhide-x86_64/07608493-shotcut/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment on attachment 2037082 [details]
The .spec file difference from Copr build 7585836 to 7608493

License:        GPL-3.0-or-later AND LGPL-2.1-or-later

This still needs to be updated to include the Apache and MIT license tags, e.g.:

License:        GPL-3.0-or-later AND LGPL-2.1-or-later AND Apache-2.0 AND MIT

-Patch0:         Force_X.patch
+#Patch0:         Force_X.patch

Please just drop it instead of commenting out.

+# Remove duplicate files and create hardlinks

You can use the hardlink tool to do that automatically, i.e.:

BuildRequires: hardlink

%install
...
hardlink -v %{buildroot}/usr/share/shotcut/qml/filters

(In reply to MartinKG from comment #11)
> - Move libCuteLogger.so out of linker search path

You did that without patching in the new path, so now it fails to run:

$ shotcut
shotcut: error while loading shared libraries: libCuteLogger.so: cannot open shared object file: No such file or directory

I'd do it by adding a RUNPATH pointing to %{_libdir}/shotcut.

built new package.

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/shotcut.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/shotcut-24.06.02-3.fc40.src.rpm

%changelog
* Sun Jun 16 2024 Martin Gansser <martinkg> - 24.06.02-3
- license update
- drope unused patch
- Remove duplicate files and create hardlinks
- Add RUNPATH pointing to %%{_libdir}/shotc

> - Add RUNPATH pointing to %%{_libdir}/shotc

I think you cut the changelog line short above. It runs fine now, however:
$ rpm -q --provides shotcut|grep Cute
libCuteLogger.so()(64bit)
$ rpm -qR shotcut|grep Cute
libCuteLogger.so()(64bit)

Please filter this internal library out from Provides: and Requires:, for example by adding:

%global __provides_exclude_from ^%{_libdir}/%{name}/libCuteLogger\\.so
%global __requires_exclude ^libCuteLogger\\.so

See https://docs.fedoraproject.org/en-US/packaging-guidelines/AutoProvidesAndRequiresFiltering/
for more details.

> # The entire source code is GPLv3+

The comment is wrong now, change it to, for example, "# Main code is GPLv3+" or drop entirely.

> License:        GPL-3.0-or-later AND LGPL-2.1-or-later AND Apache-2.0 AND MIT

As I mentioned earlier, CuteLogger is LGPL2-2.1-only. Please correct the license string.

Other than that, it looks good.

Created attachment 2037535 [details]
The .spec file difference from Copr build 7608493 to 7619071

Copr build:
https://copr.fedorainfracloud.org/coprs/build/7619071
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2254709-shotcut/fedora-rawhide-x86_64/07619071-shotcut/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

built new package.

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/shotcut.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/shotcut-24.06.02-4.fc40.src.rpm

%changelog
* Sun Jun 16 2024 Martin Gansser <martinkg> - 24.06.02-4
- Filter out internal library from Provides: and Requires:
- Correct license type

Created attachment 2037541 [details]
The .spec file difference from Copr build 7619071 to 7619451

Copr build:
https://copr.fedorainfracloud.org/coprs/build/7619451
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2254709-shotcut/fedora-rawhide-x86_64/07619451-shotcut/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Thanks for bearing with me, Martin, but the License: field is still incorrect:

> License:        GPL-3.0-or-later AND LGPL-2.1-or-later AND Apache-2.0 AND MIT

It should read:

License:        GPL-3.0-or-later AND LGPL-2.1-only AND Apache-2.0 AND MIT

If you can correct the above, the the package will be APPROVED.

built new package, hopefully solved.
many thanks for your great help.

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/shotcut.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/shotcut-24.06.02-5.fc40.src.rpm

%changelog
* Mon Jun 17 2024 Martin Gansser <martinkg> - 24.06.02-5
- Fixed license type

Created attachment 2037628 [details]
The .spec file difference from Copr build 7619451 to 7621612

Copr build:
https://copr.fedorainfracloud.org/coprs/build/7621612
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2254709-shotcut/fedora-rawhide-x86_64/07621612-shotcut/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Looks good now, APPROVED.

Good job, thanks!

The Pagure repository was created at https://src.fedoraproject.org/rpms/shotcut