Bug 468797

Summary: Review Request: jrosetta - A common base to build a graphical console
Product: [Fedora] Fedora Reporter: Nicolas Chauvet (kwizart) <kwizart>
Component: Package ReviewAssignee: Dominik 'Rathann' Mierzejewski <dominik>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: dominik, fedora-package-review, mycae, notting
Target Milestone: ---Flags: dominik: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-01-22 11:32:08 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:
Bug Depends On:    
Bug Blocks: 472639, 496433    

Description Nicolas Chauvet (kwizart) 2008-10-28 02:12:59 UTC
Spec URL: 
http://kwizart.fedorapeople.org/SPECS/jrosetta.spec
SRPM URL: 
http://kwizart.fedorapeople.org/SRPMS/jrosetta-1.0.1-1.fc8.kwizart.src.rpm
Description: JRosetta provides a common base for graphical component

Comment 1 Dominik 'Rathann' Mierzejewski 2008-11-02 16:50:05 UTC
Full review, relevant items only.

MUST Items:

- MUST: rpmlint must be run on every package. The output should be posted in the review.
$ rpmlint /var/lib/mock//fedora-rawhide-i386/result
2 packages and 0 specfiles checked; 0 errors, 0 warnings.

- MUST: The package must meet the Packaging Guidelines .

Summary:        JRosetta provides a common base for graphical component

No need to repeat package name in Summary. Suggestion:

Summary:        A common base for graphical user interface components


  ln -fs ${j}-%{version}.jar $RPM_BUILD_ROOT%{_javadir}/${j}.jar

Why does it need the symlinks?


All patches should have an upstream bug link or comment, so please add a comment before
Patch0:         jrosetta-1.0.1-rm_classpath.patch


- MUST: The License field in the package spec file must match the actual license.

Licence is GPLv2 only, not v2+. Please correct.

- MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this.

Source matches upstream:
0f42365004065d01923bd55ca458fb47  jrosetta-1.0.1-GPL.zip

Please add a comment in the specfile with either the URL of the download page or the full URL of the zip archive,
i.e.:
# http://dev.artenum.com/projects/JRosetta/download/JRosetta-1-0-1/data/src-gpl?action=download&nodecorator

- MUST: The package must successfully compile and build into binary rpms on at least one supported architecture.

Builds fine in mock for rawhide(F-10)/i386

Comment 2 Dominik 'Rathann' Mierzejewski 2008-11-21 22:40:55 UTC
ping?

Comment 3 D Haley 2008-12-20 02:11:41 UTC
I hope no-one minds, but I have taken the liberty of updating this package, as it is blocking a package that I am keen to complete.

SPEC URL: http://dhd.selfip.com/427e/jrosetta-2.spec
SRPM URL: http://dhd.selfip.com/427e/jrosetta-1.0.1-2.fc9.src.rpm 

*Sat Dec 20 2008  <mycae(a!t)yahoo.com> 1.0.1-2
- Fix build paths to prevent setup from asking for user input
  after multiple builds
- Refresh source to match MD5.
- Update patch to match path change
- Corrected licence from GPL2+ to GPL2

I agree with the symlinking, as this is done in many java packages, where the version of interest is installed in %{name}-%{version}.jar, and then a symlink is made to this from %{name}.jar . Unless I have mis-interpreted the intent of the command.

Comment 4 Nicolas Chauvet (kwizart) 2009-01-14 10:11:38 UTC
Spec URL: 
http://kwizart.fedorapeople.org/SPECS/JRosetta.spec
SRPM URL: 
http://kwizart.fedorapeople.org/SRPMS/JRosetta-1.0.2-1.fc10.src.rpm
Description: A common base to build a graphical console

Changelog
- Fix License (GPLv2 only) (was confirmed by phone call with upstream, the shortname license can be seen in the MANIFEST of the jar files.)
- Fix Summary
- Update to 1.0.2 - previous patch merged upstream
- Rename to JRosetta

Koji scratch build for Rawhide:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1051591

rpmlint is quiet on local build from F-10

Comment 5 Nicolas Chauvet (kwizart) 2009-01-14 15:19:06 UTC
@Haley
-----------
%setup -qc
[...snip]
pushd .
[...snip]
popd

