Bug 212974 - Review Request: aspell-ar - Word list/dictionaries for Arabic
Summary: Review Request: aspell-ar - Word list/dictionaries for Arabic
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2006-10-30 13:27 UTC by Dan Kenigsberg
Modified: 2007-11-30 22:11 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-04-02 08:17:30 UTC
Type: ---
Embargoed:
panemade: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)
Use this modified SPEC and test functionality of Package (1.29 KB, application/octet-stream)
2007-03-27 07:41 UTC, Parag AN(पराग)
no flags Details

Description Dan Kenigsberg 2006-10-30 13:27:19 UTC
Spec URL: http://ivrix.org.il/redhat/aspell-ar.spec
SRPM URL: ivrix.org.il/redhat/aspell-ar-1.2-1.src.rpm
Description: Provides the word list/dictionaries for Arabic.

# Note that this package, like other aspell's language packs, does not come up
# cleanly through rpmlint, but with the following errors:
# E: aspell-he no-binary
# E: aspell-he only-non-binary-in-usr-lib
# This is because the package contains only data files which sit under /usr/lib.
# They have to stay there, as they are architecture-dependent (due to
# byte-ordering issues).

Comment 1 Jens Petersen 2006-11-06 06:26:13 UTC
Can aspell-ar not be included in Fedora Core?

Comment 2 Dan Kenigsberg 2006-11-06 07:54:14 UTC
I certainly wouldn't mind if aspell-ar (as well as aspell-he) is included in Core.

Comment 3 Bill Nottingham 2006-11-09 15:58:04 UTC
Having it reviewed for Extras is fine to get the review done. I suspect enough
things will change around Core + Extras soon enough that we can handle the
question of 'where' later.

Comment 4 Lior Kaplan 2007-01-01 20:00:30 UTC
Any news with this package?

I've already seen it in Debian. I'll be happy to see it in Fedora as well.

Thanks.

Comment 5 Lior Kaplan 2007-01-01 20:47:21 UTC
I'm pasting a part of a private mail between me and Dan, at his request. 

On Mon, Jan 01, 2007 at 09:53:49PM +0200, Lior Kaplan wrote:
> > 
> > About the spec file - why do you manually edit the Makefile instead of
> > running ./configure during %build ?
> > 
> > and in %install
> > $(MAKE) DESTDIR=$(DEB_DESTDIR) install
> > 
> > $(DEB_DESTDIR) has the path for the debian package build root.
> > 
> > I think it will look much more cleaner with these changes.

Suggestions are based on aspell-ar package from Debian (which I maintain with
others).

Comment 6 Parag AN(पराग) 2007-03-27 07:30:56 UTC
rpmlint reported following output on SRPM and RPM

E: aspell-ar no-binary
The package should be of the noarch architecture because it doesn't contain
any binaries.
==> Ok for aspell packages

E: aspell-ar only-non-binary-in-usr-lib
There are only non binary files in /usr/lib so they should be in /usr/share.
==> Ok for aspell packages

W: aspell-ar rpm-buildroot-usage %build DEST_DIR = $RPM_BUILD_ROOT
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it
will break short circuiting.

W: aspell-ar rpm-buildroot-usage %build dictdir =
${RPM_BUILD_ROOT}%{_libdir}/aspell-0.60
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it
will break short circuiting.

W: aspell-ar rpm-buildroot-usage %build datadir =
${RPM_BUILD_ROOT}%{_libdir}/aspell-0.60
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it
will break short circuiting.

E: aspell-ar no-cleaning-of-buildroot %install
You should clean $RPM_BUILD_ROOT in the %clean section and just after the
beginning of %install section. Use "rm -Rf $RPM_BUILD_ROOT".


Comment 7 Parag AN(पराग) 2007-03-27 07:41:14 UTC
Created attachment 151002 [details]
Use this modified SPEC and test functionality of Package

Comment 8 Dan Kenigsberg 2007-03-27 11:44:57 UTC
It works for me.
(Though do note that my functionality test is as good as anyone's. I'm no
authority on Arabic spelling.)

--
Memo to self: update aspell-he.spec accordingly.

Comment 9 Parag AN(पराग) 2007-03-27 11:59:43 UTC
(In reply to comment #8)
> It works for me.
  What??
> (Though do note that my functionality test is as good as anyone's. I'm no
> authority on Arabic spelling.)
  I mean aspell-ar package is working fine with new changes to SPEC. I have
already reviewed few aspell-* packages in Extras which have no problems so I
think even I know its functionality test :)
> 
> --
> Memo to self: update aspell-he.spec accordingly.
  What changes you are going to do in aspell-he.spec?



Comment 10 Dan Kenigsberg 2007-03-27 13:10:08 UTC
(In reply to comment #9)

Sorry to be so unclear to warrent two consecutive question marks :-)

> > It works for me.
>   What??

I checked aspell-ar with your spec file against a test case that I used before:
a page from the Arabic Wikipedia. As expected, aspell recognized Arabic words
from the text, and rejected foreign or misspelled words (such as "wikipedia").

> > Memo to self: update aspell-he.spec accordingly.
>   What changes you are going to do in aspell-he.spec?

You suggested a couple of cleaning advice to the Arabic spec file. I should
apply the same advice to my long-ago-accepted aspell-he package, since its spec
file includes all the drawbacks that you pointed. Come to think of it, since I
stole the spec file from the French package, maybe it, too, could use a facelift.

Please tell me if/when aspell-ar is approved to enter Extras.


Comment 11 Parag AN(पराग) 2007-03-28 03:22:53 UTC
Submit a new package SRPM link with changes I suggested to you and I will do
review of it immediately.

Comment 12 Dan Kenigsberg 2007-03-28 13:46:03 UTC
Ok, it's in http://www.ivrix.org.il/redhat/aspell-ar-1.2-2.src.rpm
Do note that with your cleaned up spec file, rpmlint of the SRPM cries about
configure-without-libdir-spec: the configure script ignores %{_libdir}.

Comment 13 Parag AN(पराग) 2007-03-29 04:01:37 UTC
Review:
+ package builds in mock (development i386).
- rpmlint is NOT silent for SRPM and RPM.
  But following messages are ignorable
  E: aspell-ar no-binary
  E: aspell-ar only-non-binary-in-usr-lib
  E: aspell-ar configure-without-libdir-spec
+ SPEC file contains explanation about above warnings.
+ source files match upstream.
154cf762bafdd02db419b62191138738  aspell6-ar-1.2-0.tar.bz2
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc is small; no -doc subpackage required.
+ %doc does not affect runtime.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code, not content.
+ no headers or static libraries.
+ no .pc file present.
+ no -devel subpackage exists.
+ no .la files.
+ no translations are available.
+ Dose owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ Requires: aspell >= 12:0.60
+ Provides: aspell-ar = 1.2-2
+ Not a GUI APP.

APPROVED.

Comment 14 Dan Kenigsberg 2007-03-30 08:41:10 UTC
New Package CVS Request
=======================
Package Name: aspell-ar
Short Description: Arabic word list for Aspell
Owners: danken.ac.il
Branches: FC-6 devel
InitialCC: danken.ac.il


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