Spec URL: http://muep.fedorapeople.org/glm/glm.spec SRPM URL: http://muep.fedorapeople.org/glm/glm-0.9.3.1-1.fc16.src.rpm Description: GLM is a C++ library for doing math operations required in many OpenGL based applications. Its interface has been designed to resemble the built-in matrix and vector types of the OpenGL shading language. Note: Rpmlint will complain about a couple of empty .inl files. These were not removed because other code in the package uses #include with their names.
Sorry for sending the package for review a bit too early. Here is an improved version: Spec URL: http://muep.fedorapeople.org/glm/glm.spec SRPM URL: http://muep.fedorapeople.org/glm/glm-0.9.3.1-2.fc16.src.rpm * Mon Feb 06 2012 Joonas Sarajärvi <muep> - 0.9.3.1-2 - Build and run the self-test suite shipped with glm - Add subpackage for manual and reference docs
The Summary is bad. Please use something like "A C++ mathematics library for graphics programming". Also, please fix your tabbing on the Source0 line.
Thanks, Jussi, for your comments. I changed the summary according to your suggestion, and reformatted the Source0 line to align with the other lines. Because the Summary of the main package does not actually seem to end up in any of the binaries, the -devel package was also given the same Summary. Updated package: Spec URL: http://muep.fedorapeople.org/glm/glm.spec SRPM URL: http://muep.fedorapeople.org/glm/glm-0.9.3.1-3.fc16.src.rpm
> # The library consists of headers only > %define debug_package %{nil} Which effectively makes it a static-only package that requires special handling: https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries_2 > The %{name}-devel package contains libraries and header files for > developing applications that use %{name}. Contradictory. No library and no %{name} package exist. > %package doc > Summary: Documentation for %{name} > >%description doc > The %{name}-doc package contains documentation for the %{name} library. Same here. No %{name} package exists. * Build log on x64_64 contains lots of warnings about type-punning. Be careful, even if the test suite seems to pass. $ rpmlint glm* glm-devel.x86_64: E: zero-length /usr/include/glm/gtx/ocl_type.inl glm-devel.x86_64: E: zero-length /usr/include/glm/gtx/vec1.inl
Thanks, Michael. Just to clarify, should the library word in descriptions be reserved exclusively for shared object files, or is it acceptable to use it for a headers of a header-only C++ library? Regardless, it indeed is unnecessary talk about headers AND libraries in the same sentence.
Fixed issues pointed to by Michael. Updated package: Spec URL: http://muep.fedorapeople.org/glm/glm.spec SRPM URL: http://muep.fedorapeople.org/glm/glm-0.9.3.1-4.fc16.src.rpm (In reply to comment #4) > $ rpmlint glm* > glm-devel.x86_64: E: zero-length /usr/include/glm/gtx/ocl_type.inl > glm-devel.x86_64: E: zero-length /usr/include/glm/gtx/vec1.inl Did you add this just for information of participants, or do you desire this to be addressed further than what was noted in my original post? These would probably be very easy to patch out, but I would prefer not adding a patch that does not affect the reliability nor functionality of the package at all.
Please use %global instead of %define: http://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define What is a "zib bomb"? rm -rf $RPM_BUILD_ROOT is no longer necessary.
Thank you, Volker, for the comments. Updated package: Spec URL: http://muep.fedorapeople.org/glm/glm.spec SRPM URL: http://muep.fedorapeople.org/glm/glm-0.9.3.1-5.fc16.src.rpm * Mon Feb 13 2012 Joonas Sarajärvi <muep> - 0.9.3.1-5 - Use global instead of define - Clarify the comment about GLM zip archives - Remove the unnecessary rm command from install section - Remove misleading reference to non-existing glm package
Re: comment 5 First of all, no strong objections with regard to using the term "library" in a generic way. Similar to the term "archive", which is not limited to tar/tgz/zip/… file types. It's okay to write "GLM is a C++ library ...", although it is just a collection of header files. I didn't object to doing that. That's an _is a_ relation, whereas a _contains_ relation is more complicated: The spec file said: "... package contains libraries and header files", which is plural (=> "library files and header files"), which was duplicate and very ambiguous as the package contains only header files. There's a description at the web page, which also explains the "GLM" acronym, btw: | OpenGL Mathematics (GLM) is a header only C++ mathematics library | for graphics software based on the OpenGL Shading Language (GLSL) | specification. Re: comment 6 Current rpmlint was only included for completeness, making it explicit which two files are affected. It could be that other builds would end up with a different set of empty files, for exaple. ;)
A new upstream version, glm 0.9.3.2, has been released. It mostly had some documentation changes, but it also fixes a couple of minor issues. Spec URL: http://muep.fedorapeople.org/glm/glm.spec SRPM URL: http://muep.fedorapeople.org/glm/glm-0.9.3.2-1.fc17.src.rpm
Hi, I am taking this package for review. I think it's in a pretty good condition after suggestions from everybody in this bugzilla. There is a new release out on upstream's website. It probably would be worth it to update the spec to it. It concerns me a bit that bugfixes in glm will require rebuilds of everything that uses it to propagate. But there clearly is nothing we can do about that. Upstream puts a lot of binary files into the release zip file including doc/build folder which has Win32 binaries. It would be awesome if you could convince the upstream that this shouldn't be in the release file.
Although upstream puts binary dlls and one exe into the zip archive they are not installed so I think it's not an issue except it bloats the lookaside cache. I think that including the two zero length files is OK, I would suggest that upstream puts a comment in there saying that they are "// empty for now" or something to avoid any future confusion. Regarding find $RPM_BUILD_ROOT -name '*.la' -exec rm -f {} ';' find $RPM_BUILD_ROOT -name CMakeLists.txt -exec rm -f {} ';' The first should be an option in the cmake file, the second is clearly a mistake and upstream should be notified. Nice to haves but unimportant: [!]: SHOULD Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [!]: SHOULD %check is present and all tests pass (EDIT: The latter is actually correct and I just missed it in the .spec file, sorry!) MD5-sum check ------------- http://downloads.sourceforge.net/ogl-math/glm-0.9.3.2/glm-0.9.3.2.zip : CHECKSUM(SHA256) this package : ee66ab8336b9b6b3dff69268c497688268cf5a9d2b3a14e1aa6fbd7f48c911be CHECKSUM(SHA256) upstream package : ee66ab8336b9b6b3dff69268c497688268cf5a9d2b3a14e1aa6fbd7f48c911be Approved, full review here: http://pastebin.com/yR4Sjz9Z
Thank you for reviewing the package. I have taken a look at the two more recent upstream releases, but currently they have a failing test case which I would prefer to look into before pushing a new release. The problem does not look very significant, since it only seems to concern integer operations and glm is likely usually used for floating point computations. It may also be the case that the bug is actually in the test code and not glm itself, but I have not yet figured out which participant of the test case is the faulty one. I will try to communicate issues raised in this bug upstream. I can also try to look at the failing test more myself, in hopes of fixing the issue.
(In reply to comment #12) > Approved, full review here: http://pastebin.com/yR4Sjz9Z This pretty much goes against all guidelines. Bugzilla exists for the reviews.
(In reply to comment #14) > (In reply to comment #12) > > Approved, full review here: http://pastebin.com/yR4Sjz9Z > > This pretty much goes against all guidelines. Bugzilla exists for the > reviews. I only included fedora-review output for completeness. All points it raises are in the bugzilla. I don't think pasting the entire output here is of any value. If you feel otherwise, feel free to copy paste it in there. I would appreciate it if you could point the guideline that I crossed with this. Thanks!
One more thing that I forgot to mention. If upstream doesn't remove the doc/build folder you should remove it in %prep, see https://fedoraproject.org/wiki/Packaging:Guidelines#No_inclusion_of_pre-built_binaries_or_libraries
Version 0.9.3.2-2 removes the doc/build directory in %prep. Spec URL: http://muep.fedorapeople.org/glm/glm.spec SRPM URL: http://muep.fedorapeople.org/glm/glm-0.9.3.2-2.fc16.src.rpm
Doh, should have copy-pasted the URL from the correct place.. SRPM URL: http://muep.fedorapeople.org/glm/glm-0.9.3.2-2.fc18.src.rpm
New Package SCM Request ======================= Package Name: glm Short Description: C++ mathematics library for graphics programming Owners: muep Branches: f17 f18 InitialCC:
Git done (by process-git-requests).
Package has been built for Rawhide and Fedora 17 and Fedora 18. I intend to post this into updates of F17 and F18 after spending some more effort in the gtx_integer test failure. Thanks to everyone involved.