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 195585
Summary: | Review Request: tetex-fonts-hebrew | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Dan Kenigsberg <danken> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | panemade |
Target Milestone: | --- | Flags: | kevin:
fedora-cvs+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-06-25 15:32:24 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 |
Description
Dan Kenigsberg
2006-06-15 20:28:30 UTC
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. |