Bug 225978

Summary: Merge Review: kudzu
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Susi Lehtola <susi.lehtola>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: notting, redhat-bugzilla, susi.lehtola
Target Milestone: ---Flags: susi.lehtola: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-06-18 15:37:32 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:

Description Nobody's working on this, feel free to take it 2007-01-31 19:17:20 UTC
Fedora Merge Review: kudzu

http://cvs.fedora.redhat.com/viewcvs/devel/kudzu/
Initial Owner: notting

Comment 1 Susi Lehtola 2009-03-20 20:59:42 UTC
rpmlint output:
kudzu.src: E: non-utf8-spec-file /tmp/kudzu-1.2.85-3.src.rpm.3418/kudzu.spec
kudzu.src:8: W: unversioned-explicit-obsoletes rhs-hwdiag
kudzu.src:8: W: unversioned-explicit-obsoletes setconsole
kudzu.src:9: E: prereq-use /sbin/chkconfig
kudzu.src:12: E: buildprereq-use pciutils-devel >= 2.2.3-4, python-devel python gettext zlib-devel popt-devel
kudzu.src:30: W: setup-not-quiet
kudzu.src:506: W: macro-in-%changelog d
kudzu.src: W: summary-ended-with-dot The Red Hat Linux hardware probing tool.
kudzu.src: E: tag-not-utf8 %changelog
kudzu.src: W: no-url-tag
kudzu.x86_64: W: summary-ended-with-dot The Red Hat Linux hardware probing tool.
kudzu.x86_64: E: tag-not-utf8 %changelog
kudzu.x86_64: W: no-url-tag
kudzu.x86_64: W: obsolete-not-provided rhs-hwdiag
kudzu.x86_64: W: obsolete-not-provided setconsole
kudzu-debuginfo.x86_64: E: tag-not-utf8 %changelog
kudzu-debuginfo.x86_64: W: no-url-tag
kudzu-devel.x86_64: W: no-documentation
kudzu-devel.x86_64: W: summary-ended-with-dot Development files needed for hardware probing using kudzu.
kudzu-devel.x86_64: E: tag-not-utf8 %changelog
kudzu-devel.x86_64: W: no-url-tag
4 packages and 0 specfiles checked; 7 errors, 14 warnings.

- Change source line to use %{version}

- Change buildroot to
%(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)

- Requires, obsoletes etc need to be cleaned up

After these have been fixed the package would seem to be good to go.

