Bug 232792 - Review Request: mapserver - Environment for building spatially-enabled internet applications
Summary: Review Request: mapserver - Environment for building spatially-enabled intern...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-03-17 22:00 UTC by Balint Cristian
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-05-10 20:15:54 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)
rpmlint log for 4.10.1-2 (with gdal-config issue modified) (1.58 KB, text/plain)
2007-04-20 14:22 UTC, Mamoru TASAKA
no flags Details
mock build log of mapserver 4.10.1-2 (157.09 KB, text/plain)
2007-04-20 14:24 UTC, Mamoru TASAKA
no flags Details
mock build log of mapserver 4.10.1-2 with gdal-config issue fixed (202.59 KB, text/plain)
2007-04-20 14:25 UTC, Mamoru TASAKA
no flags Details

Description Balint Cristian 2007-03-17 22:00:11 UTC
Spec URL: http://openrisc.rdsor.ro/mapserver.spec
SRPM URL: http://openrisc.rdsor.ro/mapserver-4.10.1-1.src.rpm
Description: Environment for building spatially-enabled internet applications

Website: http://mapserver.gis.umn.edu/

Comment 1 Balint Cristian 2007-03-17 22:34:34 UTC
updated.
Spec URL: http://openrisc.rdsor.ro/mapserver.spec
SRPM URL: http://openrisc.rdsor.ro/mapserver-4.10.1-2.src.rpm

Comment 2 Mamoru TASAKA 2007-04-20 14:22:35 UTC
Created attachment 153185 [details]
rpmlint log for 4.10.1-2 (with gdal-config issue modified)

Well, actually I don't know about PHP at all!! So I don't know
how to use this...
However as this is heavily related with grass, gdal.. so I will
review this.

For 4.10.1-2:
* Mock build failure/gdal-config related issue
  - First of all, -2 won't be rebuilt.
-------------------------------------------------------
/usr/bin/ld: cannot find -lodbcinst
collect2: ld returned 1 exit status
make: *** [shp2img] Error 1
error: Bad exit status from /var/tmp/rpm-tmp.41559 (%build)
-------------------------------------------------------
    (Well, the previous line is so long, so I will attach a full mock
     build log)
     This is because `$GDAL_CONFIG --dep-libs` in configure adds
     unnecessary linkages.
     Applying a patch for configure to remove the above seems
     good. For sed usage,
-------------------------------------------------------
%{__sed} -i.libs -e 's|`\$GDAL_CONFIG --dep-libs`||' configure
-------------------------------------------------------

* License
  - Well, while most files are licensed under MIT, one file is
    licensed under BSD.
-------------------------------------------------------
strptime.c
-------------------------------------------------------
    Currently I do not disagree with writing "BSD" for the license
    of this.

Then after the fix above is applied..
* rpmlint - attached.
  Summary:
  * Fix improper permissions.

Next for spec file:
A. Description entry
   - Well, while there is a php releated subpackage which requires
     php, does main package also require php?
     Please explain because currently I don't know how to use this
     at all.
   - Do perl/python subpackage have no dependency for main package?
   - Requires: python/Requires: perl are redundant.
   - Current Fedora packaging policy requires that BuildRoot includes
     release number (according to the section "BuildRoot tag" of
     http://fedoraproject.org/wiki/Packaging/Guidelines )
   - By the way, there seems to be java/ruby binding. Would you try
     to enable this?

B. Prep/Build/Install stage
   - not a big problem, however fedora compilation flags is passed
     twice for main and python subdirectory build (not a blocker)
   - If this support parallel make, then please use. Otherwise
     add some comments in spec file.

C. Scripts
   - While no shared libraries are installed by main package, why
     does main package call ldconfig?

D. File entry
    - README.CONFIGURE is for people who want to build this software
      by themselves and so this is not needed for fedora rpm.
    - Vera related fonts under tests/ should not be installed because
      these fonts are provides system-wide by bitstream-vera-fonts
    - (I say this although I know *very little* about httpd)
      Please consider to move files under /var/www to %{_datadir}
      Check: the section "Web Applications" of
      http://fedoraproject.org/wiki/Packaging/Guidelines
    - It seems that mapscript/php3/README should be added as %doc
      to php subpackage.
    - On my system %{_libdir}/php4/ is not owned by any package.
      Please check if this directory is correct.
    - %{perl_vendorarch}/auto/mapscript/ is not owned by any package.

Comment 3 Mamoru TASAKA 2007-04-20 14:24:08 UTC
Created attachment 153186 [details]
mock build log of mapserver 4.10.1-2

Comment 4 Mamoru TASAKA 2007-04-20 14:25:59 UTC
Created attachment 153187 [details]
mock build log of mapserver 4.10.1-2 with gdal-config issue fixed

Comment 5 Mamoru TASAKA 2007-04-20 18:11:59 UTC
One more comment:
Why do you exclude %{python_sitearch}/mapscript.py? ?

Comment 6 Balint Cristian 2007-04-24 13:09:26 UTC
(In reply to comment #2)
> Created an attachment (id=153185) [edit]
> rpmlint log for 4.10.1-2 (with gdal-config issue modified)
> 
> Well, actually I don't know about PHP at all!! So I don't know
> how to use this...
> However as this is heavily related with grass, gdal.. so I will
> review this.

 Well, this software package provedes both cgi-bin shell like interpreter and
a nice php plugin, loaded and registered by apache server.
  Its easy we should have the .so registered in apach , thats all. this .so
library exports for apache all necesary bindings, and provide a higher level
programing functions in php specialized for GIS.


> -------------------------------------------------------
> %{__sed} -i.libs -e 's|`\$GDAL_CONFIG --dep-libs`||' configure
> -------------------------------------------------------
applied.


> * License
>   - Well, while most files are licensed under MIT, one file is
>     licensed under BSD.
> -------------------------------------------------------
> strptime.c
> -------------------------------------------------------
erghh ...
author fault, i should notify him.

>     Currently I do not disagree with writing "BSD" for the license
>     of this.
> 
> Then after the fix above is applied..
> * rpmlint - attached.
>   Summary:
>   * Fix improper permissions.

fixed all.

> 
> Next for spec file:
> A. Description entry
>    - Well, while there is a php releated subpackage which requires
>      php, does main package also require php?
yes must require php, and especialy php-gd, it use some functions from php-gd

>      Please explain because currently I don't know how to use this
>      at all.
well, this mapserver.so extension have some external reference to php-gd 
extension so its mandatory to have php-gd at all.
I removed php since php-gd itself olso require php

>    - Do perl/python subpackage have no dependency for main package?
no, its just a wrapper.

>    - Requires: python/Requires: perl are redundant.
removed.

>    - Current Fedora packaging policy requires that BuildRoot includes
>      release number (according to the section "BuildRoot tag" of
>      http://fedoraproject.org/wiki/Packaging/Guidelines )
updated.

>    - By the way, there seems to be java/ruby binding. Would you try
>      to enable this?

ok i try, i notice down on my TODO.

> 
> B. Prep/Build/Install stage
>    - not a big problem, however fedora compilation flags is passed
>      twice for main and python subdirectory build (not a blocker)
>    - If this support parallel make, then please use. Otherwise
>      add some comments in spec file.
ouch, i will workaround put on my TODO as non-trivial.
 
> C. Scripts
>    - While no shared libraries are installed by main package, why
>      does main package call ldconfig?

removed.
 
> D. File entry
>     - README.CONFIGURE is for people who want to build this software
>       by themselves and so this is not needed for fedora rpm.
not included for now.

>     - Vera related fonts under tests/ should not be installed because
>       these fonts are provides system-wide by bitstream-vera-fonts
not included for now.

>     - (I say this although I know *very little* about httpd)
>       Please consider to move files under /var/www to %{_datadir}
update my TODO for now.

>       Check: the section "Web Applications" of
>       http://fedoraproject.org/wiki/Packaging/Guidelines
>     - It seems that mapscript/php3/README should be added as %doc
>       to php subpackage.
>     - On my system %{_libdir}/php4/ is not owned by any package.
If I own it than i break ownage for other php modules. 
I saw no other php modules olso own it, this is a
place where all php modules go to be picked up by apache.
Its owned _default_ by php-common !
 
>       Please check if this directory is correct.
>     - %{perl_vendorarch}/auto/mapscript/ is not owned by any package.
now its owned.


Comment 7 Balint Cristian 2007-04-24 13:10:01 UTC
(In reply to comment #5)
> One more comment:
> Why do you exclude %{python_sitearch}/mapscript.py? ?

done.

Comment 8 Balint Cristian 2007-04-24 13:11:00 UTC
I got 3 TODO for now, i proceed into, hope within hours i solve those lefting 
issues.

Comment 9 Balint Cristian 2007-04-24 14:50:24 UTC
Spec URL: http://openrisc.rdsor.ro/mapserver.spec
SRPM URL: http://openrisc.rdsor.ro/mapserver-4.10.1-3.src.rpm

solved all blockers.
ruby is not enabled becouse i wasnt able to build it, java is now enabled.

Comment 11 Mamoru TASAKA 2007-04-24 16:49:48 UTC
For -3:

* php module directory
> >     - On my system %{_libdir}/php4/ is not owned by any package.
> If I own it than i break ownage for other php modules. 
> I saw no other php modules olso own it, this is a
> place where all php modules go to be picked up by apache.
> Its owned _default_ by php-common !
  - Still I don't know why this is happening
    * In your opinion it is the bug of php side that %{_libdir}/php4
      is not owned by any package?
    * And what does php"4" means? This "4" is of no relation with
      php version (currently 5.2.1)?
    * And as far as I saw some php modules rpms, php modules (which
      I think so) are installed under %{_libdir}/php/modules/, and this
      directory (%{_libdir}/php/modules) is owned by php-common.

* perl .so module permission
  - Well, actually you fixed the permission by:
----------------------------------------------------------
%attr(0755,root,root) %{perl_vendorarch}/auto/mapscript/*
----------------------------------------------------------
    However, this method leaves the following message which
    I don't know I can ignore:
----------------------------------------------------------
+ /usr/lib/rpm/check-buildroot
+ /usr/lib/rpm/redhat/brp-compress
+ /usr/lib/rpm/redhat/brp-strip-static-archive /usr/bin/strip
+ /usr/lib/rpm/redhat/brp-strip-comment-note /usr/bin/strip /usr/bin/objdump
/usr/bin/strip: unable to copy file
'/var/tmp/mapserver-4.10.1-3.fc7-root-mockbuild/usr/lib/perl5/vendor_perl/5.8.8/i386-linux-thread-multi/auto/mapscript/mapscript.so'
reason: Permission denied
+ /usr/lib/rpm/brp-python-bytecompile
+ /usr/lib/rpm/redhat/brp-java-repack-jars
----------------------------------------------------------
   To avoid this, it seems that the permission of mapscript.so must
   be changed to 0755 at the install stage, not by setting attr.

* documentation
----------------------------------------------------------
 %files
 %defattr(-,root,root)
-%doc COMMITERS GD-COPYING HISTORY.TXT INSTALL
-%doc MIGRATION_GUIDE.TXT README README.CONFIGURE
+%doc COMMITERS GD-COPYING HISTORY.TXT 
+%doc INSTALL MIGRATION_GUIDE.TXT
----------------------------------------------------------
  - While ReADME.CONFIGURE is not needed, IMO README should be
    left as documentation.

Comment 12 Balint Cristian 2007-04-24 17:05:20 UTC
>     * In your opinion it is the bug of php side that %{_libdir}/php4
>       is not owned by any package?

It _should be owned by:
[cbalint@cbalint ~]$ rpm -qf /usr/lib64/php
php-common-5.2.1-5

So i should _not_ own it !

>     * And what does php"4" means? This "4" is of no relation with
SHIIT.... 
Sorry, ok i should remove 4, and put all in 
/usr/lib64/php/modules/
Seems thigs changed since a while ...

>       php version (currently 5.2.1)?
Ya right, sorry for confusion.

>     * And as far as I saw some php modules rpms, php modules (which
>       I think so) are installed under %{_libdir}/php/modules/, and this
>       directory (%{_libdir}/php/modules) is owned by php-common.
correct !
 
> * perl .so module permission
>   - Well, actually you fixed the permission by:
> ----------------------------------------------------------
> %attr(0755,root,root) %{perl_vendorarch}/auto/mapscript/*
> ----------------------------------------------------------
>     However, this method leaves the following message which
>     I don't know I can ignore:
> ----------------------------------------------------------
> + /usr/lib/rpm/check-buildroot
> + /usr/lib/rpm/redhat/brp-compress
> + /usr/lib/rpm/redhat/brp-strip-static-archive /usr/bin/strip
> + /usr/lib/rpm/redhat/brp-strip-comment-note /usr/bin/strip /usr/bin/objdump
> /usr/bin/strip: unable to copy file
> '/var/tmp/mapserver-4.10.1-3.fc7-root-mockbuild/usr/lib/perl5/vendor_perl/5.8.8/i386-linux-thread-multi/auto/mapscript/mapscript.so'
> reason: Permission denied
> + /usr/lib/rpm/brp-python-bytecompile
> + /usr/lib/rpm/redhat/brp-java-repack-jars
> ----------------------------------------------------------
>    To avoid this, it seems that the permission of mapscript.so must
>    be changed to 0755 at the install stage, not by setting attr.
> 
> * documentation
> ----------------------------------------------------------
>  %files
>  %defattr(-,root,root)
> -%doc COMMITERS GD-COPYING HISTORY.TXT INSTALL
> -%doc MIGRATION_GUIDE.TXT README README.CONFIGURE
> +%doc COMMITERS GD-COPYING HISTORY.TXT 
> +%doc INSTALL MIGRATION_GUIDE.TXT
> ----------------------------------------------------------
>   - While ReADME.CONFIGURE is not needed, IMO README should be
>     left as documentation.

Yay, i did stupid F6 instead of F5 at a point.

Ok i fix all issues and upload things for tomorrow.

(head out for now in a rush)

Comment 13 Ville Skyttä 2007-04-24 19:08:05 UTC
(In reply to comment #12)
> >     * In your opinion it is the bug of php side that %{_libdir}/php4
> >       is not owned by any package?
> 
> It _should be owned by:
> [cbalint@cbalint ~]$ rpm -qf /usr/lib64/php
> php-common-5.2.1-5

Note "php4" vs "php".


Comment 14 Mamoru TASAKA 2007-05-04 08:01:39 UTC
ping?

Comment 15 Balint Cristian 2007-05-07 09:35:47 UTC
7 day holiday.
today i try repush.

Comment 17 Balint Cristian 2007-05-10 12:58:20 UTC
- Solved all mentioned issue in the thread, except ruby build (that one need 
investigation).
- New upstream release is just bugfix, it not affect way of packing.

Comment 18 Mamoru TASAKA 2007-05-10 13:50:57 UTC
Rebuild finished. I will check new rpms later.

Comment 19 Mamoru TASAKA 2007-05-10 15:40:02 UTC
Well, for 4.10.2-1:

* Dependency for main package:
  - Well, for unknown reason I didn't notice, however
    none of 4 subpackages have no dependency for main
    (mapserver) package. Please check if this is correct.

    IMO at least perl/python/java binding subpackage should
    have the release number dependent dependency for main
    package (i.e. should have: 
    "Requires: %{name} = %{version}-%[release}").

* Source
  - The URL of the source returns "not found". Maybe:
    http://download.osgeo.org/mapserver/mapserver-4.10.2.tar.gz ?

* (minor) Macros
  - /usr/sbin/ can be replaced with %{_sbindir} (preferred)
  - And in the line "mkdir -p %{buildroot}/etc/php.d" 
    /etc should be %{_sysconfdir}

Other things are okay.

Comment 20 Balint Cristian 2007-05-10 15:52:39 UTC
(In reply to comment #19)
> Well, for 4.10.2-1:
> 
> * Dependency for main package:
>   - Well, for unknown reason I didn't notice, however
>     none of 4 subpackages have no dependency for main
>     (mapserver) package. Please check if this is correct.

updated.
php one not require this, it embed everything inside that apache
module, so thats exception.

> 
>     IMO at least perl/python/java binding subpackage should
>     have the release number dependent dependency for main
>     package (i.e. should have: 
>     "Requires: %{name} = %{version}-%[release}").
yes updated.

> 
> * Source
>   - The URL of the source returns "not found". Maybe:
>     http://download.osgeo.org/mapserver/mapserver-4.10.2.tar.gz ?
umm, updated. (they changed truely)

> 
> * (minor) Macros
>   - /usr/sbin/ can be replaced with %{_sbindir} (preferred)
>   - And in the line "mkdir -p %{buildroot}/etc/php.d" 
>     /etc should be %{_sysconfdir}
updated every places.

> 
> Other things are okay.

Thank you Tasaka !

Comment 22 Mamoru TASAKA 2007-05-10 15:59:07 UTC
Only checked by diff.

The last issue (minor) is the line:
----------------------------------------------
%files
%defattr(-,root,root)
%doc README COMMITERS GD-COPYING HISTORY.TXT  
%doc INSTALL MIGRATION_GUIDE.TXT
%doc rfc symbols tests
%doc fonts
%{_bindir}/shp2img
%{_bindir}/shptree
%{_bindir}/sortshp
%{_bindir}/tile4ms
/usr/sbin/mapserv <----- THIS LINE (please use macro)
------------------------------------------------
Please fix in CVS. Other things are okay.

------------------------------------------------------
   This package (mapserver) is APPROVED by me
------------------------------------------------------

Comment 23 Balint Cristian 2007-05-10 16:04:13 UTC
http://openrisc.rdsor.ro/mapserver.spec
http://openrisc.rdsor.ro/mapserver-4.10.2-3.fc7.src.rpm

Minor todo for my list:
- When pdflib is in fedora enable mapserver against it.
- When ming is in fedora enable mapserver against it.
- Fix and enable ruby also in future.

Comment 24 Balint Cristian 2007-05-10 16:09:11 UTC
New Package CVS Request
=======================
Package Name: mapserver
Short Description: Environment for spatially-enabled internet applications
Owners: cbalint
Branches: FC-5 FC-6
InitialCC:


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