Bug 205955 - Review Request: gdal - A translator library for raster geospatial data formats
Review Request: gdal - A translator library for raster geospatial data formats
Status: CLOSED DUPLICATE of bug 222042
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
: Reopened
: 168719 (view as bug list)
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2006-09-10 14:58 EDT by Joost Soeterbroek
Modified: 2007-11-30 17:11 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-02-07 00:26:52 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Patch to install binary, not corrupted shell script (369 bytes, patch)
2006-09-12 15:15 EDT, Mamoru TASAKA
no flags Details | Diff
gdal.spec I used (9.45 KB, text/plain)
2006-09-12 15:18 EDT, Mamoru TASAKA
no flags Details
Patch to install binary, not corrupted shell script (modified) (835 bytes, patch)
2006-09-12 15:31 EDT, Mamoru TASAKA
no flags Details | Diff
gdal.spec I used (modified) (9.56 KB, text/plain)
2006-09-12 15:32 EDT, Mamoru TASAKA
no flags Details
support for libdap and fix for hdf (810 bytes, patch)
2006-09-14 11:52 EDT, Patrice Dumas
no flags Details | Diff
gdal.spec I used (9.76 KB, text/plain)
2006-09-14 13:55 EDT, Mamoru TASAKA
no flags Details
fix hdf link, and add support for libdap for FC5 and devel (1.10 KB, text/plain)
2006-09-14 15:46 EDT, Patrice Dumas
no flags Details

  None (edit)
Description Joost Soeterbroek 2006-09-10 14:58:48 EDT
Spec URL: http://www.soeterbroek.com/linux/fedora/extras/gdal/gdal.spec
SRPM URL: http://www.soeterbroek.com/linux/fedora/extras/gdal/gdal-1.3.2-1.src.rpm
Description: GDAL is a translator library for raster geospatial data formats. As a library, 
it presents a single abstract data model to the calling application for all
supported formats. The related OGR library (which lives within the GDAL source
tree) provides a similar capability for simple features vector data.

