Andrew Overholt made some suggestions on IRC.  New files are here:

Spec Name or Url: http://people.redhat.com/green/FE/devel/azureus.spec
SRPM Name or Url: http://people.redhat.com/green/FE/devel/azureus-2.3.0.6-9.src.rpm

I'll pick it up, i'm almost getting used to this java packaging thing :-)
Changing to FE-REVIEW.

It builds cleanly on fedora-devel-i386, and functionally it seems well in order too.

However during compile i got this message:
/usr/bin/build-classpath: error: could not find xml-commons-apis Java extension
for this JVM
/usr/bin/build-classpath: error: All specified jars were not found

Is this a missing BuildRequires? Or an ignorable error? Doing "yum install
xml-commons-api" seemed to make it happy again, so it appears to be a missing
BuildRequire, if so please add it

Spacing in the header of the spec file is a little confused, seems your not
using spaces (especially for the patches) but tabs, in different editors this
then looks messy; Could you please re-indent it with spaces?

It might be better to: install -m644 %{SOURCE2} and 3 instead of copy, this way
your always sure the permissions are correct, and same with -m775 for the script

**** edit *** I see that you just corrected this in release 9 :-)

If you could please look at the above mentioned issues (spacing in header and
probable missing build requires) i'll post the formal reviewlist

Ps i see your not using install yet for:
mkdir -p $RPM_BUILD_ROOT%{_datadir}/applications
cp %{SOURCE2} $RPM_BUILD_ROOT%{_datadir}/applications

mkdir -p $RPM_BUILD_ROOT%{_datadir}/application-registry
cp %{SOURCE3} $RPM_BUILD_ROOT%{_datadir}/application-registry

Could you please change this to:
mkdir -p $RPM_BUILD_ROOT%{_datadir}/applications
install -m644 %{SOURCE2} $RPM_BUILD_ROOT%{_datadir}/applications

mkdir -p $RPM_BUILD_ROOT%{_datadir}/application-registry
install -m644 %{SOURCE3} $RPM_BUILD_ROOT%{_datadir}/application-registry

File permissions of the %doc files come out all wrong too (775, should be 644).
You could do a simple chmod 644 below the dos2unix lines to fix this up.

Also fedora extra's guidelines say md5sum of the source package should match the
one from upstream, but upstream i can't seem to find the _nocrypto source, could
you please provide a link to it?

If its custom patched, the normal way to do this is thru a patch, and including
the original upstream source and patching it on the fly, only exceptions to this
i've seen so far is when patents prevent RH from distributing the original source

If neither those is the case (not on the upstream download server & can't patch
it or script it and not for legal reasons) please at the very least add a
comment to the spec file why not, and how people can reproduce this based on
future upstream release versions

(In reply to comment #4)
> Also fedora extra's guidelines say md5sum of the source package should match the
> one from upstream, but upstream i can't seem to find the _nocrypto source, could
> you please provide a link to it?
> 
> If its custom patched, the normal way to do this is thru a patch, and including
> the original upstream source and patching it on the fly, only exceptions to this
> i've seen so far is when patents prevent RH from distributing the original source

No, stuff like crypto, MP3 support, etc. must be removed from the tarball
entirely. See bmp for an example.

I've made all your suggested changes, and more:

Spec Name or Url: http://people.redhat.com/green/FE/devel/azureus.spec
SRPM Name or Url: http://people.redhat.com/green/FE/devel/azureus-2.3.0.6-8.src.rpm

Regarding the source ball... I wanted to remove export controlled crypto code
from the azureus distributions.  We're trying to make it use existing crypto
package (GNU Crypto and Jessie, for instance).  If testing tells us that we
absolutely need bouncycastle, then that should be a separate package.  I put a
comment in the spec file explaining what directory I deleted from the source
distribution.

(In reply to comment #6)
> I've made all your suggested changes, and more:
> 
> Spec Name or Url: http://people.redhat.com/green/FE/devel/azureus.spec
> SRPM Name or Url:
http://people.redhat.com/green/FE/devel/azureus-2.3.0.6-8.src.rpm

Oops - that was supposed to read:

http://people.redhat.com/green/FE/devel/azureus-2.3.0.6-10.src.rpm

Taking a look at it now.

Reason for excluding crypto is definatly good enough (i mentioned that in my
original comment, patent & export restrictions are solid reasons :-)

I just noticed btw you forgot:
Requires(post):   desktop-file-utils                                           
                                        
Requires(postun): desktop-file-utils

Which are required for 'update-desktop-database'.

With the link you probably meant:
http://people.redhat.com/green/FE/devel/azureus-2.3.0.6-10.src.rpm 
But i found it ok :-)

Ps the desktop file is supposed to be installed thru:

desktop-file-install --vendor fedora                            \
        --dir ${RPM_BUILD_ROOT}%{_datadir}/applications         \
        --add-category X-Fedora                                 \
        %{SOURCE2}

this adds the correct vendor and category tags

More details on this here:
http://fedoraproject.org/wiki/PackagingGuidelines#desktop

Then you also need:
BuildRequires: desktop-file-utils 

Formal review list so far:

MUST review items:
- Builds cleanly on FC5 devel.
- Source included doesn't match upsteam source, but for a good reason :-)
- Package name meets guidelines
- spec file name is in %{name}.spec format
- Licence is fedora extra's compatible & is included
- Spec file is in (american) english
- Does not list buildrequires that are excepted in the package guidelines
- All build dependencies are listed
- No ldconfig needed
- All files have proper permissions
- Package is not relocatable
- No duplicate files in %files section
- No missing files in %files section
- Has a proper %clean section with rm -rf $RPM_BUILD_ROOT
- Uses macro's described in PackagingGuidelines
- No entries in %doc that are required for standard program operation
- No -devel package needed
- Proper directory-ownerships
- **ERROR: No complete desktop file & install handling yet

Should items:
- Includes upstream licence(s) file
- No insane scriplets
- No unnescesarry requires
- Mock build cleanly

Almost there!

rpmlint's only warning is about libswt3-gtk, but since so auto-depedencies don't
pick this up rpmlint is wrong in this case.

Small side note: Why is it that it leaves 'plugins' dirs in any directory you
start this up in? Shouldn't it use ~/.Azureus/plugins/ for that? (which also exists)

(In reply to comment #11)
> rpmlint's only warning is about libswt3-gtk, but since so auto-depedencies don't
> pick this up rpmlint is wrong in this case.

Ok, try..
http://people.redhat.com/green/FE/devel/azureus-2.3.0.6-11.src.rpm
 
I think it has eveything you've suggested.

> Small side note: Why is it that it leaves 'plugins' dirs in any directory you
> start this up in? Shouldn't it use ~/.Azureus/plugins/ for that? (which also
exists)

I'm not sure, but I have an idea.  That would be a good issue to file in
bugzilla once this is accepted. :-)

