Bug 222039 - Review Request: ogdi - Open Geographic Datastore Interface
Review Request: ogdi - Open Geographic Datastore Interface
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Patrice Dumas
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT 222042
  Show dependency treegraph
 
Reported: 2007-01-09 14:04 EST by Rick Niles
Modified: 2007-11-30 17:11 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-02 10:36:11 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
wtogami: fedora‑cvs+


Attachments (Terms of Use)
Mock build log of ogdi-3.1.5-2.fc7 (170.86 KB, text/plain)
2007-02-10 02:37 EST, Mamoru TASAKA
no flags Details
use INST_LIB instead of hardcoding MODULES_PATH (1.34 KB, patch)
2007-02-10 08:56 EST, Patrice Dumas
no flags Details | Diff

  None (edit)
Description Rick Niles 2007-01-09 14:04:11 EST
Spec URL: http://www.gnupooh.org/RPM/ogdi.spec
SRPM URL: http://www.gnupooh.org/RPM/ogdi-3.1.5-mh1.src.rpm
Description: Open Geographic Datastore Interface
This is my first Fedora package and I need a reviewer.  This package is a prereq for GDAL, the Geospatial Data Abstraction Library.
Source URL: http://ogdi.sourceforge.net/

Someone else was suppose to be working on this years ago, but I haven't seen any activity so I'm going to maintain it if no one else will.
Comment 1 John Mahowald 2007-01-11 02:02:04 EST
Not building in mock on FC6 x86_64:

