Review for this package:-
         Mock build for i386 gives no error
MUST Items:
     - MUST: rpmlint shows no error 
     - MUST: The package is named according to the Package Naming Guidelines.
     - MUST: The spec file name matching the base package tetex-fonts-hebrew, in
the format tetex-fonts-hebrew.spec
      - MUST: This package meets the Packaging Guidelines.
      - MUST: The package is licensed with an open-source compatible license GPL.
      - MUST: The License field GPL, in the package tetex-fonts-hebrew.spec file
is NOT included under %doc section as well NOT in tarball.
      - MUST: The sources used to build the package matches the upstream source,
as provided in the spec URL. md5sum is correct.
      - MUST: This package owns all directories that it creates. 
      - MUST: This package did not contain any duplicate files in the %files
listing.
      - MUST: This package have a %clean section, which contains %{__rm} -rf
%{buildroot}.
      - MUST: This package used macros.
      - MUST: Document files are NOT included like README.

SHOULD Items:
      - SHOULD: Package should include License file in upstream package.

I am not sure I understand all your comments. Anyway, I added the GPL to the
package, and cleaned one warining by rpmlint. The other warning are
indespensible (I think) because this package *should* and a soft link to an
other package - the fotns-hebrew package.

I uploaded the corrected version to
http://ivrix.org.il/redhat/tetex-fonts-hebrew.spec and
http://ivrix.org.il/redhat/tetex-fonts-hebrew-0.1-2.src.rpm

Should it be further corrected?

Parag, if you'reviewing this package, you should assign it to yourself.

Above is Not an official review as I'm not yet sponsored

Package failes to build in mock on x86_64, both FC5 and development.  The build
fails at:

cp: cannot stat `*.tfm': No such file or directory

Indeed, there are no .tfm files in the current directory when that is run.  This
is due to failures in %build:

+ make
./mkCLMtfm.sh
rm: cannot remove `culmus.map': No such file or directory
afm2tfm: fatal: afm file `/usr/share/fonts/hebrew/AharoniCLM-Bold.afm' not found.
afm2tfm: fatal: afm file `/usr/share/fonts/hebrew/AharoniCLM-Bold.afm' not found.
./mkCLMtfm.sh: line 47: vptovf: command not found
./mkCLMtfm.sh: line 48: vptovf: command not found
(repeated for each font).

I think you're missing a BuildRequires: tetex.  tetex-afm does not pull it in. 
Adding it lets things build, although I then see a bunch of errors from mkCLMtfm.sh:

./mkCLMtfm.sh
rm: cannot remove `culmus.map': No such file or directory
afm2tfm: fatal: afm file `/usr/share/fonts/hebrew/AharoniCLM-Bold.afm' not found.
afm2tfm: fatal: afm file `/usr/share/fonts/hebrew/AharoniCLM-Bold.afm' not found.
afm2tfm: fatal: afm file `/usr/share/fonts/hebrew/AharoniCLM-BoldOblique.afm'
not found.
afm2tfm: fatal: afm file `/usr/share/fonts/hebrew/AharoniCLM-BoldOblique.afm'
not found.
(and so on for many files)

I think this points to a missing BuildRequires: fonts-hebrew.  I still get:

./mkCLMtfm.sh
rm: cannot remove `culmus.map': No such file or directory

and a bunch of "I had to round some hights by X units" but it looks like the
package builds OK now.  (It actually built OK but didn't contain any tfm files
without the BR: fonts-hebrew; you might want to add some error-checking
somewhere.)  I am at a loss to see how this would build at all in mock as stated
in a previous comment.

Can you verify that you are the upstream for the source tarball?  Generally your
Source: tag includes a URL to the upstream source, but it's possible that for a
package like this the package is the upstream source.  I'm going to assume that
there is no upstream source here.

Once built, rpmlint has this to say:
W: tetex-fonts-hebrew incoherent-version-in-changelog 0.1-1 0.1-2.fc6

You don't seem to have added a changelog entry for what went into release 2.

W: tetex-fonts-hebrew dangling-symlink
/usr/share/texmf/fonts/type1/public/culmus /usr/share/fonts/hebrew
W: tetex-fonts-hebrew dangling-symlink /usr/share/texmf/fonts/afm/public/culmus
/usr/share/fonts/hebrew

Symlinks into packages which are dependencies are OK.

W: tetex-fonts-hebrew symlink-should-be-relative
/usr/share/texmf/fonts/type1/public/culmus /usr/share/fonts/hebrew
W: tetex-fonts-hebrew symlink-should-be-relative
/usr/share/texmf/fonts/afm/public/culmus /usr/share/fonts/hebrew

Absolute symlinks aren't OK; these should be relative (a blocker).

You have various scriptlets which call texhash and updmap-sys but you don't
specify appropriate requirements for them:

Requires(post): tetex (or /usr/bin/texhash) (updmap-sys comes from tetex-fonts
which is a dependency of tetex; it should be OK to leave it out but you're free
to be more explicit if you like)
Requires(preun): tetex-fonts (or /usr/bin/updmap-sys)

(the postun requirement on /usr/bin/texhash is picked up by rpm automatically)

This package doesn't seem to own /usr/share/texmf/fonts/tfm/public/culmus and
/usr/share/texmf/fonts/vf/public/culmus/, and nothing else in the repository
seems to either.  (/usr/share/texmf/fonts/tfm/public and
/usr/share/texmf/fonts/vf/public are owned by tetex-fonts which is in the
dependency tree).

