Spec URL: http://remi.fedorapeople.org/monitor-edid.spec SRPM URL: http://remi.fedorapeople.org/monitor-edid-1.16-1.fc8.src.rpm Koji scratch build : http://koji.fedoraproject.org/koji/taskinfo?taskID=518430 Description: Tool for probing and parsing EDID : http://en.wikipedia.org/wiki/EDID This package will try to read the monitor details directly from the monitor. -- This is an optional package for ocsinventory-agent which allow the retrieval of monitor informations, specially the Serial Number.
License: GPLv2 is incorrect. The tarball includes COPYING which is the LGPLv2 text, but none of the source files have any GPL or LGPL references, instead they have various BSD/MIT like notices. Providing a checkout script would be more convenient than including comments in the specfile. Could also use svn export instead of checkout (does not checkout .svn directories), and bzip2 or lzma the tarball instead of gzipping to save space. Looks also like there's a private copy of lrmi in the tarball, is there a reason why the Fedora packaged one is not used instead? Including the word "monitor" in the Summary would be good. Will continue the review after the license issues have been sorted out.
Thanks for the comments Spec URL: http://remi.fedorapeople.org/monitor-edid.spec SRPM URL: http://remi.fedorapeople.org/monitor-edid-1.16-2.fc8.src.rpm - Fix licenses : GPLv2 + MIT + DSB - add LICENSE.x86emu in doc - add monitor-edid-makesource.sh (I hope I well understand your comment) lrmi is only an i386 package. The version provided is build with x86emu to run under x86_64 (i don't think we can use a i386 in mock/koji).
Augh, *another* LRMI/x86emu port? So this, radeontool, etc., etc. have to all include separate versions? Note that you really want to use x86emu everywhere.
(In reply to comment #2) > - Fix licenses : GPLv2 + MIT + DSB I still don't see anything GPLv2 in the package. AFAICT GPLv2 should be replaced by LGPLv2+, see COPYING included in the tarball and http://fedoraproject.org/wiki/Licensing
@Bill a solution could be to ask kevin to provide and lrmi version for x86_64 ? @Ville Argh, you're right. Spec URL: http://remi.fedorapeople.org/monitor-edid.spec SRPM URL: http://remi.fedorapeople.org/monitor-edid-1.16-3.fc8.src.rpm
@Ville Skyttä is there is still a problem with this package ?
Created attachment 301389 [details] Use system lrmi Using the private lrmi is not nice, something like the attached patch could do the trick. I have no way to check if the result actually works on i386, but it does build. If you apply this, also add %ifarch %{ix86} "BuildRequires: lrmi-devel" to the specfile and "rm -f lrmi.h lrmi.c" in %prep to make sure the bundled lrmi is not used. MIT should probably be dropped from licenses even if system lrmi is not used; lrmi gets built into an LGPLv2+ executable which I think makes the combined work LGPLv2+ and we only list licenses for binaries. %description could be improved, suggestion: Monitor-edid is a tool for probing and parsing Extended display identification data (EDID) from monitors. For more information about EDID, see http://en.wikipedia.org/wiki/EDID
Thanks for the patch. I was working on the same solution, but i've also try to remove x86emu (not possible, used even on i386). Patch applied and send upstream. Test on FC5.i386 (my last i386 available OS) : run well. Scratch build : http://koji.fedoraproject.org/koji/taskinfo?taskID=551901 Spec URL: http://remi.fedorapeople.org/monitor-edid.spec SRPM URL: http://remi.fedorapeople.org/monitor-edid-1.16-4.fc8.src.rpm
Approved. There's no need to %ifarch the removal of lrmi.h and lrmi.c though, you can do that for all arches for clarity.
Thanks for the review. New Package CVS Request ======================= Package Name: monitor-edid Short Description: Tool for probing and parsing EDID Owners: remi Branches: F-7, F-8, EL-5, EL-4 InitialCC: Cvsextras Commits: yes
cvs done.