[Bug 1386938] Review Request: libprelude - Prelude Library

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

 



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



--- Comment #13 from Thomas Andrejak <thomas.andrejak@xxxxxxxxx> ---
(In reply to Zbigniew Jędrzejewski-Szmek from comment #12)
> # Force default attrs because libprelude force others
> %defattr(- , root, root, 755)
> → I think you don't need this anymore.

=> Just try it, I still need this

> 
> %{python3_sitearch}/_prelude.*so
> %{python3_sitearch}/prelude.py
> → Not a packaging issue, but still something to reconsider upstream. I think
> putting a private module at the top level is rather ugly. Imaging the mess
> if everybody did that ;). Why not structure this as
> %{python3_sitearch}/prelude/__init__.py
> %{python3_sitearch}/prelude/_prelude.*so
> ? (Please note that I'm just complaining here, this review is not contingent
> on this in any way.)

Good point, I will put this upstream

> 
> BR: perl-generators is needed according to
> http://fedoraproject.org/wiki/Packaging:Perl#Build_Dependencies,
> and also R: perl(:MODULE_COMPAT), see
> http://fedoraproject.org/wiki/Packaging:
> Perl#Versioned_MODULE_COMPAT_Requires.

Done

> 
> - Provides: bundled(gnulib) in place as required.
>   Note: Bundled gnulib but no Provides: bundled(gnulib)
>   See:
>  
> http://fedoraproject.org/wiki/Packaging:
> No_Bundled_Libraries#Requirement_if_you_bundle

Done

> 
> - Note: License file LICENSE.README is not marked as %license
> Yeah, it seems reasonable to include that in %license too.
> Same goes for HACKING.README.

Done

> 
> - Large documentation must go in a -doc subpackage. Large could be size
>   (~1MB) or number of files.
>   Note: Documentation size is 5703680 bytes in 53 files.
>   See:
>   http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation
> 
> I'm not sure how exactly fedora-review arrives at this number, but it seems
> that there's indeed a few MBs of documentation. You might want to split out
> libprelude-doc subpackage with /usr/share/doc/libprelude-devel/libprelude.
> (Also not that there's an extra level of directories nesting here that looks
> accidental.)

Done

> 
> rpmlint also says:
> libprelude-debuginfo.x86_64: E: incorrect-fsf-address
> /usr/src/debug/libprelude-3.1.0/src/libprelude-error/code-to-errno.h
> libprelude-debuginfo.x86_64: E: incorrect-fsf-address
> /usr/src/debug/libprelude-3.1.0/src/libprelude-error/err-sources.h
> libprelude-debuginfo.x86_64: E: incorrect-fsf-address
> /usr/src/debug/libprelude-3.1.0/src/libprelude-error/strsource.c
> libprelude-debuginfo.x86_64: E: incorrect-fsf-address
> /usr/src/debug/libprelude-3.1.0/src/libprelude-error/code-from-errno.h
> libprelude-debuginfo.x86_64: E: incorrect-fsf-address
> /usr/src/debug/libprelude-3.1.0/src/libprelude-error/err-codes.h
> libprelude-debuginfo.x86_64: E: incorrect-fsf-address
> /usr/src/debug/libprelude-3.1.0/src/libprelude-error/strerror.c
> It's not a big issue, but probably to fix upstream at some point.

I can't change this, this is not part of libprelude

> 
> libprelude.x86_64: W: crypto-policy-non-compliance-gnutls-2
> /usr/lib64/libprelude.so.23.3.0 gnutls_priority_init
> prelude-tools.x86_64: W: crypto-policy-non-compliance-gnutls-1
> /usr/bin/prelude-admin gnutls_priority_set_direct
> 
> This one is a bigger problem. It's been a while since I looked at the
> details, but basically you need to call gnutls_set_default_priority or 
> gnutls_priority_set_direct("@SYSTEM")
> [https://fedoraproject.org/wiki/Packaging:CryptoPolicies]. If this policy
> does not apply to this package for some reason, please explain in a comment
> in the spec file. Looking at ./prelude-admin/server.c, it should be easy
> enough to patch.

Done. I explain in the spec why it is OK for libprelude.so. For prelude-admin,
I made a patch to use what is required in the Wiki but I still have the warning

> 
> libprelude.x86_64: W: unused-direct-shlib-dependency
> /usr/lib64/libprelude.so.23.3.0 /lib64/libdl.so.2
> libprelude.x86_64: W: unused-direct-shlib-dependency
> /usr/lib64/libprelude.so.23.3.0 /lib64/libgpg-error.so.0
> libprelude.x86_64: W: unused-direct-shlib-dependency
> /usr/lib64/libpreludecpp.so.8.1.0 /lib64/libgnutls.so.30
> libprelude.x86_64: W: unused-direct-shlib-dependency
> /usr/lib64/libpreludecpp.so.8.1.0 /lib64/libgcrypt.so.20
> libprelude.x86_64: W: unused-direct-shlib-dependency
> /usr/lib64/libpreludecpp.so.8.1.0 /lib64/libdl.so.2
> libprelude.x86_64: W: unused-direct-shlib-dependency
> /usr/lib64/libpreludecpp.so.8.1.0 /lib64/libgpg-error.so.0
> libprelude.x86_64: W: unused-direct-shlib-dependency
> /usr/lib64/libpreludecpp.so.8.1.0 /lib64/libltdl.so.7
> libprelude.x86_64: W: unused-direct-shlib-dependency
> /usr/lib64/libpreludecpp.so.8.1.0 /lib64/libm.so.6
> 
> Overlinking? Not a big issue, because those libraries are going to be
> installed anyway, but removing it might reduce memory usage and startup time
> but some minuscule amount.

Done

> 
> And one more suggestion for upstream reconsideration:
> custom autoconf macros are horrible. The problem is that any project that
> wants to use them, must either bundle them (which is annoying if you have
> more than two or three dependencies), or wrap the calls to those macros in
> ugly and brittle m4 macros for the case when the dependency is not
> installed. Please consider providing a pkgconfig file, which is easier to
> write, easier to use, and as a bonus, works with other build systems like
> meson. (Please note that I'm just complaining here, this review is not
> contingent on this in any way.)

Interesting, I will share this with upstream

-- 
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




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]