Bug 652533 - Review Request: pgRouting - Provides routing functionality to PostGIS/PostgreSQL
Review Request: pgRouting - Provides routing functionality to PostGIS/PostgreSQL
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michel Alexandre Salim
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-11-12 01:41 EST by viji
Modified: 2010-11-25 20:17 EST (History)
3 users (show)

See Also:
Fixed In Version: pgRouting-1.03-3.fc14
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-11-25 20:10:11 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
michel: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)
Removes the preset CFLAGS (460 bytes, patch)
2010-11-12 14:16 EST, Michel Alexandre Salim
no flags Details | Diff
Generates a patch given a "purpose" and backup files named .purpose (136 bytes, text/plain)
2010-11-14 08:19 EST, Michel Alexandre Salim
no flags Details

  None (edit)
Description viji 2010-11-12 01:41:03 EST
Spec URL: http://viji.fedorapeople.org/SPECS/postgresql-pgrouting.spec
SRPM URL: http://viji.fedorapeople.org/SRPMS/postgresql-pgrouting-1.03-1.fc14.src.rpm
Description: The main objective is to provide routing functionality to PostGIS/ PostgreSQL. pgRouting is part of  PostLBS, which provides core tools for Location Based Services (LBS) as Open Source Software (OSS). Its tools are similar to those found on proprietary software.
Comment 1 viji 2010-11-12 01:49:02 EST
$ 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.
Comment 2 Michel Alexandre Salim 2010-11-12 04:36:47 EST
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.)
Comment 3 Michel Alexandre Salim 2010-11-12 14:15:05 EST
#+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
Comment 4 Michel Alexandre Salim 2010-11-12 14:16:44 EST
Created attachment 460129 [details]
Removes the preset CFLAGS

This way, cmake will just pick up the CFLAGS and CXXFLAGS set by the %cmake macro
Comment 5 viji 2010-11-13 14:51:25 EST
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.
Comment 6 Michel Alexandre Salim 2010-11-13 18:20:27 EST
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?
Comment 7 viji 2010-11-14 06:42:01 EST
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.
Comment 8 Michel Alexandre Salim 2010-11-14 08:18:32 EST
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 :)
Comment 9 Michel Alexandre Salim 2010-11-14 08:19:36 EST
Created attachment 460368 [details]
Generates a patch given a "purpose" and backup files named .purpose
Comment 10 viji 2010-11-14 13:29:41 EST
New Package SCM Request
=======================
Package Name: pgRouting
Short Description: Provides routing functionality to PostGIS/PostgreSQL
Owners: viji
Branches: f13 f14
InitialCC: salimma
Comment 11 Jason Tibbitts 2010-11-15 09:43:41 EST
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?
Comment 12 Michel Alexandre Salim 2010-11-15 09:50:35 EST
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!
Comment 13 viji 2010-11-15 10:45:15 EST
New Package SCM Request
=======================
Package Name: pgRouting
Short Description: Provides routing functionality to PostGIS/PostgreSQL
Owners: viji
Branches: f13 f14
InitialCC: salimma
Comment 14 Jason Tibbitts 2010-11-15 10:55:12 EST
Git done (by process-git-requests).
Comment 15 Fedora Update System 2010-11-16 02:20:53 EST
pgRouting-1.03-3.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/pgRouting-1.03-3.fc14
Comment 16 Fedora Update System 2010-11-16 02:21:00 EST
pgRouting-1.03-3.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/pgRouting-1.03-3.fc13
Comment 17 Fedora Update System 2010-11-16 18:14:12 EST
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
Comment 18 Fedora Update System 2010-11-25 20:10:06 EST
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.
Comment 19 Fedora Update System 2010-11-25 20:17:25 EST
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.

Note You need to log in before you can comment on or make changes to this bug.