Some additional information:
============================
Picking up where a previous orphaned review request left of (see https://bugzilla.fedora.us/show_bug.cgi?id=168719 for history):

* Sun Sep 10 2006 Joost Soeterbroek <fedora@soeterbroek.com> 1.3.2-1
- new upstream version 1.3.2
- excluded *.pyc and *.pyo files
- removed patch2, fixed upstream in gdal 1.3.2
- removed ChangeLog, no longer present in from source
- add $RPM_OPT_FLAGS to %configure
- moved man1/pct2rgb.1.gz, man1/rgb2pct.1.gz to *-python
Comment 1 Mamoru TASAKA 2006-09-11 04:03:56 EDT
Okay. I will review this package.
First review:

1. From http://fedoraproject.org/wiki/Packaging/Guidelines :

* License:
  - Well, how can I know that this package can be distributed under
    MIT? It seems that main package does not include any licence
    document.

* rpmlint:
  - is not silent.
W: gdal macro-in-%changelog configure
W: gdal macro-in-%changelog configure
W: gdal mixed-use-of-spaces-and-tabs
  - Use %% in changelog so as the description in changelog is not
    expanded.
  - if it is difficult finding where tabs are used, use
    "sed -i -e 's|\t|  |g' gdal.spec", which automatically changes
    tabs in spec file into two spaces.

* Tags:
  - use %?dist tag

* Requires:
  - -python package requires python-numeric as gdal_merge.py includes
    the line: "import Numeric" (this dependency cannot be automatically
    checked).

* BuildRequires:
  - libjpeg-devel, zlib-devel, netcdf-devel <- required by hdf-devel
    (ditto requires for -devel package)

* Encoding
  - Several text files are encoded in ISO-8859. Change the encoding
    to UTF-8 unless it is necessary.
  - ./gdal-1.3.2-1/usr/share/gdal/seed_2d.dgn: Microstation
    ./gdal-1.3.2-1/usr/share/gdal/seed_3d.dgn: data
    Well, what are these files?

* Why the %makeinstall macro should not be used
  - Don't use %makeinstall

* Timestamps
  - These (gdal) package contains lots of text files, so keeping
    timestamps is preferable. Keep timestamps of those files (usually
    'make install' accepts the option 'INSTALL="install -c -p"'.

* File and Directory Ownership
  - Don't own the directory %python_sitearch itself.

2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines

* MUST: The sources used to build the package must match....
  - Specify the URL of the source. I cannot verify is the source is
    correct.

* MUST: Packages must NOT contain any .la libtool archives,  
  - /usr/lib/python2.4/site-packages/_gdalmodule.la - should be removed.

* SHOULD: The reviewer should test that the package functions as described
  - I have not yet checked if these packages functionally work well
    as desired. Please give me some simple examples if you can.


3. Other things I have noticed:
* Packaging issue:
  - in %prep stage:
    - %patch0 -p0: it is preferable that the suffix for original files is
      specified.
    - cd %{_builddir}/%{name}-%{version} is not necessary. The working
      directory at this stage is it.

  - in file entry:
    - Don't use %exclude unless there is no way to avoid 
      using %exclude (such as %exclude %{_bindir}/*.pyc)
      as it makes file entry somewhat complicate.
      For example, specify %{_bindir}, %{_mandir}/man1/
      entry so as not to use %exclude.
      Especially:
      - Explicitly REMOVE the files which are not included in any
        packages in %install stage (such as static archive)
     - I doubt that -devel package should have html directory.
       Owning files under html directory is sufficient. i.e:
       I suspect that %doc html should be %doc html/*
Comment 2 Ralf Corsepius 2006-09-12 09:27:12 EDT
(In reply to comment #1)
> Okay. I will review this package.
> First review:
> 
> 1. From http://fedoraproject.org/wiki/Packaging/Guidelines :
> 
> * License:
>   - Well, how can I know that this package can be distributed under
>     MIT? It seems that main package does not include any licence
>     document.
By checking/examining the source files - Only them are legally binding.
Comment 3 Joost Soeterbroek 2006-09-12 11:30:00 EDT
(In reply to comment #1)

> Okay. I will review this package.

Thank you for reviewing.
I have fixed most, but not all, issues (see replies below for details):

-----------------------------------------
Spec URL: http://www.soeterbroek.com/linux/fedora/extras/gdal/gdal.spec
SRPM URL:
http://www.soeterbroek.com/linux/fedora/extras/gdal/gdal-1.3.2-2.fc6.src.rpm

* Sun Sep 10 2006 Joost Soeterbroek <fedora@soeterbroek.com> 1.3.2-2
- change macros in changelog to prevent expansion
- use %%{?dist} tag
- change all tabs into two spaces
- remove python-abi req
- add python-numeric req to -python package
- drop %%makeinstall macro
- add timestamps to make install with option 'INSTALL="install -c -p"'
- drop %%{python_sitearch} from %%files python
- add full URL to Source
- exclude /usr/lib/python2.4/site-packages/_gdalmodule.la
- fixed misc. packaging issues
-----------------------------------------

> First review:
>
> 1. From http://fedoraproject.org/wiki/Packaging/Guidelines :
>
> * License:
>   - Well, how can I know that this package can be distributed under
>     MIT? It seems that main package does not include any licence
>     document.

see comment #2

> * rpmlint:
>   - is not silent.
> W: gdal macro-in-%changelog configure
> W: gdal macro-in-%changelog configure
>   - Use %% in changelog so as the description in changelog is not
>     expanded.

done

> W: gdal mixed-use-of-spaces-and-tabs
>   - if it is difficult finding where tabs are used, use
>     "sed -i -e 's|\t|  |g' gdal.spec", which automatically changes
>     tabs in spec file into two spaces.

done

> * Tags:
>   - use %?dist tag

done

> * Requires:
>   - -python package requires python-numeric as gdal_merge.py includes
>     the line: "import Numeric" (this dependency cannot be automatically
>     checked).

done

> * BuildRequires:
>   - libjpeg-devel, zlib-devel, netcdf-devel <- required by hdf-devel
>     (ditto requires for -devel package)

I am unsure what you mean with this one. Afaik all BuildRequires are allready
present in the spec file.

> * Encoding
>   - Several text files are encoded in ISO-8859. Change the encoding
>     to UTF-8 unless it is necessary.
>   - ./gdal-1.3.2-1/usr/share/gdal/seed_2d.dgn: Microstation
>     ./gdal-1.3.2-1/usr/share/gdal/seed_3d.dgn: data
>     Well, what are these files?

Could you give me some examples? I am not really clear on what I am supposed to
do here.

> * Why the %makeinstall macro should not be used
>   - Don't use %makeinstall

done

> * Timestamps
>   - These (gdal) package contains lots of text files, so keeping
>     timestamps is preferable. Keep timestamps of those files (usually
>     'make install' accepts the option 'INSTALL="install -c -p"'.

done

> * File and Directory Ownership
>   - Don't own the directory %python_sitearch itself.

done

> 2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
>
> * MUST: The sources used to build the package must match....
>   - Specify the URL of the source. I cannot verify is the source is
>     correct.

done

> * MUST: Packages must NOT contain any .la libtool archives,
>   - /usr/lib/python2.4/site-packages/_gdalmodule.la - should be removed.

done

> * SHOULD: The reviewer should test that the package functions as described
>   - I have not yet checked if these packages functionally work well
>     as desired. Please give me some simple examples if you can.

Ok, I'll play around with some of the example data files in
ftp://ftp.remotesensing.org/pub/gdal/data/. I will need some help from
(upstream) community though, in order to make some sense of input and (expected)
output data.



> 3. Other things I have noticed:
> * Packaging issue:
>   - in %prep stage:
>     - %patch0 -p0: it is preferable that the suffix for original files is
>       specified.

done

>     - cd %{_builddir}/%{name}-%{version} is not necessary. The working
>       directory at this stage is it.

done

>   - in file entry:
>     - Don't use %exclude unless there is no way to avoid
>       using %exclude (such as %exclude %{_bindir}/*.pyc)
>       as it makes file entry somewhat complicate.
>       For example, specify %{_bindir}, %{_mandir}/man1/
>       entry so as not to use %exclude.
>       Especially:
>       - Explicitly REMOVE the files which are not included in any
>         packages in %install stage (such as static archive)

done

>      - I doubt that -devel package should have html directory.
>        Owning files under html directory is sufficient. i.e:
>        I suspect that %doc html should be %doc html/*

done


One major issue I still have is with some programs in the *-bin package:

 $ gdalinfo
 mkdir: cannot create directory `/usr/bin/.libs': Permission denied
/usr/bin/ld: cannot open output file /usr/bin/.libs/22786-lt-gdalinfo: No such
file or directory
 collect2: ld returned 1 exit status
Comment 4 Mamoru TASAKA 2006-09-12 12:06:39 EDT
Before checking 1.3.2-2:

(In reply to comment #3)
> > * BuildRequires:
> >   - libjpeg-devel, zlib-devel, netcdf-devel <- required by hdf-devel
> >     (ditto requires for -devel package)
> 
> I am unsure what you mean with this one. Afaik all BuildRequires are allready
> present in the spec file.

I meant that: "libjpeg-devel, zlib-devel, netcdf-devel" is required
by hdf-devel, so adding "libjpeg-devel, zlib-devel, netcdf-devel" for
BuildRequires (or Requires on -devel package) is unnecessary because
hdf-devel pulls them anyway.

> 
> > * Encoding
> >   - Several text files are encoded in ISO-8859. Change the encoding
> >     to UTF-8 unless it is necessary.
> Could you give me some examples? 
Well,

gdal-1.3.2-1/usr/share/gdal/cubewerx_extra.wkt: ISO-8859 English text, with very
long lines
gdal-1.3.2-1/usr/share/gdal/s57expectedinput.csv: ISO-8859 English text
gdal-1.3.2-1/usr/share/gdal/prime_meridian.csv:  ISO-8859 text
gdal-debuginfo-1.3.2-1/usr/src/debug/gdal-1.3.2/frmts/rmf/rmfdataset.cpp:
ISO-8859 English text
gdal-devel-1.3.2-1/usr/include/gdal/cpl_win32ce_api.h: ISO-8859 English text
gdal-devel-1.3.2-1/usr/include/gdal/cpl_wince.h:ISO-8859 English text

Also, one file has wrong end-of-line encoding.
gdal-debuginfo-1.3.2-1/usr/src/debug/gdal-1.3.2/frmts/nitf/nitffile.c: ASCII
English text, with CRLF, LF line terminators

> >   - ./gdal-1.3.2-1/usr/share/gdal/seed_2d.dgn: Microstation
> >     ./gdal-1.3.2-1/usr/share/gdal/seed_3d.dgn: data
> >     Well, what are these files?

I simply want to know what these files are.

 done
> 
> One major issue I still have is with some programs in the *-bin package:
> 
>  $ gdalinfo
>  mkdir: cannot create directory `/usr/bin/.libs': Permission denied
> /usr/bin/ld: cannot open output file /usr/bin/.libs/22786-lt-gdalinfo: No such
> file or directory
>  collect2: ld returned 1 exit status
> 

I will check this after.
Comment 5 Mamoru TASAKA 2006-09-12 15:15:33 EDT
Created attachment 136100 [details]
Patch to install binary, not corrupted shell script

(In reply to comment #4)
> (In reply to comment #3)
> > 
> > One major issue I still have is with some programs in the *-bin package:
> > 
> >  $ gdalinfo
> >  mkdir: cannot create directory `/usr/bin/.libs': Permission denied
> > /usr/bin/ld: cannot open output file /usr/bin/.libs/22786-lt-gdalinfo: No
such
> > file or directory
> >  collect2: ld returned 1 exit status
> > 
> 
> I will check this after.
> 

Well, all the bash shell scripts under /usr/bin in -bin rpm are
corrupted. Check if this patch fixes this issue.
Comment 6 Mamoru TASAKA 2006-09-12 15:18:40 EDT
Created attachment 136101 [details]
gdal.spec I used

gdal.spec I used.

I have not checked this in detail, so please check my spec,
verify if the patch above work, and resubmit. Then I will
review it again.
Comment 7 Mamoru TASAKA 2006-09-12 15:31:18 EDT
Created attachment 136103 [details]
Patch to install binary, not corrupted shell script (modified)

Well, the previous patch is not sufficient.
Please use this patch and verify the result.
Comment 8 Mamoru TASAKA 2006-09-12 15:32:22 EDT
Created attachment 136104 [details]
gdal.spec I used (modified)

And the spec file I used (again modified)
Comment 9 Rudolf Kastl 2006-09-13 08:30:53 EDT
* cfitsio support could be enabled (builds and is available in extras).
* in my eyes libgeotiff should be compiled with external library (its currently 
under review).
* pc raster support is currently compiled with internal library (should it stay 
like that?)


Comment 10 Mamoru TASAKA 2006-09-13 10:25:09 EDT
Joost, please check my patch and spec file (in comment #7 and
in comment #8), add some needed change you think, and re-sumbit
spec and srpm. I will then re-check your spec.

At least:
%exclude %{_libdir}/*.la
%exclude %{_libdir}/*.a
%exclude %{_libdir}/python*/site-packages/*.a
%exclude %{_libdir}/python*/site-packages/*.la
Well, don't do these ways. Explicitly remove these files at %install stage.
i.e. add
rm -f %{buildroot}%{_libdir}/*.{a,la}
rm -f %{buildroot}%{_libdir}/python*/site-packages/*.{a,la}
to %install stage and don't use %exclude if it is possible.
( %exclude %{_bindir}/*.pyc
  %exclude %{_bindir}/*.pyo is needed anyway)

And check my previous comment (comment #4 and comment #1).
Also, I want to know your opinion against comment #9 (by Rudolf).

Comment 11 Joost Soeterbroek 2006-09-13 15:07:02 EDT
Spec URL: http://www.soeterbroek.com/linux/fedora/extras/gdal/gdal.spec
SRPM URL:
http://www.soeterbroek.com/linux/fedora/extras/gdal/gdal-1.3.2-3.fc6.src.rpm

* Wed Sep 13 2006 Joost Soeterbroek <fedora@soeterbroek.com> 1.3.2-3
- enable cfitsio support, add cfitsio-devel BuildRequires
- remove unwated files in %%install phase (rather than exclude in %%files)

* Tue Sep 12 2006 Mamoru Tasaka <mtasaka@ioa.s.u-tokyo.ac.jp> 1.3.2-2.3
- Fix corrupted shell scripts problem more.

* Tue Sep 12 2006 Mamoru Tasaka <mtasaka@ioa.s.u-tokyo.ac.jp> 1.3.2-2.2
- Fix BuildRequires, encoding, end-of-line encoding.
- Install binary in -bin rpm, not corrupted shell scripts.
Comment 12 Joost Soeterbroek 2006-09-13 15:08:42 EDT
(In reply to comment #9)
> * cfitsio support could be enabled (builds and is available in extras).

done in 1.3.2-3

> * in my eyes libgeotiff should be compiled with external library (its currently 
> under review).

I have no idea/preference either way

> * pc raster support is currently compiled with internal library (should it stay 
> like that?)

I have no idea/preference either way
Comment 13 Patrice Dumas 2006-09-13 15:51:17 EDT
(In reply to comment #12)
> (In reply to comment #9)

> I have no idea/preference either way

It is always very preferable to have external tools, it is very 
hard to fix bugs, and especially security bugs when internal
libraries are used. 

There are exception, when the internal library isn't really
an independent project, and when the internal library is patched. 
Comment 14 Rudolf Kastl 2006-09-13 17:41:00 EDT
well if the external library is completly forked alright... otherwise in my 
eyes those patches should rather be brought into the upstream library. just my 
opinion.
Comment 15 Joost Soeterbroek 2006-09-14 08:26:15 EDT
(In reply to comment #14)
> well if the external library is completly forked alright... otherwise in my 
> eyes those patches should rather be brought into the upstream library. just my 
> opinion.

I'll leave it as it is for the time being..

Comment 16 Mamoru TASAKA 2006-09-14 08:51:36 EDT
(In reply to comment #13 and comment #14)
Well, generally I agree with your comments. Joost, generally you must consider
using external libraries when it is available (mainly for security reasons, see
http://fedoraproject.org/wiki/Packaging/Guidelines#head-17396a3b06ec849a7c0c6fc3243673b17e5fed90
).

However,
1. both libcsf (pcraster) and libgeotiff are not available in FC/FE. So this
   is NOT a blocker.
2. Apparantly the internal pcraster library seems newer than the one available
   (and some many modifications are made on internal libraries)
3. When libgeotiff is available on FE, you should consider using external
   libgeotiff libraries. It seems that the codes included in this package and
   libgeotiff project package are actually SAME. However, currently this is
   not a blocker.

So, anyway I won't make extenal/internal issue block this bug.
Well, again I will check this package (in the comment #11)
Comment 17 Mamoru TASAKA 2006-09-14 08:56:57 EDT
Before checking comment #11:

Well, build log says:

  LIBTOOL support:           yes

  LIBZ support:              external
  GRASS support:             no
  CFITSIO support:           external
  PCRaster support:          internal
  NETCDF support:            yes
  LIBPNG support:            external
  LIBTIFF support:           external
  LIBGEOTIFF support:        internal
  LIBJPEG support:           external
  LIBGIF support:            external
  OGDI support:              no
  HDF4 support:              no
  HDF5 support:              yes
  KAKADU support:            no
  JASPER support:            no
  ECW support:               no
  MrSID support:             no
  POSTGRESQL support:        yes
  MySQL support:             yes
  XERCES support:            no
  ODBC support:              yes
  OCI support:               no
  SDE support:               no
  DODS support:              no
  SQLite support:            yes
  DWGdirect support          no
  PANORAMA GIS support:      no
  GEOS support:              yes

  Statically link PROJ.4:    no

  Traditional Python:        yes
  NG SWIG Bindings:          

  enable OGR building:       yes

... It seems that jasper-devel-1.701.0 seems available on extras.
I quite don't know about this package, however, if you want consider
to support jasper support.
Comment 18 Patrice Dumas 2006-09-14 09:29:39 EDT
(In reply to comment #17)
> Before checking comment #11:
> 
> Well, build log says:
> 

>   HDF4 support:              no

hdf4 is in extras (hdf/hdf-devel).

>   DODS support:              no

DODS is also in extras, more precisely, opendap is there, which
is what replace DODS. It is possible that gdal looks for older
versions (as a side note, I took some code from gdal m4 macros to do a 
detection of old dods version in a macro for detection of dods/dap, 
it is in upstream libdap...). Packages are libdap/libdap-devel for
the C++ interface or libnc-dap/libnc-dap-devel for the nectdf dods 
enabled interface.
Comment 19 Mamoru TASAKA 2006-09-14 09:42:04 EDT
(In reply to comment #18)
> >   HDF4 support:              no
> 
> hdf4 is in extras (hdf/hdf-devel).

Strange. In spec file (in comment #11), it says:

<snip>
BuildRequires: hdf-devel
<snip>
%configure \
<snip>
--with-hdf4=yes \
<snip>

So I think that Joost want to support JDF4 support. Well, Joost,
can you check what is happening?
Comment 20 Patrice Dumas 2006-09-14 09:46:43 EDT
(In reply to comment #16)

> 1. both libcsf (pcraster) and libgeotiff are not available in FC/FE. So this
>    is NOT a blocker.

libgeotiff is blocked by a legal issue, so it is something that should
be looked at carefully... Is there the same issue for the internal 
geotiff of gdal?
Comment 21 Jason Tibbitts 2006-09-14 10:19:54 EDT
The problem with libgeotiff seems to be the license on the data distributed as
part of the package.  Is that data included here?  If not, there shouldn't be a
problem, but then there shouldn't be a problem with getting just the library
portion of libgeotiff in either.
Comment 22 Mamoru TASAKA 2006-09-14 10:55:46 EDT
(In reply to comment #20 and comment #21)
Patrice and Jason, good catch, thank you.

Actually libgeotiff part in gdal code REALLY USES EPSG/POSC database.
"REALLY USES" I said means that:

Under gdal-1.3.2/frmts/gtiff/libgeotiff, the EPSG/POSC database
epsg*.inc exist and these files are included in geonames.h,
geovalues.h and these header files are used in .c source code.

So, if using libgeotiff part in gdal code, it is ALSO
legal issue. Joost, will you remove libgeotiff support?
Comment 23 Mamoru TASAKA 2006-09-14 11:11:57 EDT
Third review.

1. From http://fedoraproject.org/wiki/Packaging/Guidelines :
   * Legal
     - Legal issue needs checking (as argued above).

   * Code Vs Content
     - Well, again, what are the following files?
         /usr/share/gdal/seed_2d.dgn (in gdal)
         /usr/share/gdal/seed_3d.dgn (in gdal)
       I don't know what these files are. If you also don't know
       what these are, remove these files.

2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
   = Nothing.

3. Other things I have noticed:
   * Well, when including man files in rpm package, the format like
           "gdal_contour.1*" 
     (not gdal_contour.1.gz) is prefered.
   * Also, when includes all .py, .pyo, .pyc files, the format like
           "%{python_sitearch}/gdal.py*" 
     (not writing 3 files seperately) is prefered.
   * Check again what you want to support.
   * Perhaps it must be considered why HDF4 support is disabled.
Comment 24 Patrice Dumas 2006-09-14 11:52:31 EDT
Created attachment 136274 [details]
support for libdap and fix for hdf

This fix the hdf4 issue, and add support for dods/libdap.

This should work for devel libdap, for previous FC releases
`dap-config --client-libs` should not be in LDFLAGS
(and shouldn't need to be in LDFLAGS).
Comment 25 Patrice Dumas 2006-09-14 12:20:53 EDT
(In reply to comment #24)
> Created an attachment (id=136274) [edit]
> support for libdap and fix for hdf

I forgaot to add a

Requires: libdap-devel

for gdal-devel
Comment 26 Joost Soeterbroek 2006-09-14 13:24:09 EDT
(In reply to comment #22)
> (In reply to comment #20 and comment #21)

> So, if using libgeotiff part in gdal code, it is ALSO
> legal issue. Joost, will you remove libgeotiff support?

Ok, I will. 
If I understand the configure options correctly, this would translate in these
options (among others):

  --with-libtiff=no
  --with-geotiff=no
  --with-geos=yes

Correct?
Comment 27 Joost Soeterbroek 2006-09-14 13:30:22 EDT
(In reply to comment #24)
 
> This should work for devel libdap, for previous FC releases
> `dap-config --client-libs` should not be in LDFLAGS
> (and shouldn't need to be in LDFLAGS).

My dap-config does not have '--client-libs' option:
  $ dap-config --client-libs
  unknown option: --client-libs

Suppose I should use:
  $ dap-config --libs
  -L/usr/lib -ldap  -L/usr/lib -lcurl -L/usr/kerberos/lib -lssl -lcrypto -ldl
-lz -lgssapi_krb5 -lkrb5 -lk5crypto -lkrb5support -lcom_err -lresolv -lidn -lssl
-lcrypto -lz -L/usr/lib -lxml2 -lz -lm -lpthread




Comment 28 Joost Soeterbroek 2006-09-14 13:41:23 EDT
(In reply to comment #26)
> (In reply to comment #22)
> > (In reply to comment #20 and comment #21)
> 
> > So, if using libgeotiff part in gdal code, it is ALSO
> > legal issue. Joost, will you remove libgeotiff support?
> 
> Ok, I will. 
> If I understand the configure options correctly, this would translate in these
> options (among others):
> 
>   --with-libtiff=no
>   --with-geotiff=no
>   --with-geos=yes
> 
> Correct?
> 

Actually, using options:

    --with-libtiff=yes
    --with-geotiff=no

results in configure error:

 using pre-installed libtiff.
 checking for XTIFFClientOpen in -lgeotiff... no
 configure: error: We require at least GeoTIFF 1.2.1. Consider using the one
supplied with GDAL

 
Comment 29 Mamoru TASAKA 2006-09-14 13:49:45 EDT
(In reply to comment #28)

> Actually, using options:
> 
>     --with-libtiff=yes
>     --with-geotiff=no
> 
> results in configure error:
> 
>  using pre-installed libtiff.
>  checking for XTIFFClientOpen in -lgeotiff... no
>  configure: error: We require at least GeoTIFF 1.2.1. Consider using the one
> supplied with GDAL
> 
>  

Well, actually I have the same error. I suppose this needs a
patch. I will check it.

And... adding "dap-config --client-libs" will not enable DODS support.
I will check is "dap-config --libs" works.

Then, will someone review my review request:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=206487 ?
Comment 30 Mamoru TASAKA 2006-09-14 13:51:55 EDT
(In reply to comment #29)
> 
> And... adding "dap-config --client-libs" will not enable DODS support.
> I will check is "dap-config --libs" works.
> 

Sorry, DODS support was enabled by including libdap-devel.
Comment 31 Mamoru TASAKA 2006-09-14 13:53:00 EDT
Well, again sorry. For me, "dap-config --client-libs" works.
Comment 32 Mamoru TASAKA 2006-09-14 13:55:10 EDT
Created attachment 136286 [details]
gdal.spec I used

gdal.spec I used.

The big problem left is how to disable libgeotiff support.
Comment 33 Joost Soeterbroek 2006-09-14 13:57:12 EDT
(In reply to comment #31)
> Well, again sorry. For me, "dap-config --client-libs" works.

I am using libdap-devel-3.6.0-1.fc5:

  $ /usr/bin/dap-config --client-libs
  unknown option: --client-libs
Comment 34 Mamoru TASAKA 2006-09-14 14:23:55 EDT
Well, it is found that:

* on FC5, 'dap-config --libs' is okay (and 'dap-config --client-libs' bears
  error)

* on FC6-devel, 'dag-config --libs' won't work!!
/bin/sh /builddir/build/BUILD/gdal-1.3.2/libtool --mode=link    g++  ogrinfo.o
/builddir/build/BUILD/gdal-1.3.2/libgdal.la -o ogrinfo
g++ ogrinfo.o -o .libs/ogrinfo 
/builddir/build/BUILD/gdal-1.3.2/.libs/libgdal.so -L/usr/lib/netcdf-3
-L/usr/lib/hdf -L/usr/lib -L/usr/kerberos/lib -lgeos -lodbc -ljasper -lhdf5
-lmfhdf -ldf -lgif -ljpeg -ltiff -lpng -lnetcdf -L/usr/include/cfitsio
-L/usr/include/cfitsio/lib -lcfitsio -lpq -lrt -ldap -lcurl -lgssapi_krb5 -lkrb5
-lk5crypto -lcom_err -lresolv -ldl -lidn -lxml2 -lpthread -lsqlite3
-L/usr/lib/mysql -lmysqlclient -lz -lcrypt -lnsl -lssl -lcrypto
/builddir/build/BUILD/gdal-1.3.2/.libs/libgdal.so: undefined reference to
`AISConnect::AISConnect(std::basic_string<char, std::char_traits<char>,
std::allocator<char> > const&)'
/builddir/build/BUILD/gdal-1.3.2/.libs/libgdal.so: undefined reference to
`RCReader::instance()'
collect2: ld returned 1 exit status
make[1]: *** [ogrinfo] Error 1
make[1]: Leaving directory `/builddir/build/BUILD/gdal-1.3.2/ogr'
make: *** [ogr-apps] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.80418 (%build)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.80418 (%build)

  on FC6-devel, it must "dap-config --client-libs".

So, a switch is needed so if dap-config accepts '--client-libs', use
it and otherwise, use '--libs'
Comment 35 Patrice Dumas 2006-09-14 15:37:09 EDT
(In reply to comment #34)
> Well, it is found that:
> 
> * on FC5, 'dap-config --libs' is okay (and 'dap-config --client-libs' bears
>   error)

This is what is done by the configure.in, so on fc5 there is
nothing to add in LDFLAGS to have dap support.

> * on FC6-devel, 'dag-config --libs' won't work!!

In the version in devel the libdap library has been 
split in 3: libdapclient, libdapserver and libdap. 
gdal needs to link against what is provided by libdapclient.
In that version `dap-config --libs` provides only libdap.
So it doesn't work, and that's why dap-config --clients-libs
has to be added in LDFLAGS.

In the previous version (in FC5), the libdap library is linked 
with dap-config --libs and since it is not split everything
is okay.


> RPM build errors:
>     Bad exit status from /var/tmp/rpm-tmp.80418 (%build)
> 
>   on FC6-devel, it must "dap-config --client-libs".
> 
> So, a switch is needed so if dap-config accepts '--client-libs', use
> it and otherwise, use '--libs'

That's approximately what is done in the latest autoconf macro.
I'll propose a patch for the spec file shortly.
Comment 36 Patrice Dumas 2006-09-14 15:46:06 EDT
Created attachment 136299 [details]
fix hdf link, and add support for libdap for FC5 and devel

I haven't tested on FC5, only on devel
Comment 37 Mamoru TASAKA 2006-09-14 20:26:55 EDT
Well, no way to fix for libgeotiff dependency.

Now I make this bug block FE-legal.
Comment 38 Ralf Corsepius 2006-09-14 21:21:18 EDT
Further issues:

* BLOCKER:
gdal-devel ships an autoheader-generated header file
(/usr/include/gdal/cpl_config.h). This violates autoconf's working principles
and prevents autoconf-based packages from using gdal.

* SHOULDFIX: The way cfitsio is being configed is broken:
ATM, you pass --with-cfitsio=/usr/include/cfitso

This causes gdal to use -I/usr/include/cfits and -L/usr/include/cfits/lib. The
latter is incorrect.

A proper way would be to pass --with-cfitsio
and to append -I/usr/include/cfitsio to CPPFLAGS
Comment 39 Mamoru TASAKA 2006-09-23 13:30:03 EDT
*** Bug 168719 has been marked as a duplicate of this bug. ***
Comment 40 Mamoru TASAKA 2006-10-15 01:21:42 EDT
Removing FE-Legal blocker as spot checked this license issue
(and declared that this package is surely wrong with license issue).
Comment 41 Mamoru TASAKA 2006-10-30 10:27:50 EST
Umm.. for now I will have to close this bug as "WONTFIX" as libgeotiff
review request (bug 178162) was closed as "NOTABUG" due to   EPSG data
license problem. I have already contacted libgeotiff upstream to
resolve license problem, however I have not yet received any response.

Closing as WONTFIX, removing the blocker for FE-REVIEW and adding the
blocker for FE-DEADREVIEW. When I receive any response from upstream,
I will report it.
Comment 42 Balint Cristian 2007-02-06 15:22:51 EST
Why cant we disable geotiff ? is some tehnical reason ?
Comment 43 Balint Cristian 2007-02-06 16:41:20 EST
ok.
Tooked latest gdal-1.4.0, made a patch that add --with-geotiff=no option.
It compiles fine, we can get rid of geotiff, till we have update on licence 
issue.

Anyone interested for a respin on this gdal ?

I am really looking forward to add grass http://grass.itc.it/ into -extras.
~cristian

Comment 44 Devrim GUNDUZ 2007-02-06 16:52:36 EST
Hi Cristian,

http://www.mammothpostgresql.org/browser/mammothpostgresql/FC6/SRPMS/grass-6.0.2-1CMD.src.rpm?format=raw

may be a good starting point for grass. I have packaged it for our product some
monts ago.

Regards, Devrim
Comment 45 Mamoru TASAKA 2007-02-07 00:26:52 EST
Hello, Cristian and Devrim:

Currently another person submitted another GDAL review request,
so I close this bug as DUPLICATE. Please attach a patch on the 
bug if you have a patch, thanks.

*** This bug has been marked as a duplicate of 222042 ***

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