Bug 430441 - Review Request: xautolock - Launches a program when your X session has been idle
Review Request: xautolock - Launches a program when your X session has been idle
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Patrice Dumas
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-01-27 22:42 EST by Ian Weller
Modified: 2008-03-19 22:45 EDT (History)
5 users (show)

See Also:
Fixed In Version: 2.2-4.fc8
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-02-04 23:52:35 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
pertusus: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Ian Weller 2008-01-27 22:42:37 EST
Spec URL: http://ianweller.org/rpm/xautolock/xautolock.spec
SRPM URL: http://ianweller.org/rpm/xautolock/xautolock-2.2-1.fc8.src.rpm
Description:  A program that launches a given program when
your X session has been idle for a given time.
Comment 1 Rajeesh 2008-02-01 06:29:24 EST
Hi Ian,
   I cannot sponsor you, but I can unofficially review your package.
   rpm was build successfully from srpm and rpmlint didn't complain anything.
   Your spec file also look fine for me, except the point that you are putting
the man file in directory "%{_mandir}/manl" (read man-el), not "man1" (read
man-one). Is that intended, or a typo?
Comment 2 Ian Weller 2008-02-01 10:46:21 EST
i looked at the xautolock man page and it was XAUTOLOCK(l), as in el. so i'm 
presuming this is correct, and it is intentionally done.
Comment 3 Mamoru TASAKA 2008-02-01 21:08:40 EST
Removing NEEDSPONSOR (bug 417711)
Comment 4 Ian Weller 2008-02-01 21:54:04 EST
New Package CVS Request
=======================
Package Name: flam3
Short Description: Launches a program when your X session has been idle for some
time
Owners: ianweller
Branches: F-7 F-8
InitialCC: ianweller
Cvsextras Commits: yes
Comment 5 Ian Weller 2008-02-01 21:55:29 EST
Crud, I messed up the CVS request. Package Name should read `xautolock'.

Here's a new one.
New Package CVS Request
=======================
Package Name: xautolock
Short Description: Launches a program when your X session has been idle for some
time
Owners: ianweller
Branches: F-7 F-8
InitialCC: ianweller
Cvsextras Commits: yes
Comment 6 Mamoru TASAKA 2008-02-01 22:09:02 EST
Ah... no, even if you are sponsored, this request must be
reviewed by someone. The difference is that as you are sponsored,
this review request can be reviewed by non-sponsor members.
Comment 7 Ian Weller 2008-02-01 23:57:05 EST
OK.
Comment 8 Patrice Dumas 2008-02-02 05:15:59 EST
gzip xautolock.man 
should not be done in the spec, it is done automatically by rpmbuild.
Also the permission for man page should be 0644.

Man pages are automatically considered %doc. 

I personally prefer to use globs like
%{_mandir}/manl/xautolock.l*
to catch all possibility of compression and no compression.

The l for man page is a bit strange. Currently on my box all the pages
in manl are owned by lapack and blas (and the directory itself is not
owned). So it is likely that it is an upstream mistake. It is not
an issue for the review, but may be worth asking upstream if there is
a typo in the man page.
Comment 9 Patrice Dumas 2008-02-02 05:31:26 EST
To install the man page, there is a make target, install.man,
and it installs in man1, so it is certainly a typo. I tested
that the following works:
make install install.man DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"

Also for the build, the build flags are not the right ones.
I found that the following works:
make %{?_smp_mflags} CDEBUGFLAGS="$RPM_OPT_FLAGS" CC=%{__cc}


The summary is a bit too long in my opinion, but it is not 
a blocker.
Comment 10 Ian Weller 2008-02-02 11:45:08 EST
* Sat Feb 02 2007 Ian Weller <ianweller@gmail.com> 2.2-2
- Shortened summary
- Fixed make flags
- Fixed installation of man page

Spec: http://ianweller.fedorapeople.org/SRPMS/xautolock/2.2-2/xautolock.spec
SRPM:
http://ianweller.fedorapeople.org/SRPMS/xautolock/2.2-2/xautolock-2.2-2.fc8.src.rpm

Comment 11 Ian Weller 2008-02-02 15:54:12 EST
koji build ok
http://koji.fedoraproject.org/koji/taskinfo?taskID=391426
Comment 12 Patrice Dumas 2008-02-02 18:16:44 EST
Source archive timestamp ois not kept:

$ ls -l ./xautolock-2.2.tgz ../SOURCES/xautolock-2.2.tgz 
-rw-r--r-- 1 dumas dumas 35288 jan 27 23:18 ../SOURCES/xautolock-2.2.tgz
-rw-rw-r-- 1 dumas dumas 35288 déc 31 23:36 ./xautolock-2.2.tgz

You can use wget -N or spectool -g on the spec file.


Also the install is now done twice, once in 
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
and a second time in
make install install.man DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
(when make is given 2 targets, it does both).
Comment 13 Ian Weller 2008-02-02 23:05:41 EST
whoops. i shoulda seen that...

* Sat Feb 02 2007 Ian Weller <ianweller@gmail.com> 2.2-3
- Fixed timestamp of Source0
- Removed redundant installation
spec: http://ianweller.fedorapeople.org/SRPMS/xautolock/2.2-3/xautolock.spec
srpm:
http://ianweller.fedorapeople.org/SRPMS/xautolock/2.2-3/xautolock-2.2-3.fc8.src.rpm
Comment 14 Patrice Dumas 2008-02-03 05:34:29 EST
I get a 404 not found....
Comment 15 Terje Røsten 2008-02-03 13:29:31 EST
Is this correct:
Group:          Applications/Internet


I would changed %files lines to:
%doc Changelog License Readme Todo
%{_bindir}/%{name}
%{_mandir}/man1/%{name}.1*

but that's me.

Comment 16 Ian Weller 2008-02-03 14:15:18 EST
(In reply to comment #14)
> I get a 404 not found....
fedorapeople.org was down for some reason today, i read it on the mailing list.
should work now.

(In reply to comment #15)
> Is this correct:
> Group:          Applications/Internet
no. fixing.

* Sun Feb 03 2008 Ian Weller <ianweller@gmail.com> 2.2-4
- Fixed dates in changelog (2007 -> 2008)
- Fixed grouping
- Tightened up files list

spec: http://ianweller.fedorapeople.org/SRPMS/xautolock/2.2-4/xautolock.spec
srpm:
http://ianweller.fedorapeople.org/SRPMS/xautolock/2.2-4/xautolock-2.2-4.fc8.src.rpm

koji builds: all passed
dist-fc7: http://koji.fedoraproject.org/koji/taskinfo?taskID=393437
dist-f8:  http://koji.fedoraproject.org/koji/taskinfo?taskID=393430
dist-f9:  http://koji.fedoraproject.org/koji/taskinfo?taskID=393442
Comment 17 Patrice Dumas 2008-02-03 17:51:37 EST
* rpmlint is silent
* free software
* follow guidelines
* match upstream:
9526347a202694ad235d731d9d3de91f  xautolock-2.2.tgz
* %files section right

APPROVED
Comment 18 Ian Weller 2008-02-03 18:03:06 EST
New Package CVS Request
=======================
Package Name: xautolock
Short Description: Launches a program when your X session has been idle
Owners: ianweller
Branches: F-7 F-8
InitialCC: ianweller
Cvsextras Commits: yes
Comment 19 Kevin Fenzi 2008-02-04 14:32:15 EST
cvs done.
Comment 20 Ian Weller 2008-02-04 23:52:35 EST
thanks kevin.

srpms imported, koji build ok, added to bodhi
== NEXTRELEASE ==
Comment 21 Ian Weller 2008-02-07 18:19:52 EST
pushed to Fedora 7 and 8 stable repos
(i forgot to link the bug numbers)
Comment 22 Ian Weller 2008-03-19 18:48:33 EDT
Package Change Request
======================
Package Name: xautolock
New Branches: EL-4 EL-5
Comment 23 Kevin Fenzi 2008-03-19 22:45:55 EDT
cvs done.

Note You need to log in before you can comment on or make changes to this bug.