Spec URL: http://www.gunduz.org/epel/liblas.spec SRPM URL: http://www.gunduz.org/epel/liblas-1.8.0-1.rhel7.src.rpm Description: libLAS is a C/C++ library for reading and writing the very common LAS LiDAR format. The ASPRS LAS format is a sequential binary format used to store data from LiDAR sensors and by LiDAR processing software for data interchange and archival. Fedora Account System Username: devrim Note: I am not sure about the license in here.liblas-1.8.0-1.rhel7.src.rpm liblas.spec
rpmls (and rpm -qlvp …) are your friends. Highly recommended reading: https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership -> https://fedoraproject.org/wiki/Packaging:UnownedDirectories
Hi Michael, Thank you for the comment. I improved the spec file significantly: Spec URL: http://www.gunduz.org/epel/liblas.spec SRPM URL: http://www.gunduz.org/epel/liblas-1.8.0-2.f21.src.rpm Regards, Devrim
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.
14. SHOULD build with -DWITH_PKGCONFIG:BOOL=ON flag (unless you have some good reason not to) regarding 13 for now, you can use cmake build flag: -DCMAKE_SKIP_RPATH:BOOL=ON as a big hammer to kill rpaths. Long term, looks like apps/CMakeLists.txt has some silly hard-coded rpath handling that upstream needs to fix.
Hi, I think I fixed everything: Spec URL: http://www.gunduz.org/epel/liblas.spec SRPM URL: http://www.gunduz.org/epel/liblas-1.8.0-3.f21.src.rpm One of these changes affected the build though -- we cannot install liblas-config anymore. Regards, Devrim
I'd venture liblas-config is built only if pkgconfig is disabled (I'd argue pkgconfig is superior in that regard). That said, 15. both the pkgconfig and/or liblas-config MUST be in the -devel subpkg instead. Item 3 is not fixed correctly you added Requires: %{name}-libs%{?_isa} = %{version}-%{release} to -libs subpkg (itself! :) ) instead of -devel. it should replace -devel's: Requires: %{name} = %{version}-%{release}
Hi, Ok, put liblas-config back into the package, and fixed #3: Spec URL: http://www.gunduz.org/epel/liblas.spec SRPM URL: http://www.gunduz.org/epel/liblas-1.8.0-3.f21.src.rpm Regards, Devrim
I guess we can stick with liblas-config since that is the upstream default, but I suspect it may be prone to problems like multilib conflicts (something that pkgconfig handles well). I'd suggest you try contacting upstream why pkgconfig isn't enabled by default mostly cosmetic, but as part of item 9, main pkg still has: %dir %{_includedir}/liblas %dir %{_datadir}/cmake/libLAS-%{version} those can be dropped now (since they are properly owned by -devel subpkg now), but I won't treat that as a blocker... you can fix post-review. APPROVED
Thanks for the great and fast review!
New Package SCM Request ======================= Package Name: liblas Short Description: LAS 1.0/1.1/1.2 ASPRS LiDAR data translation toolset Upstream URL: http://www.liblas.org/ Owners: devrim Branches: epel7 el6 f21 f22 InitialCC: devrim
Git done (by process-git-requests).
liblas-1.8.0-3.el7 has been submitted as an update for Fedora EPEL 7. https://admin.fedoraproject.org/updates/liblas-1.8.0-3.el7
liblas-1.8.0-3.fc22 has been submitted as an update for Fedora 22. https://admin.fedoraproject.org/updates/liblas-1.8.0-3.fc22
liblas-1.8.0-3.fc21 has been submitted as an update for Fedora 21. https://admin.fedoraproject.org/updates/liblas-1.8.0-3.fc21
liblas-1.8.0-3.el7 has been pushed to the Fedora EPEL 7 testing repository.
liblas-1.8.0-3.fc22 has been pushed to the Fedora 22 stable repository.
liblas-1.8.0-3.fc21 has been pushed to the Fedora 21 stable repository.
liblas-1.8.0-3.el7 has been pushed to the Fedora EPEL 7 stable repository.