Bug 168032

Summary: Review Request: gnome-applet-timer - Gnome Timer Applet
Product: [Fedora] Fedora Reporter: Christoph Wickert <christoph.wickert>
Component: Package ReviewAssignee: Thorsten Leemhuis <fedora>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhide   
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
URL: http://timerapplet.sourceforge.net/
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2005-09-26 14:13:52 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 163779    
Attachments:
Description Flags
Spec file hack to get omf file installed
none
timer-applet real documentation fix
none
Fix the installed location of the applet
none
Hack to install omf and applet to libexec from specfile none

Description Christoph Wickert 2005-09-11 03:23:09 UTC
Spec Name or Url: http://home.arcor.de/christoph.wickert/fedora/extras-review/SPECS/gnome-timer-applet.spec
SRPM Name or Url: http://home.arcor.de/christoph.wickert/fedora/extras-review/i386/gnome-timer-applet-1.1.1-2.fc4.i386.rpm
Description: Timer Applet is a countdown timer applet for the GNOME panel.

I wonder if this package should be named gnome-timer-applet or gnome-applet-timer!?

This is my first package for FE, so I'm looking for a sonsor. ATM the rpm is hosted at sf.net.

Comment 1 Thorsten Leemhuis 2005-09-11 16:30:30 UTC
Please change 
>Source0:       
http://prdownloads.sourceforge.net/timerapplet/timer-applet-%{version}.tar.gz
to
Source0:       
http://download.sourceforge.net/timerapplet/timer-applet-%{version}.tar.gz
so download works in wget (after replacing version)


>%description
>Timer Applet is a countdown timer applet for the GNOME panel.
Isn't there anything more to tell? This is nearly exactly the same as in the Summary


Remove
>Requires:       gnome-panel
rpmbuild finds this requirement (libpanel-applet-2.so.0, provided by
gnome-panel-2.10.1-10.2) on its own


Mmm, this is not required but sounds okay to me:
>Provides:	timer-applet = %{version}
Maybe we should add this to the "Package Naming Guidelines" for applets that
have a differnet name upstream?


What is the reason for 
>rm -f $RPM_BUILD_ROOT%{_infodir}/dir


During build I see this:
----
for file in timer-applet-C.omf; do \
  scrollkeeper-preinstall /usr/share/gnome/help/timer-applet/C/`awk 'BEGIN {RS =
">" } /identifier/ {print $0}' ${file} | awk 'BEGIN {FS="\""} /url/ {print $2}'`
${file} ../../omf-install/${file}; \
done
I/O error : No such file or directory
touch omf_timestamp
make[3]: Leaving directory `/home/rpmqa/rpmbuild/BUILD/timer-applet-1.1.1/help/C'
Making all in fr
/home/rpmqa/rpmbuild/BUILD/timer-applet-1.1.1/help/fr
make[3]: Entering directory `/home/rpmqa/rpmbuild/BUILD/timer-applet-1.1.1/help/fr'
for file in timer-applet-fr.omf; do \
  scrollkeeper-preinstall /usr/share/gnome/help/timer-applet/fr/`awk 'BEGIN {RS
= ">" } /identifier/ {print $0}' ${file} | awk 'BEGIN {FS="\""} /url/ {print
$2}'` ${file} ../../omf-install/${file}; \
done
I/O error : No such file or directory
touch omf_timestamp
----
and this
----
for file in ./figures/*.png; do \
  basefile=`echo $file | sed -e  's,^.*/,,'`; \
  /usr/bin/install -c -m 644 $file
