205891 – Review Request: rpl - Intelligent recursive search/replace utility

Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.

Bug 205891 - Review Request: rpl - Intelligent recursive search/replace utility

Summary: Review Request: rpl - Intelligent recursive search/replace utility

Keywords:
Status:	CLOSED NEXTRELEASE
Alias:	None
Product:	Fedora
Classification:	Fedora
Component:	Package Review
Sub Component:
Version:	rawhide
Hardware:	All
OS:	Linux
Priority:	medium
Severity:	medium
Target Milestone:	---
Assignee:	Dan Horák
QA Contact:	Fedora Package Reviews List
Docs Contact:
URL:
Whiteboard:
Depends On:
Blocks:	FE-ACCEPT
TreeView+	depends on / blocked

Reported:	2006-09-09 16:53 UTC by Tim Jackson
Modified:	2007-11-30 22:11 UTC (History)
CC List:	0 users
Fixed In Version:
Doc Type:	Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed:	2006-09-10 11:19:18 UTC
Type:	---
Embargoed:
Dependent Products:
Flags:	wtogami: fedora-cvs+

Attachments	(Terms of Use)

Description Tim Jackson 2006-09-09 16:53:06 UTC

Spec URL: http://www.timj.co.uk/linux/specs/rpl.spec
SRPM URL: http://www.timj.co.uk/linux/srpms/rpl-1.5.3-1.src.rpm
Description:
rpl is a UN*X text replacement utility. It will replace strings with
new strings in multiple text files. It can work recursively over
directories and supports limiting the search to specific file
suffixes.

rpl was originally written by Joe Laffey; this is a rewritten version.

Comment 1 Tim Jackson 2006-09-09 16:55:45 UTC

rpmlint is silent on both source and noarch packages.

Comment 2 Dan Horák 2006-09-09 17:59:28 UTC

the mode for the man page rpl.1 should be 0644

Comment 3 Tim Jackson 2006-09-09 18:12:58 UTC

Of course it should. Sorry about that stupid bug.

Spec URL: http://www.timj.co.uk/linux/specs/rpl.spec
SRPM URL: http://www.timj.co.uk/linux/srpms/rpl-1.5.3-2.src.rpm

Comment 4 Dan Horák 2006-09-09 18:25:07 UTC

I have looked at the Requires of the built package and there is only
/usr/bin/env found by rpmbuild process (it is after #! in the beginning of the
script). There should be "python" too. A simple solution could be a change of
the first line of script and adding it as a patch.

Comment 5 Tim Jackson 2006-09-09 18:48:18 UTC

Hmm, yes. I don't really see the point of the "env python" there since no
variables are being set.
For now I've patched it to remove the "env" (works OK for me) and I've e-mailed
the author to ask about the reasoning.

New package:

Spec URL: http://www.timj.co.uk/linux/specs/rpl.spec
SRPM URL: http://www.timj.co.uk/linux/srpms/rpl-1.5.3-3.src.rpm

Comment 6 Jason Tibbitts 2006-09-10 00:10:56 UTC

The reason env is used is that it will search PATH to find the command it is
given.  This means that you don't have to edit your scripts if python is in
/usr/bin/python or /usr/local/bin/python or whatever.  It has, as far as I know,
always been common practise for python scripts to be called this way.

In general I think it would be better to add Requires: python rather than patch
the source, but I suppose either method works.

I'm happy to go ahead and review this, unless Dan wants to.

Comment 7 Dan Horák 2006-09-10 08:09:48 UTC

Review:
- no rpmlint output on any package
- package name OK
- spec file name OK, is in English and is legible
- package meets the Packaging Guidelines
- license OK (GPL) and is included
- source matches upstream
	7187b2c650e1522a82242c2c7f5e7123  rpl_1.5.3.tar.gz
	7187b2c650e1522a82242c2c7f5e7123  rpl_1.5.3.tar.gz.1
- builds at least on i386 (FC4 and devel)
- no BuildRequires needed
- no localized files
- has no shared lib
- do not create any directory
- no duplicates files, permissions are set properly, uses %defattr
- has %clean section
- consistent use of macros
- contains code
- no large docs, %doc is not required during runtime
- no devel subpackage required, no pkgconfig file
- no .la libtool archives
- not a GUI application
- it works

APPROVED

Comment 8 Tim Jackson 2006-09-10 11:19:18 UTC

Thanks for the feedback and comments, and for the review Dan.
Imported into Extras and built for devel (FC-5 branch requested)

Jason, your comments re:env made sense, so I removed the hack in favour of
"Requires: python".

Comment 9 Tim Jackson 2007-04-14 23:18:50 UTC

Package Change Request
======================
Package Name: rpl
New Branches: EL-4 EL-5

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