[Bug 193894] Review Request: ant-contrib - A collection of tasks for Ant
bugzilla at redhat.com
bugzilla at redhat.com
Fri Jun 23 10:19:37 UTC 2006
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: ant-contrib - A collection of tasks for Ant
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=193894
j.w.r.degoede at hhs.nl changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
AssignedTo|bugzilla-sink at leemhuis.info |j.w.r.degoede at hhs.nl
CC|j.w.r.degoede at hhs.nl |
OtherBugsDependingO|163776 |163778
nThis| |
------- Additional Comments From j.w.r.degoede at hhs.nl 2006-06-23 06:11 EST -------
Ok,
I'll start reviewing those packages then starting with this one and sorry for
being somewhat slow, I'm currently rather busy with work.
Ok, here we go this is the first java package I'm reviewing so feel free to have
a different opinion in certain cases:
MUST:
=====
* rpmlint output is:
W: ant-contrib non-standard-group Development/Libraries/Java
W: ant-contrib non-standard-group Development/Libraries/Java
W: ant-contrib class-path-in-manifest /usr/share/java/ant-contrib-1.0b2.jar
W: ant-contrib-javadoc non-standard-group Development/Documentation
W: ant-contrib-javadoc dangerous-command-in-%post rm
W: ant-contrib-manual non-standard-group Development/Documentation
W: ant-contrib-manual wrong-file-end-of-line-encoding
/usr/share/doc/ant-contrib-1.0b2/tasks/for.html
W: ant-contrib-manual wrong-file-end-of-line-encoding
/usr/share/doc/ant-contrib-1.0b2/tasks/foreach.html
These all must be fixed!
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License ok, license file included
* spec file is legible and in Am. English.
* Couldn't very if source matches upstream, sf.net gives a 500 internal serv
error.
* Compiles and builds on devel-x86_64
* BR: ok (see below)
* No locales
* No shared libraries
* Not relocatable
* Package owns / or requires all dirs (with some strangeness see Must fix below)
* No duplicate files & Permissions ok
* %clean & macro usage OK
* Contains code only
* %doc does not affect runtime, and isn't large enough to warrent a sub package!
* no -devel package needed, no libs / .la files.
* no gui -> no .desktop file required
MUST fix:
=========
* All rpmlint messages, see above
* Remove the unused section %define
* Remove "%define base_name ant-contrib", replace "Name: %{base_name}"
with "Name: ant-contrib" and replace any uses of %base_name with %name
I so no reason whatsoever for the existence and use of this macro accept
obfuscation
* For indentation / lining up the list with Name, Version .... BuildRoot you
use a mix of spaces and tabs and you seem to have your tabsize set to
something else then 8. Please just spaces everywhere, the indentation is a
mess know in my editor.
* Source1 isn't used anywhere, remove it
* Remove Epoch: 0, you should not explicitly set Epoch to 0.
* 1.0b2 contains alphanumeric, I don't know what the exact version scheme
of upstream is, does b2 stands for beta 2, or was there first a 1.0 then a
1.0a then 1.0b, 1.0b1 and finally 1.0b2? Anyways unless upstreams creative
numbering is as described above (1.0 -> 1.0a -> etc) it will break rpm's
version comparison, please in that case use just 1.0 as version and and encode
the additioan b2 into the release tag as described here:
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-e104844825856d7c45f2f0241586985c0495966b
Also see the note about Release below under Should fix.
* Replace "%setup -q -c -n %{base_name}-%{version}" with
"%setup -q -n %{name}" and remove all the pushd popd nonsense as that then
no longer is nescesarry
* Remove the 2 find lines from %setup, the first is total nonsense and the
second one doesn't do anything either as there are no jar files included.
* Don't use cp to make manual backups of patched files (the 2 .sav files
created). Instead pass " -z .backupext" to the %patch commands
* For the manual subpackage you create %{_docdir}/%{name}-%{version}
and then copy the docs there and next you put %{_docdir}/%{name}-%{version}
under %files. This isn't nescesarry if you specify %doc a dir releative to
%{builddir} (default /usr/src/redhat/BUILD/ant-contrib for this package) then
it will create the dir, copy the files and at them to %files themselves.
So:
-drop the installing of these files from %install
-under "%files manual" replace "%doc %{_docdir}/%{name}-%{version}" with
"%doc build/docs/*". Since the license is already included and the index.html
under build/docs/ contains install instructions, which we usually don't
package as the rpm does the installing for the user, I would even like to
plea to change this too: "%doc build/docs/tasks/*"
* Don't put the manual in a seperate subpackage, its only 200k and people who
really need the diskspace can tell rpm not to install anything marked %doc.
* whats this with this symlink ghosting rm-ing black voodoo, why not just plain
package the symlink, why is the symlink there at all?
Should fix:
===========
* We (Fedora) don't support building java packages using the JDK, I've checked a
couple of other packages an no other has a gcj_support conditional. Please
concider removing this and only leaving the gcj code in that will make things
much easier to read.
* The Xjpp_Yfc Release fields in other packages are only used AFAIK to allow
smooth upgrade from jpackage packages to Fedora ones, since you've upgraded
to a newer upstream version upgrading from jpackage to FE should nnot be a
problem please use a regular 1%{?dist} release instead of 1jpp_1fc.
* Why the non standard %defattr(0644,root,root,0755) under %files why not just
%defattr(-,root,root,-) ?
* Redundant BR (must ne removed): ant, alreayd implied by ant-junit.
* Are you sure it will only build with this very specific version of junit that
looks like an error to me.
--
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.
More information about the Fedora-package-review
mailing list