Fedora Merge Review: nspr http://cvs.fedora.redhat.com/viewcvs/devel/nspr/ Initial Owner: kengert
rpmlint on SRPM: Unversion obsoletes for mozilla-nspr and mozilla-nspr-devel. Probably want to correct this. nspr.src: E: no-cleaning-of-buildroot %install You should clean $RPM_BUILD_ROOT in the %clean section and just after the beginning of %install section. Use "rm -Rf $RPM_BUILD_ROOT". MUST fix this. nspr.src: W: mixed-use-of-spaces-and-tabs (spaces: line 13, tab: line 1) The specfile mixes use of spaces and tabs for indentation, which is a cosmetic annoyance. Use either spaces or tabs for indentation, not both. Cosmetic, but since you're making changes anyway. . . There are no documentation files in either the main package or -devel. If there are none, this is OK, which seems to be the case. The Source0 tag lacks a URL, and I can't locate the appropriate tarball on the upstream site, so I can't compare the upstream tarball and SRPM tarball. If this a modifed upstream tarball or cvs snapshot, please include a script that creates this tarball from upstream. Otherwise, it looks pretty good, no other blockers.
(In reply to comment #1) > Unversion obsoletes for mozilla-nspr and mozilla-nspr-devel. Probably want to > correct this. It obsoletes *any* mozilla-nspr package, because we've stopped shipping NSPR as part of the mozilla packages, but started to ship it as a standalone package. Do you still think a version number must get included? I would have to research which version number was last, at some point before FC6. Would it have to be the exact version number? > nspr.src: E: no-cleaning-of-buildroot %install > You should clean $RPM_BUILD_ROOT in the %clean section and just after the > beginning of %install section. Use "rm -Rf $RPM_BUILD_ROOT". The %clean section already has a rm statement. I've changed it to use uppercase -R I've added it to the beginning of the %install section. > nspr.src: W: mixed-use-of-spaces-and-tabs (spaces: line 13, tab: line 1) > The specfile mixes use of spaces and tabs for indentation, which is a > cosmetic annoyance. Use either spaces or tabs for indentation, not both. fixed > There are no documentation files in either the main package or -devel. > If there are none, this is OK, which seems to be the case. find . -type f | grep -v \.c$ |grep -v \.a$ | grep -v \.so$ | grep -v \.o$ | grep -vw CVS | grep -v \.h$ | grep -v \.orig$ | grep -v \.rej$ | grep -v Makefile$ |grep -v \.mk$ | grep -v \.~ | grep -v \.bck$ | grep -v \.mn | grep -v '/\.#' | grep -v \.def$ | grep -v \.chk$ | grep -v \.s$ |grep -vw tests | grep -vw OBJ | grep -v \.cfg$ | grep -v Makefile.in$ |grep -v \.cpp$ |grep -vw cvsignore |grep -v \.mk.in$ | grep -v \.sh\.in$ | grep -v \.pl$ | grep -v \.m4$ |grep -v \.asm$ I looked at all the files this command produced. ./nsprpub/pr/src/threads/combined/README ./nsprpub/lib/libc/README ./nsprpub/lib/libc/src/README These look like a developer oriented quick dump of thoughts from 1996/7, but they don't seem to be worthy to be called "documentation". > The Source0 tag lacks a URL, and I can't locate the appropriate tarball on the > upstream site, so I can't compare the upstream tarball and SRPM tarball. If > this a modifed upstream tarball or cvs snapshot, please include a script that > creates this tarball from upstream. I've added a comment that points to the download directory where official releases are available. Sometimes we do package non-release snapshots. If we do, there is no matching tarball available, but it always will be based on a CVS tag (the changelog shall make it clear what CVS tag is being used).
I think the above should be sufficent, once committed to cvs. :)
Any updates?
All changes addressed, approved. Might want to fix nspr-debuginfo.i386: W: spurious-executable-perm /usr/src/debug/nspr-4.7.1/dist/include/nspr/prvrsion.h The file is installed with executable permissions, but was identified as one that probably should not be executable. Verify if the executable bits are desired, and remove if not. but I won't hold up the review on it.