Spec URL: http://www.theknight.co.uk/releases/tzclock.spec SRPM URL: http://www.theknight.co.uk/releases/tzclock-2.51-1.src.rpm Description: While working aboard I looked for a nice graphical clock that could display the time for many locations. Because I could not find one that did all that I needed I developed TzClock.
A few comment: - How about a desktop file? - Would be nice to use %configure and %{_bindir}. - changelog entry would be nice too - you should not use the Packager field in fedora packages: your name will be in the changelog - url to source tarball should be used - I would replace "X" by "Cairo" say in the Summary. You probably want to run rpmlint on the srpm and rpm to check for more things. Very cool clock! :-)
(In reply to comment #1) > A few comment: > - How about a desktop file? > - Would be nice to use %configure and %{_bindir}. > - changelog entry would be nice too > - you should not use the Packager field in fedora packages: your name will be in > the changelog > - url to source tarball should be used > - I would replace "X" by "Cairo" say in the Summary. > > You probably want to run rpmlint on the srpm and rpm to check for more things. > > Very cool clock! :-) Thank you for your comments, I am still learning about building rpms so all help is welcome. I have updated the spec file, and uploaded a new version. I am interested in your question about a desktop file. Where can I find out more about these? Thanks again, Chris.
Guidelines are very useful. :) http://fedoraproject.org/wiki/Packaging/Guidelines#head-254ddf07aae20a23ced8cecc219d8f73926e9755
Greak link, the new version with a .desktop file has been uploaded. http://www.theknight.co.uk/releases/tzclock-2.5.3-3.src.rpm Thanks for your help, Chris
Thanks for the updates - looks better now. :) I would also suggest now removing the superfluous definitions for %name, %version and %release at the top - you will get those for free from the Name, Version and Release fields. :)
Fixed it :-) It was taken from an example on the Mandriva site, I should not trust them ;-)
Above link is broken so I use this one: http://www.theknight.co.uk/releases/tzclock-2.5.4-1.src.rpm * rpmlint is not silent: tzclock-debuginfo.x86_64: E: empty-debuginfo-package tzclock.src: E: invalid-spec-name TzClock.spec * you should use %{?dist} macro in Release tag * package fails to build in mock; missing BR: libgnomeui-devel * you should include AUTHORS and COPYING files in %doc * IMO you should change all names to lowercase to improve usability * IANAL but this doesn't look like GPLv2 /*----------------------------------------------------------------------------------------------------* * * * T Z C L O C K G T K . C * * ========================= * * * * All rights reserved. Reproduction, modification, use or * * disclosure to third parties without express authority is * * forbidden. Copyright Teligent AB, Sweden, 2006. * * * *----------------------------------------------------------------------------------------------------*/
(In reply to comment #7) > * rpmlint is not silent: > tzclock-debuginfo.x86_64: E: empty-debuginfo-package debuginfo package removed. > tzclock.src: E: invalid-spec-name TzClock.spec spec file reverted to lowercase. > * you should use %{?dist} macro in Release tag Done, always wondered how it should be done. > * package fails to build in mock; missing BR: libgnomeui-devel Added build requirement, all this for a link on the about box. > * you should include AUTHORS and COPYING files in %doc Done. > * IMO you should change all names to lowercase to improve usability spec file at least is lowercase. > * IANAL but this doesn't look like GPLv2 Oooops my auto comment program caught me out. Thanks for your feedback, it is very helpful.
BTW, simply disabling the debuginfo package isn't usually the right thing to do. You need to understand why it's ending up empty. Either the package has no binaries, the proper CFLAGS aren't getting passed to the compiler, or something is stripping the binaries. All of these indicate some problem with the package.
(In reply to comment #9) > the proper CFLAGS aren't getting passed to the compiler, This applies here. The Makefile.am is broken. It doesn't apply *_CFLAGS correctly.
(In reply to comment #9) > BTW, simply disabling the debuginfo package isn't usually the right thing to do. > You need to understand why it's ending up empty. Either the package has no > binaries, the proper CFLAGS aren't getting passed to the compiler, or something > is stripping the binaries. All of these indicate some problem with the package. (In reply to comment #10) > (In reply to comment #9) > > the proper CFLAGS aren't getting passed to the compiler, > > This applies here. The Makefile.am is broken. It doesn't apply *_CFLAGS correctly. > Well I tried to rebuild 2.5.4-2 (a bit modification needed for meaningless line "%debug_package %{nil}") http://koji.fedoraproject.org/koji/taskinfo?taskID=178885 It seems that fedora compilation flags are correctly honored http://koji.fedoraproject.org/koji/getfile?taskID=178885&name=build.log But the line: ------------------------------------------------------------ + install -s -m 755 TzClockCairo /var/tmp/tzclock-2.5.4-2.2.fc8-buildroot/usr/bin/TzClock ------------------------------------------------------------ is actually problematic.
One more note: When I tried to remove "-s" option from "install", the result is http://koji.fedoraproject.org/koji/taskinfo?taskID=178880 http://koji.fedoraproject.org/koji/getfile?taskID=178880&name=build.log ----------------------------------------------------- + /usr/lib/rpm/find-debuginfo.sh /builddir/build/BUILD/tzclock-2.5.4 extracting debug info from /var/tmp/tzclock-2.5.4-2.1.fc8-buildroot/usr/bin/TzClock 260 blocks -----------------------------------------------------
(In reply to comment #12) I have removed the -s from the latest tzclock.spec and put the debuginfo back in: http://www.tzclock.org/releases/tzclock.spec Thanks again for all you help. I hope to right this up one day so others can learn from my mistakes.
(In reply to comment #11) > (In reply to comment #9) > (In reply to comment #10) > > (In reply to comment #9) > > > the proper CFLAGS aren't getting passed to the compiler, > > > > This applies here. The Makefile.am is broken. It doesn't apply *_CFLAGS correctly. > > > > It seems that fedora compilation flags are correctly honored > http://koji.fedoraproject.org/koji/getfile?taskID=178885&name=build.log Not for me. This Makefile.am is broken. It uses *_CFLAGS = -DXXX this overrides automake's internal CFLAGS and violates automake working-principles. Correct usage would be: *_CFLAGS = $(AM_CFLAGS) <more-flags> Further abuse: -DXXX are CPPFLAGS. They do not belong into CFLAGS, they'd belong into *_CPPFLAGS For detail consult the fine manual (info Automake).
(In reply to comment #14) From reading the manual and going on what you are saying the mistake is using *_CFLAGS at all. Because -D is a pre-processor command it should be included in the *_CPPFLAGS. The flags can be modified without causing problems so... TzClockCairo_CPPFLAGS = -DBUILDCAIRO ...is OK and follows the automake rules.
(In reply to comment #15) > (In reply to comment #14) > From reading the manual and going on what you are saying the mistake is using > *_CFLAGS at all. Not quite. There are 2 small bugs interacting at the same time. 1) By using *_CFLAGS, you are _overriding_ automake's default CFLAGS's behavior. What you intend to do is extending CFLAGS. To achieve this you need to add $(AM_CFLAGS) to *_CFLAGS. 2) Passing preprocessor flags though *CFLAGS. Autoconf and automake distinguish preprocessor flags from "<language>-FLAGS". (CFLAGS, CXXFLAGS etc.) > Because -D is a pre-processor command it should be included in > the *_CPPFLAGS. Right. > The flags can be modified without causing problems so... > > TzClockCairo_CPPFLAGS = -DBUILDCAIRO > > ...is OK and follows the automake rules. Right. In particular, it avoids interferring with CFLAGS ;)
(In reply to comment #16) I have updated the sources: http://theknight.co.uk/releases/tzclock-2.5.4-4.src.rpm This is a good learning experience! :-)
Chris, It seems you are not in cvsextras member and this is the first review request you submitted. Do you have any sponsor?
(In reply to comment #18) Hello, you are right this is my first review request and I don't have a sponsor. I have been a commercial coder for 20+ years working on nearly all the different PC OS's. I am currently developing telecommunications applications than run on Redhat EL. Hence my interest in Fedora and giving something back to the community.
In this case I can't approve this package because I'm not a sponsor.
As this route is proving hard I have added it to GnomeFiles: http://www.gnomefiles.org/app.php/TzClock
Chris, thanks for your patience. Please see <http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored>.
Well, as no one is reviewing: For 2.5.4-4: * Timestamp - Please keep timestamps on installed files when possible (check the section "Timestamps" of http://fedoraproject.org/wiki/Packaging/Guidelines ) In short, when using "cp" or "install" command, please use "-p" option. * desktop-file-install vendor - We still recommend "fedora" vendor prefix ("desktop-file-install usage" section of the URL above). * macro - %_datadir/man should be %_mandir * %defattr - We now recommend %defattr(-,root,root,-) Then:
(In reply to comment #23) > Then: (sorry, my comments follow:) 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 collection review requests which are waiting for someone to review can be checked on: http://fedoraproject.org/PackageReviewStatus/NEW.html (NOTE: please don't choose "Merge Review") Review guidelines are described mainly on: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines http://fedoraproject.org/wiki/Packaging/Guidelines http://fedoraproject.org/wiki/Packaging/ScriptletSnippets ------------------------------------------------------------
(In reply to comment #23) > * desktop-file-install vendor > - We still recommend "fedora" vendor prefix > ("desktop-file-install usage" section of the URL above). Is there a macro that could be used to get the vendor (with a value of fedora)? I would like to have only one spec file for all distributions. I guess the other option is to add it to configure.
(In reply to comment #25) > (In reply to comment #23) > > > * desktop-file-install vendor > > - We still recommend "fedora" vendor prefix > > ("desktop-file-install usage" section of the URL above). > > Is there a macro that could be used to get the vendor (with a value of fedora)? I guess there is not. > I would like to have only one spec file for all distributions. Please don't stick to this idea. One more comment * %?dist tag in %changelog - Remove %?dist tag from %changelog. %?dist tag changes when Fedora version changes, which means that %changelog is changed when Fedora version changes.
ping?
(In reply to comment #26) Sorry for the slow reply. I did find a vendor in one of the macros but it was set to Readhat. Maybe one day all the distro's could get together to come up with standard macros so that a common spec file would be possible. Anyway I have uploaded a new spec file: http://tzclock.org/releases/tzclock.spec There is still one dist in the change log to stop a warning from rpmlint. Other wise all your suggestions are in there. Thank you for looking, Chris
Well, ! From next time, please make srpm and post the URL for the srpm. * %{?dist} tag in the last %changelog entry is actually not needed. When you rebuild your srpm by mock or on koji, the rpmlint should disappear. So as tzclock package itself is okay, I will wait for your pre-review or another review request as I said in comment 24.
ping again?
Again ping? It may be that this bug will be closed if no response from the reporter is received within ONE WEEK.
Once closing. If someone wants to package this software and import it into Fedora, please file another review request, thanks.
*** This bug has been marked as a duplicate of 411591 ***