Spec URL: http://sjhserv.net/~sjh/rats.spec SRPM URL: http://sjhserv.net/~sjh/rats-2.1-1.fc8.src.rpm Description: RATS scans through code, finding potentially dangerous function calls. The goal of this tool is not to definitively find bugs (yet). The current goal is to provide a reasonable starting point for performing manual security audits. The initial vulnerability database is taken directly from things that could be easily found when starting with the forthcoming book, "Building Secure Software" by Viega and McGraw. RATS is released under version 2 of the GNU Public License (GPL).
Taking this for review.
Scott: thanks for the package, it looks very well. Here are some random things that would be nice to be looked at before this this gets approval: 25 RATS is released under version 2 of the GNU Public License (GPL). This is not necessary, License: tag is for this. 30 ./configure --prefix=/usr \ 31 --datadir=%{_datadir}/rats 32 33 %build 34 CFLAGS="$RPM_OPT_FLAGS"; export CFLAGS Why not use the %configure macro? It would set the CFLAGS for you, and possibly correct libdir on 64 bit architectures, etc. 38 %install 39 [ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT Do not conditionally remove the build root. Just delete it, it is never root. 45 # rm -rf $RPM_BUILD_ROOT 46 # mkdir -p $RPM_BUILD_ROOT/usr 47 # make install prefix=$RPM_BUILD_ROOT 48 # mv bin share $RPM_BUILD_ROOT/usr/ Why is this commented? Why do you install stuff by hand instead of make install? It does not have to be wrong, just please always write comments in SPEC in case you do something nonstandard. 53 #make clean Also, never make useless comments like this. Just remove it. Also applies to what is below the %files section. 77 * Wed Sep 26 2007 Scott Henson <shenson> 78 - 2.1-1: Initial packaged version The correct format of the changelog entry would be: 77 * Wed Sep 26 2007 Scott Henson <shenson> - 2.1-1 78 - Initial packaged version
RPMLint: > $ rpmlint /home/lkundrak/rpmbuild/SRPMS/rats-2.1-1.fc8.src.rpm > rats.src:32: E: configure-without-libdir-spec Just do a $ expand rats.spec >x; mv x rats.spec > rats.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 11) This would be solved using the %configure macro, as I wrote in comment #2 > $ rpmlint /home/lkundrak/rpmbuild/RPMS/i386/rats-2.1-1.fc8.i386.rpm > $ rpmlint /home/lkundrak/rpmbuild/RPMS/i386/rats-debuginfo-2.1-1.fc8.i386.rpm > $ Fine
I can not compare the upstream version with the packaged at the time as the upstream web is not responding at the time. The patch rats-2.1-linux.diff doesn't seem right to me; 1.) files rats-2.1/lex.yy*.c are autogenerated. Why don't you regenerate them instead of patching? 2.) You seem to be patching unrelated issues in this one patch. Please eplit it into multiple patches. When I was talking about libdir in comment #2 and comment #3, I did not notice that the package doesn't make use of libdir. Please ingore it. Another bad thing in .SPEC: 8 URL: http://www.securesw.com/rats/rats-%{version}.tar.gz 9 Source0: rats-2.1.tar.gz The URL tag is for the web page of the software's author, Source0 is for URIs of the files. It should look like this instead: 8 URL: http://www.securesw.com/rats/ 9 Source0: http://www.securesw.com/rats/rats-%{version}.tar.gz Apart from these issues, the package meets the Packing Guidelines, and I will approve it when the issues are resolved.
Builds fine in mock for fedora-devel-i386
Sorry I haven't responded, I didn't see the update. As for the upstream, securesw was bought out by fortify. I was contacting them with the Debian maintainer while I put the package together so that never got updated. The next version will have the corrected URL. I will take care of the other problems now as well. Thank you.
A new verion is now avaliable at http://sjhserv.net/~sjh/Rats/. I believe all the above problems should be fixed. Thank you.
Just a note, so this doesn't look stalled; we discussed the patch split on IRC and are waiting for a response from Debian patch maintainer. Also, Scott: Mentioning that the patch comes from Debian should be appropriate in patch's header.
New version avaliable. SRPM: http://sjhserv.net/~sjh/Rats/rats-2.1-3.fc8.src.rpm SPEC: http://sjhserv.net/~sjh/Rats/rats.spec * Tue Oct 9 2007 Scott Henson <shenson> - 2.1-3 - Break the monolithic patch into pieces - Build Clean, contains build cleanups and spelling corrections - Php, adds support for php3 and php4 files - Report, adds some html output cleanup - Lex, some lex bug fixes - GTK-Vuln, adds some gtk vulnerabilities - Also generate lex output files on each build. As for the Debian maintainer, the lex-yy*.c stuff just slipped in as a result of the Debian build process. I missed him calling make lex in the debian/rules file. I now call this in the spec, so its now building the lex files every time.
The latest version doesn't work because the configure macro sets datadir to /usr/share and rats wants it at /usr/share/rats. I can do a configure myself, but then I get the following error. rats.src:40: E: configure-without-libdir-spec It appears above as well and I think you said I could ignore that since rats doesn't use lib. Is that right? Or shold I do something to take care of this? If I can just ignore it, new version: SRPM: http://sjhserv.net/~sjh/Rats/rats-2.1-3.fc8.src.rpm SPEC: http://sjhserv.net/~sjh/Rats/rats.spec
New version with fixes for what was discussed in irc. SRPM: http://sjhserv.net/~sjh/Rats/rats-2.1-5.fc8.src.rpm SPEC: http://sjhserv.net/~sjh/Rats/rats.spec * Tue Oct 9 2007 Scott Henson <shenson> - 2.1-5 - Change the Makefile.in so we can use the configure macro - Rename all patches to .patch to be more standard
Tadaa, the package is beuatiful; ready to go to CVS. APPROVED
New Package CVS Request ======================= Package Name: rats Short Description: Rough Auditing Tool for Security Owners: shenson Branches: F-7 InitialCC: Cvsextras Commits: no
cvs done
Package Change Request ====================== Package Name: rats New Branches: f20 Owners: cicku
Branch exists, ownership is handled in pkgdb.