Bug 512106 (rpmconf) - Review Request: rpmconf - Tool to handle rpmnew and rpmsave files
Summary: Review Request: rpmconf - Tool to handle rpmnew and rpmsave files
Keywords:
Status: CLOSED NEXTRELEASE
Alias: rpmconf
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Susi Lehtola
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-07-16 11:34 UTC by Miroslav Suchý
Modified: 2009-08-29 03:28 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-07-20 07:43:11 UTC
Type: ---
Embargoed:
susi.lehtola: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Miroslav Suchý 2009-07-16 11:34:01 UTC
SPEC:
http://miroslav.suchy.cz/fedora/rpmconf/rpmconf.spec
SRPM:
http://cloud.github.com/downloads/xsuchy/rpmconf/rpmconf-0.1.2-1.src.rpm

Description: 
This tool seach for .rpmnew and .rpmsave files and ask you what to do with 
them:
Keep current version, place back old version or watch the diff.

rpmlint is silent

Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1478674

Comment 1 Susi Lehtola 2009-07-16 11:49:44 UTC
- Change 
 BuildRequires: /usr/bin/docbook2man
to
 BuildRequires: docbook-utils
as that package provides the binary at least on F-11 and EL-5.

- Change
 /usr/bin/docbook2man rpmconf.sgml
 /usr/bin/gzip rpmconf.8
to
 docbook2man rpmconf.sgml
(rpm will automatically compress the man page)

- and you can shorten
 mkdir -p $RPM_BUILD_ROOT/%{_mandir}/man8
 mkdir -p $RPM_BUILD_ROOT/%{_usr}/sbin
 install -m 755 rpmconf $RPM_BUILD_ROOT%{_usr}/sbin
 install -m 644 rpmconf.8.gz $RPM_BUILD_ROOT%{_mandir}/man8/
to
 install -D -p -m 755 rpmconf $RPM_BUILD_ROOT%{_sbindir}/rpmconf
 install -D -p -m 644 rpmconf.8 $RPM_BUILD_ROOT%{_mandir}/man8/rpmconf.8
N.B. the use of the %{_sbindir} macro.

- Also, you could write out the file names in %files, i.e.
 %{_sbindir}/rpmconf
 %{_mandir}/man8/rpmconf.8.*
(a wildcard is used for the man page in case the compression format is changed in the future)

Comment 2 Miroslav Suchý 2009-07-16 12:04:59 UTC
All but second suggestion implemented. Second suggestion do not work. At least on Red Hat Enterprise Linux 5, therefore I leave it as is.

New SRPM:
http://cloud.github.com/downloads/xsuchy/rpmconf/rpmconf-0.1.4-1.src.rpm
Update SPEC: http://miroslav.suchy.cz/fedora/rpmconf/rpmconf.spec

