Bug 626175

Summary: Review Request: DeTex - is a filter program that removes the LaTeX control sequences from tex files
Product: [Fedora] Fedora Reporter: Didi Hoffmann <didi>
Component: Package ReviewAssignee: Christoph Wickert <cwickert>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: alexis.lameire, cwickert, fedora-package-review, notting, sanjay.ankur
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: 2011-03-06 08:08:25 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 201449    

Description Didi Hoffmann 2010-08-22 10:09:01 EDT
Spec URL: http://www.ribalba.de/dump/detex.spec
SRPM URL: http://www.ribalba.de/geek/port/src/detex-2.8-1.src.rpm
Description:
DeTeX is a filter program that removes the LaTeX (or TeX)
control sequences from the input so that the real content can be
passed to a spell or diction checker
Comment 1 Christoph Wickert 2010-08-22 10:16:17 EDT
I#m going to take this over, stay tuned.
Comment 2 Lameire Alexis 2010-08-22 10:32:29 EDT
Although this review is in progress, I see two (very) little issues:
- on the summary, you must write TeX and LaTeX, not Tex/LaTex ;)
- you should indent all the values in your preamble.
Comment 3 Christoph Wickert 2010-10-31 10:24:40 EDT
I'm extremely sorry for the delay Didi!


REVIEW FOR 458b5603a1c21a383285e4aceeb4f6b7  detex-2.8-1.src.rpm

FIX - MUST: rpmlint must be run on every package. 

$ rpmlint /var/lib/mock/fedora-14-x86_64/result/detex-*
detex.src:24: W: setup-not-quiet

Use %setup -q instead of %setup

detex.src:32: W: macro-in-comment %{__make}
detex.src:32: W: macro-in-comment %{buildroot}

This is the line that is commented out, remove it. Macros in comments confuse rpm. Even though things are commented out, the macros get resolved. When you have a macro like %setup, that consists of several lines (use 'rpm --eval %setup' to check), the build will break.

detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: normal or special character expected (got a space)
detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: warning: numeric expression expected (got `,')
detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: normal or special character expected (got a space)
detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: normal or special character expected (got a space)
detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: cannot use a space as a starting delimiter
detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: normal or special character expected (got a space)
detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: normal or special character expected (got a space)
detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: normal or special character expected (got a space)
detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: normal or special character expected (got a space)
detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: normal or special character expected (got a space)
detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: normal or special character expected (got a space)
detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: normal or special character expected (got a space)
detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: normal or special character expected (got a space)
detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: normal or special character expected (got a space)
detex.x86_64: W: manual-page-warning /usr/share/man/man1/detex.1.gz 57: normal or special character expected (got a node)

Not sure about this one.

detex-debuginfo.x86_64: E: debuginfo-without-sources

Read http://fedoraproject.org/wiki/Packaging/Debuginfo

OK - MUST: named according to the Package Naming Guidelines
OK - MUST: spec file name matches the base package %{name}
OK - MUST: package meets the Packaging Guidelines
OK - MUST: Fedora approved license and meets the Licensing Guidelines (NCSA)
FIX - MUST: License field in spec file matches the actual license: According to  its http://www.cs.purdue.edu/homes/trinkle/detex/ NCSA not BSD.

FIX - MUST: license file is not included in %doc: Include the COPYRIGHT file in %doc.

OK - MUST: spec is in American English
FIX - MUST: spec is legible, but could be better if the tags in the header were properly indented.

OK - MUST: sources match the upstream source by MD5 7a96b647f43bb077323cde92faa1e893
FIX - MUST: does not successfully compile and build into binary rpms:
error: %patch without corresponding "Patch:" tag

You are using Patch0:, therefor the patch needs to be applied with %patch0 too, not only %patch.

N/A - MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch.
OK - MUST: all build dependencies are listed in BuildRequires.
N/A - MUST: handles locales properly with %find_lang
N/A - MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun.
OK - MUST: Package does not bundle copies of system libraries.
N/A - MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package.
OK - MUST: owns all directories that it creates (none)
OK - MUST: no duplicate files in the %files listing
OK - MUST: Permissions on files are set properly, includes %defattr(...)
OK - MUST: consistently uses macros
OK - MUST: package contains code, or permissable content
N/A - MUST: Large documentation files should go in a -doc subpackage
OK - MUST: Files included as %doc do not affect the runtime of the application
N/A - MUST: Header files must be in a -devel package
N/A - MUST: Static libraries must be in a -static package
N/A - MUST: library files that end in .so are in the -devel package.
N/A - MUST: devel packages must require the base package using a fully versioned dependency
OK - MUST: The package does not contain any .la libtool archives.
N/A - MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section.
OK - MUST: package does not own files or directories already owned by other packages.
OK - Should: at the beginning of %install, the package runs rm -rf %{buildroot}.
OK - MUST: all filenames valid UTF-8


SHOULD Items:
OK - SHOULD: Source package includes license text(s) as a separate file.
N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available.
OK - SHOULD: builds in mock (once the naming of the patch is fixed)
OK - SHOULD: compiles and builds into binary rpms on all supported architectures, see http://koji.fedoraproject.org/koji/taskinfo?taskID=2566981
OK - SHOULD: functions as described.
N/A - SHOULD: Scriptlets are sane (no scriptlets used).
N/A - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
N/A - SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg
OK - SHOULD: no file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin
OK - SHOULD: package should contain man pages for binaries/scripts.


Other items:
OK - latest stable version
OK - SourceURL valid
OK - Compiler flags ok
OK - Debuginfo complete
OK - SHOULD: package has a %clean section, which contains rm -rf %{buildroot}.
N/A - SHOULD: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'.


TODO:

- Fix all isues marked with FIX

- Naming of the patch should be <name-of-package>-<version-when-introduced>-<name-of-patch>.patch. In this case Patch0 would be named detex-2.8-malloc-and-troff.patch

- Do not use macros like %{__rm}, %{__make} and %{__install} for simple commands. You never know if/how these marcos are defined.

- %defattr should be (-,root,root,-) (cosmetic change)

- No need to tag the manpage as %doc. Whatever is in %{_mandir} will automatically be tagged %doc.

- Include COPYRIGHT


If you have any problems, please let me know.
Comment 4 Mohamed El Morabity 2010-12-28 00:59:13 EST
*** Bug 665956 has been marked as a duplicate of this bug. ***
Comment 5 Ankur Sinha (FranciscoD) 2010-12-28 01:25:51 EST
Hello,

Ah! I somehow missed this review. I packaged for personal use sometime back. The spec and srpm are here, if you want to reference them. It appears to be quite simple package. 

http://ankursinha.fedorapeople.org/detex/

Thanks, 
regards,
Ankur
Comment 6 Christoph Wickert 2010-12-28 03:22:50 EST
Usually we follow the rule "first come, first served" so I'd like Didi to finish this review instead of doing another one.

Didi, you need help?
Comment 7 Christoph Wickert 2011-02-20 18:17:02 EST
Didi, are you still there? Please respond to this review request, otherwise I think Ankur should take over. I'd like to see this package in Fedora.
Comment 8 Didi Hoffmann 2011-02-21 05:38:37 EST
Hey

I sent you a mail ages ago. Let me write up all the changes I made and resubmit it here. I have taken Ankurs stuff into account. I really like this tool too. Give me a few min(hours) and I will post the new package.

Cheers Didi
Comment 9 Christoph Wickert 2011-02-25 16:07:39 EST
Ping?
Comment 10 Christoph Wickert 2011-03-06 08:08:25 EST
Didi contacted me and told me that he is sorry to have no more time to maintain packages in Fedora because of his dayjob.

Ankur, please file a new review request and submit your package. Then change the resolution of this bug to duplicate of yours. You can assign the review to me if you want.
Comment 11 Ankur Sinha (FranciscoD) 2011-03-07 03:06:20 EST
Hello!

Aye aye Sir! I'll get down to this immediately. 

Thanks!
Regards,
Ankur
Comment 12 Ankur Sinha (FranciscoD) 2011-03-07 03:09:52 EST

*** This bug has been marked as a duplicate of bug 682666 ***