Bug 241403

Some initial comments:

First please check:
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
Then:

* Please write all BuildRequires in main section.
* %makeinstall is highly forbidden.
* stripping binaries is strictly forbidden to create debuginfo
  rpm
* For python/test_export.py, verify which is proper,
  to make this script have executable permission, or
  to remove shebang.
* Fix directory ownership. For example, %{_libdir}/%{name} is
  not owned by any package.
* Shipping static archive is highly forbidden unless you have
  some reasonable reason.
* Also shipping libtool archive is highly forbidden.
* %defattr(-, root, root, -) is recommended
* Using disttag is recommended

I just glanced at your spec file and I didn't try to
rebuild this package.

Thanks for the comments; I believe I've addressed your initial concerns and 
updated files are here:
Spec URL: http://www.silfreed.net/download/repo/packages/qgis/qgis.spec
SRPM URL: 
http://www.silfreed.net/download/repo/packages/qgis/qgis-0.8.0-4.src.rpm

> * Please write all BuildRequires in main section.

Moved.

> * %makeinstall is highly forbidden.

Updated to `make DESTDIR=%{builddir} install`

> * stripping binaries is strictly forbidden to create debuginfo
>   rpm

Removed and enabled build of debuginfo package.

> * For python/test_export.py, verify which is proper,
>   to make this script have executable permission, or
>   to remove shebang.

I'm working with upstream to resolve this:
http://lists.qgis.org/pipermail/qgis-developer/2007-May/thread.html
They currently say this test script should be executable.

> * Fix directory ownership. For example, %{_libdir}/%{name} is
>   not owned by any package.

I believe this should be fixed.  Let me know if I missed any directories.

> * Shipping static archive is highly forbidden unless you have
>   some reasonable reason.
> * Also shipping libtool archive is highly forbidden.

These are the .a and .la files, correct?  They have been removed.

> * %defattr(-, root, root, -) is recommended

Fixed.

> * Using disttag is recommended

Added to Release.