Comment 3 Susi Lehtola 2009-07-16 12:11:31 UTC
(In reply to comment #2)
> All but second suggestion implemented. Second suggestion do not work. At least
> on Red Hat Enterprise Linux 5, therefore I leave it as is.

Do you have redhat-rpm-config installed?

Comment 4 Miroslav Suchý 2009-07-16 12:20:17 UTC
$ rpm -q redhat-rpm-config
redhat-rpm-config-8.0.45-29.el5

Comment 5 Susi Lehtola 2009-07-16 12:33:32 UTC
%build
docbook2man rpmconf.sgml

%install
rm -rf $RPM_BUILD_ROOT
install -D -p -m 755 rpmconf $RPM_BUILD_ROOT%{_sbindir}/rpmconf
install -D -p -m 644 rpmconf.8 $RPM_BUILD_ROOT%{_mandir}/man8/rpmconf.8


$ mock -r epel-5-x86_64 rpmconf-0.1.4-1.fc11.src.rpm 
INFO: mock.py version 0.9.16 starting...
State Changed: init plugins
State Changed: start
INFO: Start(rpmconf-0.1.4-1.fc11.src.rpm)  Config(epel-5-x86_64)
State Changed: lock buildroot
State Changed: clean
State Changed: init
State Changed: lock buildroot
Mock Version: 0.9.16
INFO: Mock Version: 0.9.16
INFO: enabled root cache
State Changed: unpacking root cache
INFO: enabled yum cache
State Changed: cleaning yum metadata
INFO: enabled ccache
State Changed: running yum
State Changed: setup
State Changed: build
INFO: Done(rpmconf-0.1.4-1.fc11.src.rpm) Config(epel-5-x86_64) 0 minutes 31 seconds
INFO: Results and/or logs in: /var/mock//epel-5-x86_64/result

so no problem. There must be something missing on your system if you're doing a local build instead of using mock.

Comment 6 Miroslav Suchý 2009-07-16 12:57:59 UTC
OK. I must done something wrong previosly. When I tried it second time it works.
Updated.
Spec on the same place.
SRPM: http://cloud.github.com/downloads/xsuchy/rpmconf/rpmconf-0.1.5-1.src.rpm

Comment 7 Susi Lehtola 2009-07-16 13:30:28 UTC
rpmlint output is clean.

MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK

MUST: The spec file for the package is legible and macros are used consistently. NEEDSWORK
- BuildRoot used is archaic. Use
%(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
(I still don't see why you would want to use "/usr/bin/docbook2man" instead of "docbook2man". )

MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK

MUST: The License field in the package spec file must match the actual license. NEEDSWORK
- License is GPLv2 not GPLv3. (Source code header is binding.)

MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK
MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. N/A
MUST: Optflags are used and time stamps preserved. OK
MUST: Packages containing shared library files must call ldconfig. N/A
MUST: A package must own all directories that it creates or require the package that owns the directory. OK
MUST: Files only listed once in %files listings. OK
MUST: Debuginfo package is complete. N/A
MUST: Permissions on files must be set properly. OK
MUST: Clean section exists. OK
MUST: Large documentation files must go in a -doc subpackage. N/A
MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. OK
MUST: Header files must be in a -devel package. N/A
MUST: Static libraries must be in a -static package. N/A
MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A
MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A
MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. N/A
MUST: Packages does not contain any .la libtool archives. N/A
MUST: Desktop files are installed properly. N/A
MUST: No file conflicts with other packages and no general names. OK
MUST: Buildroot cleaned before install. OK
SHOULD: %{?dist} tag is used in release. OK

SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. NEEDSWORK
- Included COPYING is GPLv3 not GPLv2.

SHOULD: The package builds in mock. OK

Comment 8 Miroslav Suchý 2009-07-17 08:34:21 UTC
> BuildRoot used is archaic. Use
It was one of recommended in Guideliness. But if you insist on the most prefered... Changed.

> I still don't see why you would want to use "/usr/bin/docbook2man" instead of
"docbook2man"
I prefer full paths as it prevent path spoofing. Just my opinion. I'm not dogmatic about it. Changed.

> License is GPLv2 not GPLv3. (Source code header is binding.)
Since I'm upstrem, then what I want is binding :)
Changed source header to GPLv3. Mention of GPLv2 was mistake.

Update SPEC:
http://miroslav.suchy.cz/fedora/rpmconf/rpmconf.spec
Update SRPM:
http://cloud.github.com/downloads/xsuchy/rpmconf/rpmconf-0.1.6-1.src.rpm

Comment 9 Susi Lehtola 2009-07-17 09:05:42 UTC
(In reply to comment #8)
> > BuildRoot used is archaic. Use
> It was one of recommended in Guideliness. But if you insist on the most
> prefered... Changed.

It was missing at least %{release}.

> > I still don't see why you would want to use "/usr/bin/docbook2man" instead of
> "docbook2man"
> I prefer full paths as it prevent path spoofing. Just my opinion. I'm not
> dogmatic about it. Changed.

But that shouldn't be an issue with the build system.

> > License is GPLv2 not GPLv3. (Source code header is binding.)
> Since I'm upstrem, then what I want is binding :)
> Changed source header to GPLv3. Mention of GPLv2 was mistake.

OK.

**

The package has been

APPROVED

Comment 10 Miroslav Suchý 2009-07-17 09:57:10 UTC
Thx Jussi for the work.

New Package CVS Request
=======================
Package Name: rpmconf
Short Description: Tool to handle rpmnew and rpmsave files
Owners: msuchy
Branches: F-11, EL-4, EL-5
InitialCC:

Comment 11 Jason Tibbitts 2009-07-17 15:48:27 UTC
CVS done.

Comment 12 Milos Jakubicek 2009-08-28 22:55:00 UTC
Package Change Request
======================
Package Name: rpmconf
New Branches: F-10
Owners: mjakubicek msuchy

(has been discussed with msuchy)

Comment 13 Kevin Fenzi 2009-08-29 03:28:39 UTC
cvs done.


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