Bug 652533

$ rpmlint postgresql-pgrouting.spec 
postgresql-pgrouting.spec: W: no-cleaning-of-buildroot %clean
postgresql-pgrouting.spec: W: no-buildroot-tag
postgresql-pgrouting.spec: W: no-%clean-section
0 packages and 1 specfiles checked; 0 errors, 3 warnings.

$ rpmlint ../SRPMS/postgresql-pgrouting-1.03-1.fc14.src.rpm 
postgresql-pgrouting.src: W: no-cleaning-of-buildroot %clean
postgresql-pgrouting.src: W: no-buildroot-tag
postgresql-pgrouting.src: W: no-%clean-section
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

$ rpmlint ../RPMS/x86_64/postgresql-pgrouting-1.03-1.fc14.x86_64.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Taking this review -- will try and do it this evening.

PS you don't need to rpmlint the .spec file -- when you rpmlint the SRPM, the spec file gets checked too (in addition to permissions on the bundled files, etc.)

#+TODO: TODO(t) WAIT(w@/!) FAIL(f@) | DONE(d) N/A(n)

* TODO Review [75%]
** DONE Names [2/2]
*** DONE Package name
    pgRouting or pgrouting would be good package names too, but I see
    their bundled Debian configuration uses postgres-X.Y-pgrouting so
    I guess postgres-pgrouting is a good choice.
