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=731898 --- Comment #14 from Steve Jenkins <steve@xxxxxxxxxxxxxxxx> 2011-08-22 20:46:46 EDT --- (In reply to comment #13) > c) explicitly patch the initfile from the upstream tarball using Patch0: > %{name}-2.6.2-initscript.patch, and a %patch0 -p1 later. Yep. Even better. I looked at the keygen logic in the sshd init file and did something very similar (albeit a tad bit simpler, since there are fewer cases to handle). >The spec is getting much better, thanks for the quick turnaround. >A few more items to note: You're an excellent mentor, Matt. Shoving me in the right direction without spoon-feeding everything, and politely telling me to RTFM where appropriate. :) As a non-programmer, I imagine I need more hand-holding than most, so I appreciate your patience. Thank you. > Summary: doesn't tell me much at present. How about: > Summary: DomainKeys Identified Mail (DKIM) Signature milter and library Fixed. > As the readme tarball is personal, rather than upstream, no need for a full URL > on it then. Ok, changed back - but I think I may just do a patch of the existing INSTALL file instead. Seems simpler. > Does the -devel package need to include static libraries for a good reason? > http://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries_2 > simply don't package the *.a and *.la files. Again - me being guilty of just leaving stuff there that was in the original spec file in the upstream's contrib. I removed these two files from %files -n libopendkim-devel: %{_libdir}/*.a %{_libdir}/*.la And then explicitly removed them in %install with: rm -r %{buildroot}%{_libdir}/*.a rm -r %{buildroot}%{_libdir}/*.la to avoid build errors saying I had unpackaged files left in the buildroot. If there's a better way to do this, please let me know. > See if you can get away without %defining LIBTOOL. It's not clear to me when > that define gets evaluated - it may well be when outside the buildroot, which > isn't what you really want. Hmm... looks like it's working now! Some of the other cleanup we did must have allowed it to "just work." > The bit to include kerberos on the include path. That should come from > pkgconfig instead, yes? On F14, /usr/lib64/pkgconfig/openssl.pc doesn't pull > it in, though I see on EL5 it does. Can you Require: pkgconfig, and have > upstream make use of it, instead of the current directory existence test? Fixed. > The %install test to clear the buildroot doesn't need to be tested - you've > explicitly set the buildroot above. Guidelines say simply to: > %install > rm -rf %{buildroot} > > Likewise for %clean. Fixed and fixed. > You can remove the spaces among all the mkdirs. (trivial nit I know...) Fixed! I am totally on board with nits. :) > Your %pre calls exit 0 before calling usermod. Simply add the -G mail flag to > the useradd command, and delete the usermod line all together. Fixed. Big "DUH" on that one. :) > You can simplify the invocations of ldconfig as noted here: > http://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries Nice. Clean and efficient is good. Fixed. > The long echo in %post will likely never be seen by a user - PackageKit, > anaconda, and the like won't display it. Consider removing it. Sigh... users. :) Fixed. > in %preun, why are you deleting the socket file and then the directory? Now > that the dir is owned by the package, it'll get deleted as the package is > uninstalled. Besides, the app itself should be deleting the .sock file when it > no longer is listening for connections on it. Good point. Fixed. > Look again at the preun and post scriptlets, compare against > http://fedoraproject.org/wiki/Packaging/SysVInitScript#InitscriptScriptlets > in particular, if you have Requires: chkconfig, then you don't have to test for > it, and you can also ignore the LSB stuff all together. Fixed, although I'm not sure if I put the new Requires in the right place. The wiki wasn't very specific. >You need to also > condrestart the service, yes? See the scriptlets link above for examples. As a matter of fact, I do! :) Fixed. Question: I'd really like to move my monster conf file echo into a patch against of the sample conf files that's included in the upstream source. However, I like that I can use variables in the spec file, but the samples have hard-coded paths. Which approach is preferable? Updated spec file and SRPM in: http://packages.stevejenkins.com/opendkim/2.4.2/working/ -- 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