#The zip file is a bit messy. 
#We reshuffle the top level dir
mv %{name}-%{version}-GPL/* .
mv %{name}-%{version}-GPL/.[A-z]* .
-----------
This is weird to fix problem you have introduced with not using the correct %setup macro:
%setup -q -n %{name}-%{version}-GPL

Along with your debuginfo tweak in the scilab package (that, again, isn't needed), You seem to have a strange buildsys. At this time, I don't know if you got inattentive or your buildsys is buggy. The result is that I cannot trust any packages that comes from your side.
I hope you can solve this, because we really need hands for scilab and others...

Comment 6 D Haley 2009-01-14 23:21:14 UTC
Nicholas, If you have specific problems with my packages, I would be most apprreciative if you could please comment on them one by one and where appropriate.

My packages are subject to review and can be rejected as required. Personal attacks are unprofessional, and finally it is not up to you to decide whether to accept packages or not. My personal policy is that others are allowed to modify my work, if I fail to do so in a timely fashion.]

A bug report forum is not a place to conduct such aggressive behaviour. If you dont like it, please say so in a review, or contact me personally.

Comment 7 Dominik 'Rathann' Mierzejewski 2009-01-15 00:27:36 UTC
(In reply to comment #4)
> Spec URL: 
> http://kwizart.fedorapeople.org/SPECS/JRosetta.spec
> SRPM URL: 
> http://kwizart.fedorapeople.org/SRPMS/JRosetta-1.0.2-1.fc10.src.rpm
> Description: A common base to build a graphical console
> 
> Changelog
> - Fix License (GPLv2 only) (was confirmed by phone call with upstream, the
> shortname license can be seen in the MANIFEST of the jar files.)
> - Fix Summary
> - Update to 1.0.2 - previous patch merged upstream
> - Rename to JRosetta

Full review:

rpmlint clean:
$ rpmlint /var/lib/mock/fedora-rawhide-i386/result
2 packages and 0 specfiles checked; 0 errors, 0 warnings.

Naming:
Please rename it back to jrosetta. There is no good reason to have mixed-case name. In fact, no java package present in Fedora has mixed-case name.

Pre-built binaries:
The 1.0.2 source tarball contains pre-built binaries in build/*, please remove them in %prep and ask upstream to provide a source-only tarball, if possible. 1.0.1 was shipped without those binaries and the tarball was half the size.

Source MD5 matches upstream:
ef0f9208202762c93c8c415d8472aa76  jrosetta-1.0.2-GPL.zip
ef0f9208202762c93c8c415d8472aa76  JRosetta-1.0.2-GPL.zip

Other than that, it seems fine.

Comment 8 Nicolas Chauvet (kwizart) 2009-01-16 17:15:55 UTC
Spec URL: 
http://kwizart.fedorapeople.org/SPECS/jrosetta.spec
SRPM URL: 
http://kwizart.fedorapeople.org/SRPMS/jrosetta-1.0.1-1.fc8.kwizart.src.rpm
Description: A common base to build a graphical console

I'm reverting back to jrosetta as package named as the .jar files are named jrosetta*.jar anyway.
Im' not bumping the release since they were not a package named jrosetta 1.0.2 previously.

Comment 9 Dominik 'Rathann' Mierzejewski 2009-01-16 21:05:16 UTC
(In reply to comment #8)
> Spec URL: 
> http://kwizart.fedorapeople.org/SPECS/jrosetta.spec

Specfile says 1.0.2, but...

> SRPM URL: 
> http://kwizart.fedorapeople.org/SRPMS/jrosetta-1.0.1-1.fc8.kwizart.src.rpm

this is 1.0.1.

Comment 10 Nicolas Chauvet (kwizart) 2009-01-16 23:02:10 UTC
SRPM URL: 
http://kwizart.fedorapeople.org/SRPMS/jrosetta-1.0.2-1.fc10.src.rpm

Comment 11 Dominik 'Rathann' Mierzejewski 2009-01-19 23:04:43 UTC
Very nice, APPROVED.

Comment 12 Nicolas Chauvet (kwizart) 2009-01-20 11:30:50 UTC
New Package CVS Request
=======================
Package Name: jrosetta
Short Description: A common base to build a graphical console
Owners: kwizart
Branches: F-10 EL-5
Cvsextras Commits: yes

Comment 13 Kevin Fenzi 2009-01-20 21:19:32 UTC
cvs done.