Bug 190071

I was not completly satisfied with the resulting package so I have a new 
versio available:

* Thu Apr 27 2006 José Matos <jamatos.pt> - 1.1-2
- Add new entries to %%doc and expand description

Spec URL: http://www.fc.up.pt/pessoas/jamatos/fedora-extras/dvipost.spec
SRPM URL: 
http://www.fc.up.pt/pessoas/jamatos/fedora-extras/dvipost-1.1-2.src.rpm

I'll review this package

Here is a patch to make rpmlint happy.

Created attachment 128299 [details]
cleanings for rpmlint

(In reply to comment #4)
> Created an attachment (id=128299) [edit]
> cleanings for rpmlint
> 

-Requires(post): /usr/bin/texhash
-Requires(pre):  /usr/bin/texhash

What is the purpose of that?
I know texhash is supplied by tetex-fonts which is required by tetex, and that
any tetex install is going to have to have texhash, but I personally think it is
a good idea to explicitly require what is run the post/postun scripts.

Other changes in the patch are good.

mock build error:

RPM build errors:
    File not found by glob: /var/tmp/dvipost-1.1-2.fc5-root-mockbuild/usr/share/
texmf/tex/latex/misc/*

It seems that in mock, it can't identify the TEXMF and installs it into
dvipost-1.1-2.fc5-root-mockbuild./

The purpose of texhash is to create an hash with .sty files paths archived.

That is why we need to run it after installing and after removing it.

In general I agree with you, but in this case the relation between tetex and 
tetex-fonts is such that I don't expect ever to able to run tetex without 
tetex-fonts (famous last words ;-).

In reply to #4, Patrice I have incorporated your patch in release 3.

In reply to #6 /usr/share/texmf/tex/latex/misc/ belongs to tetex-latex, so 
better requiring it for building and install.

From the makefile inside the mock build -

# Directory to install LaTeX styles

LATEX=  $(DESTDIR).

which is incorrect.

-=-

This is how configure does it:

if      texpath=`kpsewhich latex.ltx 2>/dev/null`
then    kpseflag=""
elif    texpath=`kpsewhich tex latex.ltx 2>/dev/null`
then    kpseflag="-DKPSEWHICH_NEED_TYPE"
else    texpath="."; kpseflag=""
fi


texpath=`echo $texpath | sed -e 's%/[^/][^/]*/[^/]*$%/misc%'`


if      test "$texpath" = "."
then    echo "$ac_t"""broken"" 1>&6
elif    test "$kpseflag" != ""
then    echo "$ac_t""kpsewhich needs type" 1>&6
else    echo "$ac_t""ok" 1>&6
fi

latex.ltx is tetex-latex - which is probably why the mock build failed.

* Thu Apr 27 2006 José Matos <jamatos.pt> - 1.1-3
- Capitalize Summary, fix spell error in description, rework
  invocation of post and postun calls (thanks to Patrice Dumas)
- Add tetex-latex to Requires and BuildRequires.
- Add tetex-fonts to Requires to satisfy direct dependency on texhash

Spec URL: http://www.fc.up.pt/pessoas/jamatos/fedora-extras/dvipost.spec
SRPM URL: 
http://www.fc.up.pt/pessoas/jamatos/fedora-extras/dvipost-1.1-3.src.rpm

Good:

md5sum matches upstream : 2ec79283a8348312bc72831ca80ae3a2  dvipost.tar.gz
Builds in mock (fc5 x86)
rpmlint clean on all packages
spec file written in proper English
spec file easy to read and understand
cleanly installs and removes w/ no unowned directories
spec file name matches package name
consistent use of macros
Appropriate license (GPL), matches package COPYING file.
Package works.

Suggestions (non blocking):
1) The spec file explicitly specifies /usr/share/texmf in the %files.
That is the location in every fedora install - but some other spec files detect
the texmfmain directory in a macro and use that instead.

If a user has for whatever reason changed their texmfmain - the src.rpm would
have a build error when rebuilt.

2) The html documentation might want to placed into texmf/doc somewhere so that
texdoc dvipost will launch a browser window to the documentation.