*** DONE Spec name
** DONE Meets [[http://fedoraproject.org/wiki/Packaging/Guidelines][guidelines]]
** DONE source files match upstream
   $ md5sum pgRouting-1.03.tgz ../SOURCES/pgRouting-1.03.tgz 
   ee700d18a984b8fd78c1a739ca078683  pgRouting-1.03.tgz
   ee700d18a984b8fd78c1a739ca078683  ../SOURCES/pgRouting-1.03.tgz

** TODO License [2/3]
*** DONE License is Fedora-approved
*** FAIL License field accurate
    Should be "GPLv2+ and Boost"; make a note that shooting_star* are under the
    latter license
*** DONE License included iff packaged by upstream
** DONE rpmlint [2/2]
*** DONE on src.rpm
*** DONE on x86_64.rpm
** DONE Language & locale [3/3]
*** DONE Spec in US English
*** DONE Spec legible
*** N/A Use %find_lang to handle locale files
** DONE Build [3/3]
*** DONE Koji results
    http://koji.fedoraproject.org/koji/taskinfo?taskID=2597933
*** DONE BRs complete
*** DONE Directory ownership
** TODO Spec inspection [8/9]
*** N/A ldconfig for libraries
*** DONE No duplicate files
*** DONE File permissions
*** DONE Filenames must be UTF-8
*** DONE no BuildRoot ([[https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag][except if targeting EPEL5]])
*** DONE Has %clean section
    (except F-13+:
    https://fedoraproject.org/wiki/Packaging/Guidelines#.25clean)
*** DONE %buildroot cleaned on %install
*** FAIL Macro usage consistent
    see cmake guidelines:
    http://fedoraproject.org/wiki/Packaging/cmake
    - no need to test for %{?_lib} == lib64; the cmake macro does that
    - no need to override CMAKE_INSTALL_PREFIX, likewise done by the macro
    - please use VERBOSE=1 and %{?_smp_mflags}. The former makes sure
      the build logs are detailed (useful to see, e.g., if the
      compiler flags are being used properly). The latter speeds up
      the build (the build servers use -j 16, normally).

      If the build cannot be parallelized, comment out the %{?_smp_mflags}
      part, put a comment and notify upstream.
    - even with VERBOSE=1 it does not look like CFLAGS and CXXFLAGS are
      being used. See attached patch -- looks like the cmake configuration
      sets CFLAGS and CXXFLAGS regardless of what the user passed in.

      Please report the problem upstream once we're done with the review.
    - no need to BuildRequires gcc-c++:
      see http://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2

*** DONE Documentation [2/2]
**** N/A If large docs, separate -doc
**** DONE %doc files are non-essential

Created attachment 460129 [details]
Removes the preset CFLAGS

This way, cmake will just pick up the CFLAGS and CXXFLAGS set by the %cmake macro

Updated with the suggested changes, please check

Spec URL: http://viji.fedorapeople.org/SPECS/postgresql-pgrouting.spec
SRPM URL: http://viji.fedorapeople.org/SRPMS/postgresql-pgrouting-1.03-2.fc14.src.rpm

$ rpmlint /home/viji/rpmbuild/RPMS/x86_64/postgresql-pgrouting-1.03-2.fc14.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

$ rpmlint /home/viji/rpmbuild/SRPMS/postgresql-pgrouting-1.03-2.fc14.src.rpm
postgresql-pgrouting.src: W: no-cleaning-of-buildroot %clean
postgresql-pgrouting.src: W: no-buildroot-tag
postgresql-pgrouting.src: W: no-%clean-section
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

Also, I will update the upstream regarding the CFLAGS issue.

Just a couple more things:
- it's normally a good idea to have your patches be named
  (packagename|tarballname)-version-purpose.patch

  That way when someone rpm -i the srpm on their local machine, and the files
  end up in ~/rpmbuild/SOURCES, it's easy to see which patch belongs to which
  package.

  The versioning helps knowing which patch might be out of date (though a lot
  of time the same patch would apply across several new releases!)

- Likewise, when applying the patch, it's useful to use the -b SUFFIX option
  ( backs up the file being patched with the SUFFIX, so my suffix is normally
  .purpose )

- Have we settled on a new name? pgRouting or pgrouting perhaps?

Spec URL: http://viji.fedorapeople.org/SPECS/pgRouting.spec
SRPM URL: http://viji.fedorapeople.org/SRPMS/pgRouting-1.03-3.fc14.src.rpm

Updated the package, please verify.

Also, regarding the name, I have changed it to pgRouting. Now it is matching with upstream project name and tarball name - What do you think?

$ rpmlint /home/viji/rpmbuild/SRPMS/pgRouting-1.03-3.fc14.src.rpm
pgRouting.src: W: no-cleaning-of-buildroot %clean
pgRouting.src: W: no-buildroot-tag
pgRouting.src: W: no-%clean-section
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

$ rpmlint /home/viji/rpmbuild/RPMS/x86_64/pgRouting-1.03-3.fc14.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

That looks good. I normally use the exact version number (e.g. 1.03) but that's partly an artifact of generating the patches automatically (I'm attaching my patch-generating script here for reference). But as long as the version number is unambiguous that's good enough as a practice.

Everything looks good. APPROVED. As before, add me to the Cc: list :)

Created attachment 460368 [details]
Generates a patch given a "purpose" and backup files named .purpose

New Package SCM Request
=======================
Package Name: pgRouting
Short Description: Provides routing functionality to PostGIS/PostgreSQL
Owners: viji
Branches: f13 f14
InitialCC: salimma

The bug summary gives one package name, the SCM request specifies another.
Could you make them match what you actually desire and re-raise the
fedora-cvs-flag?

We had a discussion in the packaging list and Tom Lane (pgsql developer) strongly argued against using postgresql- for third party packages.

I've updated the summary -- package name is pgRouting. Thanks!

New Package SCM Request
=======================
Package Name: pgRouting
Short Description: Provides routing functionality to PostGIS/PostgreSQL
Owners: viji
Branches: f13 f14
InitialCC: salimma

Git done (by process-git-requests).

pgRouting-1.03-3.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/pgRouting-1.03-3.fc14

pgRouting-1.03-3.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/pgRouting-1.03-3.fc13

pgRouting-1.03-3.fc14 has been pushed to the Fedora 14 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update pgRouting'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/pgRouting-1.03-3.fc14

pgRouting-1.03-3.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

pgRouting-1.03-3.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.