https://bugzilla.redhat.com/show_bug.cgi?id=2213078 Felix Kaechele <felix@xxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(topazus@outlook.c | |om) --- Comment #7 from Felix Kaechele <felix@xxxxxxxxxxx> --- (In reply to Felix Wang from comment #5) > Thanks for your clear and detailed comments on the package review. > > > - Package conflicts with files from goldendict but doesn't specify a Conflicts tag. See https://docs.fedoraproject.org/en-US/packaging-guidelines/Conflicts/ for details on the process. > > Done. If the conflict could be avoided (i.e. by renaming the binary and data directories) that would be preferred. This would support a scenario in which a user may decide to install both variants of the package. That would fulfill the "As a general rule, Fedora packages must NOT contain any usage of the Conflicts: field." part of the packaging guidelines. > > issues about license and bundled of JavaScript libraries > > Many thanks for the extensive clarification of JavaScript libraries > licensing and their bundled things. > Accutually, I am not familiar with these things. I've updated the spec file > accordingly, hope they are correct now. Technically, JS libraries need to be unbundled like any other bundled library. In this instance it doesn't make a lot of sense as the build process then bundles the JS files into the binary. So any change to the distribution version of the JS files would have to trigger a reboot of this package regardless. So I believe bundling in this instance is an acceptable solution. > > - Package uses an ExclusiveArch tag for reasons related to qt6-webengine, this is permissible from my perspective but the approach mentioned in the referenced specfile could be used (BuildRequires qt6-srpm-macros and use %qt6_qtwebengine_arches macro). Your call. > > %qt6_qtwebengine_arches macro includes the %{ix86} architecure, which is not > supported by qt6-webengine, so I use `ExclusiveArch: aarch64 x86_64` to > explicitly set the supported architectures. > > Ref: https://src.fedoraproject.org/rpms/qt6/blob/rawhide/f/macros.qt6-srpm#_8 OK, that makes sense then. > >- CMake uses pkg-config to identify the library devel packages to install. The spec file should therefor express library >dependencies in that way: https://docs.fedoraproject.org/en-US/packaging-guidelines/PkgConfigBuildRequires/ > > This has the added benefit that the package will build even when the user has ffmpeg installed through a third-party >repository, which may conflict with ffmpeg-free-devel. > > I converted most of the dependencies to the corresponding pkgconfig() or > cmake() format, but I do not find a way to how to use pkgconfig() to express > the dependency of ffmpeg-free-devel. In the upstream sources you can find the ffmpeg headers pulled in inside the file src/ffmpegaudio.cc: #include <libavcodec/avcodec.h> #include <libavformat/avformat.h> #include <libavutil/avutil.h> #include "libswresample/swresample.h" As such you'd be looking at: pkgconfig(libavcodec) pkgconfig(libavformat) pkgconfig(libavutil) pkgconfig(libswresample) These dependencies are provided by the respective lib*-free-devel packages but alternatively also by ffmpeg-devel from RPMFusion. By using the pkgconfig BuildRequires a user can build the package locally using either Fedora or RPMFusion FFmpeg without creating a conflict. The pkgconfig BuildRequires for xz-devel is pkgconfig(liblzma) as can be seen by running dnf repoquery --provides xz-devel > > - rpmlint warning: unused-direct-shlib-dependency could be fixed by patching the CMake files to pass the as-needed flag for the fmt library, see https://stackoverflow.com/a/65819681 (this patch should be suggested upstream) > > The --as-needed flag already has been added to LDFLAGS by default. I do not > have any idea about how to fix this warning. > > Ref: https://fedoraproject.org/wiki/Changes/RemoveExcessiveLinking This one is certainly a bit weird. The source references fmt but the symbol doesn't show up in the binary. I'll need to investigate this a bit further. > (In reply to shenlebantongying from comment #4) > > > - qtsingleapplication > > > - No justification given in spec file > > > > The upstream made incompatible changes to the lib; thus have to use the > > bundled one :) > > Here is the comment from the maintainer of the upstream project, so I set > the qtsingleapplication dependency as bundled. Understood. I think bundling is acceptable in this case. ---- Summarizing the To Dos: - Please resolve the Conflicts by renaming the binaries - Introduce the pkgconfig requires for the remaining libraries. - I will investigate the linking warning a bit further. I don't think it's a real issue but I'd like to understand better why this is happening. May be an issue with the ordering of the linker flags or an overlap from fmt being supported in the stdlib of newer compiler versions. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2213078 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202213078%23c7 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
