Bug 232792
Summary: | Review Request: mapserver - Environment for building spatially-enabled internet applications | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Balint Cristian <cbalint> | ||||||||
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> | ||||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||||
Severity: | medium | Docs Contact: | |||||||||
Priority: | medium | ||||||||||
Version: | rawhide | CC: | 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: | --- | Target Upstream Version: | |||||||||
Embargoed: | |||||||||||
Attachments: |
|
Description
Balint Cristian
2007-03-17 22:00:11 UTC
updated. Spec URL: http://openrisc.rdsor.ro/mapserver.spec SRPM URL: http://openrisc.rdsor.ro/mapserver-4.10.1-2.src.rpm 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. Created attachment 153186 [details]
mock build log of mapserver 4.10.1-2
Created attachment 153187 [details]
mock build log of mapserver 4.10.1-2 with gdal-config issue fixed
One more comment: Why do you exclude %{python_sitearch}/mapscript.py? ? (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. (In reply to comment #5) > One more comment: > Why do you exclude %{python_sitearch}/mapscript.py? ? done. I got 3 TODO for now, i proceed into, hope within hours i solve those lefting issues. 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. 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.
> * 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) (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". ping? 7 day holiday. today i try repush. - 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. Rebuild finished. I will check new rpms later. 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. (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 ! 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 ------------------------------------------------------ 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. New Package CVS Request ======================= Package Name: mapserver Short Description: Environment for spatially-enabled internet applications Owners: cbalint Branches: FC-5 FC-6 InitialCC: |