Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.

Bug 232792

Summary: Review Request: mapserver - Environment for building spatially-enabled internet applications
Product: [Fedora] Fedora Reporter: Balint Cristian <cbalint>
Component: Package ReviewAssignee: Mamoru TASAKA <mtasaka>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: lemenkov
Target Milestone: ---Flags: mtasaka: fedora-review+
wtogami: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-05-10 20:15:54 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Attachments:
Description Flags
rpmlint log for 4.10.1-2 (with gdal-config issue modified)
none
mock build log of mapserver 4.10.1-2
none
mock build log of mapserver 4.10.1-2 with gdal-config issue fixed none

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@redhat.com
Branches: FC-5 FC-6
InitialCC: