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

Re: RFS: liboauth (Updated package)



Paul Wise <pabs <at> debian.org> writes:
> 
> On Thu, May 20, 2010 at 1:52 PM, Bilal Akhtar <bilalakhtar96 <at> yahoo.com>
wrote:
>

Hi there,
Upstream liboauth-developer here.

 
> > in Debian was the reasons why many app developers copied the source code
into their programs.
> 
> Way to get my attention! If any of these apps are already in Debian,
> please notify the security team about the embedded code copies. Since
> you got my attention, here is a review (I don't have time to commit
> ongoing sponsorship, sorry):
> 
> Please send the patches upstream if you haven't already.
> 
> You can replace aclocal/autoheader/autoconf/automake with autoreconf.
> Probably you want dh-autoreconf instead of doing it manually. What is
> the reason for running autotools anyway?
> 
> The paths in debian/patches/acandam.diff are specific to your personal
> machine, that is probably a bad idea.
> 
> debian/patches/changeencodinglocale.diff is not present in
> debian/patches/series so it will not get applied, is that what you
> wanted to do?
> 
> Insert my standard comment about library package descriptions, think
> about the audience for each one. -dev package will be manually
> installed by people developing apps using liboauth and also as part of
> build-depends. liboauth0 should only be installed automatically so it
> doesn't need a verbose description.
> 
> You forgot to build-depend on autoconf/libtool.
> 
> debian/docs should probably be named debian/liboauth-dev.examples
> 
> No need to be so specific with the manual page path, usr/share/man/
> should do it.
> 
> xmalloc is GPL not LGPL so I'm wondering why upstream and
> debian/copyright refer to the LGPL.

xmalloc is 'generated' by autotools (autotools replaces the project-name in
there) and /usr/share/doc/autoproject/README reads:

"autoproject itself is distributed under the GNU General Public License.
As a special exception to the GNU General Public License, you may
use the files generated by autoproject without any restriction."

> 
> xmalloc reduces the amount of software that can link with liboauth
> (due to GPL licensing incompatibilities), it would be nice if upstream
> could use plain malloc. You may want to send upstream a patch.

no need to send a patch: 
xmalloc is only used IFF liboauth is configured with --enable-gpl.
 
> Uhh, actually since you are linking xmalloc and OpenSSL (GPL & OpenSSL
> licenses are not compatible), the liboath0 binary package is not
> distributable!

Thanks for bringing this to my attention, I've added an exemption as suggested
by http://lists.debian.org/debian-legal/2004/05/msg00595.htm
 
> It is best to license debian/ under the same license as upstream so
> that they can make use of your patches etc.
> 
> Should you be depending on locales/locales-all too?
> 
> Do you need to install the static library and .la file? Debian seems
> to be moving towards not installing either of these.
> 
> autotools warnings (send upstream):
> 
> libtoolize: Consider adding `AC_CONFIG_MACRO_DIR([m4])' to configure.ac and

Thanks, added in v0.7.0 (r46 in the sf.net SVN repo)

> libtoolize: rerunning libtoolize, to keep the correct libtool macros in-tree.
> libtoolize: Consider adding `-I m4' to ACLOCAL_AMFLAGS in Makefile.am.

?? I don't use ACLOCAL_AMFLAGS in Makefile.am.
 
> gcc warnings (send upstream):
> 
> oauthbodyhash.c: In function 'main':
> oauthbodyhash.c:65: warning: unused variable 'bh'

Also fixed. Actually that's example code and 'bh' (aka body-hash) is only unused
in the default example (there's some #ifdef in there).
 
> lintian complaints (send most upstream):
> 
> I: liboauth0: no-symbols-control-file usr/lib/liboauth.so.0.5.2
> X: liboauth0: shlib-calls-exit usr/lib/liboauth.so.0.5.2
> I: liboauth-dev: hyphen-used-as-minus-sign usr/share/man/man3/oauth.3.gz:374
> I: liboauth-dev: hyphen-used-as-minus-sign usr/share/man/man3/oauth.3.gz:376
> I: liboauth-dev: hyphen-used-as-minus-sign usr/share/man/man3/oauth.3.gz:403
> I: liboauth-dev: hyphen-used-as-minus-sign usr/share/man/man3/oauth.3.gz:405
> I: liboauth-dev: hyphen-used-as-minus-sign usr/share/man/man3/oauth.3.gz:863

Well, these are tricky! Said man-page is generated by Doxygen. Any ideas how to
tell doxygen to properly escape those hyphens?

I've found
http://www.mail-archive.com/debian-mentors@lists.debian.org/msg39606.html
but replacing them is not really an option since doxygen is also generating html
from the same sources.


> I: liboauth-dev: spelling-error-in-manpage
> usr/share/man/man3/oauth.3.gz paramater parameter
> I: liboauth-dev: spelling-error-in-manpage
> usr/share/man/man3/oauth.3.gz recieve receive
> I: liboauth0: spelling-error-in-binary ./usr/lib/liboauth.so.0.5.2
> environement environment

LOL. I've corrected those.

Cheers!
robin  



> uscan warning:
> 
> pabs <at> chianamo:~/tmp/liboauth-0.6.3$ uscan
> Use of uninitialized value $2 in split at /usr/bin/uscan line 1503,
> <WATCH> line 2.
> 
> Seems that can be fixed by adding a slash to the URL.
> 





Reply to: