Bug 469471

Files:
Spec URL: http://dhd.selfip.com/427e/skinlf-2.spec
SRPM URL: http://dhd.selfip.com/427e/skinlf-6.7-2.fc9.src.rpm  

- Fix version numbering
- Fix top level dir when building jar

#001: remove the copyright paragraph, else I won't review this package

#002: avoid renaming spec files : %{name}-X.spec. Always keep the spec filenme as %{name}.spec. I have seen you did the same for all your packages.

#003: rpmlint issues
chitlesh(~)[0]$rpmlint /home/chitlesh/rpmbuild/RPMS/i386/skinlf-6.7-2.fc9.i386.rpm
skinlf.i386: W: no-documentation
skinlf.i386: E: description-line-too-long Today many applications must be skinnable: your CD player, your emailnotifier, even your operating system! Now with Skin Look And Feel, your Java application is skinnable! Skin Look And Feel is able to read GTK (The Gimp ToolKit) and KDE (The K Desktop Environment) Skins to enhance your application GUI controls such as Buttons, Checks, Radios, Scrollbars, Progress Bar, Lists, Tables, Internal Frames, Colors, Background Textures, Regular Windows.
skinlf.i386: W: non-standard-group skinlf
skinlf.i386: W: invalid-license Apache
skinlf.i386: E: no-binary

Break the description paragraph into seperate lines.

004: Build errors, possible missing BuildRequires:
     [javac]                       ^                                                                             
    [javac] /home/chitlesh/rpmbuild/BUILD/skinlf-6.7/build/src/com/l2fprod/tools/msstyles/MsStylesToSkinLF.java:78: warning: com.sun.org.apache.xpath.internal.XPathAPI is Sun proprietary API and may be removed in a future release                                                                                                           
    [javac]       (Element)XPathAPI.                                                                            
    [javac]                ^

Spec URL:
SRPM URL:

> #001: remove the copyright paragraph, else I won't review this package
Done.

>#002: avoid renaming spec files : %{name}-X.spec. Always keep the spec filenme
>as %{name}.spec. I have seen you did the same for all your packages.

I'm going to do it again, just to be difficult -- my local copy is always blahblah.spec, only the bugzilla copies are blahblah-rev.spec. Sorry. How do I make sure other people on the 'net can see the changes in the spec files after I have made them? Is there a good way to do this?

>#003: rpmlint issues
>skinlf.i386: W: no-documentation
Fixed.

>skinlf.i386: E: description-line-too-long
Fixed.

>skinlf.i386: W: non-standard-group skinlf
Fixed.

>004: Build errors, possible missing BuildRequires:
>...
>warning: com.sun.org.apache.xpath.internal.
>XPathAPI is Sun proprietary API and may be removed in a future release  
Not technically errors, these warnings arise due to my usage of the sun xpath api as a substitute for the org.apache.xpath api (patch #3), which was removed as of sun JRE1.5 [1]. I can look at this again to see if I can make it work without the patch, but that will have to wait. The package itself will still work with the sun API, it just means that I may have to revisit and update later. I'd rather continue with the current sun API if no-one knows how to solve this properly, and doesn't object to doing so. Just so we can lift the block on other bugs


>skinlf.i386: W: invalid-license Apache
Not fixed -- I am unsure how to proceed here. If you examine the LICENSE file in the source, it isn't Apache, contrary to what's on their website [2]. Its a redistributable with attribution licence, which doesn't appear up in the rpmlint -iv output. Which licence should I select??


Current RPMlint output:
$ rpmlint -i skinlf-6.7-3.fc9.src.rpm 
skinlf.src: W: invalid-license Apache 2.0


Thanks for taking a look at the package!

[1] http://java.sun.com/j2se/1.5.0/docs/guide/xml/jaxp/JAXP-Compatibility_150.html#New
[2] https://skinlf.dev.java.net/

*sigh* One more time... 

URLS:
Spec: http://www.dhd.selfip.com/427e/skinlf-3.spec
SRPM: http://www.dhd.selfip.com/427e/skinlf-6.7-3.fc9.src.rpm

*sigh* again. There is no www prefix. 

URLS:
Spec: http://dhd.selfip.com/427e/skinlf-3.spec
SRPM: http://dhd.selfip.com/427e/skinlf-6.7-3.fc9.src.rpm

build seems to error out aswell without laf-plugin installed so shouldnt that also have a BuildRequires on it?

>build seems to error out aswell without laf-plugin installed so shouldnt that
>also have a BuildRequires on it?

Does Requires not imply BuildRequires? I was under the (maybe incorrect) assumption that buildrequires was just a weaker form of requires (ie Requires, but only at build time)

(In reply to comment #7)
> >build seems to error out aswell without laf-plugin installed so shouldnt that
> >also have a BuildRequires on it?
> 
> Does Requires not imply BuildRequires? 

No.

Spec: http://dhd.selfip.com/427e/skinlf-4.spec
SRPM: http://dhd.selfip.com/427e/skinlf-6.7-4.fc9.src.rpm

* Sat Nov 29 2008 <mycae(a!t)yahoo.com> 6.7-4
- Updated BuildRequires to inlcude laf-plugin
- Silence several rpmlint errors
	- ASL 2.0 vs Apache Source Licence 2.0
	- Fix arch
	- Fix EOL on docs.

>build seems to error out aswell without laf-plugin installed so shouldnt that
>also have a BuildRequires on it?

Fixed

rpmlint:

$ rpmlint -iv ../SRPMS/skinlf-6.7-4.fc9.src.rpm 
skinlf.src: I: checking
$ rpmlint -iv ../RPMS/noarch/skinlf-6.7-4.fc9.noarch.rpm 
skinlf.noarch: I: checking
$ rpmlint -iv skinlf.spec 

Known issues :

comment 3
>>skinlf.i386: W: invalid-license Apache
>Not fixed -- I am unsure how to proceed here. If you examine the LICENSE file
>in the source, it isn't Apache, contrary to what's on their website [2]. Its a
>redistributable with attribution licence, which doesn't appear up in the
>rpmlint -iv output. Which licence should I select??

* Licensing
  - I guess src/examples/Clock.java is non-free. So if this file is not
    needed, please remove this file (and also .jar/.dll files) and
    repackage the zip source:
    https://fedoraproject.org/wiki/Packaging/SourceURL#When_Upstream_uses_Prohibited_Code

    * Then the license tag must be "ASL 1.1 and ASL 2.0 and zlib".
---------------------------------------------------------------------------
LICENSE:		ASL 1.1
LICENSE_nanoxml:	zlib

src/com/l2fprod/gui/plaf/skin/LafPluginSupport.java	ASL 2.0
src/com/l2fprod/util/WeakImageIcon.java                 ASL 2.0
src/examples/Clock.java	???
---------------------------------------------------------------------------
    * Also, add "LICENSE_nanoxml" to %doc.

  - If src/examples/Clock.java is needed, I wil ask spot whether the
    license is free or not.

* BuildRequires, Requires
  - For java depedency, please follow:
    https://fedoraproject.org/wiki/Packaging/Java#BuildRequires_and_Requires
    If this package needs Java openjdk, the BuildRequires should have
    "BuildRequires: java-devel >= 1:1.6.0".

SPEC URL: http://dhd.selfip.com/427e/skinlf-5.spec
SRPM URL: http://dhd.selfip.com/427e/skinlf-6.7-5.fc9.srpm

RPMLint on SRPM, SPEC and RPM are empty

>* Also, add "LICENSE_nanoxml" to %doc.
Done.

> If src/examples/Clock.java is needed, I will ask spot whether the
> license is free or not.
Done. Removed by deletion in %prep -- the examples are only compressed into the jar and are not required at compile. 

Q: How do I provide the srpm? With Clock.java or without -- if I make it with, it still contains it (just not the RPM). If I make it without then it will require me to recompress the source tarball, making the Source0 line invalid. 

> (and also any .jar.dll files)
Not needed/implicit. The output of find ./ -name  blah, where blah is *.jar, *so and *dll, is empty. This was the case for the prior revision. The prior revision included the standard check for java files from the packaging guidelines to ensure that the package will not build if any file *.jar is located within the build tree. This check is performed at %prep time.

> BuildRequires, Requires
Done. Imported any missing java deps from previous package "laf-plugin". Specifically jpackage-utils and updated java lines for req & buildreq.

just two things I noticed (I'm about to create a package that will depend on skinlf):

1) there jar should be named %{name}-%{version}.jar with a symlink %{name}.jar to it. Right now, only %{name}.jar is installed to the %{_javadir}. ls -al /usr/share/java will show you, that (most?) packages do it like that.

2) the jar should contain the class files, not the source files ;)

instead of:
pushd .
cd build/src
jar cf %{name}-%{version}.jar .
popd

do:
pushd build/classes
jar cf %{name}-%{version}.jar .
popd

of course, you'll need to change the path in the install cmd after that, too.

I've tested that change and it works fine.

I'm actually away for the better part of a week, however I have access to my webserver, but not my build machine (aka my desktop). As such I have uploaded a modified "tentative" spec file. Be warned that I cannot even test for approximate correctness.
URL: http://dhd.selfip.com/427e/skinlf-6-tentative.spec


>1) there jar should be named %{name}-%{version}.jar with a symlink %{name}.jar
>to it. Right now, only %{name}.jar is installed to the %{_javadir}. ls -al
>/usr/share/java will show you, that (most?) packages do it like that.

Changed name & linked. I did look at this and in my java dir, IIRC, it was split between %{name}-%{version} and %{name} for the primary file.

>2) the jar should contain the class files, not the source files ;)
Fixed. Well that was stupid. 

I hope this package is almost there. My packages seem to have a nasty tendency of being a mess at the start -- something I need to attend to in the future.

Thanks.

Sorry for delay...

Seems good, however as I wrote in my comment 10 (and as you wondered)
since Clock.java must be removed from source zip file, the zip file
must be re-packaged. Please follow:

https://fedoraproject.org/wiki/Packaging/SourceURL#When_Upstream_uses_Prohibited_Code

SPEC URL: http://dhd.selfip.com/427e/skinlf-6.spec
SRPM URL: http://dhd.selfip.com/427e/skinlf-6.7-6.fc9.src.rpm 

>Seems good, however as I wrote in my comment 10 (and as you wondered)
>since Clock.java must be removed from source zip file, the zip file
>must be re-packaged.
Done.

rpmlint output is empty for spec, SRPM and RPM. I set the permissions on the tarball generator to 644, so if you wish to use it, you have to set execute permissions.

Seems good, approved.
---------------------------------------------------------
   This package (skinlf) is APPROVED by mtasaka
---------------------------------------------------------

New Package CVS Request
=======================
Package Name: skinlf
Short Description: Java look and feel for swing 
Owners: mycae
Branches: F-9 F-10
InitialCC:

cvs done.

skinlf-6.7-6.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/skinlf-6.7-6.fc9

skinlf-6.7-6.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/skinlf-6.7-6.fc10

Please also rebuild for dist-f11.

Do I need to repeat my CVS req?

No. "devel" branch is already created on Fedora CVS and
you can already rebuild this package for dist-f11.

skinlf-6.7-6.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update skinlf'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2008-11321

skinlf-6.7-6.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing-newkey update skinlf'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-11295

skinlf-6.7-6.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

skinlf-6.7-6.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.