[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 #15 from Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> ---
(In reply to Thomas Andrejak from comment #13)
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #12)
> > 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

> # - crypto-policy-non-compliance-gnutls-2 in libprelude.so is normal
> #   because the user can configure it

That's the only important issue. While it is true that the value
can be overridden, most users of the library will not override the
default, so the "default" provided by the library is what will be
used most often in the end. IIUC, libprelude.so has
tls_auth_init_priority() which calls gnutls_priority_init(tlsopts ?: "NORMAL"),
and tls_auth_init_priority() is called in a few places as 
tls_auth_init_priority(NULL), e.g. from tls_auth_connection().
So effectively "NORMAL" is the default. I think it should be changed
to "@SYSTEM".

For a similar case, consider libmicrohttpd. We patched it
[http://pkgs.fedoraproject.org/cgit/rpms/libmicrohttpd.git/tree/gnutls-utilize-system-crypto-policy.patch]
to use @SYSTEM, and all users of libmicrohttpd magically became
compliant with the policy without any further work.

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

> > - 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: it'd be nicer to include a specific "version", something like
Provides: bundled(gnulib) = 20160508
but if it's hard to determine or unclear or whatever, it's fine as is.

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

Those same files in libgpg-error sources are quite different. It seems that
libprelude has an independent forked copy, so you *could* fix the address.
But anyway, it doesn't matter much.

I noticed another issue: valgrind is used in tests, but is not listed in
BuildRequires. Unfortunately, valgrind is not available on all architectures.
There's a macro %{valgrind_arches}, but it is only available in rawhide.
That could be used in the future, but for now:
%ifnarch s390
BuildRequires: valgrind
%endif
... and you also have to make sure that the test still run without valgrind.

>> And one more suggestion for upstream reconsideration:
>> custom autoconf macros are horrible.
> Interesting, I will share this with upstream
Oh, I now see that there's a pkg-config file already. So my rant wasn't
necessary. Sorry.

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