Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.

Bug 190071

Summary: Review Request: tetex-dvipost - latex post filter command
Product: [Fedora] Fedora Reporter: José Matos <jamatos>
Component: Package ReviewAssignee: Michael A. Peters <mpeters>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: pertusus
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-05-06 06:41:25 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 163779    
Attachments:
Description Flags
cleanings for rpmlint none

Description José Matos 2006-04-27 10:45:19 UTC
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-1.src.rpm

Description:
dvipost - a latex post filter command to support change bars
and overstrike mode.

Comment 1 José Matos 2006-04-27 11:15:20 UTC
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

Comment 2 Michael A. Peters 2006-04-27 11:52:24 UTC
I'll review this package

Comment 3 Patrice Dumas 2006-04-27 11:55:23 UTC
Here is a patch to make rpmlint happy.

Comment 4 Patrice Dumas 2006-04-27 11:56:20 UTC
Created attachment 128299 [details]
cleanings for rpmlint

Comment 5 Michael A. Peters 2006-04-27 12:05:26 UTC
(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.

Comment 6 Michael A. Peters 2006-04-27 12:14:00 UTC
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./




Comment 7 José Matos 2006-04-27 12:21:38 UTC
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.


Comment 8 Michael A. Peters 2006-04-27 12:27:14 UTC
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.

Comment 9 José Matos 2006-04-27 12:36:37 UTC
* 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

Comment 10 Michael A. Peters 2006-04-27 13:16:20 UTC
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.

Comment 11 José Matos 2006-04-27 14:37:54 UTC
(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.

Comment 12 Michael A. Peters 2006-04-27 15:06:43 UTC
(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.

Comment 13 José Matos 2006-04-27 15:41:11 UTC
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

Comment 14 Michael A. Peters 2006-04-27 16:05:35 UTC
Approved

Comment 15 Patrice Dumas 2006-04-28 09:24:02 UTC
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.

Comment 16 Michael A. Peters 2006-04-28 09:52:18 UTC
kpsewhich is provided by tetex-fonts.
It would need to be there if explicit tetex-fonts dependency is removed.

Comment 17 Patrice Dumas 2006-04-28 10:18:41 UTC
Yes, that's what I said above. I think 

BuildRequires: /usr/bin/kpsewhich

is better than

BuildRequires: tetex-fonts

Not a big deal.

Comment 18 José Matos 2006-05-06 06:41:25 UTC
(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 

Comment 19 Michael A. Peters 2006-05-06 06:58:44 UTC
(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)

Comment 20 José Matos 2006-05-19 12:42:44 UTC
(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. :-)