[Bug 1199298] Review Request: liblas - LAS 1.0/1.1/1.2 ASPRS LiDAR data translation toolset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



--- Comment #3 from Rex Dieter <rdieter@xxxxxxxxxxxx> ---
naming: ok

1. license: NOT ok.  close though, there's some bundled modified boost headers
included, so MUST use:
License: BSD and Boost
in that vein, due to how dependencies work out, I'd suggest droping
%license COPYING
(it's an empty) file, and include in -libs instead:
%license LICENSE.txt

sources: ok
599881281d45db4ce9adb2d75458391e  libLAS-1.8.0.tar.bz2

2. macros: NOT ok, MUST use %cmake macro in place of bare 'cmake', doing so,
you can drop -DCMAKE_INSTALL_PREFIX:PATH=/usr flag.  while you're at it, can
omit:
"../%{name}-%{version}/" too

3. MUST use %{?_isa} macro in subpkg dependencies, so main pkg and -devel
should use instead:
Requries: %{name}-libs%{?_isa} = %{version}-%{release}

4. main pkg MUST drop not needed explicit dep:
Requires: laszip

5.  SHOULD omit deprecated rpm Group: tags and %clean section

6. %build section MUST envoke 'make', include:
make %{?_smp_mflags}
(after cmake call)

7.  %install section, SHOULD use
make install/fast DESTDIR=%{buildroot}
(in preference over %make_install macro, which is generally intended for
autoconf-based pkgs)

8. MUST move liblas.so and liblas_c.so symlinks to -devel pkg

9. -devel MUST own
%dir %{_includedir}/liblas
%dir %{_datadir}/cmake/libLAS-%{version}
%{_datadir}/%{name}/doc/
dirs, I'd suggest removing these %dir calls and include in -devel just:
%{_includedir}/liblas/
%{_datadir}/cmake/libLAS-%{version}/
%{_datadir}/%{name}/doc/
(while we're at it, will probably have to patch this to put cmake files under
%_libdir instead of %_datadir, but I can help with that later, possibly
post-review)

10. SHOULD drop INSTALL from %doc

11. in %build SHOULD use
-DWITH_LASZIP:BOOL=ON
instead of
-DWITH_LASZIP:BOOL=OFF

12. in %build SHOULD replace hard-coded:
-DGEOTIFF_INCLUDE_DIR:STRING="/usr/include/libgeotiff"
with
-DGEOTIFF_INCLUDE_DIR:PATH="$(pkg-config --variable=includedir libgeotiff)"

13. MUST fix rpath.  there's some rpath's getting set somewhere, e.g.
ERROR   0001: file '/usr/bin/las2las' contains a standard rpath '/usr/lib' in
[/usr/lib]
I'll try to figure out where this is coming from.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]