Question:

From http://fedoraproject.org/wiki/Packaging/NamingGuidelines
----
If a new package is considered an "addon" package that enhances or adds a new
functionality to an existing Fedora Core or Fedora Extras package without being
useful on its own, its name should reflect this fact.

The new package ("child") should prepend the "parent" package in its name, in
the format: %{parent}-%{child}.
----

Since this package isn't useful without tetex, and is used in conjunction with
tetex, should it be called tetex-dvipost ?

-=-
Misc suggestion for upstream - filter out the cgi-bin references in the man2html
conversion of the man page.

(In reply to comment #10)
> Good:
> 
> md5sum matches upstream : 2ec79283a8348312bc72831ca80ae3a2  dvipost.tar.gz
> Builds in mock (fc5 x86)
> rpmlint clean on all packages
> spec file written in proper English
> spec file easy to read and understand
> cleanly installs and removes w/ no unowned directories
> spec file name matches package name
> consistent use of macros
> Appropriate license (GPL), matches package COPYING file.
> Package works.
> 
> Suggestions (non blocking):
> 1) The spec file explicitly specifies /usr/share/texmf in the %files.
> That is the location in every fedora install - but some other spec files 
detect
> the texmfmain directory in a macro and use that instead.
> 
> If a user has for whatever reason changed their texmfmain - the src.rpm 
would
> have a build error when rebuilt.

  Something like:

 %{!?_texmf: %define _texmf %(eval "echo 
`kpsewhich -expand-var '$TEXMFMAIN'`")}

> 2) The html documentation might want to placed into texmf/doc somewhere so 
that
> texdoc dvipost will launch a browser window to the documentation.

  That makes sense but then it would imply to Require: tetex-doc. That would 
mean that a 40 KB package could potencially require an 100 MB package. I don't 
think this is worth it. :-)

> Question:
> 
> From http://fedoraproject.org/wiki/Packaging/NamingGuidelines
> ----
> If a new package is considered an "addon" package that enhances or adds a 
new
> functionality to an existing Fedora Core or Fedora Extras package without 
being
> useful on its own, its name should reflect this fact.
> 
> The new package ("child") should prepend the "parent" package in its name, 
in
> the format: %{parent}-%{child}.
> ----
> 
> Since this package isn't useful without tetex, and is used in conjunction 
with
> tetex, should it be called tetex-dvipost ?

  Actually I think that dvipost requires a tex installation, there is nothing 
exclusive from tetex. That was the reason why I have proposed dvipost and not 
tetex-dvipost.

  If you feel strongly about this I will rename it.

> -=-
> Misc suggestion for upstream - filter out the cgi-bin references in the 
man2html
> conversion of the man page.

  I agree.

