Bug 432808 - Review Request: figtoipe - FIG to IPE conversion tool
Review Request: figtoipe - FIG to IPE conversion tool
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Orion Poplawski
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2008-02-14 10:08 EST by Laurent Rineau
Modified: 2008-06-05 13:37 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-06-05 13:37:48 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
orion: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Laurent Rineau 2008-02-14 10:08:23 EST
Spec URL: http://www.normalesup.org/~rineau/Fedora/figtoipe.spec
SRPM URL: http://www.normalesup.org/~rineau/Fedora/figtoipe-20071004-3.fc9.src.rpm
Figtoipe is a program that reads FIG files (as generated by
xfig) and generates an XML file readable by the Ipe editor.

Note #1: That tool was previously shipped with the Ipe editor itself. From ipe-6.0pre30, figtoipe is released in another tarball. Maybe I should add a field "Obsoletes: ipe < 6.0-0.23.pre30". I am not used to the semantic of Obsoletes: and I am not sure of that. What do you think?

Note #2: the spec file is really simple. No call to configure or make. Just one call to g++, and a manual installation. I use perl to extract a changelog from the .cpp file. That way I can make it a %doc.
Comment 1 Orion Poplawski 2008-02-14 18:28:48 EST
Well, the BR needs to be zlib-devel, not zlib. 

Also, I'd like to see a copy of the license in the final package.

* source files match upstream:
ea0d1ff62c7c342450fdb0676a2b6192  figtoipe-20071004.tar.gz
ea0d1ff62c7c342450fdb0676a2b6192  figtoipe-20071004.tar.gz.new

* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
* license field matches the actual license.
* license is open source-compatible. 
XXX License text not included in package.
* latest version is being packaged.
X BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock ( ).
* package installs properly
* debuginfo package looks complete.
* rpmlint is silent.
* final provides and requires are sane:

* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* 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.
Comment 2 Laurent Rineau 2008-02-15 02:21:00 EST
Update (figtoipe-20071004-4.fc9):

(In reply to comment #1)
> Well, the BR needs to be zlib-devel, not zlib. 

Sorry for that stupid error. My laptop is too short in free space today to run 

> Also, I'd like to see a copy of the license in the final package.

I have added a copy of http://www.gnu.org/licenses/gpl-2.0.txt
Maybe should I add a copy of the GPLv3, as the licensing is GPLv2+ (with an 
exception for CGAL).
Comment 3 Orion Poplawski 2008-02-15 11:39:53 EST
(In reply to comment #2)
> I have added a copy of http://www.gnu.org/licenses/gpl-2.0.txt
> Maybe should I add a copy of the GPLv3, as the licensing is GPLv2+ (with an 
> exception for CGAL).

I really meant the license information from the top of the .cpp file so that
people know what the exception is - though it doesn't apply in this case.
Comment 4 Orion Poplawski 2008-02-26 19:24:47 EST
Comment 5 Laurent Rineau 2008-05-16 14:40:22 EDT
Update (figtoipe-20071004-4.fc10):

Sorry Orion, because of my daily job (too busy in February) I forgot that 
package submission!

Here is an update, in respond to your suggestion in comment #3.

* Fri May 16 2008 Laurent Rineau <laurent.rineau__fedora@normalesup.org> - 
- Add a copy of the figtoipe.cpp header in license.txt
Comment 6 Orion Poplawski 2008-05-16 15:49:10 EDT
Looks good, but it also looks like a new version (20080505) is available from
Comment 7 Laurent Rineau 2008-05-17 13:16:47 EDT

You are right. Upstream has not announced it on the mailing list. Upstream 
tarball changed "readme.txt" to "README". I have changed the name of the 
generated files "CHANGES" and "LICENSE" accordingly.
Comment 8 Orion Poplawski 2008-05-19 12:33:51 EDT
You don't need to gzip the man page, that is done automatically.

-- figtoipe.spec       2008-05-17 11:12:35.000000000 -0600
+++ figtoipe.spec.new   2008-05-19 10:31:09.000000000 -0600
@@ -12,7 +12,7 @@
 Source1:        http://www.gnu.org/licenses/gpl-2.0.txt
 BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
-BuildRequires:  zlib-devel, gzip, perl
+BuildRequires:  zlib-devel, perl

 Figtoipe is a program that reads FIG files (as generated by
@@ -39,7 +39,6 @@
 install -m 755 figtoipe $RPM_BUILD_ROOT%{_bindir}
 mkdir -p $RPM_BUILD_ROOT%{_mandir}/man1
 install -m 644 figtoipe.1 $RPM_BUILD_ROOT%{_mandir}/man1
-gzip $RPM_BUILD_ROOT%{_mandir}/man1/figtoipe.1

@@ -49,7 +48,7 @@
 %doc README CHANGES gpl-2.0.txt LICENSE

 * Sat May 17 2008 Laurent Rineau <laurent.rineau__fedora@normalesup.org> -
Comment 9 Laurent Rineau 2008-05-19 13:55:14 EDT

You are right. I forgot to have a look at the rpm scripts. New release, with 
your patch, but the last line (I avoid glob in %files).
Comment 10 Orion Poplawski 2008-05-19 14:01:01 EDT
Looks good.  APPROVED.
Comment 11 Laurent Rineau 2008-05-19 17:19:08 EDT
Dear cvs admins,

New Package CVS Request
Package Name: figtoipe
Short Description: FIG to IPE conversion tool
Owners: rineau
Branches: F-8 F-9
Comment 12 Orion Poplawski 2008-05-19 17:46:54 EDT
Laurent -

 What I don't understand - figtoipe is already in the ipe package.  Why the
separate package.
Comment 13 Laurent Rineau 2008-05-20 03:59:47 EDT
ipe-6.0pre30 (the last upstream version, already in F-9 and Rawhide) no longer 
has figtoipe included. When I saw that missing, I first planned to make 
figtoipe reviewed before the F-9 freeze. But my daily job got more difficult 
in February (and then I forgot that review request :-$ ).

I want to update ipe to ipe-6.0pre30 in F-8 also, that is why i request a F-8 
branch for figtoipe.
Comment 14 Laurent Rineau 2008-05-20 04:50:42 EDT
The current version of ipe in F-9 and Rawhide is ipe-6.0-0.25.pre30%{?dist}.

Maybe I will update all ipe versions in F-8, F-9 and rawhide to 
ipe-6.0-0.26.pre30%{?dist} and add:
  Obsoletes: ipe < ipe-6.0-0.26.pre30%{?dist}
to figtoipe.

That way, F-9 and rawhide users will recover that missing /usr/bin/figtoipe 
that disappeared during the upgrade to F-9, and F-8 users will upgrade 
smoothly to the new release of ipe.
Comment 15 Kevin Fenzi 2008-05-20 11:59:36 EDT
cvs done.
Comment 16 Orion Poplawski 2008-06-05 13:37:48 EDT
I'm closing since a rawhide build has been done.

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