Bug 456972

Just to note that this is Sean's first package and I am sponsoring him. :)
But if someone with more experience on eclipse packaging wants to review that is
ok by me.

Andrew, if you have any initial comments or input on the packaging that would be
useful.

This isn't really Eclipse packaging since you're just dropping files into the
correct place, so I think it's fine for you to review it, Jens.  fetch-babel.sh
will obviously have to be updated once 3.4 is in rawhide (sorry, I'm working on
it ... stupid ppc64 is killing me) and will perhaps require an s/x86/%{arch} but
otherwise I think it's okay from the Eclipse side of things.  I don't know all
the fancy langpack specfile tricks you're using but if you say they're good,
I'll buy it :)

Ah, I see I left the wrong path for ECLIPSE_BIN in fetch-babel.sh, so yes, that
will need changing when 3.4 is in.  Was there something else, too?

As for s/x86/%{arch}, I don't believe Babel has different versions for different
architectures - I left in the "-p2.arch x86" because I assumed the update
manager required it.  (I don't suppose P2 understands noarch?).  I certainly
hope we don't need to download everything multiple times.  It already takes a
couple of hours to download 150MB.

Andrew, what do you think of the location I'm storing the langpacks in:
%{eclipse_base}/dropins/babel-$LOCALE/eclipse/{plugins,features}.  I gather from
your post to linux-distros-dev that we would probably use dropins for this sort
of thing, at least in Fedora 10.  I'm extremely new to P2, but I think it's
neater to use a subdir/subdirs of dropins, rather than dropins itself.   And it
makes the %files section nice and short!

I should probably mention that putting Babel into dropins (eg via my package),
and then installing Babel through the update manager too, tends to break
Ganymede: <https://bugs.eclipse.org/bugs/show_bug.cgi?id=242327>.  Eclipse's
current integration builds are okay though, so I'm hoping this will change.

I'm beginning to think that this little optimisation/hack in my spec file might
be a problem:
----------------------
# Disable repacking of jars, since it takes forever for all the little jars, 
# and we don't need multilib anyway:
%define __jar_repack %{nil}
----------------------

since it looks to me as if eclipse 3.4 is doing away with noarch support
(replacing %{_datadir}/eclipse/ with %{_libdir}/eclipse/).

I don't really understand why we repack jars, except that it's something to do
with multilib support.  I was hoping to avoid repacking 8,962 (really!) jars by
making eclipse-nls noarch.

Do I need to make eclipse-nls multi-arch?  And does anyone know if we really
need to be repacking jars in that case?

> since it looks to me as if eclipse 3.4 is doing away with noarch support
> (replacing %{_datadir}/eclipse/ with %{_libdir}/eclipse/).

I think we need %{_datadir}/eclipse/.  Andrew?

> Do I need to make eclipse-nls multi-arch?  And does anyone know if we really
> need to be repacking jars in that case?

It should be noarch, surely, but may still need to repack jar?

Sean pointed out that this is discussed in:

https://www.redhat.com/archives/fedora-devel-java-list/2008-June/msg00031.html

New Spec/SRPM/fetcher script are all here:

http://seanf.fedorapeople.org/eclipse-nls/0.2.0-0.2/

* Mon Aug 11 2008 Sean Flanigan <sflaniga> - 0.2.0-0.2.20080807snap
- Fixed version in changelog
- Updated snapshot of Babel translation plugins
- Changed code for Hebrew to he (not iw); changed fetch-babel.sh to compensate
- Renamed eclipse_base macro to eclipse_data

Updates and notes:
Andrew's latest package for Fedora Eclipse eclipse-*-3.4.0-18 has proper dropins support for %{_datadir}/eclipse/.

I noticed that eclipse.spec disables jar repacking, so I'm going to assume I can get away with it too, at least for eclipse-nls's very simple needs (no compilation).

https://bugs.eclipse.org/bugs/show_bug.cgi?id=242327 (mentioned previously) doesn't seem to happen with Fedora Eclipse, for some reason.  (The splash screen flashes on and off for about a minute after the update, but then it settles down.)

Also, I was having trouble getting Fedora Eclipse to pick up the translations from any directory at all, but then it started happening with old revisions (eg eclipse-*.3.4.0-15) that *used to work*.  I spent a lot of time trying to work out what was wrong, but got nowhere.  Today I tried the rawhide version of eclipse-*-3.4.0-18.fc10.i386.rpm and everything is working again, so I'd better move things along!

rpmlint says:

eclipse-nls.src:36: W: unversioned-explicit-obsoletes \
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
eclipse-nls-uk.noarch: W: no-documentation
eclipse-nls-uk.noarch: W: obsolete-not-provided eclipse-sdk-nls-uk
eclipse-nls-fr.noarch: W: no-documentation
eclipse-nls-fr.noarch: W: obsolete-not-provided eclipse-sdk-nls-fr
eclipse-nls-pt.noarch: W: no-documentation
eclipse-nls-pt.noarch: W: obsolete-not-provided eclipse-sdk-nls-pt
eclipse-nls-nl.noarch: W: no-documentation
eclipse-nls-nl.noarch: W: obsolete-not-provided eclipse-sdk-nls-nl
eclipse-nls-no.noarch: W: no-documentation
eclipse-nls-no.noarch: W: obsolete-not-provided eclipse-sdk-nls-no
eclipse-nls-ar.noarch: W: no-documentation
eclipse-nls-ar.noarch: W: obsolete-not-provided eclipse-sdk-nls-ar
eclipse-nls-ko.noarch: W: no-documentation
eclipse-nls-ko.noarch: W: obsolete-not-provided eclipse-sdk-nls-ko
eclipse-nls-ro.noarch: W: no-documentation
eclipse-nls-ro.noarch: W: obsolete-not-provided eclipse-sdk-nls-ro
eclipse-nls-sv.noarch: W: no-documentation
eclipse-nls-sv.noarch: W: obsolete-not-provided eclipse-sdk-nls-sv
eclipse-nls-hu.noarch: W: no-documentation
eclipse-nls-hu.noarch: W: obsolete-not-provided eclipse-sdk-nls-hu
eclipse-nls-it.noarch: W: no-documentation
eclipse-nls-it.noarch: W: obsolete-not-provided eclipse-sdk-nls-it
eclipse-nls-ja.noarch: W: no-documentation
eclipse-nls-ja.noarch: W: obsolete-not-provided eclipse-sdk-nls-ja
eclipse-nls-fi.noarch: W: no-documentation
eclipse-nls-fi.noarch: W: obsolete-not-provided eclipse-sdk-nls-fi
eclipse-nls-zh.noarch: W: no-documentation
eclipse-nls-zh.noarch: W: obsolete-not-provided eclipse-sdk-nls-zh
eclipse-nls-pl.noarch: W: no-documentation
eclipse-nls-pl.noarch: W: obsolete-not-provided eclipse-sdk-nls-pl
eclipse-nls-el.noarch: W: no-documentation
eclipse-nls-el.noarch: W: obsolete-not-provided eclipse-sdk-nls-el
eclipse-nls-de.noarch: W: no-documentation
eclipse-nls-de.noarch: W: obsolete-not-provided eclipse-sdk-nls-de
eclipse-nls-es.noarch: W: no-documentation
eclipse-nls-es.noarch: W: obsolete-not-provided eclipse-sdk-nls-es
eclipse-nls-tr.noarch: W: no-documentation
eclipse-nls-tr.noarch: W: obsolete-not-provided eclipse-sdk-nls-tr
eclipse-nls-zh_TW.noarch: W: no-documentation
eclipse-nls-zh_TW.noarch: W: obsolete-not-provided eclipse-sdk-nls-zh_TW
eclipse-nls-he.noarch: W: no-documentation
eclipse-nls-he.noarch: W: obsolete-not-provided eclipse-sdk-nls-he
eclipse-nls-da.noarch: W: no-documentation
eclipse-nls-da.noarch: W: obsolete-not-provided eclipse-sdk-nls-da
eclipse-nls-bg.noarch: W: no-documentation
eclipse-nls-bg.noarch: W: obsolete-not-provided eclipse-sdk-nls-bg
eclipse-nls-ru.noarch: W: no-documentation
eclipse-nls-ru.noarch: W: obsolete-not-provided eclipse-sdk-nls-ru
eclipse-nls-pt_BR.noarch: W: no-documentation
eclipse-nls-pt_BR.noarch: W: obsolete-not-provided eclipse-sdk-nls-pt_BR
eclipse-nls-cs.noarch: W: no-documentation
eclipse-nls-cs.noarch: W: obsolete-not-provided eclipse-sdk-nls-cs
26 packages and 0 specfiles checked; 0 errors, 52 warnings.

I think these can probably be waived.

Minor housekeeping in the new version here:
http://seanf.fedorapeople.org/eclipse-nls/0.2.0-0.3/eclipse-nls.spec

* Mon Sep 9 2008 Sean Flanigan <sflaniga> - 0.2.0-0.3.20080807snap
- Added eclipse_version macro
- Changed the Obsoletes version to be slightly higher than the last release of 
  eclipse-sdk-nls

I have built another SRPM, but I haven't uploaded it.  If anyone wants it, just let me know!

Created attachment 316267 [details]
eclipse-nls.spec.patch-1

minor suggested cleanup

requested by Jens Petersen (#27995)

Applied patch from Jens, result: http://seanf.fedorapeople.org/eclipse-nls/0.2.0-0.4/eclipse-nls.spec

Here is the review:

 +:ok, =:needs attention

MUST Items:
[+] MUST: rpmlint must be run on every package. The output should be posted in the review.
[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines.
[+] MUST: The License field in the package spec file must match the actual license.
[+] MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[NA] 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.

The tarball is created with from upstream updates site with a script included, since there are other 400 .jar fragments per language.

[+] MUST: The package must successfully compile and build into binary rpms on at least one supported architecture.
[+] MUST: All build dependencies must be listed in BuildRequires
[+] MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory.
[+] MUST: A package must not contain any duplicate files in the %files listing.
[+] MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line.
[+] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
[+] MUST: Each package must consistently use macros
[+] MUST: The package must contain code, or permissable content.
[+] MUST: Packages must not own files or directories already owned by other packages.
[+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
[+] MUST: All filenames in rpm packages must be valid UTF-8.

SHOULD Items:
[=] SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.

It would be nice to have a license file from upstream to include in the packages.

[+] SHOULD: The reviewer should test that the package builds in mock.
[=] SHOULD: The reviewer should test that the package functions as described.

There are some know issues with eclipse plugins that are stopping this plugin from working in common cases, but it would be good to have this package included so that they can be straightened out.  A bug should be opened to track that issue is there isn't one already.

Andrew, or any other Java packager: is it ok to not to jar_repack since this is noarch anyway?

Package is APPROVED as packaged.

Created attachment 316289 [details]
eclipse-nls.spec.patch-2

some very minor comment suggestions

(In reply to comment #14)
> Andrew, or any other Java packager: is it ok to not to jar_repack since this is
> noarch anyway?

I think we should run this by fedora-devel-java-list to get opinions and if people are generally okay with it, we can add it to the guidelines.  Can you please send an email to the list to get comments?  Thanks.

Another version here:

http://seanf.fedorapeople.org/eclipse-nls/0.2.0-0.5/eclipse-nls.spec

* Thu Sep 11 2008 Sean Flanigan <sflaniga> - 0.2.0-0.5.20080807snap
- Applied another tidy-up patch from Jens Petersen and added a comment
 about the licence doc files

New Package CVS Request
=======================
Package Name: eclipse-nls
Short Description: Babel translations for Eclipse
Owners: seanf
Branches: 
InitialCC: petersen

(In reply to comment #16)
> I think we should run this by fedora-devel-java-list to get opinions and if
> people are generally okay with it, we can add it to the guidelines.

Okay, let's turn on jar repacking for now in builds until that gets cleared up.

cvs admin done

(In reply to comment #20)
> cvs admin done

Thanks Jens!  I have re-enabled jar repacking for now, and told Koji to get to work:

http://koji.fedoraproject.org/koji/buildinfo?buildID=62602

So eclipse-nls-* should show up in rawhide some time in the next 24 hours.

thanks