Red Hat Bugzilla – Bug 236849
Review Request: lcdproc - LCDproc displays real-time system information on a 20x4 backlit LCD
Last modified: 2007-11-30 17:12:02 EST
Spec URL: http://kwizart.free.fr/fedora/6/testing/lcdpro/lcdproc.spec SRPM URL: http://kwizart.free.fr/fedora/6/testing/lcdpro/lcdproc-0.5.1-1.kwizart.fc6.src.rpm Description: LCDproc displays real-time system information on a 20x4 backlit LCD There is still a need to tweak init scripts W: lcdproc no-reload-entry /etc/rc.d/init.d/LCDd W: lcdproc service-default-enabled /etc/rc.d/init.d/LCDd
The URLs above do not seem valid, the site is not accessible. Could you please take care of that, Nicolas?
Well... indeed my website is down (isp server side). I really hope this can be solved quickly Until then, here is the SRPMS: http://kwizartchitecture.free.fr/fedora/6/SRPMS/lcdproc-0.5.1-1.kwizart.fc6.src.rpm And the spec file from my personnal workstation (mean not always online - but often ) SPECS: http://kwizart.hd.free.fr/SPECS/browser/SPECS/lcdproc.spec
I've mirrored both files on my personal server, which is more or less always online, just in case someone wishes to do a review and your site(s) are not available. http://wdl.lug.ro/linux/rpms/lcdproc/lcdproc.spec http://wdl.lug.ro/linux/rpms/lcdproc/lcdproc-0.5.1-1.kwizart.fc6.src.rpm Nicolas, let me know if you do not agree with this and I'll remove them at once.
Just a few notes, before a more comprehensive review: - version 0.5.2 has been released by upstream a couple of weeks ago. any reason to not update the rpm, too ? - the following quote from the spec does not seem right: %makeinstall make install DESTDIR=$RPM_BUILD_ROOT Since the whole ./configure process is done twice, it looks like the first line is just a leftover from a previous version or something... - I'd say the file CREDITS should also be included as %doc - I get a lot of warnings during configure: checking for libg15render.h... no configure: WARNING: The g15driver needs libg15render.h checking for main in -lirman... no configure: WARNING: The irman driver needs the irman library. checking for main in -llirc_client... no configure: WARNING: The lirc driver needs the lirc client library checking for vga.h... no configure: WARNING: The svga driver needs vga.h and vgagl.h checking for main in -lftdi... no configure: WARNING: The ula200 driver needs the ftdi library ... and others. They look to me like missing build requires, or at least screaming for a couple more packages needed. I do not know about some of them, but lirc at least is available in Fedora, any reason to not use it? - rpmlint has a small cosmetic complaint: W: lcdproc mixed-use-of-spaces-and-tabs (spaces: line 2, tab: line 1) - however rpmlint of lcdproc is a bit more serious: W: lcdproc service-default-enabled /etc/rc.d/init.d/lcdproc --> please fix that, a one line sed to patch the script should do it W: lcdproc no-reload-entry /etc/rc.d/init.d/lcdproc --> can be ignored, but it would be nice to update the initscript instead W: lcdproc service-default-enabled /etc/rc.d/init.d/LCDd W: lcdproc no-reload-entry /etc/rc.d/init.d/LCDd -->same as above - the postuninstall scriptlet is not particulary friendly. Shouldn't the service be stopped prior to deleting it? - I did not look very attentive, but there seem to exist some additional docs and programs/examples available (and built by the make scripts). Any particular reason to not include those (eventually as separate packages) ?
Well thx for you interest! I was working on 5.2 and noticed some of theses errors.. I will update another version tomorrow.. thx Nicolas
SRPMS: http://kwizartchitecture.free.fr/fedora/7/testing/lcdproc/lcdproc-0.5.2-1.kwizart.fc7.src.rpm SPECS: http://kwizartchitecture.free.fr/fedora/7/testing/lcdproc/lcdproc.spec Description: LCDproc displays real-time system information on a 20x4 backlit LCD There is others BR that are not present in Fedora yet, indeed. I May take a look on these, but this is not the first topic on my TODO list. lcdproc can be used for freevo (currently testing on my test repository) Since i do not own such lcd screen i'm not able to test it.. I will provide it on my repository until someone report it work fine... Feel free to enahance this spec file... (about doc , i'v installed them but not uses a separate sub-package, this could be done eventually - examples are installed in bindir but they uses name that are too generics, there is maybe a need to rename them...)
Created attachment 155060 [details] F7 x86_64 mock build.log
Not very important but we have a duplicate BR: libX11-devel (by xosd-devel), libXext-devel (by xosd-devel). Package Review ============== Key: - = N/A x = Check ! = Problem ? = Not evaluated === REQUIRED ITEMS === [x] Package is named according to the Package Naming Guidelines. [x] Spec file name must match the base package %{name}, in the format %{name}.spec. [x] Package meets the Packaging Guidelines. [x] Package successfully compiles and builds into binary rpms on at least one supported architecture. Tested on:devel/x86_64 [x] Rpmlint output: empty for all packages [x] Package is not relocatable. [x] Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)) [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x] License field in the package spec file matches the actual license. License type:GPL [x] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [x] Spec file is legible and written in American English. [x] Sources used to build the package matches the upstream source, as provided in the spec URL. SHA1SUM of package: 924fca84eb5a07464a3d38df86d9a4427fd06dd7 [x] Package is not known to require ExcludeArch, OR: Arches excluded: Why: [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [-] The spec file handles locales properly. [-] ldconfig called in %post and %postun if required. [x] Package must own all directories that it creates. [-] Package requires other packages for directories it uses. [x] Package does not contain duplicates in %files. [x] Permissions on files are set properly. [x] Package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [x] Package consistently uses macros. [x] Package contains code, or permissable content. [ ] Large documentation files are in a -doc subpackage, if required. [x] Package uses nothing in %doc for runtime. [-] Header files in -devel subpackage, if present. [-] Static libraries in -devel subpackage, if present. [-] Package requires pkgconfig, if .pc files are present. [-] Development .so files in -devel subpackage, if present. [-] Fully versioned dependency in subpackages, if present. [-] Package does not contain any libtool archives (.la). [-] Package contains a properly installed %{name}.desktop file if it is a GUI application. [x] Package does not own files or directories owned by other packages. === SUGGESTED ITEMS === [x] Latest version is packaged. [x] Package does not include license text files separate from upstream. [-] Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x] Reviewer should test that the package builds in mock. Tested on:devel/x86_64 [x] Package should compile and build into binary rpms on all supported architectures. Tested on:devel/x86_64 [! ] Package functions as described. See Note 1 [x] Scriptlets must be sane, if used. [-] The placement of pkgconfig(.pc) files are correct. [x] File based requires are sane. === Issues === 1. === Final Notes === 1. I have not the ability to actually test the program for the simple reason that I do not have access to the required hardware. 2. Acording to the build log, there are some unfulfilled dependencies. However none of the needed resources seem to be available in Fedora, so unless someone adds packages with them, the situation is as good as it could be. 3. Given the rather large number of files, I would split the developer-guide and the user-guide to a separate -doc package. However, I would not consider this as a blocker, because the mere size is small. I see no blockers so I unless someone else sees something I've missed, I approve this package. ================ *** APPROVED *** ================
New Package CVS Request ======================= Package Name: lcdproc Short Description: LCDproc displays real-time system information on a 20x4 backlit LCD Owners: kwizart@gmail.com Branches: FC-6 F-7 devel InitialCC: