[Bug 205955] Review Request: gdal - A translator library for raster geospatial data formats

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: gdal - A translator library for raster geospatial data formats


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=205955





------- Additional Comments From fedora at soeterbroek.com  2006-09-12 11:30 EST -------
(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 at 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


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.