/var/tmp/gnome-timer-applet-1.1.1-2-root-rpmqa/usr/share/gnome/help/timer-applet/C/figures/$basefile;
\
done
/usr/bin/install: cannot stat `./figures/*.png': No such file or directory
----
Can you check what's wrong there?


(In reply to comment #0)
> I wonder if this package should be named gnome-timer-applet or
gnome-applet-timer!?
According to the Package Naming Guidelines is should be gnome-applet-timer. I
first thought we should make a exception here, because gnome-timer-applet is
closer to the upstream name and (not that important) the mandrake-package is
named gnome-timer-applet, too. But I think I changed my mind: IMHO it should be
named gnome-applet-timer.

Readers on fedora-extras-list: What's your opinion on this?

Comment 2 Christoph Wickert 2005-09-16 12:42:24 UTC
updated Specfile:
http://home.arcor.de/christoph.wickert/fedora/extras-review/SPECS/gnome-applet-timer.spec
new SRPM:
http://home.arcor.de/christoph.wickert/fedora/extras-review/SRPMS/gnome-applet-timer-1.1.1-3.fc4.src.rpm

Changes:
- changed name to gnome-applet-time. There has a similar discussion on
fedora-packaging-list, see this thread.
http://www.redhat.com/archives/fedora-packaging/2005-September/msg00053.html
- changed "Source0:" to
http://download.sourceforge.net/timerapplet/timer-applet-%{version}.tar.gz
- extended Description:
- removed gnome-panel Requires:
- added Requires(pre|post|preun|postun): for scrollkeeper
- added scrollkeeper scriptlet sniplets

Questions/Problems/Comments
- name (see above)
- the build errors still exist. These are upstream packages errors due to
missing/incomplede documentation. I've contaced the author of timerapplet.
- "Provides: timer-applet" is only there for upgrading purposes from my previous
package. I can remove it if you like.

Comment 3 Thorsten Leemhuis 2005-09-17 12:25:58 UTC
(In reply to comment #2)
> Changes:
> - extended Description:

Now it's a nearly to long :| But it's okay. 

> - added Requires(pre|post|preun|postun): for scrollkeeper
> - added scrollkeeper scriptlet sniplets

Uhhh, why? From you spec:
> scrollkeeper-update -q -o %{_datadir}/omf/%{name} || :
This file is not installed in %files so this will always fail until the build
errors (see below) are resolved. And probably it should be
%{_datadir}/omf/timer-applet and not %{_datadir}/omf/%{name} 

> Questions/Problems/Comments
> - name (see above)
gnome-applet-timer is okay for me

> - the build errors still exist. These are upstream packages errors due to
> missing/incomplede documentation. I've contaced the author of timerapplet.

If they do no real harm we could just ignore them. But then the scrollkeeper
stuff must be removed for now.

> - "Provides: timer-applet" is only there for upgrading purposes from my 
> previous package. I can remove it if you like.

No, in fact I even like it, because people using the upstream name with yum "yum
install" will get the package then, too.

Comment 4 Toshio Kuratomi 2005-09-17 20:28:59 UTC
Created attachment 118936 [details]
Spec file hack to get omf file installed

It should be easy enough to install the omf files into the proper directories
for now and then add scrollkeeper -q -o %{_datadir}/omf/timer-applet || : in
the scriptlets.

I'm attaching a patch to the spec file for that solution.

Comment 5 Toshio Kuratomi 2005-09-17 20:39:31 UTC
Created attachment 118937 [details]
timer-applet real documentation fix

Here's the real fix to the timer-applet install problem.  It updates the doc
install scripts to a more recent one from gnome-common.  It requires running
autoreconf to regenerate the Makefile's though.  I'll send this upstream with
the note that autoreconf is necessary to run before the next upstream tarball.

Comment 6 Christoph Wickert 2005-09-17 21:48:08 UTC
Thanks for the patch Toshio! Now timer-applet builds fine.

New SRPM:
http://home.arcor.de/christoph.wickert/fedora/extras-review/SRPMS/gnome-applet-timer-1.1.1-4.fc4.src.rpm
updated SPEC:
http://home.arcor.de/christoph.wickert/fedora/extras-review/SPECS/gnome-applet-timer.spec

- name will be gnome-applet-timer, no more changes
- added timer-applet-docinstall.patch and run autoreconf
- cleaned up the description once again, hope it's short enough now
- added %{_datadir}/omf/timer-applet to %files

Comment 7 Toshio Kuratomi 2005-09-17 23:48:56 UTC
Created attachment 118939 [details]
Fix the installed location of the applet

Here's another patch.  Gnome panel applets normally go into /usr/libexec [1]_

Here's a patch to put the applet in the proper place.  I'm sending this
upstream as well.

If Thorsten is ready to approve, this can go in after the package hits CVS.  If
not, it can go in on the next iteration of fixes.

[1]_
  $ locate applet |grep bin|wc -l
  3
  $ locate applet |grep libexec|wc -l
  36
Also see this post by Havoc on the Debian/Gnome list:
http://lists.debian.org/debian-gtk-gnome/2002/11/msg00428.html

Comment 8 Christoph Wickert 2005-09-18 00:57:57 UTC
applied libexec-patch and included you specfike hack.

uploaded updated specfile and SRPM
http://home.arcor.de/christoph.wickert/fedora/extras-review/SRPMS/gnome-applet-timer-1.1.1-5.fc4.src.rpm

Comment 9 Toshio Kuratomi 2005-09-18 07:00:26 UTC
Err... The specfile hack didn't need to be added.  That was an either-or with
the timer-applet-docinstall.patch.  basically, if you felt like autoreconf
didn't belong in the spec file, then the specfile hack would get the omf file
installed correctly.  Since you're okay with autoreconf, you can just have use
the -docinstall and -libexec patches to fix things.

Comment 10 Thorsten Leemhuis 2005-09-18 08:39:09 UTC
First: Toshio thanks for your help.

(In reply to comment #9)
> Err... The specfile hack didn't need to be added.  That was an either-or with
> the timer-applet-docinstall.patch.  basically, if you felt like autoreconf
> didn't belong in the spec file, then the specfile hack would get the omf file
> installed correctly.

I would prefer not to run autoreconf in the spec file. But Christoph, that's up
to you.

(In reply to comment #7)
> If Thorsten is ready to approve, this can go in after the package hits CVS. 
> If  not, it can go in on the next iteration of fixes.

Let get the package ready first. But Christoph, I think it does no harm if you
sign up in the fedora accounts system and sign the CLA already. I'll sponsor you.

Comment 11 Christoph Wickert 2005-09-18 16:34:03 UTC
> (In reply to comment #9)
> 
> I would prefer not to run autoreconf in the spec file. But Christoph, that's up
> to you.
> 
I'm not really happy with running autoreconf, but it's needed for the
libexec-patch anyway. So I removed the specfile hacks instead, still using both
patches and running autoreconf afterwards. Updated specfile, new SRPM
http://home.arcor.de/christoph.wickert/fedora/extras-review/SRPMS/gnome-applet-timer-1.1.1-6.fc4.src.rpm

> (In reply to comment #7)
> > If Thorsten is ready to approve, this can go in after the package hits CVS. 
> > If  not, it can go in on the next iteration of fixes.
> 
> Let get the package ready first. But Christoph, I think it does no harm if you
> sign up in the fedora accounts system and sign the CLA already. I'll sponsor you.

Thanks, I've created an account, signed the cla and added myself to cvsextras group.

Comment 12 Toshio Kuratomi 2005-09-18 18:54:44 UTC
Created attachment 118948 [details]
Hack to install omf and applet to libexec from specfile

If you want to avoid autoreconf, you can do everything from the spec file
instead of patching the build scripts.	The current 1.1.1-6.fc4 looks good if
you want to use autoreconf (Upstream has replied that he'll look into
incorporating the patches when he has time for the next release.) If you'd
rather avoid autoreconf, this patch will change the spec file to doing things
from the spec instead of via patches to the build scripts.

Comment 13 Christoph Wickert 2005-09-18 21:23:22 UTC
Thanks. Added specfile hacks, removed patches and antoreconf.
http://home.arcor.de/christoph.wickert/fedora/extras-review/SRPMS/gnome-applet-timer-1.1.1-7.fc4.src.rpm

Comment 14 Thorsten Leemhuis 2005-09-19 16:20:11 UTC
Some last things before this can get approved:

Please remove the %{?dist} in the changelog section before importing to cvs. It
is unneeded there. 

rpmlint currently gives:
E: gnome-applet-timer script-without-shellbang
/usr/share/omf/timer-applet/timer-applet-fr.omf
E: gnome-applet-timer script-without-shellbang
/usr/share/omf/timer-applet/timer-applet-C.omf

These files are marked as executable. You should fix that by using "-m 0644"
when installing those.

W: gnome-applet-timer non-standard-dir-in-usr libexec

Can be ignored afaik

W: gnome-applet-timer dangerous-command-in-%post install
Uhh? Seems to be caused by this comment:
# For GConf apps: install schemas as system default
We could ignore this but I think it better to remove it.

Comment 16 Thorsten Leemhuis 2005-09-19 17:37:08 UTC
APPROVED:
0d8fc37810ced0b38e6f6218c424eb80  gnome-applet-timer-1.1.1-8.fc4.src.rpm

Source matches upstream:
db9dacfa6bd35904dc88740764b1750e  timer-applet-1.1.1.tar.gz

rpmlint:
W: gnome-applet-timer non-standard-dir-in-usr libexec
(can be ignored)

Good:
- package meets naming guidelines
- package meets packaging guidelines
- URL and Source url are valid
- license (GPL) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on mach-fc4 (x86)
- no missing BR
- no unnecessary BR
- locales use find_lang
- 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 because it's an applet
- scriptlets match examples from wiki

Comment 17 Christian Iseli 2006-10-18 09:40:56 UTC
Normalize summary field for easy parsing