Spec URL: http://smani.fedorapeople.org/review/engrid.spec SRPM URL: http://smani.fedorapeople.org/review/engrid-1.4.0-1.gite6d55f5.fc21.src.rpm Description: Mesh generation tool Fedora Account System Username: smani Note: packaging git snapshot since official 1.4.0 release does not have VTK 6 support.
Created attachment 911739 [details] epel6 patch I built it on EPEL6 with the attached patch.
Thanks!
Taking review.
Hi Dominik, any chance of doing the review? Thanks.
Sorry, I was quite busy lately. Some comments while fedora-review is processing: # Rename licence file... mv licence.txt license.txt Why? The guidelines say the spec file must be written in American English, but that doesn't mean the filenames must be, too. # Desktop file, icon echo "Icon=engrid.png" >> engrid.desktop sed -i 's/Education;/Science;/' engrid.desktop The two lines above can be done with appropriate options in desktop-file-install invocation, please use them (--set-icon and --add-category/--remove-category). Also, the Icon= option should be set to engrid, not engrid.png, as required by desktop file guidelines. Please also try adding appdata, as the guideline has been approved recently (http://people.freedesktop.org/~hughsient/appdata/), Out of curiosity, are there any consumers of that shared library apart from this package? I don't see a -devel subpackage...
Additionally: What's the origin of Source1: engrid.png? I can see that the included setup_fedora.bash script uses src/libengrid/resources/icons/G.png, so why not do the same instead of shipping another copy in the SRPM? The engrid_build.patch contains whitespace-only change, which should be dropped to improve readability, namely this part (changes to src/engrid.pro.app): @ -46,11 +46,12 @@ win32-msvc* { LIBS += ../../netCDF/lib/netcdfcxx.lib } } else { - QMAKE_CXXFLAGS += -Wno-deprecated -g + CONFIG += link_pkgconfig + QMAKE_CXXFLAGS += -Wno-deprecated -g INCLUDEPATH += $(VTKINCDIR) LIBS += -L$(VTKLIBDIR) - LIBS += -L./netgen_svn -lng LIBS += -L./libengrid -lengrid + PKGCONFIG += netgen-mesher brlcad { INCLUDEPATH += $(BRLCADINCDIR) INCLUDEPATH += $(BRLCADINCDIR)/openNURBS You can skip adjusting formatting of the QMAKE_CXXFLAGS line. rpmlint issues: engrid.x86_64: W: incoherent-version-in-changelog 1.4-1 ['1.4.0-1.gite6d55f5.fc20', '1.4.0-1.gite6d55f5'] -> easyfix engrid.x86_64: W: shared-lib-calls-exit /usr/lib64/libengrid.so.1.0.0 exit.5 -> please report upstream engrid.x86_64: W: undefined-non-weak-symbol /usr/lib64/libengrid.so.1.0.0 nc_put_vara_short [...] engrid.x86_64: W: undefined-non-weak-symbol /usr/lib64/libengrid.so.1.0.0 nc_inq_var engrid.x86_64: W: undefined-non-weak-symbol /usr/lib64/libengrid.so.1.0.0 typeinfo for vtkInteractorStyle [...] -> looks like missing libraries during linking stage engrid.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libengrid.so.1.0.0 /lib64/libQtNetwork.so.4 -> drop from link flags Source checksums ---------------- https://github.com/enGits/engrid/tarball/e6d55f564c20a8d13bee6bba6280a32320f1bde2/enGits-engrid-1.4.0-264-ge6d55f5.tar.gz : CHECKSUM(SHA256) this package : 27119148fbcefe8654c213bdeb503bec323bd5a72181dd17ae8035492e80cc26 CHECKSUM(SHA256) upstream package : 27119148fbcefe8654c213bdeb503bec323bd5a72181dd17ae8035492e80cc26 Licence seems to be ok, but there are some files without licence information: src/dialoglineedit/* Please ask upstream to clarify the licence on those.
Thank you for the careful review. Concerning the icon, the reason I include it as a separate source is because the original icon is not a standard sized (46x51). I suppose I could BR imagemagick and resize the image in %prep, I feel it more robust to include it as a separate source but am fine with either method. Concerning other users of the shared library: none that I know of for sure at the moment, OpenFOAM might be one. But in any case I'd rather keep the upstream build scripts as far as possible. Concerning the shared-lib-calls-exit and license files, upstream report is here: https://github.com/enGits/engrid/issues/47 Spec URL: http://smani.fedorapeople.org/review/engrid.spec SRPM URL: http://smani.fedorapeople.org/review/engrid-1.4.0-2.gite6d55f5.fc21.src.rpm %changelog * Wed Jul 30 2014 Sandro Mani <manisandro> - 1.4.0-2.gite6d55f5 - Don't rename license file - Use desktop-file-install to set icon and fix category - Add doc subpackage - Add blender subpackage - Update build patch to also link against vtk libraries, and not QtNetwork
$ rpmlint engrid-debuginfo engrid-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/enGits-engrid-e6d55f5/src/libengrid/egvtkinteractorstyle.h engrid-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/enGits-engrid-e6d55f5/src/libengrid/egvtkinteractorstyle.cpp engrid-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/enGits-engrid-e6d55f5/src/libengrid/createvolumemesh.cpp Please chmod -x these files in %prep. Other highlighted rpmlint issues are gone, great. I noticed that there's a manpage (engrid.1) in debian/ subdirectory, please install it, too. You can fix these issues upon package import, as they're pretty minor. This package is APPROVED.
Thanks a lot, issues will be fixed when importing. New Package SCM Request ======================= Package Name: engrid Short Description: Mesh generation tool Owners: smani Branches: f20 InitialCC:
Git done (by process-git-requests). Added f21.
engrid-1.4.0-3.gite6d55f5.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/engrid-1.4.0-3.gite6d55f5.fc20
engrid-1.4.0-3.gite6d55f5.fc20 has been pushed to the Fedora 20 testing repository.
engrid-1.4.0-3.gite6d55f5.fc20 has been pushed to the Fedora 20 stable repository.