gcc -shared  -O  -o /builddir/build/BUILD/ogdi-3.1.5/bin/Linux/libogdi31.so ecs_
dyna.o ecsregex.o ecssplit.o ecsassoc.o ecshash.o ecstile.o server.o ecsdist.o e
cslist.o ecsinfo.o ecsgeo.o ecs_xdr.o ecs_xdrz.o gmath.o client.o ecs_capabiliti
es.o -ldl  -lz -lexpat  -lproj  -lm 
/usr/bin/ld: ecs_dyna.o: relocation R_X86_64_32 against `a local symbol' can not
 be used when making a shared object; recompile with -fPIC
ecs_dyna.o: could not read symbols: Bad value
collect2: ld returned 1 exit status


Don't use Packager field. You may put your contact information in your
%changelog entries.

See http://fedoraproject.org/wiki/Packaging/Guidelines  and
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines for reference.
Comment 2 Mamoru TASAKA 2007-01-19 18:02:49 EST
(Add myself to Cc list as I want to review bug 222042.
 Please update your spec/srpm according to John)
Comment 3 Mamoru TASAKA 2007-02-01 11:31:32 EST
ping?
Comment 4 Balint Cristian 2007-02-08 06:32:36 EST
Spec: http://openrisc.rdsor.ro/ogdi.spec
SRPMS: http://openrisc.rdsor.ro/ogdi-3.1.5-1.src.rpm

Updated.
Tested the build in mock, and olso on various arches: 
i386,x86_64,sparc,alpha,ppc

becouse it required an extra -fPIC flag to link objects correctly, i make 
those test across arches.

  Only minor issue, to add -soname versioning, anyway rpmlint complain as W: 
only.I will add soname versioning but not right now, anyway the project seem 
to update their source once in severeal year, so -soname doesnt make sense for 
an exact -compat versioning between releases.

can review please ?
thank you,
/cristian
Comment 5 Balint Cristian 2007-02-09 10:45:46 EST
updated.
http://openrisc.rdsor.ro/ogdi.spec
http://openrisc.rdsor.ro/ogdi-3.1.5-2.src.rpm

- add -soname versioning on shared libs
- remove pl lang from spec
- fix packing of libs
- tcl is plugin dont separate package name

+rpmlint report zero bugs.
+mock can build it.
Comment 6 Mamoru TASAKA 2007-02-09 11:19:55 EST
Well, who is the current submitter??
Comment 7 Balint Cristian 2007-02-09 11:50:02 EST
Is Rick Niles (niles@rickniles.com)

  I just helped him to fix that package, since 2007-01-11 
nothing happened, Rick doesn't updated the package, and
this is blocker for grass + gdal.
 Basicly it required a mass rework of evrithing inside 
.spec and the source to align to nowdays standards.

Anyway, Tasaka, John can review it PLS ?
Maybe would be better Tasaka to review it since he is reviewing all deps
for grass. 

Or have to resubmit from scratch for this ?

/cristian
Comment 8 Mamoru TASAKA 2007-02-09 12:06:32 EST
Umm..

current fedora policy is that "we have to wait one month plus
one week at most for submitter's response"...
http://fedoraproject.org/wiki/Extras/Policy/StalledReviews

Well, actually I want to have all packages required by grass
reviewed by someone (including me). John, what do you think
of current status?

Anyway I go to sleep because it is 2AM in Japan now.

By the way.. what is PLS?
Comment 9 Balint Cristian 2007-02-09 12:17:41 EST
>Anyway I go to sleep because it is 2AM in Japan now.
No problem. tommorow is a day too.
Till than i will come up with geotiff-less version of gdal.

>By the way.. what is PLS?
PLS is please. I am sorry if i confuse with my abreviation, i will avoid 
it ;-)
Comment 10 Ralf Corsepius 2007-02-10 00:52:30 EST
Some remarks:

- What is the reason to run autoconf?
I don't see any reason to do so.

- What is the reason for the cp -af ...?
AFAIS, the cause is you running a modern autoconf on a poorly written
autoconf-2.13-based configure.in, which seems to break it.
Comment 11 Mamoru TASAKA 2007-02-10 02:37:22 EST
Created attachment 147831 [details]
Mock build log of ogdi-3.1.5-2.fc7

(In reply to comment #5)
> updated.
> http://openrisc.rdsor.ro/ogdi.spec
> http://openrisc.rdsor.ro/ogdi-3.1.5-2.src.rpm
Mockbuild log of 3.1.5-2 on FC7 i386 attached.

From a very quick check:
* compiler flags + debuginfo rpm
  - Fedora specific compilation flags are not passed
  - debuginfo rpm is useless because of missing "-g"
    option
* multilib condition
  - ogdi-driversdir.patch includes
-------------------------------------------
+#define MODULES_PATH "/usr/lib/ogdi/"
-------------------------------------------
    This is questionable on x86_64 arch.

* Documentation
  Add more documentation. Especially, Changelog README
  is rather mandatory.

* autotool
  - Please don't use autotool unless it is necessary.

* header file location
  - Please consider to hide header files into
    %{_includedir}/%{name}
  - By the way, ecs_util.h includes the line:
-----------------------------------------------------
    90	#include "projects.h"
-----------------------------------------------------
    However, I cannot find projects.h
Comment 12 Balint Cristian 2007-02-10 04:19:33 EST
updated.

> +#define MODULES_PATH "/usr/lib/ogdi/"
add lib64 path search too.

Spec: http://openrisc.rdsor.ro/ogdi.spec
SRPMS: http://openrisc.rdsor.ro/ogdi-3.1.5-3.src.rpm
- add dlopen path for lib64 too.
- add more docs
- fix export of CFLAGS
- move include files and add pkgconf module

>
>* autotool
>  - Please don't use autotool unless it is necessary.

removed.

>    90  #include "projects.h"
>-----------------------------------------------------
>    However, I cannot find projects.h

This is from proj-devel, so i change to <projects.h>.
Comment 13 Patrice Dumas 2007-02-10 06:15:47 EST
I may be wrong, but from my reading of config/common.mak and of the makefile
it seems that config/Linux.mak is already used, and not config/linux.mak,
since TARGET=Linux. So the cp -af are unneeded. As a suggestion, I think it is
more readable to avoid the {..,..} in the cp (although it may be less elegant).
Also cp -p should in general be better suited than cp -af.

make %{?_smp_mflags}
fails for me.

am I right in thinking that objects in /usr/lib/ogdi/ are dlopened? In
that case it would be better if they hadn't a soname.

/usr/lib/libecs_tcl.so.1
is in the main package, I guess this is a mistake.

README-BIN.TXT is certainly unusefull.

The requires for other subpackages should generally be with full version, ie:
Requires:       %{name} = %{version}-%{release}

Why isn't the sed 's/\"projects.h\"/\<projects.h\>/g' a patch
instead? It is not a blocker, but I suggest to do patches when possible
since sed one liners fail without notice.

I also suggest using rm instead of rm -f when possible such that it
breaks if the files to remove aren't there anymore. Alternatively 
you can use %exclude in %files. I would also remove the -f to mv.

In the pkgconfig file, there is certainly some .private requires or
flags missing, for expat, zlib and proj. And the -devel subpackage
depends certainly on zlib-devel, expat-devel and proj-devel.

There is a scary amount of compiler warnings most of them seem
serious. Maybe you could try to work patches for upstream?

There is a Requires on pkgconfig for -devel missing.

It seems to be a matter of style, but the template defattr is
%defattr(-,root,root,-)

If there are some examples, maybe their codes could be in %doc in
-devel?
Comment 14 Patrice Dumas 2007-02-10 06:16:46 EST
Also for documentation, it should certainly be

cp -p %{SOURCE1} .
Comment 15 Balint Cristian 2007-02-10 06:31:19 EST
updated.
Spec: http://openrisc.rdsor.ro/ogdi.spec
SRPMS: http://openrisc.rdsor.ro/ogdi-3.1.5-4.src.rpm
Comment 16 Patrice Dumas 2007-02-10 06:54:42 EST
Maybe you missed the Comment #13... Comment #14 was a rather minor 
addition....
Comment 17 Balint Cristian 2007-02-10 07:58:57 EST
Thanks Patrice !

Well, come again with an update:
Spec: http://openrisc.rdsor.ro/ogdi.spec
SRPMS: http://openrisc.rdsor.ro/ogdi-3.1.5-5.src.rpm

>I may be wrong, but from my reading of config/common.mak and of the makefile
i rennounce. Was useless in fact.
>Also cp -p should in general be better suited than cp -af.
updated.

>make %{?_smp_mflags}
>fails for me.
noticed, disabled.

>am I right in thinking that objects in /usr/lib/ogdi/ are dlopened? In
>that case it would be better if they hadn't a soname.

well, its difficult, i have to mess up make-files, i would prefer to leave it 
like this, and convince upstream people to add -soname suport. I done the 
soname work over to not warrn rpmlint.

>/usr/lib/libecs_tcl.so.1
>is in the main package, I guess this is a mistake.

oh, yes it was fixed.

>README-BIN.TXT is certainly unusefull.
:-)) not included, removed.

>The requires for other subpackages should generally be with full version, ie:
>Requires:       %{name} = %{version}-%{release}
oh. sorry for that.


>Why isn't the sed 's/\"projects.h\"/\<projects.h\>/g' a patch
>instead? It is not a blocker, but I suggest to do patches when possible
>since sed one liners fail without notice.
convert to patch.

>I also suggest using rm instead of rm -f when possible such that it
no force flags anymore.


>In the pkgconfig file, there is certainly some .private requires or
>flags missing, for expat, zlib and proj. And the -devel subpackage
>depends certainly on zlib-devel, expat-devel and proj-devel.
updated.

>There is a scary amount of compiler warnings most of them seem
>serious. Maybe you could try to work patches for upstream?
Well, yes. I would like to clean it up closest to no warrnings,
but not right now, it will take time anyway.

>There is a Requires on pkgconfig for -devel missing.
added.

>It seems to be a matter of style, but the template defattr is
>%defattr(-,root,root,-)
hmm, ok :-)

>If there are some examples, maybe their codes could be in %doc in
>-devel?
added those olso.

can re-review ?
Comment 18 Patrice Dumas 2007-02-10 08:42:02 EST
The .pc changes are wrong. Here is something that seems to
be right to me:

Libs.private: -lproj -lexpat -lz


libecs_tcl.so.1 is still in the main package. Maybe you could avoid that
by having in %files, instead of %{_libdir}/lib[^l]*.so.*:

%{_libdir}/libogdi*.so.*

The makefiles should certainly be removed from examples shipped
in %doc. When doing that, make sure that --short-circuit works.

As a suggestion, the removal of .cvsignore could also be in %setup,
those files shouldn't have been shipped in the first place.

Comment 19 Patrice Dumas 2007-02-10 08:56:22 EST
Created attachment 147834 [details]
use INST_LIB instead of hardcoding MODULES_PATH

This is a proposed replacement for ogdi-driversdir.patch. It uses
INST_LIB to set MODULES_PATH instead of hardcoding.

It applies with -p1.

This also means that INST_LIB should be set to the right value when 
compiling, I think that it means that make, in the .spec should
be called like

make INST_LIB=%{_libdir}/ogdi

This would be more acceptable for upstream.
Comment 20 Patrice Dumas 2007-02-10 09:31:41 EST
I think that the soname patch should be dropped for 2 reasons

* for dlopened libs it add a soname, it is wrong
* if upstream adds a shlib it may use another soname and then
  you are in big trouble. 

Maybe what could be done, besides working with upstream such that
they include sanely a shared library ogdi public libraries,
would be to keep the dlopened objects as-is in %_libdir/ogdi,
remove libogdi.so (and libogdi31.so?) and keep only static libraries
in -devel in %_libdir, and link statically against ogdi.

It also seems to me that te tcl interface object file is dlopened and
should be loaded. So it doesn't needs to be in %_libdir, nor have
a soname. It may be left in %_libdir/ogdi, or moved to the appropriate 
tcl/tk directory.
Comment 21 Michael Schwendt 2007-02-10 13:43:48 EST
The inlined pkgconfig file is broken. In its "Requires" line it
creates a dependency on non-existant pkgconfig template files.
You cannot put RPM package names there. Be aware that for an
installed system, a broken pkgconfig dependency-chain breaks
many pkg-config queries globally. The damage then is not local
to this ogdi package.
Comment 22 Balint Cristian 2007-02-10 22:36:28 EST
(In reply to comment #18)
> The .pc changes are wrong. Here is something that seems to
> be right to me:
> 
> Libs.private: -lproj -lexpat -lz

added.
sorry was confused when told me first.

> 
> 
> libecs_tcl.so.1 is still in the main package. Maybe you could avoid that
> by having in %files, instead of %{_libdir}/lib[^l]*.so.*:
> 
> %{_libdir}/libogdi*.so.*

excluded.
 
> The makefiles should certainly be removed from examples shipped
> in %doc. When doing that, make sure that --short-circuit works.

its done, short-circuiting install works fine, olso build works.

> 
> As a suggestion, the removal of .cvsignore could also be in %setup,
> those files shouldn't have been shipped in the first place.

moved in setup, olso added some tab checking lookups, and UTF8 converters.

> 
> 

i come up later , thinking how to solve soname.
Comment 23 Balint Cristian 2007-02-11 12:51:16 EST
  I get rid of _all_ warrnings from the code, and wait for author reviews.
It pretty large of cleanup patch ~182k, there was some pretty serious 
issues too.

 Well, i am in wait now, olso proposed a soname versioning for the 
author.
Comment 24 Balint Cristian 2007-02-12 05:54:40 EST
preliminary update.
Spec: http://openrisc.rdsor.ro/ogdi.spec
SRPMS: http://openrisc.rdsor.ro/ogdi-3.1.5-5.src.rpm

Preliminary fix:
a) zero compile warrn
b) do the right thing with dlopen. (re-work)
c) fix --target=i386 build.
d) test program functionality.

Working with maintainer to get in:
0) cleanup all sources: a. UTF8 b. remuve junks c. dos2unix d. chmod fix.
1) cleanup patch
2) soname patch (i proposed to him a final one)
3) dlopen patch (reworked, and proposed final one)
4) hide plugins by default into  /usr/lib/ogdi folder.
Comment 25 Patrice Dumas 2007-02-12 07:34:37 EST
(In reply to comment #24)
> preliminary update.
> Spec: http://openrisc.rdsor.ro/ogdi.spec
> SRPMS: http://openrisc.rdsor.ro/ogdi-3.1.5-5.src.rpm

it is
http://openrisc.rdsor.ro/ogdi-3.1.5-6.src.rpm

> Preliminary fix:
> a) zero compile warrn
> b) do the right thing with dlopen. (re-work)
> c) fix --target=i386 build.
> d) test program functionality.
> 
> Working with maintainer to get in:
> 0) cleanup all sources: a. UTF8 b. remuve junks c. dos2unix d. chmod fix.

Having stuff not in UTF-8 isn't a bug. It is upstream choice. Same
may be true or not for dos2unix, it depends on the precise situation.

> 1) cleanup patch
> 2) soname patch (i proposed to him a final one)
> 3) dlopen patch (reworked, and proposed final one)
> 4) hide plugins by default into  /usr/lib/ogdi folder.

You should also give them the pkgconfig file. For that you could send them
A file named ogdi.pc.in with something along:
 
prefix=@prefix@
exec_prefix=@prefix@
libdir=@libdir@
includedir=@includedir@

Name: @PACKAGE@
Description: Open Geographic Datastore Interface
Version: @VERSION@
Cflags: -I${includedir}/@PACKAGE@
Libs: -L${libdir} -logdi
Libs.private: -lproj -lexpat -lz


And you should add it to the files generatd by configure.ac,
and also maybe add a installation target, or let it to the
packager/end user. 

Also you could also propose the projects include patch to upstream.
Comment 26 Balint Cristian 2007-02-12 09:20:09 EST
+soname proposal, final.

Spec: http://openrisc.rdsor.ro/ogdi.spec
SRPMS: http://openrisc.rdsor.ro/ogdi-3.1.5-7.src.rpm

  I think this is the final shape for now, including all patches that need 
to be upstream.

Comment 27 Balint Cristian 2007-02-12 10:27:15 EST
 Ok you can review now this -7 ?

  Those patches will be merged soon as win32 build is proven olso, agreed with 
the maintainer and actualy i became co-maintainer :-)

/cristian

Comment 28 Ralf Corsepius 2007-02-12 11:05:53 EST
comment #10 is still valid.
Comment 29 Balint Cristian 2007-02-12 11:12:56 EST
updated. ver -8, same URL.

BTW, merged half of patches mainline.
/cristian
Comment 30 Balint Cristian 2007-02-12 11:26:13 EST
> Having stuff not in UTF-8 isn't a bug. It is upstream choice. Same
> may be true or not for dos2unix, it depends on the precise situation.

 It will be not the case. unfortunately he port olso for win32 using old 
tools so dos2unix and UTF8 will not be merged.
Comment 31 Balint Cristian 2007-02-12 11:53:14 EST
SRPMS reloaded.
was corrupt on ftp, please update.

~cristian
Comment 32 Balint Cristian 2007-02-12 20:24:27 EST
ogdi 3.1.6 released.
http://sourceforge.net/project/showfiles.php?group_id=11181

Spec: http://openrisc.rdsor.ro/ogdi.spec
SRPMS: http://openrisc.rdsor.ro/ogdi-3.1.6-1.src.rpm

- new upstream version.
- drop all patches, now they are upstream.
- remove useless source code cleanup from spec.
- pkgconfig is now autogenerated.
Comment 33 Patrice Dumas 2007-02-13 07:35:59 EST
* in %files, there is no need of * for odbc, so
%{_libdir}/%{name}/liblodbc.so*
should be replaced with
%{_libdir}/%{name}/liblodbc.so

* no need to duplicate the documentation in all the subpackages.
LICENSE NEWS ChangeLog README
could be only in the main package.

* HOWTO-RELEASE isn't usefull in the fedora package

* The following is useless and should be removed:
%post tcl -p /sbin/ldconfig
%postun tcl -p /sbin/ldconfig

* there is a license issue for vpflib. In LICENSE, there is:

vpflib/*:

No license mentioned, public domain?



However some files in vpflib have an author, which means a copyright
owner. Without license they are under the default license which is
a restrictive license (no redistribution, no modification).

* there is also a license issue for ogdi/c-api/gmath.c:

ogdi/c-api/gmath.c is:
   Derived from Numerical methods in C



Looking at this book it seems that the codes are under a proprietary
license.


Suggestions:

* The timestamps of source file aren't the same that those 
  spectool -g gets (but otherwise source match upstream)

* use
%defattr(-,root,root,-)
instead of
%defattr(-,root,root)
Comment 34 Balint Cristian 2007-02-13 08:10:33 EST
(In reply to comment #33)
> * in %files, there is no need of * for odbc, so
> %{_libdir}/%{name}/liblodbc.so*
> should be replaced with
> %{_libdir}/%{name}/liblodbc.so

fixed.

> 
> * no need to duplicate the documentation in all the subpackages.
> LICENSE NEWS ChangeLog README
> could be only in the main package.

fixed, leaved only README in the odbc/tcl package (at last need something).

 
> * HOWTO-RELEASE isn't usefull in the fedora package
get rid
> * The following is useless and should be removed:
> %post tcl -p /sbin/ldconfig
> %postun tcl -p /sbin/ldconfig

oh yes. Those are now private libs.

> * there is a license issue for vpflib. In LICENSE, there is:
> 
> vpflib/*:
> 
> No license mentioned, public domain?
> 
> 
> 
> However some files in vpflib have an author, which means a copyright
> owner. Without license they are under the default license which is
> a restrictive license (no redistribution, no modification).
> 
> * there is also a license issue for ogdi/c-api/gmath.c:
> 
> ogdi/c-api/gmath.c is:
>    Derived from Numerical methods in C

> 
> Looking at this book it seems that the codes are under a proprietary
> license.
> * there is a license issue for vpflib. In LICENSE, there is:
>vpflib/*:
>No license mentioned, public domain?

>However some files in vpflib have an author, which means a copyright
>owner. Without license they are under the default license which is
>a restrictive license (no redistribution, no modification).

I try contact tham and sort this out.

>* there is also a license issue for ogdi/c-api/gmath.c:
>ogdi/c-api/gmath.c is:
>   Derived from Numerical methods in C

Is this insuficcient (from .c header):

Looking at this book it seems that the codes are under a proprietary
******************************************************************************
 * Derived from Numerical methods in C.
 *
 * Copyright (C) 1995 Logiciels et Applications Scientifiques (L.A.S.) Inc
 * Permission to use, copy, modify and distribute this software and
 * its documentation for any purpose and without fee is hereby granted,
 * provided that the above copyright notice appear in all copies, that
 * both the copyright notice and this permission notice appear in
 * supporting documentation, and that the name of L.A.S. Inc not be used
 * in advertising or publicity pertaining to distribution of the software
 * without specific, written prior permission. L.A.S. Inc. makes no
 * representations about the suitability of this software for any purpose.
 * It is provided "as is" without express or implied warranty.
 
******************************************************************************

> 
> Suggestions:
> 
> * The timestamps of source file aren't the same that those 
>   spectool -g gets (but otherwise source match upstream)
> * use
> %defattr(-,root,root,-)
> instead of
> %defattr(-,root,root)

fixed. 

-2 updated.
Comment 35 Patrice Dumas 2007-02-13 08:27:22 EST
(In reply to comment #34)

> fixed, leaved only README in the odbc/tcl package (at last need something).

If there is nothing specific for these sub-packages, the right thing is
not to have any documentation. There will be a rpmling warning but it 
can be ignored.
 

> Is this insuficcient (from .c header):

Yes, it is insufficient since it covers only the new code.

> Looking at this book it seems that the codes are under a proprietary
> ******************************************************************************
>  * Derived from Numerical methods in C.
>  *
>  * Copyright (C) 1995 Logiciels et Applications Scientifiques (L.A.S.) Inc
>  * Permission to use, copy, modify and distribute this software and
>  * its documentation for any purpose and without fee is hereby granted,
>  * provided that the above copyright notice appear in all copies, that
>  * both the copyright notice and this permission notice appear in
>  * supporting documentation, and that the name of L.A.S. Inc not be used
>  * in advertising or publicity pertaining to distribution of the software
>  * without specific, written prior permission. L.A.S. Inc. makes no
>  * representations about the suitability of this software for any purpose.
>  * It is provided "as is" without express or implied warranty.
>  
> ******************************************************************************
Comment 36 Mamoru TASAKA 2007-02-13 08:49:17 EST
(In reply to comment #34)
> Looking at this book it seems that the codes are under a proprietary
> ******************************************************************************
>  * Derived from Numerical methods in C.
>  *
>  * Copyright (C) 1995 Logiciels et Applications Scientifiques (L.A.S.) Inc
>  * Permission to use, copy, modify and distribute this software and
>  * its documentation for any purpose and without fee is hereby granted,
>  * provided that the above copyright notice appear in all copies, that
>  * both the copyright notice and this permission notice appear in
>  * supporting documentation, and that the name of L.A.S. Inc not be used
>  * in advertising or publicity pertaining to distribution of the software
>  * without specific, written prior permission. L.A.S. Inc. makes no
>  * representations about the suitability of this software for any purpose.
>  * It is provided "as is" without express or implied warranty.
>  
> ******************************************************************************

This is "Standard ML of New Jersey Copyright License" and
GNU says this is "permissive non-copyleft free software license, 
compatible with the GNU GPL"
Comment 37 Balint Cristian 2007-02-13 09:05:50 EST
 
******************************************************************************
> >  * Derived from Numerical methods in C.
> >  *
> >  * Copyright (C) 1995 Logiciels et Applications Scientifiques (L.A.S.) Inc
> >  * Permission to use, copy, modify and distribute this software and
> >  * its documentation for any purpose and without fee is hereby granted,
> >  * provided that the above copyright notice appear in all copies, that
> >  * both the copyright notice and this permission notice appear in
> >  * supporting documentation, and that the name of L.A.S. Inc not be used
> >  * in advertising or publicity pertaining to distribution of the software
> >  * without specific, written prior permission. L.A.S. Inc. makes no
> >  * representations about the suitability of this software for any purpose.
> >  * It is provided "as is" without express or implied warranty.
> >  
> > 
******************************************************************************
> 
> This is "Standard ML of New Jersey Copyright License" and
> GNU says this is "permissive non-copyleft free software license, 
> compatible with the GNU GPL"

So to understand this part is basicaly OK ?

Regarding vrf i called the author to help sort out issue, i waiting for his 
response.

Comment 38 Balint Cristian 2007-02-13 09:09:43 EST
> If there is nothing specific for these sub-packages, the right thing is
> not to have any documentation. There will be a rpmling warning but it 
> can be ignored.

Ah, ok :-)

In this case i removed.
Updated to -3
Comment 39 Patrice Dumas 2007-02-13 09:14:07 EST
(In reply to comment #37)
>  
> > This is "Standard ML of New Jersey Copyright License" and
> > GNU says this is "permissive non-copyleft free software license, 
> > compatible with the GNU GPL"

Indeed, but it doesn't cover the original code which is from a book.

> So to understand this part is basicaly OK ?

No, I think it is not OK. See also in the C file the comment. It is 
said that the code is:

   Derived from Numerical methods in C

but it seems to me that this is illegal given the license of 
Numerical methods in C.
Comment 40 Balint Cristian 2007-02-13 09:22:33 EST
> > > This is "Standard ML of New Jersey Copyright License" and
> > > GNU says this is "permissive non-copyleft free software license, 
> > > compatible with the GNU GPL"
> 
> Indeed, but it doesn't cover the original code which is from a book.
> 
> > So to understand this part is basicaly OK ?
> 
> No, I think it is not OK. See also in the C file the comment. It is 
> said that the code is:
> 
>    Derived from Numerical methods in C
> 
> but it seems to me that this is illegal given the license of 
> Numerical methods in C.

Hmm, have idea how to sort out ?
I have no idea :-(

Comment 41 Patrice Dumas 2007-02-13 09:25:50 EST
(In reply to comment #40)

> Hmm, have idea how to sort out ?
> I have no idea :-(

Ask upstream for clarification? This kind of issue is certainly 
best solved by upstream.
Comment 42 Balint Cristian 2007-02-13 09:34:31 EST
(In reply to comment #41)
> (In reply to comment #40)
> 
> > Hmm, have idea how to sort out ?
> > I have no idea :-(
> 
> Ask upstream for clarification? This kind of issue is certainly 
> best solved by upstream.

Looks on google there are some projects that use same looking code under GPL
http://www.google.com/codesearch?q=copy_dmatrix&hl=en&btnG=Search+Code

What do you think ?
Should i replace with this with pices from other GPL project ?

Are those legal ?
Comment 43 Patrice Dumas 2007-02-13 09:38:42 EST
(In reply to comment #42)

> Looks on google there are some projects that use same looking code under GPL
> http://www.google.com/codesearch?q=copy_dmatrix&hl=en&btnG=Search+Code
> 
> What do you think ?
> Should i replace with this with pices from other GPL project ?

I don't think so, since ogdi is basically BSD. The replacements should
either be BSD-like or public domain.

> Are those legal ?

It depends on the precise case. It is legal to reimplement the same
interface, not to reuse the code under another license.
Comment 44 Mamoru TASAKA 2007-02-13 09:44:16 EST
Well, I have not yet checked the code of ogdi actually (sorry
I will be busy for today and tommorrow), however including
numerical recipe C code seems a problem.

From http://www.numerical-recipes.com/infotop.html#distinfo
---------------------------------------------------------
#  You want to distribute, noncommercially and free on the internet, an
application that uses NR routines. You need to distribute source code, so that
your application can be recompiled on different machines. Can you include
Numerical Recipes routines as part of that source code, including a notice that
they are only allowed to be used with your application?

    * Sorry, no. We never give permission for Numerical Recipes source code to
be posted on any public server, or distributed with any freeware or shareware
package. If you encounter such a distribution, we'd be grateful if you'd tell us
about it. There are good freely redistributable numerical libraries on Netlib
that can be used, instead of Numerical Recipes, in such cases. The Numerical
Recipes Multi-Language Code CDROM includes the entire freely redistributable
SLATEC library, for this kind of use.
--------------------------------------------------------------
Comment 45 Patrice Dumas 2007-02-13 09:51:58 EST
It doesn't seems to be the same book, but the redistributions
conditions are the same.
Comment 46 Balint Cristian 2007-02-13 10:04:57 EST
From a conversation with author:

> vpflib was written by ESRI (www.esri.com) for the department of defence.

(those guys with guns and tanks ! *just joking* )

> I believe we will need to dig up a public software release from DOD
> (I can't quite remember the package that includes vpflib) to confirm
> that it is completely in the public domain including vpflib.

    I will look forward over this issues. Will try my best to escaladate.
Comment 47 Balint Cristian 2007-02-14 09:34:04 EST
1) Sorted out vpflib license.

Look at this project (http://www.ivtools.org/vhclmaps/), they used the _same_ 
vpfutil whatever we call in ogdi vpflib but have a good explonation of the 
sources:
From their LICENSE file:
-----------------------/////////---------------->
The VpfUtil library contains the public domain VPFVIEW C code included
with the Digital Chart of the World, Edition 1, which was pruned and ported
to work on "POSIX"-like systems.

The line-of-sight and viewshed capability was borrowed and retrofitted
from the DMA (NIMA) MUSE2 package.
--------------///////-------------------------------------<
From Nima license site:

http://earth-info.nga.mil/geospatial/SW_TOOLS/NIMAMUSE/dist/VPFViewdist.html
Is that OK ?


2)Sorted out to replace "numeric C recipes"-'s  ultimate wisdom !

Err, what  ogdi use from that crappy gmath.c:

-2D matrix mult.
-2D matrix invert.

Rest of gmath.c is unused, all functions are just declared but unused.
'blas' or 'lapack' project has functions for matrix 2d manipulation, so i try 
to come up with a replacemant in ogdi, but it will imply to add blas to build 
requirement,so i will try escaladete the final replacement patch until 
confgure.in olso.

c'mmon, its not so hard to mult 2 matrices especialy in 2D without floating or 
whatever super precision,and to invert matrix is piece of cake.

http://www.google.com/codesearch?q=multiply+matrices&hl=en&btnG=Search+Code
Look at grass one which link to 'blas','lapack'.


Guys,

What do you think ? 
Make sense go for it ? 
Comment 48 Balint Cristian 2007-02-14 10:48:21 EST
Source Code Disclaimer

 
 1. The VPFView source code ("the software") is provided free of charge via 
the Internet by the National Imagery and Mapping Agency (NIMA) of the United 
States Department of Defense. Although NIMA makes no copyright claim under 
Title 17 U.S.C., NIMA claims copyrights in the source code under other legal 
regimes. NIMA hereby grants to each user of the software a license to use and 
distribute the software, and develop derivative works.
Comment 49 Rick Niles 2007-02-14 11:16:01 EST
I'm so sorry I've been out of touch, I give approval to whomever wants to take
the ball on this.  I just want to get it done.  I really appreciate all the effort.

Rick.
Comment 50 Patrice Dumas 2007-02-14 11:22:50 EST
The vpflib license issue seems to be solved, please include all
the documentation you found in a file to explain the issue, where
you found the information and so on. Maybe even better would be 
to propose a patch for the LICENSE file for upstream.


For gmath.c, using blas/lapack seems fine to me. I guess that
the gsl could also be used instead. Use whatever you prefer.
Comment 51 Balint Cristian 2007-02-14 12:43:57 EST
I will update mainstream.
Olso need to get rid of "math recipies", i will use lapack for this.

  Let me breath, and in max 2 days i come up with final 3.2.0beta release.
BTW, i fixed mainstream to be able run on 64bit platform too, x86_64 was 
broken, last night i merge a patch the fix it forever ;-), and again over the 
top i got zero gcc warnings despite the factmaintainer request to took breath 
into win32 port too ;-) It was pain in the ass, but its done.

So, for tomorrow i got new tasks.
Comment 52 Balint Cristian 2007-02-19 06:30:18 EST
  I got rid of math recipes in the weekend, i re-wrote tham.

   Discuss now the merge of code into CVS, (i inquiry author what license tag 
want he to see for these pieces, thats the only issue now)
  Tomorrow i will re-join ogdi+gdal+grass with full force :-), today i have to 
run random places to manage personal stuff.

 ogdi - will contain a singe patch wich solve 64bit issue+gmath.c 
issue,basicaly as a difference between CVS and 3.1.6 version. Soon 3.2.0beta 
will come and patch will be dropped, but maintainer insist to wait with CVS 
until some bugs will be found.

 gdal - still have r-path problem (i will remove it, and retest within mock).

 grass - pretty unreviewed, if have the time take a look into it, i am sure 
there you will find lists of issues, its pretty _big_ beast, but it will be 
_great_ to see it in -extras



  
Comment 53 Balint Cristian 2007-02-19 15:31:50 EST
Attached new version with license sorted out.

It cointain olso 2 splitted patches from latest CVS, one is 64bit fix
second is gmath replacement with a new GPLv2 one.

Spec: http://openrisc.rdsor.ro/ogdi.spec
SRPMS: http://openrisc.rdsor.ro/ogdi-3.1.6-4.src.rpm

can review ?
Comment 54 Balint Cristian 2007-02-21 10:38:25 EST
no review ?
Comment 55 Balint Cristian 2007-02-21 14:49:21 EST
sorry mistake new license is not GPLv2 is Public Domain, as maintainer 
requested to do.
Comment 56 John Mahowald 2007-02-22 23:17:41 EST
Who is this assigned to now?

You mean the gmath replacement is public domain? LICENSE in the 3.1.6-4 sources
says GPL 2.
Comment 57 Balint Cristian 2007-02-23 07:10:50 EST
(In reply to comment #56)
> Who is this assigned to now?

You can re-pick up if want. Perseus was reviewing it intesive last time,
but if want you can re-pick it up. I just want get this into -extras, 
i really put lot off eforts to clean up upstream version.

> 
> You mean the gmath replacement is public domain? LICENSE in the 3.1.6-4 
sources
> says GPL 2.

gmath.c,.h are gone forever !

matrix.c Its not GPL2 its Public Domain. (look in ogdi-matrix.patch derivated 
from CVS version of ogdi) That was the wish of maintainer to be Public Domain
instead GPLv2

Even mainstream gmath.c..g is gone forever.

Spec: http://openrisc.rdsor.ro/ogdi.spec
SRPMS: http://openrisc.rdsor.ro/ogdi-3.1.6-5.src.rpm

Comment 58 Patrice Dumas 2007-02-23 08:58:59 EST
I hadn't time to review ogdi that week, maybe this week-end, but if I 
remember well, for me, the license issues where the only remaining 
issues.
Comment 59 Patrice Dumas 2007-02-24 11:18:49 EST
* devel package should require
Requires:       %{name} = %{version}-%{release}

but should not require %{name}-tcl.

* the license issues have been solved, but the package cannot be 
accepted as is. Indeed the code copyrighted by the book editor
is still in the srpm. I would have accepted similar issue if the 
license issue was between, say 2 authors of the software unknowingly
using incompatible licenses. But here there is a third party
which may sue fedora if we include that code, I think that is 
a risk we shouldn't take. 

To avoid that issue, you can either
 - use a release that doesn't contain the problematic code
 - or modify sources prior from doing the srpm by removing the
   problematic code. In that case you should provide a script 
   that allows to recreate the tarball you ship with the original 
   tarball. You can have a look at what I did for grads or the cernlib
   if you want examples.


I don't see any other issue.
Comment 60 Balint Cristian 2007-02-24 11:37:13 EST
   Yes. i thinked that it can be an issue.

solution:

   Ok, within 1 hour i go release 3.2.0beta tarball, anyway this was approved 
by Frank the maintainer ;-) 
   It will olso contain a fix of malloc by google, i defenatly give a shot
for google's fix proposal, than i pack 3.2.0 ;-)

/cristian
Comment 61 Balint Cristian 2007-02-24 12:57:46 EST
updated from mainstream.
Spec: http://openrisc.rdsor.ro/ogdi.spec
SRPMS: http://openrisc.rdsor.ro/ogdi-3.2.0.beta1-1.src.rpm

;-)
Comment 62 Patrice Dumas 2007-02-24 13:38:21 EST
The version-release is wrong. It should be along

Version: 3.2.0
Release: 0.1.beta1

http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-d97a3f40b6dd9d2288206ac9bd8f1bf9b791b22a
Comment 63 Patrice Dumas 2007-02-24 13:43:20 EST
Also 
* devel package should require
Requires:       %{name} = %{version}-%{release}

but should not require %{name}-tcl.

Comment 64 Balint Cristian 2007-02-24 14:01:11 EST
updated.

Spec: http://openrisc.rdsor.ro/ogdi.spec
SRPMS: http://openrisc.rdsor.ro/ogdi-3.2.0-0.2.beta1.src.rpm
Comment 65 Patrice Dumas 2007-02-24 14:21:10 EST
in main package %files, there is no need of * in
%exclude %{_libdir}/%{name}/liblodbc.so*

Otherwise
* rpmlint output ignorable
W: ogdi-odbc no-documentation
W: ogdi-tcl no-documentation
* follow packaging and naming guidelines
* license acceptable, with a license file summarizing the conditions 
  for the source files
* sane provides (I consider those .so provides bogus, but that's common)
Provides: libadrg.so libdtcanada.so libdted.so libdtusa.so libgdal.so
libogdi.so.3 libremote.so librpf.so libskeleton.so libvrf.so
Provides: liblodbc.so
Provides: libecs_tcl.so
* match upstream:
193da3f154985d37bb5aaa886e78f650  ogdi-3.2.0.beta1.tar.gz
029a8cdcd36bee73df92196ee769040e  ogdi.pdf
* library packaged rightly (no .la, devel stuff in -devel, right 
  Requires for -devel)
* %files section right

APPROVED


The source file timestamps are not the same than what I get with
spectool -g. I get

Feb 24 18:48 ogdi-3.2.0.beta1.tar.gz
Nov  3  2000 ogdi.pdf

while in the SRPM, there is
Feb 24 19:00 ../SOURCES/ogdi-3.2.0.beta1.tar.gz
Nov 27  2003 ../SOURCES/ogdi.pdf



Do you need to be sponsored?
Comment 66 Balint Cristian 2007-02-24 14:32:55 EST
reuploaded the same package with correct timestamp.

yes i need.
Comment 67 Patrice Dumas 2007-02-24 19:44:07 EST
Ok, I'll sponsor you.
Comment 68 Balint Cristian 2007-02-26 14:27:22 EST
Need to do more things from my side ? (i ask, really dont know the further 
procedure, must stick with some flags in in this bz ?!)

BTW, i re-uploaded the package with right timestamp + that small '*' removed,
but not increased -ver.

Anyway must thank you a lot for the very good review over the package !
Really impressive fidelity !
Comment 69 Patrice Dumas 2007-02-26 14:32:41 EST
Now you should follow
http://fedoraproject.org/wiki/Extras/Contributors
You are already at 
http://fedoraproject.org/wiki/Extras/Contributors#head-bb3314e7b80fd98f037edd46f6d1efafbb611752

(but I suggest that you reread the whole document and follow
the links in case you didn't do that already).
Comment 70 Balint Cristian 2007-02-27 06:38:29 EST
New Package CVS Request
=======================
Package Name: ogdi
Short Description: Open Geographic Datastore Interface
Owners: cbalint@redhat.com
Branches: FC-5 FC-6 devel
InitialCC: 
Comment 71 Patrice Dumas 2007-02-27 06:46:23 EST
I haven't seen your name appearing in the list of members to
be sponsored. Did I miss something?
Comment 72 Balint Cristian 2007-02-27 06:50:21 EST
i did it a bit later.
Can re-check now ?
Comment 73 Patrice Dumas 2007-02-27 06:57:36 EST
You should be sponsored now. I don't know how much time it takes
to propagate, and unless I'm wrong there is also some action to be
taken to create the branch and add you in owners.list. 
Comment 74 Mamoru TASAKA 2007-03-02 10:24:04 EST
Balint, please close this bug when rebuilding is done
(perhaps now you can close this).

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