Review:
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text included in package.
O can't compare source against upstream as there is none.
O can't compare version agains upstream.
O BuildRequires are proper (one amended as above)
* package builds in mock (development, x86_64).
X rpmlint has valid complaints (see above).
* final provides and requires are sane:
   tetex-fonts-hebrew = 0.1-2.fc6
  =
   /bin/sh
   /usr/bin/texhash
   fonts-hebrew
   tetex
* no shared libraries are present.
* package is not relocatable.
X owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* %clean is present.
* %check is not present; no test suite.
X scriptlets present; dependencies are not correct.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.
* not a GUI app.

Thanks for the detailed and helpful review.

> I think you're missing a BuildRequires: tetex.  tetex-afm does not
> pull it in.  I think this points to a missing BuildRequires:
> fonts-hebrew.  I still get:

Indeed, I implicitly expected that both tetex and fonts-hebrew are
there on build time.

> ./mkCLMtfm.sh rm: cannot remove `culmus.map': No such file or
> directory

I eliminated this error by replacing the rm with a cp of /dev/null

> and a bunch of "I had to round some hights by X units" but it looks
> like the package builds OK now. 

I don't know what is this LaTeX problem. I hope to have some Hebrew
support, it does not have to be flawless.

> without the BR: fonts-hebrew; you might want to add some
> error-checking somewhere.)  

Maybe I should, but I don't know what and how. Checking whether the
number of generated files is nonzero sounds very silly...

> Can you verify that you are the upstream for the source tarball?
> Generally your Source: tag includes a URL to the upstream source,
> but it's possible that for a package like this the package is the
> upstream source.  I'm going to assume that there is no upstream
> source here.

Indeed, I am the one packing the tarball. Note however, that this
package is a (simple) repackaging of the Culmus fonts for the use of
tetex.

> Once built, rpmlint has this to say: W: tetex-fonts-hebrew
> incoherent-version-in-changelog 0.1-1 0.1-2.fc6
> 
> You don't seem to have added a changelog entry for what went into
> release 2.

You got me.

> Absolute symlinks aren't OK; these should be relative (a blocker).

Should I use ../../share/fonts/hebrew instead?

> You have various scriptlets which call texhash and updmap-sys but
> you don't specify appropriate requirements for them:
> 
> Requires(post): tetex (or /usr/bin/texhash) (updmap-sys comes from
> tetex-fonts which is a dependency of tetex; it should be OK to leave
> it out but you're free to be more explicit if you like)
> Requires(preun): tetex-fonts (or /usr/bin/updmap-sys)
> 
> (the postun requirement on /usr/bin/texhash is picked up by rpm
> automatically)

I should be RTFMing about it, but are my scriptlets sane? I would like
to run texhash after any change to the TeX tree, and updmap-sys if
culmus.map is added (or going to be removed). Would you take a look?
 
> This package doesn't seem to own
> /usr/share/texmf/fonts/tfm/public/culmus and
> /usr/share/texmf/fonts/vf/public/culmus/, and nothing else in the
> repository seems to either.  (/usr/share/texmf/fonts/tfm/public and
> /usr/share/texmf/fonts/vf/public are owned by tetex-fonts which is
> in the dependency tree).

I hope this is solved by adding the directories explicitly to the file
list.

Please see the update SRPM at
http://ivrix.org.il/redhat/tetex-fonts-hebrew-0.1-3.src.rpm

I answered my own scriptlet question by stealing the skiptlets of
tetex-font-kerkis rpm. Please review
http://ivrix.org.il/redhat/tetex-fonts-hebrew-0.1-4.src.rpm istead.

rpmlint is down to these:

W: tetex-fonts-hebrew dangling-relative-symlink
/usr/share/texmf/fonts/type1/public/culmus ../../../../fonts/hebrew
W: tetex-fonts-hebrew dangling-relative-symlink
/usr/share/texmf/fonts/afm/public/culmus ../../../../fonts/hebrew

which are OK.

Only a couple of issues I can see; sorry if I missed these earlier.

The summary is a bit confusing given the name of the package, because Culmus
seems to be a non-sequitur (to someone who doesn't know what Culmus is).  How
about "Culmus Hebrew font support for tetex"?

I'm seeing the file:
/usr/share/texmf/tex/generic/0babel/hebrew.ldf
which I must have overlooked from earlier.  Nothing seems to own the directory
/usr/share/texmf/tex/generic/0babel, so this package will need to own it.

Thanks for your comments. They are applied into
http://ivrix.org.il/redhat/tetex-fonts-hebrew-0.1-5.src.rpm

OK, everything looks good now.  The summary is more descriptive and there are no
longer any unowned directories.  As those were the only outstanding issues, this
package is

APPROVED

The component field for package reviews must stay to "Package Review".

Package Change Request
======================
Package Name: tetex-fonts-hebrew
New Branches: el5


A colleague wants to typeset Hebrew document on a RHEL-5 machine. Let's help him.

You need to specify the owner in the request.

Package Change Request
======================
Package Name: tetex-fonts-hebrew
New Branches: el5
Owners: danken

package owner is unchange (me). does this provide the missing info?

Git done (by process-git-requests).

You may consider taking over the devel/rawhide version too. 
It's currently orphaned.

tetex-fonts-hebrew-0.1-12.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/tetex-fonts-hebrew-0.1-12.el5

tetex-fonts-hebrew-0.1-12.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.