Comment 2 Bill Nottingham 2009-03-23 16:49:18 UTC
(In reply to comment #1)
> rpmlint output:
> kudzu.src: E: non-utf8-spec-file /tmp/kudzu-1.2.85-3.src.rpm.3418/kudzu.spec

Fixed.

> kudzu.src:8: W: unversioned-explicit-obsoletes rhs-hwdiag
> kudzu.src:8: W: unversioned-explicit-obsoletes setconsole

Nuked. (They're ridiclously ancient.)

> kudzu.src:9: E: prereq-use /sbin/chkconfig

'Fixed'; expressing it correctly isn't possible, so I went for something that should work the same.

> kudzu.src:12: E: buildprereq-use pciutils-devel >= 2.2.3-4, python-devel python gettext zlib-devel popt-devel

Fixed.

> kudzu.src:30: W: setup-not-quiet

Fixed.

> kudzu.src:506: W: macro-in-%changelog d

Fixed.

> kudzu.src: W: summary-ended-with-dot The Red Hat Linux hardware probing tool.

Not fixed, string freeze. Needs completely rewritten anyway.

> kudzu.src: E: tag-not-utf8 %changelog

Fixed, see above.

> kudzu.src: W: no-url-tag

Fixed.

> kudzu.x86_64: W: summary-ended-with-dot The Red Hat Linux hardware probing
> tool.
> kudzu.x86_64: E: tag-not-utf8 %changelog
> kudzu.x86_64: W: no-url-tag
> kudzu.x86_64: W: obsolete-not-provided rhs-hwdiag
> kudzu.x86_64: W: obsolete-not-provided setconsole
> kudzu-debuginfo.x86_64: E: tag-not-utf8 %changelog
> kudzu-debuginfo.x86_64: W: no-url-tag
> kudzu-devel.x86_64: W: no-documentation
> kudzu-devel.x86_64: W: summary-ended-with-dot Development files needed for
> hardware probing using kudzu.
> kudzu-devel.x86_64: E: tag-not-utf8 %changelog
> kudzu-devel.x86_64: W: no-url-tag

See above for all of these.

All fixed on http://git.fedorahosted.org/git/?p=kudzu.git . Will get around to building at some point.

Comment 3 Susi Lehtola 2009-03-23 20:08:51 UTC
(In reply to comment #2)
> > kudzu.src: W: summary-ended-with-dot The Red Hat Linux hardware probing tool.
> 
> Not fixed, string freeze. Needs completely rewritten anyway.

Does string freeze really affect this? There are no translations in the spec file anyway for the summary!

IIUC the explanations at http://fedoraproject.org/wiki/L10N/Freezes#string_freeze the string freeze only affects content to be translated.

Comment 4 Bill Nottingham 2009-03-23 20:33:23 UTC
It's translated as part of the specspo package.

Comment 5 Susi Lehtola 2009-03-23 20:37:22 UTC
(In reply to comment #4)
> It's translated as part of the specspo package.  

Okay, then it has to wait until F11 is released. You can add a comment to the spec file about the need to fix the summaries, however!

Review for 1.2.86-1 as follows:

rpmlint output:
kudzu.src: W: summary-ended-with-dot The Red Hat Linux hardware probing tool.
kudzu.x86_64: W: summary-ended-with-dot The Red Hat Linux hardware probing
tool.
kudzu-devel.x86_64: W: summary-ended-with-dot Development files needed for
hardware probing using kudzu.
4 packages and 0 specfiles checked; 0 errors, 3 warnings.

- Is it really necessary to have the documentation in the devel package too?
I'd prune these.

Review:
? source files match upstream (no srpm provided)
X package meets naming and versioning guidelines.
X specfile is properly named, is cleanly written and uses macros consistently.

- dist tag is present.
 MUST: Add this.

X build root is correct.
X license field matches the actual license.
X license is open source-compatible.
X license text included in package.
X latest version is being packaged.

X BuildRequires are proper.
 * Please clean the conflicts, requires and buildrequires. ideally they should
be given one per line in alphabetical order

X compiler flags are appropriate.
X %clean is present.
X package builds in mock.
X package installs properly.
X debuginfo package looks complete.

? rpmlint is silent.
 * The summaries still need fixing.

X final provides and requires are sane
X no check available
X no shared libraries are added to the regular linker search paths.
X owns the directories it creates.
X doesn't own any directories it shouldn't.
X no duplicates in %files.
X file permissions are appropriate.
X no scriptlets present.
X code, not content.
X documentation is small, so no -docs subpackage is necessary.
X %docs are not necessary for the proper functioning of the package.
X headers are in devel package

- static libraries are in static package
 * The devel package needs to Provides: kudzu-static=%{version}-%{release}

X no pkgconfig files.
X no libtool .la droppings.
X no desktop files

Comment 6 Bill Nottingham 2009-03-24 16:46:51 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > It's translated as part of the specspo package.  
> 
> Okay, then it has to wait until F11 is released. You can add a comment to the
> spec file about the need to fix the summaries, however!
> 
> Review for 1.2.86-1 as follows:

You know, you could have mentioned more of these items in the initial review. :)

> - Is it really necessary to have the documentation in the devel package too?
> I'd prune these.

There's no dependnecies between the subpackages, and given the recent discussions on where license files need to be, it seems more prudent to put them in both.

> X specfile is properly named, is cleanly written and uses macros consistently.
> 
> - dist tag is present.
>  MUST: Add this.

It's not listed in Packaging/Guidelines as a must, so I don't think it's really needed.

> X BuildRequires are proper.
>  * Please clean the conflicts, requires and buildrequires. ideally they should
> be given one per line in alphabetical order

Also seems a bit nitpicky, but sure.

> - static libraries are in static package
>  * The devel package needs to Provides: kudzu-static=%{version}-%{release}

Added.

Comment 7 Robert Scheck 2009-03-24 16:57:24 UTC
> > X specfile is properly named, is cleanly written and uses macros consistently.
> > 
> > - dist tag is present.
> >  MUST: Add this.
> 
> It's not listed in Packaging/Guidelines as a must, so I don't think it's 
> really needed.

Notting is right, it's a SHOULD, but no MUST.

> > X BuildRequires are proper.
> >  * Please clean the conflicts, requires and buildrequires. ideally they 
> > should be given one per line in alphabetical order
> 
> Also seems a bit nitpicky, but sure.

Requiring a line for each conflicts, requires and buildrequires seems a waste
of lines. Using space or colon as separator seems much more better to me.

Comment 8 Susi Lehtola 2009-03-24 17:19:57 UTC
(In reply to comment #7)
> > > - dist tag is present.
> > >  MUST: Add this.
> > 
> > It's not listed in Packaging/Guidelines as a must, so I don't think it's 
> > really needed.
> 
> Notting is right, it's a SHOULD, but no MUST.

True; it isn't even in the packaging or the review guidelines; it's buried in http://fedoraproject.org/wiki/Packaging/DistTag, also it appears in the spec file example. My apologies.

Still, I think it should be in, since when you upgrade from an older distribution the old distro's rpm may not be replaced if its EVR is otherwise the same as in the older distro release.

> > > X BuildRequires are proper.
> > >  * Please clean the conflicts, requires and buildrequires. ideally they 
> > > should be given one per line in alphabetical order
> > 
> > Also seems a bit nitpicky, but sure.
> 
> Requiring a line for each conflicts, requires and buildrequires seems a waste
> of lines. Using space or colon as separator seems much more better to me.  

It's okay, as long as the separators are the same throughout the spec file. Now there's both commas and spaces.

(In reply to comment #6)
> (In reply to comment #5)
> > - Is it really necessary to have the documentation in the devel package too?
> > I'd prune these.
> 
> There's no dependnecies between the subpackages, and given the recent
> discussions on where license files need to be, it seems more prudent to put
> them in both.

Okay, I guess this is an exception to the rule that devel packages require %{name} = %{version}-%{release} and thus the doc files should be replicated in the devel package.

Comment 9 Susi Lehtola 2009-03-27 19:03:45 UTC
Well, if you have added
 Provides: kudzu-static = %{version}-%{release}
to the devel subpackage, then all of the requirements are fulfilled and the package is

APPROVED.

Remember to push the new version to CVS; at least now the fixes you made are not there yet. Also, I'd still like to see the %{?dist} tag in the release, although it is not a requirement.

Comment 10 Susi Lehtola 2009-04-24 20:51:25 UTC
Ping, I still don't see the new version in CVS.

Comment 11 Bill Nottingham 2009-04-27 20:42:15 UTC
It didn't seem worth a freeze break to add a kudzu-static provide without any other functional changes.

Comment 12 Susi Lehtola 2009-06-14 16:07:53 UTC
Rawhide freeze is over, please push the new release and close this bug.

Comment 13 Bill Nottingham 2009-06-18 15:37:32 UTC
Tagged, built.