(In reply to comment #11)

>   Something like:
> 
>  %{!?_texmf: %define _texmf %(eval "echo 
> `kpsewhich -expand-var '$TEXMFMAIN'`")}

Well, except that defining _texmf would in this case cause an error as well
unless configure was patched to take it as an arguement (which I don't think is
necessary) - I guess in this case expecting to support building against modified
tetex environments might be a bit much because of the upstream configure script
which looks for a specific file and doesn't take a texmf as a switch.

> 
>   That makes sense but then it would imply to Require: tetex-doc. That would 
> mean that a 40 KB package could potencially require an 100 MB package. I don't 
> think this is worth it. :-)

/usr/bin/texdoc is owned by tetex.
The potential problem is who owns the directories within the tex documentation
tree if tetex-doc isn't installed - but other packages just own it themselves.

Since it is just the man page, and available as a man page, it isn't that big of
a deal.


> 
>   Actually I think that dvipost requires a tex installation, there is nothing 
> exclusive from tetex. That was the reason why I have proposed dvipost and not 
> tetex-dvipost.
> 
>   If you feel strongly about this I will rename it.

On fedora - tetex is what provides tex.
There are other examples of this (in core)

[mpeters@atlantis Desktop]$ rpm -qf /usr/bin/dvips
tetex-dvips-3.0-17
[mpeters@atlantis Desktop]$ rpm -qf /usr/bin/xdvi
tetex-xdvi-3.0-17

It also makes it a little easier to find when browsing repoview for tetex boltons.

What about:

%changelog
* Thu Apr 27 2006 José Matos <jamatos.pt> - 1.1-4
- Rename package to tetex-dvipost

Spec URL: http://www.fc.up.pt/pessoas/jamatos/fedora-extras/tetex-dvipost.spec
SRPM URL: 
http://www.fc.up.pt/pessoas/jamatos/fedora-extras/tetex-dvipost-1.1-4.src.rpm

Approved

Some comments:

* %post -p /usr/bin/texhash

will automatically add 
Requires(post) /usr/bin/texhash

So I think the explicit dependency on tetex-fonts is not really right 
as it may change in the future.

* A dependence on kpsewhich should be there, however, in my opinion:
BuildRequires: /usr/bin/kpsewhich

* In my opinion the following should be used to detect %_texmf, since
in configure kpsewhich is also used (even though a bit differently
but I believe the result is the same)
%{!?_texmf: %define _texmf %(eval "echo `kpsewhich -expand-var '$TEXMFMAIN'`")}

* the tetex package is picked up by tetex-latex, so I think that 
the dependencies on tetex should be removed.

kpsewhich is provided by tetex-fonts.
It would need to be there if explicit tetex-fonts dependency is removed.

Yes, that's what I said above. I think 

BuildRequires: /usr/bin/kpsewhich

is better than

BuildRequires: tetex-fonts

Not a big deal.

(In reply to comment #15)
> Some comments:
> 
> * %post -p /usr/bin/texhash
> 
> will automatically add 
> Requires(post) /usr/bin/texhash

  Right.

> So I think the explicit dependency on tetex-fonts is not really right 
> as it may change in the future.

  OK.

> * A dependence on kpsewhich should be there, however, in my opinion:
> BuildRequires: /usr/bin/kpsewhich

  Done.

> * In my opinion the following should be used to detect %_texmf, since
> in configure kpsewhich is also used (even though a bit differently
> but I believe the result is the same)
> %{!?_texmf: %define _texmf %(eval "echo 
`kpsewhich -expand-var '$TEXMFMAIN'`")}

  I agree, we should probably harmonize this in rules for tetex derived 
packages. Reading other tetex-* packages both ways are used.

> * the tetex package is picked up by tetex-latex, so I think that 
> the dependencies on tetex should be removed.

  I knew that. :-)
  I removed it.

  The changes have been commited to CVS but I don't think the changes require 
a new build.

  The package build cleanly for all branches so I will close this bug.
  The package build cleanly, so 

(In reply to comment #18)

> 
> > * In my opinion the following should be used to detect %_texmf, since
> > in configure kpsewhich is also used (even though a bit differently
> > but I believe the result is the same)
> > %{!?_texmf: %define _texmf %(eval "echo 
> `kpsewhich -expand-var '$TEXMFMAIN'`")}
> 
>   I agree, we should probably harmonize this in rules for tetex derived 
> packages. Reading other tetex-* packages both ways are used.

In this case - using %{!?_texmf: %define blah}
should probably not be used since configure doesn't take an arguement for what
texmf to use.

So if I did

rpmbuild --define '_texmf /mnt/nfs/my_texmf' --rebuild foo.src.rpm

the package might fail because kpsewhich in configure would pick up TEXMFMAIN
instead of what the macro defines.

Once upstream adds a configure switch to optionally specify the texmf, then
allowing a custom texmf in the spec file via setting a macro makes sense.

Upstream should probably be bugged about that. If I was building it from source,
I would want it in TEXMFLOCAL (or in my home dir texmf) - so it should be a
configure switch (and probably should default to TEXMFLOCAL if no arguement is
given to configure)

(In reply to comment #19)
> Upstream should probably be bugged about that. If I was building it from 
source,
> I would want it in TEXMFLOCAL (or in my home dir texmf) - so it should be a
> configure switch (and probably should default to TEXMFLOCAL if no arguement 
is
> given to configure)

I agree with your analysis, Michael. This goes to my todo list with low 
priority though. :-)