Bug 666301

Summary: Review Request: libLAS - Library and tools for the LAS LiDAR format
Product: [Fedora] Fedora Reporter: Ed Hill <ed>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting, rc040203, richmattes, volker27
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard: StalledSubmitter
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-12-13 16:13:59 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 201449    

Description Ed Hill 2010-12-29 21:50:35 EST
Spec URL: http://eh3.com/libLAS.spec
SRPM URL: http://eh3.com/libLAS-1.6.0-6.fc14.src.rpm
Description: Library and tools for the LAS LiDAR format
Comment 1 Ed Hill 2010-12-29 22:42:09 EST
The SRPM builds in mock for Fedora-devel and Fedora-14.
Comment 2 Ralf Corsepius 2010-12-30 01:58:59 EST

* /usr/bin/liblas-config is not multilib ready.
This script contains hard-coded references to architecture-specific paths
=> the *-devel.i686 package and the *-devel.x86_64 package can not be installed in parallel.

One common work-around to such issues is replacing such *-config scripts by pkg-config files and reimplementing the *-config script as a wrapper around pkg-config.

* /usr/bin/liblas-config is broken:

a) /usr/bin/liblas-config --libs
-L/usr/lib -llas -llas_c -L/usr/lib64 optimized;/usr/lib64/libboost_program_options-mt.so;debug;/usr/lib64/libboost_program_options-mt.so /usr/lib64/libgeotiff.so /usr/lib64/libtiff.so /usr/lib64/libxml2.so

  + -L/usr/lib (on x86_64).
  + optimized;...;debug;...

b) /usr/bin/liblas-config --cflags
/usr/bin/liblas-config --includes
-I/usr/include -I/usr/include/libgeotiff -I/usr/include -I/usr/include/libxml2

Using -I/usr/include disturbs the system include path and is never right.

c) /usr/bin/liblas-config --cxxflags
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -fPIC -pedantic -ansi -Wall -Wpointer-arith -Wcast-align -Wcast-qual -Wfloat-equal -Wredundant-decls -Wno-long-long -std=c++98
Comment 3 Ed Hill 2010-12-30 10:53:36 EST
Hi Ralf, thank you for the very quick and helpful feedback!  Here's an 
updated SRPM:


which does a better job of using the project-defined lib install 
directory (fixes the one rpmlint error) and, as an interim measure, 
removes the non-multilib-compliant /usr/bin/liblas-config from the 

I recently got libLAS commit permission so, pending approval by the 
project team, I'll add some CMake templates which create and install 
multilib-compliant %{_libdir}/liblas.pc files as you describe above.  
And if my bash skills aren't too pathetic I'll also write a new
liblas-config which, as you suggest, wraps pkg-config.

Would you be willing to approve the above version?
Comment 4 Volker Fröhlich 2010-12-31 19:47:37 EST
A few things I noticed:

The verbose flag for make is not necessary, because CMAKE_VERBOSE_MAKEFILE is already set to "on", when you're using %cmake. See http://www.vtk.org/Wiki/CMake_FAQ#Is_there_an_option_to_produce_more_.27verbose.27_compiling.3F and rpm -E %cmake

Please give a rationale in the spec why you're deleting $RPMBUILD/usr/share.

The chmod seems unnecessary, since all files in $RPMBUILD/usr/bin are already 755.

I think there are some tests and Python bindings as well, is that correct?
Comment 5 Ed Hill 2011-01-12 13:18:54 EST
Hi Ralf & Volker,

An update is available here:


and it addresses most of the above comments.  The pkgconfig bits are 
not yet sorted out but I'll try to have an update soon.

Aside from the pkgconfig bits, do you spot any blockers?
Comment 6 Ed Hill 2011-01-12 20:42:21 EST
Here's the promised update:


It addresses all the following:

  0) improved the pkgconfig (multi-lib) situation -- checked 
     into upstream
  1) redundant make-verbose flag removed
  2) rationale added for $RPMBUILD/usr/share
  3) description of %check section added
  4) chmod of $RPMBUILD/usr/bin removed

and there are a couple of to-do items:

  + enable -DWITH_GDAL:BOOL=ON as soon as the GDAL 1.8.x dependency
    is available and then add the necessary GDAL libs to pkgconfig
  + package the python bindings -- I will very likely need help
    with the python packaging or perhaps someone else could handle 
    it -- is anyone interested?

And thanks again for taking a look!
Comment 7 Ralf Corsepius 2011-01-15 12:46:01 EST
Further remarks:


* The *.pc is broken:

A detail, I am not sure about how pkgconfig will handle it, is the version number: Version: 1.6.0b4

* AFAICT, several "Requires:" are missing from *-devel.
E.g. libstdc++-devel (package uses libstdc++ headers)

* Some of the headers contain questionable defines:
# grep -R HAVE_ usr/include/*
usr/include/liblas/transform.hpp:#ifdef HAVE_GDAL
usr/include/liblas/transform.hpp:#ifdef HAVE_GDAL
usr/include/liblas/transform.hpp:#ifdef HAVE_GDAL
usr/include/liblas/detail/reader/zipreader.hpp:#ifdef HAVE_LASZIP
usr/include/liblas/detail/reader/zipreader.hpp:#endif // HAVE_LASZIP
usr/include/liblas/detail/zippoint.hpp:#ifdef HAVE_LASZIP
usr/include/liblas/detail/zippoint.hpp:#endif // HAVE_LASZIP
usr/include/liblas/detail/writer/zipwriter.hpp:#ifdef HAVE_LASZIP
usr/include/liblas/detail/writer/zipwriter.hpp:#endif // HAVE_LASZIP

- These defines collide with autoconf defines. 
A proper way to fix this to append such defines with a package specific prefix (e.g. _LIBLAS ...) and to let the package generate a header file providing these defines + to let all files using these defines include it.

- liblas need to defined/undefine these defines. Otherwise applications using this library are at risk of using a different API/ABI than the library. Though this occasionally may make sense, I doubt this is intentional.

* Finally, the tarball name being used is causing me headaches, but I am not familiar with mercurial and don't know off-head what the FPG says.
Comment 8 Rich Mattes 2011-02-02 13:48:17 EST
I see that libLAS 1.6.0 has been released.  Are you planning to update the submission soon?
Comment 9 Ralf Corsepius 2011-04-25 04:34:39 EDT
Ed, any update?
Comment 10 Volker Fröhlich 2011-09-12 02:18:50 EDT
Ed, are you still interested in maintaining this package?
Comment 11 Volker Fröhlich 2011-10-08 03:15:29 EDT
I guess this review is stalled. Ed, if you are still interested, please respond!

Otherwise I'll close this bug as described in http://fedoraproject.org/wiki/Policy_for_stalled_package_reviews

Feel free to re-open it any time though.