Bug 532057 - Review Request: eclipse-jgit - Eclipse JGit
Summary: Review Request: eclipse-jgit - Eclipse JGit
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Andrew Overholt
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-10-30 13:30 UTC by Alexander Kurtakov
Modified: 2009-11-10 16:52 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-11-10 16:52:48 UTC
Type: ---
Embargoed:
overholt: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Alexander Kurtakov 2009-10-30 13:30:00 UTC
Spec URL: http://akurtakov.fedorapeople.org/eclipse-jgit.spec
SRPM URL: http://akurtakov.fedorapeople.org/eclipse-jgit-0.6.0-0.git20091029.1.fc11.src.rpm
Description: A pure Java implementation of the Git version control system.

Comment 1 Peter Lemenkov 2009-10-31 18:49:53 UTC
Issues:

* First, and most important - I don't know too much about eclipse plugins, but I'm sure, that using %{_libdir} in spec-file renders package to be arch-dependent. So, I believe, that either noarch should be removed, or "%define eclipse_base   %{_libdir}/eclipse" should be fixed to not contain %{_libdir}.

* Naming scheme is somewhat wrong. Instead of "Release:        0.git20091029.1%{?dist}" it should be

Release:        0.1.git20091029%{?dist}

Other things looks sane.

Comment 2 Alexander Kurtakov 2009-10-31 22:01:03 UTC
(In reply to comment #1)
> Issues:
> 
> * First, and most important - I don't know too much about eclipse plugins, but
> I'm sure, that using %{_libdir} in spec-file renders package to be
> arch-dependent. So, I believe, that either noarch should be removed, or
> "%define eclipse_base   %{_libdir}/eclipse" should be fixed to not contain
> %{_libdir}.
> 
> * Naming scheme is somewhat wrong. Instead of "Release:       
> 0.git20091029.1%{?dist}" it should be
> 
> Release:        0.1.git20091029%{?dist}
> 
> Other things looks sane.  

%{_libdir} is used only at buildtime and it is used so we can use scripts from arch dependent package. It has nothing to do with the noarch type of the package because I'm neither doing some linking, symlinking or whatever else from %{_libdir} nor I'm installing something in %{_libdir}.
You can see the guidelines http://fedoraproject.org/wiki/Packaging/EclipsePlugins where it explained in details. It is well working for a almost every eclipse plugin included in Fedora so  I don't see it is a problem.

Comment 3 Andrew Overholt 2009-11-02 20:28:03 UTC
I thought jgit was the BSD-licensed library that egit needed.  Is it okay that this is being installed into %{eclipse_home} instead of %{java_dir}?

Comment 4 Alexander Kurtakov 2009-11-03 08:34:28 UTC
(In reply to comment #3)
> I thought jgit was the BSD-licensed library that egit needed.  Is it okay that
> this is being installed into %{eclipse_home} instead of %{java_dir}?  

It's ok because current jgit contains only parts needed by newer egit without other parts. Upstream devs are redoing their standalone jar building with maven and once they are ready I'll update it and install in %{_javadir} but meantime I want to get egit updated because current one is filling Error log when installed.

Comment 5 Andrew Overholt 2009-11-03 20:00:42 UTC
- naming fine
X please fix the release to follow:

http://fedoraproject.org/wiki/Packaging/NamingGuidelines#PreReleasePackages

Peter is right that there should be a 1. before the "git..."

- licensing fine (EDL is BSD)
- macros fine
- locations good
- %files fine
- simplicity = good :)

X rpmlint output clean except directory permission error:

0 packages and 1 specfiles checked; 0 errors, 0 warnings.
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
eclipse-jgit.noarch: E: non-standard-dir-perm /usr/share/eclipse/dropins/jgit/eclipse/features/org.eclipse.jgit_0.6.0.200911031418 0775
A standard directory should have permission set to 0755. If you get this
message, it means that you have wrong directory permissions in some dirs
included in your package.

1 packages and 0 specfiles checked; 1 errors, 0 warnings.

Comment 6 Alexander Kurtakov 2009-11-05 15:49:10 UTC
(In reply to comment #5)
> - naming fine
> X please fix the release to follow:
> 
> http://fedoraproject.org/wiki/Packaging/NamingGuidelines#PreReleasePackages
> 
> Peter is right that there should be a 1. before the "git..."
> 
Fixed.

> - licensing fine (EDL is BSD)
> - macros fine
> - locations good
> - %files fine
> - simplicity = good :)
> 
> X rpmlint output clean except directory permission error:
> 
> 0 packages and 1 specfiles checked; 0 errors, 0 warnings.
> 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
> eclipse-jgit.noarch: E: non-standard-dir-perm
> /usr/share/eclipse/dropins/jgit/eclipse/features/org.eclipse.jgit_0.6.0.200911031418
> 0775

I don't see this error when build locally

> A standard directory should have permission set to 0755. If you get this
> message, it means that you have wrong directory permissions in some dirs
> included in your package.
> 
> 1 packages and 0 specfiles checked; 1 errors, 0 warnings.  

New sources:
Spec URL: http://akurtakov.fedorapeople.org/eclipse-jgit.spec
SRPM URL:
http://akurtakov.fedorapeople.org/eclipse-jgit-0.6.0-0.git20091029.1.fc11.src.rpm

Comment 7 Andrew Overholt 2009-11-05 16:24:08 UTC
I think you meant:

http://akurtakov.fedorapeople.org/eclipse-jgit-0.6.0-0.1.git20091029.fc11.src.rpm

I don't know why I'm getting the non-standard directory permission thing.  I've seen it other times as well and couldn't reproduce on another machine so I'll chalk it up to some oddities with my setup.

APPROVED

Comment 8 Alexander Kurtakov 2009-11-06 11:35:55 UTC
New Package CVS Request
=======================
Package Name: eclipse-jgit
Short Description: Java implementation of the Git version control system.
Owners: akurtakov
Branches: 
InitialCC:

Comment 9 Kevin Fenzi 2009-11-06 20:28:40 UTC
cvs done.

Comment 10 Alexander Kurtakov 2009-11-10 16:52:48 UTC
Built in rawhide.


Note You need to log in before you can comment on or make changes to this bug.