Spec URL: http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/postgresql-pgbouncer.spec SRPM URL: http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/postgresql-pgbouncer-1.0.7-2.fc7.src.rpm Description: pgbouncer is a lightweight connection pooler for PostgreSQL. pgbouncer uses libevent for low-level socket handling.
Why did you set fedora-review to '?' here? I don't see that anyone is reviewing this package, so the only effect is that this ticket doesn't appear on the list of new review tickets and thus will never see a review.
Hello, .(In reply to comment #1) > Why did you set fedora-review to '?' here? Sorry, reverted. Regards, Devrim
Created attachment 157296 [details] New .spec file
Created attachment 157297 [details] Signed source RPM file.
Updated to 1.0.8: Spec URL: http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/postgresql-pgbouncer.spec SRPM URL: http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/postgresql-pgbouncer-1.0.8-1.fc7.src.rpm
Already update to 1.1 in my repo, but forgot to paste here: Spec URL: http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/postgresql-pgbouncer.spec SRPM URL: http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/postgresql-pgbouncer-1.1.1-1.fc8.src.rpm Regards, Devrim
A few quick comments: URLs given for URL and Source0 don't work. The hostname on its own doesn't even produce anything useful either. Description seems a little short, but maybe it's OK. If/when the website comes back, I would check it against what it says there. I have no idea what the following does. It might be useful to explain what it does and why it's needed. %define debug 0 %{?debug:%define __os_install_post /usr/lib/rpm/brp-compress} Being anal, there should be a blank line after the Summary line. I'd prefer to reformat this: %build %configure \ %if %debug --enable-debug \ --enable-cassert \ %endif --datadir=%{_datadir} ... to this: %build %configure \ %if %debug --enable-debug \ --enable-cassert \ %endif --datadir=%{_datadir} i.e. line up the options so it's more obvious they're all one command. Also I hate TABs, but that may be a personal thing. I've seen some reviews request that file permissions in %files be more explicit. Instead of relying on %install to set them correctly, you may want to do something explicit, modelled after the following: %files %defattr(0644,root,root,0755) %doc COPYING %doc README.txt %attr(0755, root, root) /sbin/fxload %{_mandir}/*/*
The source tarball doesn't match the upstream one. Devrim, care to do an update?
Any update? This has been set to needinfo for nearly six weeks, but I know Devrim is around frequently so I'll keep this open. I will close it soon if there's no response.
Sorry Ruben and Jason. I have made changes in the spec, and we got rid of rpmlint errors. Also, updated to 1.1.2 : Spec file: http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/postgresql-pgbouncer.spec SRPM: http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/postgresql-pgbouncer-1.1.2-1.f8.src.rpm
I fixed many bugs in init script and spec. Also, added a new patch for .ini file to match Red Hat defaults. This new version is now used on a production server: Spec file: http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/postgresql-pgbouncer.spec SRPM: http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/postgresql-pgbouncer-1.1.2-2.f8.src.rpm But BTW: I'm inclined to change the name to pgbouncer only. Adding postgresql- prefix to all of the packages does not seem good to me, as I felt before. Regards, Devrim
Any comments on the latest version of this package? Regards, Devrim
For 1.1.2-2: * License - For this package use "MIT and BSD" as license tag. Actually almost all source codes are under MIT, only src/md5.{h,c} are under BSD. * compile log http://koji.fedoraproject.org/koji/getfile?taskID=622998&name=build.log - Please make compile log more verbose. From outputs like ------------------------------------------------- make[1]: Leaving directory `/builddir/build/BUILD/pgbouncer-1.1.2/doc' CC src/loader.c CC src/client.c CC src/objects.c CC src/proto.c CC src/pooler.c ------------------------------------------------- we cannot check if compiler flags are passed correctly, binaries are stripped by accident, etc.. ------------------------------------------------- make %{?_smp_mflags} V=1 ------------------------------------------------- seems okay * Timestamp - When using "install" or "cp" commands, add "-p" option to keep timestamps on installed files. * Permission - Why does %_sysconfdir/sysconfig/%name have not 0644 but 0755 permission? * __os_install_post - Again from build.log currently only /usr/lib/rpm/brp-compress is executed as __os_install_post, which is wrong (you can compare with: http://koji.fedoraproject.org/koji/getfile?taskID=622988&name=build.log for example) Note: ------------------------------------------------- %{?debug:%define __os_install_post /usr/lib/rpm/brp-compress} ------------------------------------------------- means that if %debug macro _is defined_ (not is non-zero) __os_install_post is defined as /usr/lib/rpm/brp-compress. * debuginfo missing - debuginfo rpm is not correctly created * One reason is due to wrong __os_installed_post as described above * Another reason is that installed pgbouncer binary are actually stripped as "install -s". Overriding BININSTALL works for this issue. * initscript scriptlets - Please follow http://fedoraproject.org/wiki/Packaging/SysVInitScript#InitscriptScriptlets * Especially check "Requires(post)" or so * Documents - Please add the following files to %doc ------------------------------------------------- AUTHORS -------------------------------------------------
ping?
ping again?
ping again??
Please wait, I'm working on about 50 packages nowadays (mostly pg related), and I'll respond as soon as I finish working on them. FWIW, the many of the changes are done already...
Again ping? I will close this bug if no response from the reporter is received within ONE WEEK: http://fedoraproject.org/wiki/PackageMaintainers/Policy/StalledReviews#Submitter_not_responding
Ok updated: http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/postgresql-pgbouncer-1.2.3-1.fc9.src.rpm http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/postgresql-pgbouncer.spec BTW... Would you object if I change the package name to pgbouncer only?
Thank you for replying. Then before checking your latest srpm: (In reply to comment #21) > BTW... Would you object if I change the package name to pgbouncer only? Well, in my recognition when we rename a package "python-foo" "perl-baa" "ruby-XXX" it usually means that the package provides a "module" or an extension function of the main language (python, perl, ...), i.e. usually it is less useful to install the package _only_ and other application uses the package. For this package it installs some binaries under %_bindir and it seems this package is not only a module of postgresql. So I don't object to renaming.
If you want to rename the srpm, then I will wait for it before checking your srpm again.
Here is the new spec and srpm: http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/pgbouncer.spec http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/pgbouncer-1.2.3-1.f9.src.rpm Regards, Devrim
For 1.2.3-1: * Requires(preun) https://fedoraproject.org/wiki/Packaging/SysVInitScripts#Initscripts_in_spec_file_scriptlets - %preun uses /sbin/service and "Requires(preun): initscripts" is needed ! %postun - Would you consider to add %postun section as described in https://fedoraproject.org/wiki/Packaging/SysVInitScripts#Initscripts_in_spec_file_scriptlets (If there is a reason adding this %postun must be avoided, it is no problem) * cflags/install flags - build.log shows --------------------------------------------------------- 201 make[1]: Leaving directory `/builddir/build/BUILD/pgbouncer-1.2.3/doc' 202 gcc -c -o lib/client.o src/client.c -DHAVE_CONFIG_H -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wmissing-prototypes -Wpointer-arith -Wendif-labels -Wdeclaration-after-statement -Wold-style-definition -Wstrict-prototypes -Wundef -Wformat -Wnonnull -Wstrict-overflow -fomit-frame-pointer -I./include -I./include 269 mkdir -p /builddir/build/BUILDROOT/pgbouncer-1.2.3-1.fc10.i386/usr/share/doc/pgbouncer 270 install -s -m 755 ./pgbouncer /builddir/build/BUILDROOT/pgbouncer-1.2.3-1.fc10.i386/usr/bin --------------------------------------------------------- -- -fomit-frame-pointer makes debugging very difficult -- "install -s" removes debugging information and makes failure of debuginfo rpm So both are forbidden on Fedora. For this package the following can fix these issues: --------------------------------------------------------- sed -i.fedora \ -e 's|-fomit-frame-pointer||' \ -e '/BININSTALL/s|-s||' \ configure --------------------------------------------------------- * Timestamps - When installing files by "cp" or "install", add "-p" option to keep timestamps on them. * Permission --------------------------------------------------------- pgbouncer.src: W: strange-permission pgbouncer.init 0775 pgbouncer.i386: E: executable-marked-as-config-file /etc/sysconfig/pgbouncer pgbouncer.i386: E: script-without-shebang /etc/sysconfig/pgbouncer --------------------------------------------------------- - All files in the srpm must have 0644 permission. - %_sysconfdir/sysconfig/pgbouncer must have 0644 permission. * %config --------------------------------------------------------- pgbouncer.i386: W: conffile-without-noreplace-flag /etc/sysconfig/pgbouncer --------------------------------------------------------- - Would you explain why you don't want to use %config(noreplace) %{_sysconfdir}/sysconfig/%{name}?
Here is the new spec and srpm: http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/pgbouncer.spec http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/pgbouncer-1.2.3-2.f9.src.rpm rpmlint is clean, and I believe I applied all changes you suggested. Regards, Devrim
For 1.2.3-2: * %postun scriptlet - Now Requires(postun) is needed: https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_in_spec_file_scriptlets * rpmlint issue --------------------------------------------------- pgbouncer.i386: E: init-script-non-executable /etc/rc.d/init.d/pgbouncer --------------------------------------------------- - /etc/rc.d/init.d/pgbouncer must have 0755 permission. Fix these 2 issues and I guess I can approve this package.
Hi Mamoru, http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/pgbouncer.spec http://developer.postgresql.org/~devrim/rpms/other/pgbouncer/pgbouncer-1.2.3-3.f9.src.rpm Applied the two changes you suggested. Regards, Devrim
Okay. ---------------------------------------------------------------- This package (pgbouncer) is APPROVED by mtasaka ----------------------------------------------------------------
BTW, I just added >= 1.3b to libevent-devel dependency -- this version explicitly asks for that (which is available in F-8 +
New Package CVS Request ======================= Package Name: pgbouncer Short Description: Lightweight connection pooler for PostgreSQL Owners: devrim Branches: F-8 F-9 InitialCC:
cvs done.
Thanks. Pushed package to repositories.