[Bug 1972445] Review Request: servus - Zeroconf discovery in C++

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1972445



--- Comment #5 from Jaroslav Škarvada <jskarvad@xxxxxxxxxx> ---
(In reply to Otto Urpelainen from comment #1)
Guys, thanks for review.

> > For the bundling probably FESCO exception will be needed.
> 
> Not anymore, nowadays the packager can decide to leave bundled dependencies
> in if upstream does not support using system libraries instead.
> 
> ---
> 
> I would have taken this if Rafael did not beat me to it. Anyhow, I did a
> complete review, here are my findings:
> 
> > License:       LGPLv3 and RSA
> 
> Since this is a multiple licensing situation, the specfile must contain a
> comment explaining the license breakdown.
> 
> Reference:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/
> LicensingGuidelines/#_multiple_licensing_scenarios
>
Hopefully fixed.

> > Provides:      bundled(eyescale-cmake-common)
> 
> This is a build time only dependency, right? I wonder if the policy for
> bundled dependencies applies as-is to such case. Certainly the objective of
> flagging the bundling is achieved, I just wonder if it would be useful to
> also flag that it is a build time thing. Should we ask in the devel list
> about this? Reference:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling
>
I understand it as it is used to easily find packages which are affected by
security bug in the bundled code. It's unlikely the cmake macros could
introduce some runtime security bug, but I think it should be noted there. I am
trying to convince upstream to allow building with the system cmake macros. In
such case I will package the macros to standalone package.

> > %package doc
> 
> Is the doc subpackage needed? Currently, there are no %files entry for it,
> and it seems that (probably because of that) there is no rpm for the
> subpackage either. And all the docs go to the main package — which is fine,
> since there is not a lot of them. You could just remove the doc subpackage
> from the specfile.
> 
> Reference:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation
>
Ups, sorry, I was trying to build some doxygen docs, scratched the idea later,
but forgot to remove the subpackage :)

> > #%%license COPYING
> 
> This must be fixed. LGPLv3 requires distributing the full license text with
> each copy, so it must be included in the rpm. 
> Licensing guidelines allow you to add it, if you have contacted upstream,
> which you have done. So either wait for upstream response, or add the
> correct license files. According to gnu.org, the standard way would be to
> add COPYING and COPYING.LESSER with GPLv3 and LGPLv3 texts:
> https://www.gnu.org/licenses/gpl-howto.html
> 
> Reference:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/
> LicensingGuidelines/#_license_text
>
Hopefully fixed.

> > %{_libdir}/*.so.*
> 
> Globbing all shared objects like this SHOULD NOT be done. At least the major
> version number should be fixed, so that ABI breaks are noticed on updates.
> So do %{_libdir}/libSerevus.so.6* etc. instead.
> 
> Reference:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/
> #_listing_shared_library_files
>
I tried to fix it, but I noticed the upstream project has broken versioning,
i.e.:
libServus.so.1.6.0
libServus.so.6

I.e. its SONAME is libServus.so.6, ABI version 6, but library version is 1.6.0.
It's strange, I will report upstream. I think we shouldn't package it until
clarified what's the correct version is.

> > servusBrowser
> 
> This is a gui application, so it needs to have a desktop file.
> 
> Reference:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/#_desktop_files
> 
> Strangely, the review guidelines also allow to put a comment in specfile
> explaining why a desktop file is not needed. I cannot find any basis for
> that from the Packaging Guidelines.

I tried to fix it.


-- 
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
_______________________________________________
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 on the list, report it: https://pagure.io/fedora-infrastructure




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux