Spec Name or Url: http://ivrix.org.il/redhat/taarich.spec SRPM Name or Url: http://ivrix.org.il/redhat/taarich-1.20051120-1.src.rpm Description: This RPM includes two small utilities by Zvi Har'El. Taarich displays the current Hebrew date (in English or in Hebrew). Luach renders a calendar of the current Gregorian month, with Hebrew dates. Both use Gauss's algorithm to find the Gregorian date of Passover. As is bug 173683, this is attempt to make Fedora more useful for Hebrew speakers.
From the spec file: %build make - Will it build with the RPM_OPT_FLAGS ? - should use sed to add the $RPM_OPT_FLAGS to the CFLAGS in Makefile install -d %{buildroot}/{%{_docdir}/%{name},%{_bindir},%{_mandir}/man1} - Should be install -d -m755 %{buildroot}%{_bindir} install -d -m755 %{buildroot}%{_mandir}/man1 - You don't need to install the docdir - the %doc macro will do that for you. - Splitting it up instead of one liner makes it more readable with the macros - in there. gzip luach.man taarich.man cp luach.man.gz %{buildroot}%{_mandir}/man1/luach.1.gz cp taarich.man.gz %{buildroot}%{_mandir}/man1/taarich.1.gz - don't gzip the man pages, rpm will do it for you - use install to put the man pages in their place install -m644 {luach.man,taarich.man} %{buildroot}%{_mandir}/man1/ - will install them, and rpm will compress them. %files %defattr(-,root,root) %{_bindir}/taarich %{_bindir}/luach %doc gauss.txt reading.txt COPYING %{_mandir}/man1/taarich.1* %{_mandir}/man1/luach.1* - %doc is usually right below %defattr - %defattr needs to be changed to %defattr(-,root,root,-) - man pages need to be changed %files %defattr(-,root,root,-) %doc gauss.txt reading.txt COPYING %{_bindir}/taarich %{_bindir}/luach %{_mandir}/man1/* - would be much better
Thank you for your suggestions. I applied all of them (I hope so), except for the one regarding sed; I did not quite understand what you wanted. Is it enough to pass make CFLAGS="$RPM_OPT_FLAGS -s" ? An updated SRPM sits in http://ivrix.org.il/redhat/taarich-1.20051120-2.src.rpm and the spec file is again in http://ivrix.org.il/redhat/taarich.spec
(In reply to comment #2) > Thank you for your suggestions. I applied all of them (I hope so), > except for the one regarding sed; I did not quite understand what you > wanted. Is it enough to pass make CFLAGS="$RPM_OPT_FLAGS -s" ? Yeah - that works too. > > An updated SRPM sits in http://ivrix.org.il/redhat/taarich-1.20051120-2.src.rpm > and the spec file is again in http://ivrix.org.il/redhat/taarich.spec Formal review coming
What's that "-s" in CFLAGS for? If it's for stripping binaries, it should be removed as it'll result in a useless -debuginfo package.
(in reply to comment 4) Yep, that's my bad. I mistakenly understood that Michael A. Peters wanted that the upstream compilation flags would be maintained. Will be fixed during next take.
(In reply to comment #5) > (in reply to comment 4) > Yep, that's my bad. I mistakenly understood that Michael A. Peters > wanted that the upstream compilation flags would be maintained. > Will be fixed during next take. With that change - Approved Good: + (from reviewer MUST section) * rpmlint output clean: [mpeters@utility result]$ ls build.log taarich-1.20051120-2.fc4.i386.rpm mockconfig.log taarich-1.20051120-2.fc4.src.rpm root.log taarich-debuginfo-1.20051120-2.fc4.i386.rpm [mpeters@utility result]$ rpmlint *.rpm [mpeters@utility result]$ * Package named according to guidelines - no upstream tarball name, named according to primary binary * Spec file name matches binary package name * Package meets packaging guidelines * OSI Approved license (MIT) - matches source * License in %doc * Spec file written in American English - Hebrew portions obviously appropriate, and english exists * Spec file legible * Sources match upstream - I mirrored upstream ftp directory to verify md5sum of files -- lftp -e 'mirror -e' ftp://ftp.math.technion.ac.il/calendar/gauss/ * Package succesfully compiles on i386 FC4 * No un-necessary BuildRequires, builds in mock * No lang files (no need for %find_lang) * No shared libraries (no need for ldconfig) * Proper use of macros for paths * No duplicate files * Proper %files section - appropriate permissions/ownership * Proper %clean * Consistent use of macros * Package contains code * No need for separate doc package * No need for devel package * No .la files * No need for desktop file + (from reviewer SHOULD section) * Builds in mock * Hebrew description * Package works [mpeters@utility result]$ taarich 19 Heshvan 5766 [mpeters@utility result]$ -=- Needs: There should be a Hebrew summary line since there is a Hebrew description. This is not a blocker, I'm approving. But adding Summary(he): Something in Hebrew under Summary: Tells the Hebrew date, Torah readings, and generates calendars would be advised.