Thanks for all your help!

AG

According to the packaging guidelines, dos2unix shouldn't be used to convert
line endings.  The following should be used instead:

%{__sed} -i 's/\r//' filename

Woops Jeffrey is right, it reads:

E: foo-package wrong-script-end-of-line-encoding /path/to/somefile: This error
occurs because of DOS line breaks in a file. Fix it with sed in the %prep
section: %{__sed} -i 's/\r//' src/somefile -- DONT use dos2unix, that can cause
build fail on FC3.

Didn't know about the FC3 problem, could you please put that last fix in the
spec file?

(In reply to comment #9)
> Then you also need:
> BuildRequires: desktop-file-utils 

I'm still missing that part :-)

(In reply to comment #15)
> (In reply to comment #9)
> > Then you also need:
> > BuildRequires: desktop-file-utils 
> 
> I'm still missing that part :-)
> 

Ok, fixed.  And I'm using sed instead of dos2unix.

http://people.redhat.com/green/FE/devel/azureus.spec
http://people.redhat.com/green/FE/devel/azureus-2.3.0.6-12.src.rpm

Why is junit in Requires:?

In my book that's a unit test program and not used in any application
by end-users. However junit may have gained new powers that I'm not
aware of...

New reviewlist based on release 12:

MUST review items:
- Builds cleanly on FC5 devel.
- Source included doesn't match upsteam source, but for a good reason :-)
- Package name meets guidelines
- spec file name is in %{name}.spec format
- Licence is fedora extra's compatible & is included
- Spec file is in (american) english
- Does not list buildrequires that are excepted in the package guidelines
- All build dependencies are listed
- No ldconfig needed
- All files have proper permissions
- Package is not relocatable
- No duplicate files in %files section
- No missing files in %files section
- Has a proper %clean section with rm -rf $RPM_BUILD_ROOT
- Uses macro's described in PackagingGuidelines
- No entries in %doc that are required for standard program operation
- No -devel package needed
- Proper directory-ownerships
- Correct desktop file & install handling & categories

Should items:
- Includes upstream licence(s) file
- No insane scriplets
- No unnescesarry requires
- Mock build cleanly (fedora-devel-i386)

I've even tried to compile w/o junit, but can tell you that Azureus does depend
on it
(azureus-2.3.0.6/org/gudy/azureus2/ui/console/multiuser/TestUserManager.java),
the how and what and why i will leave to AG, i'm not skilled in java enough to
comment on this, but this isn't a packaging issue, more something for future
bugzilla reports (i'm sure quite a few of those will follow, but better to do
that in bugzilla/cvs). So:

FE-ACCEPTED

Thanks!

BTW, I've removed the junit dependency by removing the one bit of test code that
didn't really belong in the runtime.  So I'll be checking in a version -13 later
today.

Thanks again!

AG

Hi Anthony !

Nice work you're doing... If possible, I would like to make an Azureus gcj
source package based on your ideas, and contribute it to gentoo linux.

I hope I'm not too off-topic on this bug..

Usually, a gentoo package (ebuild) is specified in terms of compilation,
installation and dependencies, listed inside a bash script.

What method would you recommend to get the latest Azureus source built using gcj
? Is extensive patching needed ? What dependencies would I need to gcj-compile
too ? Btw, for maintenance and gentoo's java policy reasons, I would prefer to
use the original Azureus source and patch it on the fly, if possible..

Thanks in advance !

Joël

(In reply to comment #20)
> Hi Anthony !
> 
> Nice work you're doing... If possible, I would like to make an Azureus gcj
> source package based on your ideas, and contribute it to gentoo linux.

Let's take this to private mail.

AG