I also found some source files that were being pulled into the debuginfo 
package and were marked executable.  I'm not familiar with the debuginfo 
packages so I'm not sure if something's being built incorrectly for them, but 
I went ahead and made the source files non-executable and notified upstream 
(http://lists.qgis.org/pipermail/qgis-developer/2007-May/001930.html)


Also below is the rpmlint reports:

 rpmlint -vi RPMS/qgis-*0.8.0-4* SRPMS/qgis-0.8.0-4.src.rpm
I: qgis checking
I: qgis-debuginfo checking
I: qgis-devel checking
W: qgis-devel no-documentation
The package contains no documentation (README, doc, etc).
You have to include documentation files.

I: qgis-grass checking
W: qgis-grass no-documentation
The package contains no documentation (README, doc, etc).
You have to include documentation files.

I: qgis-theme-nkids checking
W: qgis-theme-nkids no-documentation
The package contains no documentation (README, doc, etc).
You have to include documentation files.

I: qgis checking
E: qgis hardcoded-library-path in /usr/lib/qt4/bin
A library path is hardcoded to one of the following paths: /lib,
/usr/lib. It should be replaced by something like /%{_lib} or %{_libdir}.


I'm not sure how to deal with the "hardcoded-library-path".  It was a bit of a 
hack for me to get this package to work with the qt4 libraries for FC6;  I'm 
not sure if there's any documentation on how to improve this or not.

-Doug

Right; that %makeinstall fix was `make DESTDIR=%{buildroot} install`; that's 
what I get for not copy/pasting.

-Doug

Created attachment 155515 [details]
mock build log of qgis 0.8.0-4 on F-7 i386

Much improvement! But still I have not tried to
install this as mock build fails.

* Mockbuild failed as attached. First check BuildRequires.
* Requires: /sbin/ldconfig is not needed.
* Some directories are still not owned
  (e.g. %{_datadir}/%{name}/themes/)

One more notes:

(In reply to comment #2)
> I: qgis checking
> E: qgis hardcoded-library-path in /usr/lib/qt4/bin
> A library path is hardcoded to one of the following paths: /lib,
> /usr/lib. It should be replaced by something like /%{_lib} or %{_libdir}.
> 
> 
> I'm not sure how to deal with the "hardcoded-library-path".  It was a bit of a 
> hack for me to get this package to work with the qt4 libraries for FC6;  I'm 
> not sure if there's any documentation on how to improve this or not.

Use %{_libdir} instead of /usr/lib. on x86_64/ppc64 arch,
qt4 moc (and others) are under /usr/lib64/qt4/bin/ 

One more comment:

--------------------------------------------
checking python/Python.h usability... no
checking python/Python.h presence... no
checking for python/Python.h... no
--------------------------------------------

I guess this package should require "BuildRequires: python-devel"

(In reply to comment #6)
> One more comment:
> 
> --------------------------------------------
> checking python/Python.h usability... no
> checking python/Python.h presence... no
> checking for python/Python.h... no
> --------------------------------------------
> 
> I guess this package should require "BuildRequires: python-devel"

The spec already defines this and it looks like it was installed when you 
tried to build it under mock.  Can you attach the config.log from the build?


On my system:

--------------------------------------------
checking for main in -lpython2.4... yes
checking python2.4/Python.h usability... yes
checking python2.4/Python.h presence... yes
checking for python2.4/Python.h... yes
  results of the Python check:
    Binary:      python2.4
    Library:     python2.4
    Include Dir: /usr/include/python2.4
    Have python:
--------------------------------------------

Oh, no... I have to point out two things:

* On F-7, python is 2.5.
-------------------------------------------------
checking for python build information... checking for python2.4... no
checking for python2.3... no
checking for python2.2... no
checking for python2.1... no
checking for python... python
checking for main in -lpython... no
checking python/Python.h usability... no
checking python/Python.h presence... no
checking for python/Python.h... no
  results of the Python check:
    Binary:      python
    Library:     no
    Include Dir: no
    Have python: 
----------------------------------------------------

* /usr/include/python2.5/Python.h is now in python-devel,
  not in python rpm (On <= FC-6, Python.h was in python rpm)

(In reply to comment #8)
> * On F-7, python is 2.5.

Okay; I'll work with upstream to get python 2.5 supported in the configure 
script and add a patch in the meantime.

> * /usr/include/python2.5/Python.h is now in python-devel,
>   not in python rpm (On <= FC-6, Python.h was in python rpm)

At least on my updated FC-6 system, Python.h is in python-devel.  Looking back 
at the python-devel package that shipped with FC-5, Python.h is also there.

-Doug

Spec URL: http://www.silfreed.net/download/repo/packages/qgis/qgis.spec
SRPM URL: 
http://www.silfreed.net/download/repo/packages/qgis/qgis-0.8.0-5.src.rpm

Changelog:
* Tue May 29 2007 Douglas E. Warner <silfreed> 0.8.0-5
- fixing more directory owernship (themes, themes-default)
- fixing qt4 hardcoded lib path
- removing Requires ldconfig
- adding BuildRequires sqlite-devel
- adding patch for supporting python 2.5 in configure script


(In reply to comment #4)
> * Mockbuild failed as attached. First check BuildRequires.
> * Requires: /sbin/ldconfig is not needed.
> * Some directories are still not owned
>   (e.g. %{_datadir}/%{name}/themes/)

I removed the Requires for ldconfig and fixed directory ownership 
for %{_datadir}/%{name}/themes/ and %{_datadir}/%{name}/themes/default/.

(In reply to comment #5)
> Use %{_libdir} instead of /usr/lib. on x86_64/ppc64 arch,
> qt4 moc (and others) are under /usr/lib64/qt4/bin/ 

Fixed.  This seems to fix the rpmlint error as well.


$ rpmlint -v RPMS/qgis-*0.8.0-5*.rpm SRPMS/qgis-0.8.0-5.src.rpm
I: qgis checking
I: qgis-debuginfo checking
I: qgis-devel checking
W: qgis-devel no-documentation
I: qgis-grass checking
W: qgis-grass no-documentation
I: qgis-theme-nkids checking
W: qgis-theme-nkids no-documentation
I: qgis checking

Created attachment 155615 [details]
mock build log of qgis 0.8.0-5 on F-7 i386

Mock build still fails on F-7 i386.

Spec URL: http://www.silfreed.net/download/repo/packages/qgis/qgis.spec
SRPM URL: 
http://www.silfreed.net/download/repo/packages/qgis/qgis-0.8.0-6.src.rpm

Changelog:
* Tue May 29 2007 Douglas E. Warner <silfreed> 0.8.0-6
- adding BuildRequires bison, flex

Created attachment 155656 [details]
mock build log of qgis 0.8.0-6 on F-7 i386

Still fails with perhaps another reason.

Created attachment 155834 [details]
config log for qgis-0.8.0-6 on F-7 i386

I attach config.log created by mockbuild of 
qgis-0.8.0-6 on F-7 i386

Thanks for the config.log.  It looks like my quick patch to enable python 2.5 
support didn't work out quite right, so I'll have to look into a couple 
options.
I checked with upstream to see how they would like to enable python 2.5 
support and found out that they've switched to cmake for their next version so 
python support should be alright 
(https://svn.qgis.org/trac/ticket/720).  Right now I think there's 
a couple options:
1) continue trying to get python 2.5 support working - this should be easier 
for me now that F7 is released.
2) drop python support for F7 for now.
3) wait for the next version.

I'm going to start with option #1, but I'll have to get an F7 box setup here 
or get my laptop upgraded first.

Well, actually you can try rebuild for devel environ
even if you are using FC-6 system.

If you want to try so, please check:
http://fedoraproject.org/wiki/PackageMaintainers/MockTricks

Any news?

Sorry; I had been on vacation and just busy with other things.  I'll be trying 
to get things going for F7 this week.

Spec URL: http://www.silfreed.net/download/repo/packages/qgis/qgis.spec
SRPM URL: 
http://www.silfreed.net/download/repo/packages/qgis/qgis-0.8.1-1.src.rpm

%changelog
* Mon Jun 19 2007 Douglas E. Warner <silfreed> 0.8.1-1
- updating version
- removed BuildRequires: python-devel due to lack of PyQt4 bindings
- updated build for use of cmake instead of autotools
- added patch for setting WORKDIR in settings.pro file
- added patch for fixing install path of man pages
- updated library names

$ rpmlint RPMS/qgis-*0.8.1-1*.rpm SRPMS/qgis-0.8.1-1.src.rpm
E: qgis invalid-soname /usr/lib/libqgis_core.so libqgis_core.so
W: qgis-devel no-documentation
W: qgis-grass no-documentation
E: qgis-grass invalid-soname /usr/lib/libqgisgrass.so libqgisgrass.so
W: qgis-theme-nkids no-documentation

Created attachment 157631 [details]
mock build log of qgis 0.8.1-1 on Fedora devel i386

Some remarks

* Mockbuild
  - Mockbuild failed on Fedora devel i386.
    At least cmake is missing for BR (I can't do a formal review
    until mockbuild passes)

* cmake
  - For cmake, please check:
    http://fedoraproject.org/wiki/Packaging/cmake
    Especially, please make the build log more verbose.

* rpm call
  - Interal rpm call in rpmbuild is forbidden because it is
    considered as dangerous due to some reason.
    Please change the following to the proper way.
-----------------------------------------------------------
%define grass_ver %(rpm -q --queryformat %{VERSION} grass)
-----------------------------------------------------------

* non-sover libraries with providing -devel subpackage.
  - Shipping non-sover libraries with providing -devel subpackage
    is unwilling because:

    Shipping -devel package means that the libraries %{_libdir}/*.so
    is allowed to be linked from other packages. So some binaries in
    other package may link to the libraries in this package.

    Then ABI of the libraries in this package may change in the future.
    At this time, as these libraries have no sover, rpm has no clue of
    whether ABI of these libraries changed, so rpm allows the upgrading
    of this package. However, this upgrade surely stop the other binaries
    linking to these libraries from working any more.

* Using %{_builddir}
---------------------------------------------------------
find %{_builddir}/%{name}-%{version}/doc -name '.cvsignore' -exec %{__rm} -f {}
';'
---------------------------------------------------------
  - %{_builddir}/%{name}-%{version} can be simply replaced with .
    as at this point the working directory is 
    %{_builddir}/%{name}-%{version}

(In reply to comment #20)
> * rpm call
>   - Interal rpm call in rpmbuild is forbidden because it is
>     considered as dangerous due to some reason.
>     Please change the following to the proper way.
> -----------------------------------------------------------
> %define grass_ver %(rpm -q --queryformat %{VERSION} grass)
> -----------------------------------------------------------

Sorry; what would the "proper way" be?  The cmake script tries to lookup the 
grass library in the wrong locations and I need to tell it that the library 
lives under a directory like /usr/lib/grass-6.2.1.

> * non-sover libraries with providing -devel subpackage.

Upstream does not provide a sover with their library but their program links 
against it:
$ ldd /usr/bin/qgis | grep libqgis_core.so
        libqgis_core.so => /usr/lib/libqgis_core.so (0x03099000)
$ ldd /usr/lib/qgis/libgrass*.so | grep libqgisgrass.so
        libqgisgrass.so => /usr/lib/libqgisgrass.so (0x00673000)
        libqgisgrass.so => /usr/lib/libqgisgrass.so (0x002fc000)

Should I still move these files into the -devel package and then have the main 
packages end up with a Requires on the -devel package, or do I need to work 
with upstream to add a sover to these libraries?

Until I figure out what to do about the missing-sover and the rpm call, I've 
built new packages to hopefully get the package building in mock.  They should 
be up on my website soon (probably by 17:30 EST), and will be:

Spec URL: http://www.silfreed.net/download/repo/packages/qgis/qgis.spec
SRPM URL: 
http://www.silfreed.net/download/repo/packages/qgis/qgis-0.8.1-2.src.rpm

%changelog
* Fri Jun 22 2007 Douglas E. Warner <silfreed> 0.8.1-2
- added BuildRequires: cmake
- updated build to use cmake macro and make verbose

$ rpmlint RPMS/qgis-*0.8.1-2* SRPMS/qgis-0.8.1-2.src.rpm
E: qgis invalid-soname /usr/lib/libqgis_core.so libqgis_core.so
W: qgis-devel no-documentation
W: qgis-grass no-documentation
E: qgis-grass invalid-soname /usr/lib/libqgisgrass.so libqgisgrass.so
W: qgis-theme-nkids no-documentation

(In reply to comment #21)
> (In reply to comment #20)
> > * rpm call
> 
> Sorry; what would the "proper way" be?  The cmake script tries to lookup the 
> grass library in the wrong locations and I need to tell it that the library 
> lives under a directory like /usr/lib/grass-6.2.1.
  - On rawhide, it is /usr/lib/grass-6.2.2RC1 and rpm EVR is
    grass-6.2.2-0.2.RC1.fc8 so anyway this usage of rpm cannot
    be used.
    The simplest way is
--------------------------------------------------------------
for dir in %{_libdir}/grass-*/ ; do
	GRASSDIR=$dir
done
%cmake \
	-D GRASS_PREFIX=$GRASSDIR \
        .........
--------------------------------------------------------------
    NOTE: /usr/lib/grass-6.2.2RC1/etc/VERSIONNUMBER is in
          grass package, however when I try mockbuild of
          0.8.1-2, this cannot be found as
--------------------------------------------------------------
grass is not installed
--------------------------------------------------------------
          ... because grass-devel requiers grass-libs, but does
              not require grass itself.

> > * non-sover libraries with providing -devel subpackage.
> 
> Upstream does not provide a sover with their library but their program links 
> against it:
> $ ldd /usr/bin/qgis | grep libqgis_core.so
>         libqgis_core.so => /usr/lib/libqgis_core.so (0x03099000)
> $ ldd /usr/lib/qgis/libgrass*.so | grep libqgisgrass.so
>         libqgisgrass.so => /usr/lib/libqgisgrass.so (0x00673000)
>         libqgisgrass.so => /usr/lib/libqgisgrass.so (0x002fc000)
   This is not what I said as a problem because this linkage
   is done *within* qgis package itself
   As I said in comment #20:

> * non-sover libraries with providing -devel subpackage.
>   - Shipping non-sover libraries with providing -devel subpackage
>     is unwilling because:
> 
>     Shipping -devel package means that the libraries %{_libdir}/*.so
>     is allowed to be linked *from other packages*. So some binaries in
>     *other package* may link to the libraries in this package.
> 
>     Then ABI of the libraries in this package may change in the future.
>     At this time, as these libraries have no sover, rpm has no clue of
>     whether ABI of these libraries changed, so rpm allows the upgrading
>     of this package. However, this upgrade surely stop the *other binaries*
>     linking to these libraries from working any more.
> 
> Should I still move these files into the -devel package 
   So this is not a solution
   - So the question is
     * First of all, why are the files under /usr/include/qsis
       needed?
     * Is libqgis_core.so meant to be linked 
       *from other packages/libraries*?
       - If YES, then libqgis_core.so should have sover
       - If NO, then -devel package should not be needed
         and all the files under /usr/include/qsis should be removed
         _unless_ qgis itself uses those files (in this case,
         all the needed files should be moved to main package).

* python
  - And even after some fixes, mockbuild stops at:
------------------------------------------------------------
+ /bin/chmod +x
/var/tmp/qgis-0.8.1-2.1.fc8-root-mockbuild//usr/share/qgis/python/test_export.py
/bin/chmod: cannot access
`/var/tmp/qgis-0.8.1-2.1.fc8-root-mockbuild//usr/share/qgis/python/test_export.py':
No such file or directory
error: Bad exit status from /var/tmp/rpm-tmp.87188 (%install)
-------------------------------------------------------------
   - Because all python stuff are not installed on mockbuild
     (you seems to have removed python-devel from BuildRequires.
      This causes the difference between your local rpmbuild
      and mockbuild)

First, thanks for being patient with me while I learn some of the proper ways 
of creating a package like this under the Fedora build system.  It's 
definitely a learning process.

(In reply to comment #23)
> > lives under a directory like /usr/lib/grass-6.2.1.
>   - On rawhide, it is /usr/lib/grass-6.2.2RC1 and rpm EVR is

What does "EVR" stand for?  I haven't found the page on fedoraproject.org that 
describes this.

Thanks for the idea on how to find the grass libraries; I'll be removing the 
rpm call shortly.


> > > * non-sover libraries with providing -devel subpackage.
> > * non-sover libraries with providing -devel subpackage.
> >   - Shipping non-sover libraries with providing -devel subpackage
> >     is unwilling because:
> > Should I still move these files into the -devel package 
>    So this is not a solution
>    - So the question is
>      * First of all, why are the files under /usr/include/qsis
>        needed?
>      * Is libqgis_core.so meant to be linked 
>        *from other packages/libraries*?
>        - If YES, then libqgis_core.so should have sover
>        - If NO, then -devel package should not be needed
>          and all the files under /usr/include/qsis should be removed
>          _unless_ qgis itself uses those files (in this case,
>          all the needed files should be moved to main package).

Thanks for clearing this up.  I'll try to get with upstream and see what their 
idea on this is.

If they do think that they need to provide a sover; is it acceptable for me to 
create a patch to push this into this release without waiting for a release 
from upstream if we both agree on what the sover should be?


> * python
>   - And even after some fixes, mockbuild stops at:
> ------------------------------------------------------------
> + /bin/chmod +x
> /var/tmp/qgis-0.8.1-2.1.fc8-root-mockbuild//usr/share/qgis/python/test_export.py
> /bin/chmod: cannot access
> 
`/var/tmp/qgis-0.8.1-2.1.fc8-root-mockbuild//usr/share/qgis/python/test_export.py':
> No such file or directory
> error: Bad exit status from /var/tmp/rpm-tmp.87188 (%install)
> -------------------------------------------------------------
>    - Because all python stuff are not installed on mockbuild
>      (you seems to have removed python-devel from BuildRequires.
>       This causes the difference between your local rpmbuild
>       and mockbuild)

Right; I removed python-devel from the BuildRequires since the python bindings 
couldn't be created anyway because there wasn't a PyQT4 package yet.

I'll try to get mock setup here to save you the hassle of dealing with all 
these problems and get back to you when I've ironed out some more of these 
problems.

Okay. This package is rather a big package in Fedora and
so it is natural to take long fix up all problems!

(In reply to comment #24)
> (In reply to comment #23)
> > > lives under a directory like /usr/lib/grass-6.2.1.
> >   - On rawhide, it is /usr/lib/grass-6.2.2RC1 and rpm EVR is
> 
> What does "EVR" stand for?  
On Fedora packaging, this means "Epoch:Version:Release".

> > > > * non-sover libraries with providing -devel subpackage.
<snip>
> > > * non-sover libraries with providing -devel subpackage.
> >    - So the question is
> >      * First of all, why are the files under /usr/include/qsis
> >        needed?
> Thanks for clearing this up.  I'll try to get with upstream 
> and see what their 
> idea on this is.
Well, my question is: is the -devel package really needed?

For example, do you know some packages which require qgis-devel
to get compiled?
If not, the simplest resolution is just not to ship -devel package.

(In reply to comment #25)
> Well, my question is: is the -devel package really needed?
> 
> For example, do you know some packages which require qgis-devel
> to get compiled?
> If not, the simplest resolution is just not to ship -devel package.

I don't have any packages that require qgis-devel, so perhaps that is the best 
bet for now until I can work with upstream to determine the solution to the 
sover problem.

I'll set up the package to delete the include files for now and disable the 
devel package.

(In reply to comment #26)
> I'll set up the package to delete the include files for now and disable the 
> devel package.
Okay, I agree.

Spec URL: http://www.silfreed.net/download/repo/packages/qgis/qgis.spec
SRPM URL: 
http://www.silfreed.net/download/repo/packages/qgis/qgis-0.8.1-3.src.rpm

%changelog
* Mon Jun 25 2007 Douglas E. Warner <silfreed> 0.8.1-3
- updating detection of grass libraries to not use RPM call
- disabling building of -devel package due to shared libraries not being
  versioned and having no other packages that compile against qgis
  (see bug #241403)
- removing chmod of test_export.py due to lack of python requirement
- removing msexport and share/python directory due to removal of python

* Fri Jun 22 2007 Douglas E. Warner <silfreed> 0.8.1-2
- added BuildRequires: cmake
- updated build to use cmake macro and make verbose

$ rpmlint RPMS/qgis-*0.8.1-3* SRPMS/qgis-0.8.1-3.src.rpm
E: qgis invalid-soname /usr/lib/libqgis_core.so libqgis_core.so
W: qgis-grass no-documentation
E: qgis-grass invalid-soname /usr/lib/libqgisgrass.so libqgisgrass.so
W: qgis-theme-nkids no-documentation

Assiging.

Sorry for delay.

For 0.8.1-3: Almost okay.

* Desktop file
  - qgis is a GUI program and the corresponding desktop
    file should be created and installed.
    Please check: "Desktop files" section of
    http://fedoraproject.org/wiki/Packaging/Guideline

    NOTE: Please remember to add "desktop-file-utils" to
          BuildRequires.

* Documents
  - IMO the following files can be added to %doc
--------------------------------------------------
CONTRIBUTORS
--------------------------------------------------

? Dependency
  - By the way, Does qgis-grass not depend on grass?
    (currently while qgis depends on grass-libs,
     it does not require grass itself. This is a
     QUESTION, not blocker)

? %check
  - tarball contains tests/ directory. Can qgis do
    some test program like "make check" at build stage?
    If so, add %check stage and do some check program
    in %check stage.

? Files with white spaces
  - Just a question: some files have names with white spaces.
    Is it okay?

Well, as this is a sponsor needed tickets:
-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on:
http://fedoraproject.org/PackageReviewStatus/NEW.html
(NOTE: please don't choose "Merge Review")


Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------

I'll have 0.8.1-4 up shortly; it's building in mock right now - I need to test 
the desktop file and upload it.  I'll reply when its done.

(In reply to comment #30)
> ? Dependency
>   - By the way, Does qgis-grass not depend on grass?
>     (currently while qgis depends on grass-libs,
>      it does not require grass itself. This is a
>      QUESTION, not blocker)

AFAICT, qgis-grass should not depend on grass, just grass-libs.  And can you 
confirm that qgis doesn't depend on grass-libs?  It doesn't on my box, just 
wanted to make sure that was a typo.


> ? %check
>   - tarball contains tests/ directory. Can qgis do
>     some test program like "make check" at build stage?
>     If so, add %check stage and do some check program
>     in %check stage.

I can't get the tests to compile; it errors out with:
g++  -o tests     -L/usr/lib/qt-3.3/lib -lqt-mt -lXext -lX11 -lm
/usr/lib/gcc/i386-redhat-linux/4.1.1/../../../crt1.o: In function `_start':
(.text+0x18): undefined reference to `main'
collect2: ld returned 1 exit status
make: *** [tests] Error 1

> ? Files with white spaces
>   - Just a question: some files have names with white spaces.
>     Is it okay?

These are provided from upstream, so I'll work with them to get any problems 
here worked out.


> Well, as this is a sponsor needed tickets:
..snip..
> Usually there are two ways to show this.
> A. submit other review requests with enough quality.

I have some other packages that I'm sitting on; I think I'll be submitting 
qtpfsgui for inclusion; I'll post the bug id here once I have it.

(In reply to comment #31)
> (In reply to comment #30)
> > ? Dependency 
> AFAICT, qgis-grass should not depend on grass, just grass-libs.
Okay.

> And can you 
> confirm that qgis doesn't depend on grass-libs?
Surely qgis does not require grass related packages.

> > ? %check
> I can't get the tests to compile; it errors out with:
> g++  -o tests     -L/usr/lib/qt-3.3/lib -lqt-mt -lXext -lX11 -lm
> /usr/lib/gcc/i386-redhat-linux/4.1.1/../../../crt1.o: In function `_start':
> (.text+0x18): undefined reference to `main'
> collect2: ld returned 1 exit status
> make: *** [tests] Error 1
This means that tests program requires qt-devel 
(i.e. Qt _3_ development packages)... why?

> > Well, as this is a sponsor needed tickets:
> I have some other packages that I'm sitting on; I think I'll be submitting 
> qtpfsgui for inclusion; I'll post the bug id here once I have it.
Okay, then let me know when new review request is ready.

(In reply to comment #32)
> > And can you 
> > confirm that qgis doesn't depend on grass-libs?
> Surely qgis does not require grass related packages.

Your previous comment said:
? Dependency
  - By the way, Does qgis-grass not depend on grass?
    (currently while ***qgis*** depends on grass-libs,
     it does not require grass itself. This is a
     QUESTION, not blocker)

(emphasis added on qgis)
I was assuming you ment to say qgis-grass in the second sentance.

> > > ? %check
> > I can't get the tests to compile; it errors out with:
> > g++  -o tests     -L/usr/lib/qt-3.3/lib -lqt-mt -lXext -lX11 -lm
> > /usr/lib/gcc/i386-redhat-linux/4.1.1/../../../crt1.o: In function 
`_start':
> > (.text+0x18): undefined reference to `main'
> > collect2: ld returned 1 exit status
> > make: *** [tests] Error 1
> This means that tests program requires qt-devel 
> (i.e. Qt _3_ development packages)... why?

I have those installed on my local machine, so I'm not sure what's going on.  
If it's alright I think I'll just leave this part out for now.

(In reply to comment #33)

> Your previous comment said:
> ? Dependency
>   - By the way, Does qgis-grass not depend on grass?
>     (currently while ***qgis*** depends on grass-libs,
>      it does not require grass itself. This is a
>      QUESTION, not blocker)
> 
> (emphasis added on qgis)
> I was assuming you ment to say qgis-grass in the second sentance.
Oh, sorry...

> > > > ? %check
> If it's alright I think I'll just leave this part out for now.
Okay.

Spec URL: http://www.silfreed.net/download/repo/packages/qgis/qgis.spec
SRPM URL: 
http://www.silfreed.net/download/repo/packages/qgis/qgis-0.8.1-4.src.rpm

%changelog
* Wed Jun 27 2007 Douglas E. Warner <silfreed> 0.8.1-4
- adding contributors to doc
- adding desktop file and icon

Few notes.

* Fix typo:

> # remove doc because it gets packged by %doc

* You should add comment about BR both grass and grass-devel because Average Joe
may consider grass as redundant BR.

Currently uploading.

Spec URL: http://www.silfreed.net/download/repo/packages/qgis/qgis.spec
SRPM URL: 
http://www.silfreed.net/download/repo/packages/qgis/qgis-0.8.1-5.src.rpm

%changelog
* Wed Jun 27 2007 Douglas E. Warner <silfreed> 0.8.1-5
- adding comment on why grass is required in addition to grass-devel for BR
- fixing typo

For qgis, only one issue is left.
* qgis.png
  - If you took this from some URL, please write the URL like
    Source0
    If not, please write a comment how you got this png file.

* So, as this is a sponsor needed review request, I will wait
  for your new review request (or your pre-review of other person's
  review request).

Currently uploading.

I fixed the icon problem by just copying it from the installed location, so 
that should provide the needed documentation.  Let me know if I should do this 
a different way since upstream doesn't provide an icon in a standard place 
(ie, should I hard-code the path in the .desktop or generate the .desktop 
instead of copying the file).

Spec URL: http://www.silfreed.net/download/repo/packages/qgis/qgis.spec
SRPM URL: 
http://www.silfreed.net/download/repo/packages/qgis/qgis-0.8.1-6.src.rpm

%changelog
* Thu Jun 28 2007 Douglas E. Warner <silfreed> 0.8.1-6
- fixed date of changelog entry for 0.8.1-5 from Wed Jun 27 2007 to
  Thu Jun 28 2007
- linking icon to included png instead of packaging it again

I've added bug 246460.

(In reply to comment #39)
> I fixed the icon problem by just copying it from the installed location, so 
> that should provide the needed documentation.  
This is okay. No problem.

(In reply to comment #40)
> I've added bug 246460.
Just glanced at your spec file and it seems almost okay
(well some points should be fixed so I will comment about that,
 however I may not be able to review bug 246460 because currently
 I am reviewing and watching about 20 review request....)

OKAY!!
-------------------------------------------------------------
  This package (qgis) is APPROVED by me
-------------------------------------------------------------

Please follow the procedure written on
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account"
When you requested someone to sponsor you (in the procedure
above), please make a note on this bug that you did so.

If you want to push this package also on F-7, you
also have to check:
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
after the URL above.

If you have some questions, please let me know.

Now I am sponsoring you.

New Package CVS Request
=======================
Package Name: qgis
Short Description: A user friendly Open Source Geographic Information System
Owners: silfreed
Branches: FC-6 F-7

Please try to rebuild this package on Fedora build system.

Building now; thanks for the prod.  I had tried building it previously and 
something had gone wrong; I hadn't had the time to try it out again until now.

I did already get a build report from plague that I need to make a patch for 
lib64 directory support; I'll be looking into that next.

qgis apparently can't be built on ppc64 right now due to bug 246324; should I 
add an ExcludeArch: ppc64 to this package for now, and open a new bug that 
blocks FE-ExcludeArch-ppc64?

(In reply to comment #46)
> qgis apparently can't be built on ppc64 right now due to bug 246324; should I 
> add an ExcludeArch: ppc64 to this package for now, and open a new bug that 
> blocks FE-ExcludeArch-ppc64?

That is reasonable for now.

added bug 247152 for ExcludeArch: ppc64

Builds successfully on x86, x86_64, and ppc.
http://koji.fedoraproject.org/koji/taskinfo?taskID=57376

qgis-0.8.1-10.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.

Thank you for getting qgis to work on Fedora. Installation was easy but I have
not done much with the package yet. If I find any problems I will post them here.

I am having one small issue. I cannot access the help pages. I get a message in
my browser saying they are located at /usr/share/qgis/doc/index.html but when I
look the directory doc does not seem to exist in /usr/share/qgis. 

I simply installed the package using the command:
 yum --enablerepo=development install qgis  

I haven't done anything else to configure qgis. 

Hope this helps, and thank you again.
Mehmet

(In reply to comment #51)
> I am having one small issue. I cannot access the help pages. I get a message 
in
> my browser saying they are located at /usr/share/qgis/doc/index.html but 
when I
> look the directory doc does not seem to exist in /usr/share/qgis. 

I'll try to take a look at that over the weekend.

The doc files should be in /usr/share/doc/qgis-0.8.1, but it doesn't look like 
they're being included correctly.  I'll try to figure out what's going on and 
get them installed correctly, as well as see if I can fix the help link so it 
points at the correct place.

(In reply to comment #51)
> I am having one small issue. I cannot access the help pages. I get a message 
in
> my browser saying they are located at /usr/share/qgis/doc/index.html but 
when I
> look the directory doc does not seem to exist in /usr/share/qgis. 

Actually, can you go ahead and file this as a new bug?

qgis-0.8.1-11.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.

qgis-0.8.1-11.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.