fedora-review denied: [Bug 226201] Merge Review: nmap
by Red Hat Bugzilla
Bug 226201: Merge Review: nmap
Product: Fedora Extras
Version: devel
Component: Package Review
Jima <jima(a)beer.tclug.org> has denied Jima <jima(a)beer.tclug.org>'s request for
fedora-review:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226201
------- Additional Comments from Jima <jima(a)beer.tclug.org>
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
ea50419f99472200c4184a304e3831ea nmap-4.20.tar.bz2
ea50419f99472200c4184a304e3831ea nmap-4.20.tar.bz2.1
OK - BuildRequires correct
See below - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
See below - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
OK - final provides and requires are sane:
SHOULD Items:
OK - Should build in mock.
OK - Should build on all supported archs
OK - Should function as described.
OK - Should have dist tag
OK - Should package latest version
0 bugs - check for outstanding bugs on package.
1. Not sure if this is a blocker, but the standard format for defattr
now is:
%defattr(-,root,root,-)
People I've talked to have had the opinion that it's a "should."
2. The recommended BuildRoot is:
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
3. rpmlint says:
rpmlint on nmap-4.20-2.fc7.src.rpm
W: nmap mixed-use-of-spaces-and-tabs (spaces: line 56, tab: line 62)
W: nmap patch-not-applied Patch1: makefile.patch
W: nmap patch-not-applied Patch0: inet_aton.patch
The first one is easy enough to fix if you want to change the multiple
spaces on lines 56-59 to tabs; I don't believe it's a blocker, though.
I don't believe the other two are, either, but if you're done with the
patches... :-)
Also, I had a couple other concerns about the package. First off, see:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194a...
fbdd2fcb7791dd002
I've successfully tested the package with this change:
-%makeinstall nmapdatadir=$RPM_BUILD_ROOT%{_datadir}/nmap
+make install nmapdatadir=%{_datadir}/nmap DESTDIR=$RPM_BUILD_ROOT
Secondly, there's a pretty major mistake in nmap-4.20-nostrip.patch. As
I said in an email to Florian La Roche (who, as far as I can tell, added
that patch):
--- snip ---
In your effort to remove -s's from install lines, you also made this
effective change:
-$(SHTOOL) mkln -f -s $(DESTDIR)$(bindir)/nmapfe $(DESTDIR)$(bindir)/xnmap
+$(SHTOOL) mkln -f $(DESTDIR)$(bindir)/nmapfe $(DESTDIR)$(bindir)/xnmap
Which has this annoying effect on the build:
ln: `nmapfe': hard link not allowed for directory
...
RPM build errors:
File not found: /var/tmp/nmap-root/usr/bin/xnmap
I'm pretty sure that's not what you meant to do. :-)
I fixed the patch in my local copy and threw it against my buildsys and it
worked fine. Just thought I'd let you know.
--- snip ---
So, please, add the -s back to the `mkln` part of that line. The build
fails otherwise. :-P
In closing, if you could address the above issues, I think nmap should
be cleared for merging.
17 years, 3 months
fedora-review granted: [Bug 225932] Merge Review: jakarta-commons-launcher
by Red Hat Bugzilla
Bug 225932: Merge Review: jakarta-commons-launcher
Product: Fedora Extras
Version: devel
Component: Package Review
Thomas Fitzsimmons <fitzsim(a)redhat.com> has granted Thomas Fitzsimmons
<fitzsim(a)redhat.com>'s request for fedora-review:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225932
------- Additional Comments from Thomas Fitzsimmons <fitzsim(a)redhat.com>
MUST:
* is this appropriate for Fedora?
X rpmlint on all rpms gives no output
$ rpmlint jakarta-commons-launcher-1.1-1jpp.1.fc7.src.rpm
W: jakarta-commons-launcher non-standard-group Development/Libraries/Java
$ rpmlint jakarta-commons-launcher-1.1-1jpp.1.fc7.i386.rpm
W: jakarta-commons-launcher non-standard-group Development/Libraries/Java
$ rpmlint jakarta-commons-launcher-javadoc-1.1-1jpp.1.fc7.i386.rpm
W: jakarta-commons-launcher-javadoc non-standard-group
Development/Documentation
W: jakarta-commons-launcher-javadoc dangerous-command-in-%post rm
W: jakarta-commons-launcher-javadoc percent-in-%postun
W: jakarta-commons-launcher-javadoc dangerous-command-in-%postun rm
$ rpmlint jakarta-commons-launcher-debuginfo-1.1-1jpp.1.fc7.i386.rpm
These are all OK for the reasons Fernando cites here:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225928
But this one is a bug:
W: jakarta-commons-launcher-javadoc percent-in-%postun
You need this patch:
--- jakarta-commons-launcher.spec 9 Feb 2007 03:56:28 -0000 1.16
+++ jakarta-commons-launcher.spec 9 Feb 2007 17:06:05 -0000
@@ -147,7 +147,7 @@
fi
%if %{gcj_support}
-%{post}
+%post
if [ -x %{_bindir}/rebuild-gcj-db ]
then
%{_bindir}/rebuild-gcj-db
@@ -155,7 +155,7 @@
%endif
%if %{gcj_support}
-%{postun}
+%postun
if [ -x %{_bindir}/rebuild-gcj-db ]
then
%{_bindir}/rebuild-gcj-db
* package is named appropriately
* specfile name matches %{name}
* package meets packaging guidelines.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package and marked with %doc
* specfile written in American English
* specfile is legible
* source files match upstream
* package successfully compiles and builds on at least x86
* BuildRequires are proper
* find_lang usage correct
* package is not relocatable
* package owns all directories and files
* no %files duplicates
* file permissions are fine; %defattrs present
* %clean present
* macro usage is consistent
* package contains code
* no large docs so no -doc subpackage
* %doc files don't affect runtime
* gcj .so files need not be in a -devel sub-package
* no pkgconfig or header files
* no -devel package
* no .la files
* desktop file
* not a web app.
* file ownership fine
* binary RPMs function on x86
* final provides and requires are sane
SHOULD:
* package includes license text
* description and summary sections don't have translations (OK)
* package builds in mock
* package builds on i386
* package functions as described
X scriptlets should be sane
See above about fixing %{post} and %{postun} references.
* no -devel package
* no pkgconfig files
I'm marking this as fedora-review+. Please build the fixed packages into Brew.
Once they hit Rawhide, I'll close this RAWHIDE.
17 years, 3 months
fedora-review denied: [Bug 226366] Merge Review: regexp
by Red Hat Bugzilla
Bug 226366: Merge Review: regexp
Product: Fedora Extras
Version: devel
Component: Package Review
Andrew Overholt <overholt(a)redhat.com> has denied Andrew Overholt
<overholt(a)redhat.com>'s request for fedora-review:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226366
------- Additional Comments from Andrew Overholt <overholt(a)redhat.com>
(In reply to comment #3)
> Can you set the assigned and flags as you see fit?
Definitely. I totally forgot about the new flag-based reviews. Sorry.
17 years, 3 months
fedora-review granted: [Bug 225928] Merge Review: jakarta-commons-el
by Red Hat Bugzilla
Bug 225928: Merge Review: jakarta-commons-el
Product: Fedora Extras
Version: devel
Component: Package Review
Thomas Fitzsimmons <fitzsim(a)redhat.com> has granted Fernando Nasser
<fnasser(a)redhat.com>'s request for fedora-review:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225928
------- Additional Comments from Thomas Fitzsimmons <fitzsim(a)redhat.com>
(In reply to comment #5)
> 1) W.r.t. Groups, please see the last message from 'spot' in the
> fedora-packaging list:
>
> "You can put whatever you would like in the Group field. Its not
> regulated in Fedora whatsoever.
> ~spot"
>
> That rpmlint warning is to be ignored as the Group use is actually deprecated
> and will be replaced by a new mechanism with the RPM revamping project.
OK.
>
> 2) W.r.t. the javadoc code it is the current mechanism used at JPackage and
> Fedora 6 for allowing multiple versions of a software and its documentation
to
> be installed. I have unearthed upstream a proposal that would simplify this
in
> future JPackage releases, but current all Java paqckages that have -javadocs
use
> the above code.
>
> Although I also want that code to be simplified, and I hope it will, it is
not
> using anything explicitly forbidden by the current Fedora Guidelines.
OK.
APPROVED.
Now we're supposed to leave this bug report open until the merged Fedora build
system comes online.
17 years, 3 months
fedora-review denied: [Bug 226002] Merge Review: libevent
by Red Hat Bugzilla
Bug 226002: Merge Review: libevent
Product: Fedora Extras
Version: devel
Component: Package Review
Jima <jima(a)beer.tclug.org> has denied Jima <jima(a)beer.tclug.org>'s request for
fedora-review:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226002
------- Additional Comments from Jima <jima(a)beer.tclug.org>
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
6cc776458ecaf9247550863702a44d7c libevent-1.1a.tar.gz
6cc776458ecaf9247550863702a44d7c libevent-1.1a.tar.gz.1
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
OK - final provides and requires are sane:
SHOULD Items:
OK - Should build in mock.
OK - Should build on all supported archs
OK - Should function as described.
See below - Should have dist tag
See below - Should package latest version
1 bug - check for outstanding bugs on package.
Issues:
1. rpmlint says:
W: libevent incoherent-version-in-changelog control 1.1a-3.2.1
I'd recommend fixing the broken changelog entry from Jesse Keating's
bump-n-build script. (Seconded by Jesse.)
2. Dist tag
Appending "%{?dist}" to the Release field is recommended. Not a
blocker, but it'd be nice.
3. Not latest version
Upstream web site indicates 1.2a is the latest version (released
2006-12-02). Please evaluate as to whether upgrading might be for the
best.
4. Bugs open:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=204990
If you can address the above, I don't see any reason why libevent can't
be approved for merging.
17 years, 3 months
[Bug 187317] Review Request: mindi
by Red Hat Bugzilla
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Review Request: mindi
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=187317
------- Additional Comments From gauret(a)free.fr 2007-02-09 10:39 EST -------
> I'm still looking at why UTF-8 is incorrect.
Open the spec file in "less", you'll see accented caracters stand out in
changelog entries for 1.0.7-1.fc5 and 1.06-1.fc5 (Rémy and Sébastien).
By the way, the spec file changelog should only contain packaging changes, not
the application's changelog. That should only be in the ChangeLog file,
installed in %doc.
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.
17 years, 3 months