Spec URL: http://www.brouhaha.com/~eric/software/fedora/fc6/lcdtest/lcdtest.spec SRPM URL: http://www.brouhaha.com/~eric/software/fedora/fc6/lcdtest/lcdtest-1.06-1.src.rpm Description: lcdtest is a utility to display LCD monitor test patterns. It may be useful for adjusting the pixel clock frequency and phase on LCD monitors when using analog inputs, and for finding pixels that are stuck on or off. lctest uses the SDL library.
Hi Eric there are a few problems with the spec, but luckily they are easy to fix - BuildRoot should have one of the values listed at http://fedoraproject.org/wiki/Packaging/Guidelines#head-b4fdd45fa76cbf54c885ef0836361319ab962473 - The spec must include a %clean section. Please add %clean rm -rf $RPM_BUILD_ROOT after the %install section - Using %defattr(-,root,root,-) is saner Please also note that SDL-devel is already pulled in by SDL_image-devel, so there is no need to BR it. And now the sad part: mock build (building for F7/x86_64) fails with the following error: + scons destdir=/var/tmp/lcdtest-1.06-build bindir=/usr/bin mandir=/usr/share/man/man1 scons: Reading SConscript files ... <type 'exceptions.AttributeError'>: SConsEnvironment instance has no attribute 'MergeFlags': File "SConstruct", line 139: exports = {'env' : env}) File "/usr/lib/scons/SCons/Script/SConscript.py", line 581: return apply(method, args, kw) File "/usr/lib/scons/SCons/Script/SConscript.py", line 508: return apply(_SConscript, [self.fs,] + files, {'exports' : exports}) File "/usr/lib/scons/SCons/Script/SConscript.py", line 239: exec _file_ in stack[-1].globals File "src/SConscript", line 67: env.MergeFlags (os.environ ['CFLAGS']) error: Bad exit status from /var/tmp/rpm-tmp.75447 (%build) RPM build errors: Bad exit status from /var/tmp/rpm-tmp.75447 (%build)
Build fails in FC6, too. Also, I have just noticed that you do not use %{?_smp_mflags}
Thank you for reviewing my package. I have updated the spec based on the review, and made the new spec and SRPM available: Spec URL: http://www.brouhaha.com/~eric/software/fedora/fc6/lcdtest/lcdtest.spec SRPM URL: http://www.brouhaha.com/~eric/software/fedora/fc6/lcdtest/lcdtest-1.06-3.src.rpm I build with mock on FC6. Apparently the reason it failed to build for you is that it is dependent on SCons >= 0.97, which was just updated in FE6 recently. I have added that version dependency to the spec.
Excellent, things are almost fine. Some cosmetic fixes are needed to make rpmlint happy: Source RPM: W: lcdtest summary-not-capitalized displays monitor test patterns ->Obvious fix W: lcdtest rpm-buildroot-usage %build destdir=$RPM_BUILD_ROOT \ ->Ignorable, scons needs this W: lcdtest macro-in-%changelog _smp_mflags ->use %% instead of % W: lcdtest mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 11) ->please stick with either spaces or tabs, if possible rpmlint of lcdtest: W: lcdtest summary-not-capitalized displays monitor test patterns -> will be fixed in the same time with the src.rpm Except for the above (and the fact that scons-0.9.7 has not yet been pushed in devel) everything seems fine, including using the program on FC6/x86_64. Have you not have been in the position of needing a sponsor I would have approved the package. here comes a formal review, to help potential sponsors: GOOD - package meets naming guidelines - package meets packaging guidelines - license (GPL) OK, text in %doc, matches source - spec file legible, in am. english - source matches upstream, is latest version, SHA1SUM is 9da6385dad834ae8073bb0ef7620e32122cec4d9 - package compiles on FC6 (x86_64) [*] - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - %clean ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - no need for .desktop file NEEDSWORK - cosmetic changes of spec, see on top of this comment [*] as of 25.05.2007, needed BR scons>=0.9.7 is not yet available in devel, probably due to F7 freeze
Some quick notes: * Disttag - Adding %{?dist} to release number is recommended for easier maintainence on different branches. http://fedoraproject.org/wiki/Packaging/DistTag * Unneeded Requires: --------------------------------------------------- Requires: SDL, SDL_image --------------------------------------------------- - This is unnessary (and *should not* needed) as rpmbuild checks libraries' dependency and automatically adds them to Requires. * Unneeded macros ----------------------------------------------------- %define _prefix /usr %define _bindir %{_prefix}/bin %define _mandir %{_prefix}/share/man ------------------------------------------------------ - These macros are already defined and are not needed. * And read the comment 4 by wolfshant-san.
Thank you for reviewing my package. I have updated the spec based on the reviews, and made the new spec and SRPM available: Spec URL: http://www.brouhaha.com/~eric/software/fedora/fc6/lcdtest/lcdtest.spec SRPM URL: http://www.brouhaha.com/~eric/software/fedora/fc6/lcdtest/lcdtest-1.07-2.fc6.src.rpm Latest changelog entries: * Tue Jun 05 2007 Eric Smith <eric> 1.07-2 - added dist tag - removed unnecessary macros - removed unnecessary Requires * Sat May 26 2007 Eric Smith <eric> 1.07-1 - update to 1.07 - capitalize summary - use spaces consistently in spec rather than tabs
Updated to upstream release 1.08, adds a new diagonal crosshatch test pattern. Spec URL: http://www.brouhaha.com/~eric/software/fedora/fc6/lcdtest/lcdtest.spec SRPM URL: http://www.brouhaha.com/~eric/software/fedora/fc6/lcdtest/lcdtest-1.08-1.fc6.src.rpm
Okay. Assigning to me. I have not checked your newest srpm in detail yet, however as far as I glance at your spec file, this package seems almost okay. Then: ------------------------------------------------------------- NOTE: Before being sponsored: This package will be accepted with another few work. But before I accept this package, someone (I am a candidate) must sponsor you. Once you are sponsored, you have the right to review other submitters' review requests and approve the packages formally. For this reason, the person who want to be sponsored (like you) are required to "show that you have an understanding of the process and of the packaging guidelines" as is described on : http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored Usually there are two ways to show this. A. submit other review requests with enough quality. B. Do a "pre-review" of other person's review request (at the time you are not sponsored, you cannot do a formal review) When you have submitted a new review request or have pre-reviewed other person's review request, please write the bug number on this bug report so that I can check your comments or review request. Fedora package review requests which are waiting for someone to review can be checked on: https://bugzilla.redhat.com/bugzilla/buglist.cgi?cmdtype=runnamed&namedcmd=mtasaka-review-noone Review guidelines are described mainly on: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines http://fedoraproject.org/wiki/Packaging/Guidelines http://fedoraproject.org/wiki/Packaging/ScriptletSnippets ------------------------------------------------------------
I have two other review requests pending: asl - macro assembler - review nearly completed https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=240807 drawtiming - Generates digital signal timing diagrams from a textual description still needs a reviewer https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=241971 asl and lcdtest were my first two attempts at contributing packages, and as such my first submissions had problems, but I've learned a lot about the requirements and think they are now in good shape. I now routinely use mock and rpmlint.
FYI, the link you supplied for review requests waiting for a reviewer does not work for me; it returns a message "The search named mtasaka-review-noone does not exist." Possibly this URL might be suitable instead: https://bugzilla.redhat.com/bugzilla/showdependencytree.cgi?id=163776
(In reply to comment #10) > FYI, the link you supplied for review requests waiting for a reviewer does not > work for me; it returns a message "The search named mtasaka-review-noone does > not exist." Still does not work for you? Maybe the link can be used only by me? > Possibly this URL might be suitable instead: > https://bugzilla.redhat.com/bugzilla/showdependencytree.cgi?id=163776 FE-NEW bugtracker is no longer used (we are still wanting for the other methods which can replace FE-NEW bugtracker, and mtasaka-review-noone mark works for me...) Well, * This package is okay * From a quick glance, the other reviews you requested seem almost okay ! NOTE: - For asl: ------------------------------------------------ make install %{_smp_mflags} DESTDIR=$RPM_BUILD_ROOT ------------------------------------------------ for make install, while I have not checked asl, there are many packages where parallel make for "make install" causes race condition problem (for example, the timing of "making directory" and "putting files in the directory") - For drawtiming: ------------------------------------------------- ./configure CFLAGS="$RPM_OPT_FLAGS" \ --bindir=%{_bindir} \ --libdir=%{_libdir} \ --mandir=%{_mandir} -------------------------------------------------- - Perhaps the above can be replaced with just %configure? (please check what %configure does by `rpm --eval %configure`) Okay. ------------------------------------------------- This package (lcdtest) is APPROVED by me ------------------------------------------------- Please follow the procedure written on http://fedoraproject.org/wiki/PackageMaintainers/Join from "Get a Fedora Account" When you requested someone to sponsor you (in the procedure above), please make a note on this bug that you did so. If you want to push this package also on F-7, you also have to check: http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT after the URL above. !! Well, recenctly Fedora package system changed a lot !! If you have some questions, please let me know.
If you're looking for open review tickets, this will be faster than making complicated bugzilla queries: http://fedoraproject.org/PackageReviewStatus/NEW.html
I have registered for a Fedora account, signed and submitted the Contributor license Agreement, verified that the account shows cla_done, and requested to join groups cvsextras and fedorabugs. Thank you for sponsoring me. I will make the changes you have requested to my submitted packages.
Okay. Now I am sponsoring you.
Are you meeting some trouble?
No trouble, I've just been a bit short on spare time this past week. I should have the latest round of changes to this and my other two submitted packages completed and checked into CVS later this week. Thanks! Eric
Again, ping?
ping again?
again?
Once removing my sponsorship. This bug will be closed if no response is received from the reporter within ONE WEEK.
Closing. If someone wants to import this package into Fedora, please file a new review request, thank you!
*** This bug has been marked as a duplicate of bug 559117 ***