Spec URL: http://www.reinform.de/download/rpm/vacation/vacation.spec SRPM URL: http://www.reinform.de/download/rpm/vacation/vacation-1.2.7.0-1.fc9.src.rpm Description: Vacation is the automatic mail answering program found on many Unix systems. This version works with the sendmail restricted shell. This is my first package for fedora. I'm seeking for a sponsor. In the past I worked with Than Ngo <than>.
Quick comments: 1. (minor) use prefered/standard Group: and BuildRoot: (see Packaging guidelines for details). 2. %build: use make CFLAGS="$RPM_OPT_FLAGS" 3. install usage: use -p flag (preserve timestamps), do *not* use -s (strip) 4. imo, the %post, %postun scriptlets are unwise/broken. My home-brewed vacation pkgs always included the /etc/smrsh/vacation symlink in the package. But that means vacation Requires: sendmail , I'm unware if it works with other mailers. If so, then perhaps a %trigger sendmail would work better. 5. %files: use %defattr(-,root,root,-) then you don't need to manually use %attr everwhere
Thanks for your comments. Here are my answers according to your numbers: 1. What do you mean? I do use a standard group and also a BuildRoot... 2. The Makefile of vacation already prepends $RPM_OPT_FLAGS to CFLAGS. Defining CFLAGS as argument to "make" undefined some additional defines (e.g. -DMAIN) which is otherwise done in the Makefile. Therefore I suggest to keep a simple "make". 3. done 4. I inserted triggers so that sendmail could now be installed after vacation and still installs/removes the link. 5. done I uploaded a new release to: Spec URL: http://www.reinform.de/download/rpm/vacation/vacation.spec SRPM URL: http://www.reinform.de/download/rpm/vacation/vacation-1.2.7.0-2.fc9.src.rpm
Your choice for buildroot does not respect https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag http://koji.fedoraproject.org/koji/getfile?taskID=982143&name=build.log says you have a missing BR: + make gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -g -Wall -DMAIN -Xlinker -warn-common -D_PATH_VACATION=\"/usr/bin/vacation\" -o vacation vacation.c strlcpy.c strlcat.c rfc822.c -lgdbm vacation.c:81:18: error: gdbm.h: No such file or directory
Again, thanks for any comments. I do now respect the BuildRoot_tag and I've added the missing BuildReq: gdbm-devel. I've uploaded again an updates SPEC/SRPMs to: Spec URL: http://www.reinform.de/download/rpm/vacation/vacation.spec SRPM URL: http://www.reinform.de/download/rpm/vacation/vacation-1.2.7.0-3.fc9.src.rpm
Could you please help me to get any further? Please review. Thanks!
$ rpmlint -vi vacation-1.2.7.0-3.fc9.src.rpm vacation.src: I: checking vacation.src:90: W: macro-in-%changelog post Macros are expanded in %changelog too, which can in unfortunate cases lead to the package not building at all, or other subtle unexpected conditions that affect the build. Even when that doesn't happen, the expansion results in possibly "rewriting history" on subsequent package revisions and generally odd entries eg. in source rpms, which is rarely wanted. Avoid use of macros in %changelog altogether, or use two '%'s to escape them, like '%%foo'. vacation.src:91: W: macro-in-%changelog postun Macros are expanded in %changelog too, which can in unfortunate cases lead to the package not building at all, or other subtle unexpected conditions that affect the build. Even when that doesn't happen, the expansion results in possibly "rewriting history" on subsequent package revisions and generally odd entries eg. in source rpms, which is rarely wanted. Avoid use of macros in %changelog altogether, or use two '%'s to escape them, like '%%foo'. ==> read and apply to your changelog vacation.src: W: strange-permission vacation-1.2.7.0.tar.gz 0640 A file that you listed to include in your package has strange permissions. Usually, a file should have 0644 permissions. ==> set the correct permissions before rpmbuild process starts 1 packages and 0 specfiles checked; 0 errors, 3 warnings. $ rpmbuild -ba SPECS/vacation.spec builds OK $ rpmlint -vi RPMS/x86_64/vacation-1.2.7.0-3.fc9.x86_64.rpm vacation.x86_64: I: checking vacation.x86_64: W: incoherent-version-in-changelog 1.2.7.0-1 ['1.2.7.0-3.fc9', '1.2.7.0-3'] The last entry in %changelog contains a version identifier that is not coherent with the epoch:version-release tuple of the package. ==> changelog entry and version must match => Release: 3%{?dist} => first line in %changelog 1.2.7.0-1 << 3 not 1 vacation.x86_64: W: dangerous-command-in-%post ln vacation.x86_64: W: dangerous-command-in-%postun rm vacation.x86_64: W: dangerous-command-in-%trigger ln vacation.x86_64: W: dangerous-command-in-%trigger rm 1 packages and 0 specfiles checked; 0 errors, 5 warnings. => probably no way around this. $ rpm -qp --requires RPMS/x86_64/vacation-1.2.7.0-3.fc9.x86_64.rpm /bin/sh /bin/sh /bin/sh /usr/bin/perl libc.so.6()(64bit) libc.so.6(GLIBC_2.2.5)(64bit) libc.so.6(GLIBC_2.3)(64bit) libc.so.6(GLIBC_2.3.4)(64bit) libc.so.6(GLIBC_2.4)(64bit) libgdbm.so.2()(64bit) perl >= 1:5 perl(GDBM_File) rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rtld(GNU_HASH) smtpdaemon => spec file misses: Requires: /usr/bin/perl I would add it, but opinions may differ. => in spec file "License: GPLv2 and BSD" : looking at vacation.c I note that we're dealing with the "Original BSD License (BSD with advertising)" and checking url http://fedoraproject.org/wiki/Licensing#SoftwareLicenses learn us that it is not compatible with GPLv2 ! That must be fixed somehow - check the documentation what is acceptable.
Ping?
OK, I hopefully managed to fix all problems mentioned in Comment #6. Anyway, thanks for them... Second I rebuild for FC11 which didn't mean a change. I've uploaded again an updates SPEC/SRPMs to: Spec URL: http://www.reinform.de/download/rpm/vacation/vacation.spec SRPM URL: http://www.reinform.de/download/rpm/vacation/vacation-1.2.7.0-4.fc11.src.rpm Please review, thanks!
- The -n %{name}-%{version} in %setup is redundant, as it is the default setup path. - Any reason why SMP make is not enabled? If it doesn't work, document it with a comment in the spec file. - Use the -p argument in all of the install commands. (At least for the files that are not generated in the %build phase.) - The %post and %trigger stuff seems very cumbersome. Two easier possibilities come to mind: Create the symlinks in %install and make the package own them. Additionally, for the dir ownership: 1) Require sendmail, which provides the directory. [If the package works also with postfix, exim &c then this is not adviseable.] 2) Make the package own the /etc/smsrh/ directory. [This creates a double provides, but a necessary one.] Or, you could even put the symlinks in a subpackage, say vacation-sendmail, which would require sendmail and own the symlinks. - You might want to change %{_mandir}/*/* to %{_mandir}/man1/*.1 to be a bit more precise, or even list the two files explicitly.
Also, you don't need to create the directories with mkdir -p $RPM_BUILD_ROOT%{_bindir} mkdir -p $RPM_BUILD_ROOT%{_mandir}/man1 if you just pass the -D switch to install.
Again, thanks for the comments. I fixed anything mentioned above. Also I dropped support for sendmail restricted shell as it seems to produce more questions than answers... I've uploaded again an updated SPEC/SRPMs to: Spec URL: http://www.reinform.de/download/rpm/vacation/vacation.spec SRPM URL: http://www.reinform.de/download/rpm/vacation/vacation-1.2.7.0-5.fc11.src.rpm Please review, thanks!
Items marked "GOOD" or "PASS" fulfil the guidelines or they don't apply to this package. Items marked "CHECK" aren't covered by the guidelines but you should check and fix them anyway in my opinion. Items marked "BAD" violate the guidelines in some point and need to be fixed. - GOOD: rpmlint run on vacation-1.2.7.0-5.fc11.src.rpm doesn't flag errors/warnings - GOOD: package name according to guidelines - GOOD: spec file named properly - BAD: package doesn't meet packaging guidelines: the source URL is wrong, it should be "downloads.sourceforge.net/..." (just verified it with spectool) - GOOD: licensed according to licensing guidelines (be it GPLv2 or GPLv2+, see below). - BAD: license in spec file doesn't match license in package, as the licensing in the source tarball isn't really clear: the README file states that it's "under the GPL (see file COPYING in the directory)" which could be construed as "GPLv2 only" (as that is the version shipped) but also "GPLv2+" (as they didn't specify a specific version). Please consult upstream about this, they really should clarify if they mean GPLv2 or GPLv2+ and state it in the source files themselves. BTW, Comment #6 isn't really correct about the original source being incompatible with the GPL as the source files are really under BSD without advertising clause (or MIT), i.e. there is no issue with this being licensed incompatibly, just that we can't be quite sure what license this is under. You can flag it as "GPLv2" in the meantime, just to stay on the safe side. If upstream tells you that it really is "GPLv2+" (as I assume), then include documentation that they did tell you that (e.g. a copy of the mail) in the package documentation when changing it in this version in a future release (this is not necessary if they clarify it in the shipped sources themselves in a future version). - GOOD: license shipped as documentation - GOOD: the spec file is written in American English - GOOD: the spec file is legible - GOOD: sources used to build the package match upstream source - GOOD: builds in mock for x86_64/Rawhide - GOOD: all build dependencies listed - PASS: doesn't ship locale files - PASS: no libraries shipped - GOOD: package is not relocatable - GOOD: all shipped directories owned by package, direct dependency or filesystem - GOOD: no duplicates in %files - GOOD: permissions on files are set properly - GOOD: package has a %clean section - GOOD: package uses macros consistently - GOOD: the package contains code, not content - PASS: no large documentation files - GOOD: %doc doesn't affect runtime - PASS: no header files - PASS: no static libraries - PASS: no pkgconfig files - PASS: no libraries included - PASS: no devel package - GOOD: no *.la libtool archives - PASS: no desktop file - GOOD: doesn't own files or directories owned by other packages - GOOD: build root is cleaned at the beginning of %install - GOOD: all file names are valid UTF-8 - CHECK: from now on, please state in the changelog what you changed (e.g. "- changed license to GPLv2 until licensing is clarified with upstream (#474802)"), just referring to a BZ ticket number doesn't explain it sufficiently. This package is APPROVED, provided that you fix the license tag to "GPLv2" (temporarily, until this is clarified with upstream) and the source URL before importing.
$ licensecheck.pl -r vacation-1.2.7.0/ vacation-1.2.7.0/tzfile.h: BSD (3 clause) vacation-1.2.7.0/rfc822.c: *No copyright* UNKNOWN vacation-1.2.7.0/html2man.pl: UNKNOWN vacation-1.2.7.0/strlcpy.c: ISC vacation-1.2.7.0/vacation.c: BSD (3 clause) vacation-1.2.7.0/vacation.h: BSD (3 clause) vacation-1.2.7.0/strlcat.c: ISC rfc822.c is under MIT license. Only the man page has any mention of a GPL (GPLv2+) license. Please ask upstream to clarify this. ** The current GPLv2+ license marking is correct: if the source code or COPYING doesn't specify a version ("released under GPL"), then the license tag is GPL+. The same is used if the COPYING is some version of the GPL without a specification what license the program is under. https://fedoraproject.org/wiki/Licensing/FAQ#How_do_I_figure_out_what_version_of_the_GPL.2FLGPL_my_package_is_under.3F
(In reply to comment #13) > $ licensecheck.pl -r vacation-1.2.7.0/ > vacation-1.2.7.0/tzfile.h: BSD (3 clause) > vacation-1.2.7.0/rfc822.c: *No copyright* UNKNOWN > vacation-1.2.7.0/html2man.pl: UNKNOWN > vacation-1.2.7.0/strlcpy.c: ISC > vacation-1.2.7.0/vacation.c: BSD (3 clause) > vacation-1.2.7.0/vacation.h: BSD (3 clause) > vacation-1.2.7.0/strlcat.c: ISC Neato. Where do I find this? > rfc822.c is under MIT license. Only the man page has any mention of a GPL > (GPLv2+) license. Please ask upstream to clarify this. > > ** > > The current GPLv2+ license marking is correct: if the source code or COPYING > doesn't specify a version ("released under GPL"), then the license tag is GPL+. > The same is used if the COPYING is some version of the GPL without a > specification what license the program is under. > https://fedoraproject.org/wiki/Licensing/FAQ#How_do_I_figure_out_what_version_of_the_GPL.2FLGPL_my_package_is_under.3F Only that the README doesn't say "under the GPL", but "under the GPL (see file COPYING in the directory)" which could be interpreted as "under the GPL as found in the file COPYING" (which would be specifying a version as this is exactly version 2 and nothing else). IANAL and all that stuff, but IMO better be safe than sorry.
Again, thanks for the comments. changelog: - changed license to GPLv2 until licensing is clarified with upstream - changed source URL from "download..." to "downloads...". I've uploaded again an updated SPEC/SRPMs to: Spec URL: http://www.reinform.de/download/rpm/vacation/vacation.spec SRPM URL: http://www.reinform.de/download/rpm/vacation/vacation-1.2.7.0-6.fc11.src.rpm Please review, thanks!
(In reply to comment #14) > (In reply to comment #13) > > $ licensecheck.pl -r vacation-1.2.7.0/ > > vacation-1.2.7.0/tzfile.h: BSD (3 clause) > > vacation-1.2.7.0/rfc822.c: *No copyright* UNKNOWN > > vacation-1.2.7.0/html2man.pl: UNKNOWN > > vacation-1.2.7.0/strlcpy.c: ISC > > vacation-1.2.7.0/vacation.c: BSD (3 clause) > > vacation-1.2.7.0/vacation.h: BSD (3 clause) > > vacation-1.2.7.0/strlcat.c: ISC > > Neato. Where do I find this? It's a Debian utility, e.g. http://packages.debian.org/source/sid/devscripts It's going to be in the next rpmdevtools release: https://bugzilla.redhat.com/show_bug.cgi?id=466353
(In reply to comment #15) > Again, thanks for the comments. > > changelog: > - changed license to GPLv2 until licensing is clarified with upstream > - changed source URL from "download..." to "downloads...". > > I've uploaded again an updated SPEC/SRPMs to: > Spec URL: http://www.reinform.de/download/rpm/vacation/vacation.spec That is actually the -5 release ;-)... > SRPM URL: > http://www.reinform.de/download/rpm/vacation/vacation-1.2.7.0-6.fc11.src.rpm the spec file in the SRPM contains the changes. > Please review, thanks! You wouldn't have had to put these up for review again, it would have been okay if you just requested CVS for your package and fixed these items in there. Anyway (and again): APPROVED. Now go, make your CVS request ;-).
New Package CVS Request ======================= Package Name: vacation Short Description: Automatic mail answering program Owners: t.bubeck Branches: F-10 F-11 InitialCC: t.bubeck
Unfortunately we can only accept FAS account IDs for Owners and InitialCC, not email addresses.
New Package CVS Request ======================= Package Name: vacation Short Description: Automatic mail answering program Owners: bubeck Branches: F-10 F-11
CVS done. BTW, you should clear the FE-NEEDSPONSOR blocker from all of your review requests, if you have any others.
Please note that Vacation 1.2.7.0 is BSD licensed and is from SourceForge (having been rebased for 1.2.7.0 from the version at savannah.nongnu.org in order to drop the BSD advertising clause in Vacation 1.2.6 on SourceForge). It is *not* the version in Debian (3.3.0), that has a different history (and codebase).
Closing this since CVS is done. Till, that would've been your job ;-).
Oh, sorry. I didn't know, that I have to close it, although it is now quite obvious... :-) Thanks.