[Bug 749299] Review Request: lcgdm-dav - HTTP/DAV frontend to the DPM/LFC services

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

 



Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


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

--- Comment #2 from Ricardo Rocha <rocha.porto@xxxxxxxxx> 2011-11-04 12:58:51 EDT ---
(In reply to comment #1)
> For the purposes of process bug #749132 details the sponsorship of you.
> 
> Quick parse:
> 1) Please include details of how the tar ball is created.

Fixed.

> 2) BuildRequires:  autoconf%{?_isa}
> BuildRequires:  automake%{?_isa}
> 
> makes no sense since they noarch, these are probably all over kill but 
> certainly these two are wrong.

Actually they're not needed at all, it's a leftover from a previous spec.
Removed.

> 3) There is no such package on curl-devel on newer than RHEL5, even if it
>    is satisfied by obsoleted provides possible. Use a dist tag to be more
>    exact. Eventually libcurl-devel should drop the curl-devel
> 
> %if %{?fedora}%{!?fedora:0} >= 10 || %{?rhel}%{!?rhel:0} >= 6
> BuildRequires:  libcurl-devel
> %else
> BuildRequires:  curl-devel
> %endif
> 
>   is what I use.

Cool thanks. I was submitting koji builds to el5/epel only, guess i have to do
it for more than that before putting any specs here.

> 
> 4) You have excessive BuildRequires, e.g (lib)curl-devel requires
>    pkgconfig so there is no need to specify it. Similarly gridsite-devel
>    requires openssl-devel, there are probably others.
> 
>    This probably goes for some of your other packages, if you can trim
>    them down preferably to the minimum the better.

Makes sense. I've cleaned it up with what looks to me like the minimum.

> 5) BuildRequires:  libtool%{?_isa} you have twice.

Fixed.

> 6) On the libs package you almost certainly don't need
>    Requires:       curl%{?_isa}
>    Requires:       gridsite-libs%{?_isa} >= 1.7
>    Requires:       gsoap%{?_isa}
>    Requires:       openssl%{?_isa}
> 
>    since they will auto generated as .so requirements. Check other sub package
>    as well.

Removed.

For the server it's a dependency on the httpd (the daemon) and gridsite (for
the delegation portType). So i believe they're needed.

And i left the ones for the -devel.

> 7) Pointless comment: #cd build
>    %post devel -p /sbin/ldconfig
>    %postun devel -p /sbin/ldconfig
>    almost certainly not needed, should show up in a rpmlint.

Actually it doesn't.

Shouldn't i do this? It does provide the .so link.

> 8) Duplication of 
>    %doc README LICENSE
>    in at least devel and libs package.

Removed (i was going to swear i had a rpmlint W before, but i'm not getting one
now).

> 9) %{_sysconfdir}/init.d/lcgdm-dav
>    is presumably an init.d script so should be in /etc/rc.d/init.d
> http://fedoraproject.org/wiki/Packaging:SysVInitScript#Initscripts_on_the_filesystem

Ups, sorry. Missed that one.

> # rpmlint items:
> 10) Fails on http://svnweb.cern.ch/trac/lcgdm, switch to https since
>     its a permanent 301 relocation.
> 
> Am surprised that rpmlint does not complain about
> $ rpm -qp --scripts  lcgdm-dav-devel-0.5.0-1.fc15.x86_64.rpm 
> postinstall program: /sbin/ldconfig
> postuninstall program: /sbin/ldconfig
> 
> http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Shared_libraries

I don't get it. Should i remove this from the devel package? From the docs
anything providing a shared library (or symlink) should invoke this?

> 11) lcgdm-dav-debuginfo.x86_64: E: debuginfo-without-sources
> 
> Hmm indeed they are not there, see:
> 
> rpmlint -I debuginfo-without-sources
> 
> for info and indeed despite %{cmake} being used it's seems something
> inside mangled your cflags and you built with
> 
> /usr/bin/gcc  -Dlcgdmhtext_EXPORTS -D_LARGEFILE64_SOURCE -D_REENTRANT
> -DNSTYPE_DPNS -Wall -fPIC -I/usr/include/httpd -I/usr/include/apr-1
> -I/usr/include/lcgdm -I/usr/include/dpm
> -I/home/steve/rpmbuild/BUILD/lcgdm-dav-0.5.0/client   -o
> CMakeFiles/lcgdmhtext.dir/htext_common.c.o   -c
> /home/steve/rpmbuild/BUILD/lcgdm-dav-0.5.0/client/htext_common.c
> 
> which is not
> 
> http://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

I've spent some time looking at this one and couldn't find where it's
happening. I'll put a new comment as soon as i find it, and a new version of
spec/srcrpm for review.

Thanks.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



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