Bug 476440 - Review Request: latexdiff - Determine and mark up significant differences between latex files
Review Request: latexdiff - Determine and mark up significant differences bet...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Orcan Ogetbil
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-12-14 13:38 EST by Dan Kenigsberg
Modified: 2009-02-04 21:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-01-14 04:40:05 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
oget.fedora: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Dan Kenigsberg 2008-12-14 13:38:11 EST
Spec URL: http://www.cs.technion.ac.il/~danken/latexdiff.spec
SRPM URL: http://www.cs.technion.ac.il/~danken/latexdiff-0.20071020-1.fc10.src.rpm
Description: Latexdiff is a Perl script for visual mark up and revision of significant differences between two latex files.  Various options are available for visual markup using standard latex packages such as color.  Changes not directly affecting visible text, for example in formatting commands, are still marked in the latex source.  A rudimentary revision facility is provided by another Perl script, latexrevise, which accepts or rejects all changes.  Manual editing of the difference file can be used to override this default behaviour and accept or reject selected changes only.
Comment 1 manuel wolfshant 2008-12-15 00:29:13 EST
I'd say that the version number is not correct. Nowhere in the scripts or on the website do I see any reference to 20071020. OTOH there are references as  "Version 0.42 November 06  Bug fixes only" (in the script) and "Version 	
0.5" (on the main web page of the project)

License is not correct. The website claims GPLv2+ but the scripts themselves include:
#    This program is free software; you can redistribute it and/or modify
#    it under the terms of the GNU General Public License Version 2 as published by
#    the Free Software Foundation.
This clearly makes them GPLv2 (not GPLv2+)
Comment 2 Dan Kenigsberg 2008-12-15 05:44:36 EST
Thanks for the review.

As I understand it, "version 0.42" in latexdiff and "version 0.3" in latexrevise relate to the versions of the separate scripts. Since I did not find any version number for the whole package, I chose the latest update date of files within.

Do you have a better idea (maybe 0.0.20071020, to let upstream choose 0.43 one day in the future ?)

See http://www.cs.technion.ac.il/~danken/latexdiff-0.0.20071020-1.fc10.src.rpm and http://www.cs.technion.ac.il/~danken/latexdiff.spec for updated version and license.
Comment 3 Orcan Ogetbil 2008-12-20 17:42:35 EST
I think it would be better to use the pre-release notation (see kismet in [1]). By the way, your SRPM link is broken.

[1] http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_packages
Comment 4 Dan Kenigsberg 2008-12-21 16:34:19 EST
That's fine by me
http://www.cs.technion.ac.il/~danken/latexdiff-0-0.1.20071020snap.fc10.src.rpm
Comment 5 Orcan Ogetbil 2008-12-22 02:31:30 EST
* On the website 
   http://tug.ctan.org/cgi-bin/ctanPackageInformation.py?id=latexdiff
I see that version is 0.5 
As you said there are different version references within the source. I would try to get in touch with the author to ask about the correct version.
I think it should be OK to go with the version of the latexdiff script for the whole package, because the whole package is named after that script.

* It looks like this package must be noarch.

* rpmlint gives bunch of errors of these types:
   W: summary-ended-with-dot
   E: description-line-too-long
   W: spurious-executable-perm
   W: incoherent-version-in-changelog
   E: no-binary
   E: empty-debuginfo-package
They are all easy to fix.

? Why don't you use the existing Makefile facility?

* I think all of doc/support/latexdiff/* files should go to %doc

? The source/latexdiff/example/ can go to %doc too. What do you think?

? Is the wrapper script source/latexdiff/contrib/latexdiff-wrap useful? Should we package it?

* We prefer %defattr(-,root,root,-)

* I don't think
   Requires: perl
is necessary. rpmbuild itself picks up the perl dependencies. (check with "rpm -qpR <package.rpm>")
Comment 6 Dan Kenigsberg 2008-12-28 18:58:03 EST
Ok, let's use version 0.5.

I hope I got all your (*)s sorted. rpm passes rpmlint cleanly now.

I did not see how the supplied Makefile could work. It relates to non-existing filenames and ignores the tarball directpry structure. avoiding seemed simpler than fixing it.

using latexdiff is so simple that I don't think the example is really useful.

I have never used latexdiff-wrap, and not anxious to try.

http://www.cs.technion.ac.il/~danken/latexdiff-0.5-1.fc10.src.rpm
Comment 7 Orcan Ogetbil 2009-01-10 08:44:23 EST
Sorry, I've been away for a while. Now I'm back and we can finish up this beast. Here are a few more issues to address.

* Please add the changes you made to the changelog in the future with the correct date(s). It'll be easier to see what is actually changed. That's why we keep a changelog.

* Also please switch to %defattr(-,root,root,-)

* We should preserve the timestamps of all the relevant files we package in the %install section. By relevant files, I mean the non-compiled files.

* Please remove the commented-out line(s) that are unnecessary. Also, in the comments and in the changelog section, we use %% for macros instead of a single %, so that the macros don't expand out during package building.
Comment 8 Dan Kenigsberg 2009-01-10 15:06:54 EST
done. see http://www.cs.technion.ac.il/~danken/latexdiff.spec
Comment 9 Orcan Ogetbil 2009-01-11 11:59:29 EST
All good.

--------------------------------------------
This package (latexdiff) is APPROVED by oget
--------------------------------------------

Please supply the SRPM (in addition to the SPEC) whenever you make an update during the review process in the future. It makes life easier for reviewers.
Comment 10 Dan Kenigsberg 2009-01-11 13:50:00 EST
New Package CVS Request
=======================
Package Name: latexdiff
Short Description: Determine and mark up significant differences between latex files
Owners: danken
Branches: F-10
InitialCC: danken
Comment 11 Kevin Fenzi 2009-01-13 15:37:14 EST
cvs done.
Comment 12 Fedora Update System 2009-01-16 15:20:33 EST
latexdiff-0.5-2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/latexdiff-0.5-2.fc10
Comment 13 Fedora Update System 2009-02-04 21:11:36 EST
latexdiff-0.5-2.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.