Bug 216299 - Review Request: libEMF - A library for generating Enhanced Metafiles
Summary: Review Request: libEMF - A library for generating Enhanced Metafiles
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT 216300
TreeView+ depends on / blocked
 
Reported: 2006-11-19 01:41 UTC by Dominik 'Rathann' Mierzejewski
Modified: 2007-12-06 20:53 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-11-21 07:16:41 UTC
Type: ---
Embargoed:
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Dominik 'Rathann' Mierzejewski 2006-11-19 01:41:46 UTC
Spec URL: http://rpm.greysector.net/extras/libEMF.spec
SRPM URL: http://rpm.greysector.net/extras/libEMF-1.0.3-1.src.rpm
Description:
libEMF is a library for generating Enhanced Metafiles on systems which                                                  
don't natively support the ECMA-234 Graphics Device Interface                                                           
(GDI). The library is intended to be used as a driver for other                                                         
graphics programs such as Grace or gnuplot. Therefore, it implements a                                                  
very limited subset of the GDI.

Comment 1 Patrice Dumas 2006-11-19 10:21:22 UTC
That's a bit strange that this package isn't already in fedora, given
that gnuplot, pstoedit and grace are already there. Once the CVS is
back to life I'll investigate a bit to understand what is 
happening.

Comment 2 Mamoru TASAKA 2006-11-19 12:45:07 UTC
Well, it seems that
* gnuplot has its original source code (not the copy of libEMF) to
  support EMF output
* http://libemf.sourceforge.net says that support of libEMF for grace
  is forthcoming
* By the way http://libemf.sourceforge.net says 'this software includes
  patches to those programs (here gnuplot) to add the EMF as an output 
  option, however, where is the patch? (not a blocker)
* Currently FE pstoedit spec file has
  %configure --disable-static --without-emf --without-swf

I will check this later anyway.

Comment 3 Mamoru TASAKA 2006-11-19 14:21:55 UTC
Well, first review of libEMF

1. From http://fedoraproject.org/wiki/Packaging/Guidelines :
* Licensing
  - Include license document(s).

* Use rpmlint
------------------------------------------------------
E: libEMF-debuginfo script-without-shebang
/usr/src/debug/libEMF-1.0.3/libemf/libemf.h
W: libEMF-devel summary-not-capitalized libEMF header files
------------------------------------------------------
  - The formar issue is permission problem. Change the permission to
    0644.
  - The latter issue can be ignored, in my opinion.

* Timestamps
  - -devel package includes many header files and keeping timestamps
    on these files is preferred as it makes clear
    - when those files are written
    - whether those files are modified by vendor
    So please keep timestamps on those files.
    Under my check, this can be done by using:
-------------------------------------------------------
%install
rm -rf $RPM_BUILD_ROOT

export CPPROG="cp -p"
%{__make} install \
        DESTDIR=$RPM_BUILD_ROOT
-------------------------------------------------------

2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
   (= Okay)

3. Other things I have noticed:
* %check
  Well, this package has tests/ directory and some tests are
  included, so I think including %check script in the spec is
  a good idea.

Comment 4 Dominik 'Rathann' Mierzejewski 2006-11-19 16:34:17 UTC
(In reply to comment #3)
> Well, first review of libEMF
> 
> 1. From http://fedoraproject.org/wiki/Packaging/Guidelines :
> * Licensing
>   - Include license document(s).

Done.

> * Use rpmlint

I already did...

> ------------------------------------------------------
> E: libEMF-debuginfo script-without-shebang
> /usr/src/debug/libEMF-1.0.3/libemf/libemf.h
> W: libEMF-devel summary-not-capitalized libEMF header files
> ------------------------------------------------------
>   - The formar issue is permission problem. Change the permission to
>     0644.

How? This is an automatically generated -debuginfo package.

>   - The latter issue can be ignored, in my opinion.

OK.

> * Timestamps
>   - -devel package includes many header files and keeping timestamps
>     on these files is preferred as it makes clear
>     - when those files are written
>     - whether those files are modified by vendor
>     So please keep timestamps on those files.
>     Under my check, this can be done by using:
> -------------------------------------------------------
> %install
> rm -rf $RPM_BUILD_ROOT
> 
> export CPPROG="cp -p"
> %{__make} install \
>         DESTDIR=$RPM_BUILD_ROOT
> -------------------------------------------------------

Done. Although I'm surprised you've asked for this. This is the first
time I've ever seen this trick.

> 2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
>    (= Okay)
> 
> 3. Other things I have noticed:
> * %check
>   Well, this package has tests/ directory and some tests are
>   included, so I think including %check script in the spec is
>   a good idea.

Added.

http://rpm.greysector.net/extras/libEMF.spec
http://rpm.greysector.net/extras/libEMF-1.0.3-2.src.rpm


Comment 5 Mamoru TASAKA 2006-11-19 16:39:59 UTC
Well, before checking 1.0.3-2:
(In reply to comment #4)
> (In reply to comment #3)
> > ------------------------------------------------------
> > E: libEMF-debuginfo script-without-shebang
> > /usr/src/debug/libEMF-1.0.3/libemf/libemf.h
> > ------------------------------------------------------
> >   - The formar issue is permission problem. Change the permission to
> >     0644.
> 
> How? This is an automatically generated -debuginfo package.

Just:
---------------------------------------------
chmod 0644 libemf/libemf.h
---------------------------------------------
at the last of %prep stage.

Comment 6 Mamoru TASAKA 2006-11-19 17:31:37 UTC
Okay, please add the line
---------------------------------------------
chmod 0644 libemf/libemf.h
---------------------------------------------
at the last of %prep to avoid rpmlint complaint of -debuginfo
rpm. All things else are okay.

----------------------------------------
   This package (libEMF) is APPROVED by me
----------------------------------------

Comment 7 Dominik 'Rathann' Mierzejewski 2006-11-19 17:39:06 UTC
Ah, sorry. I feel dumb now for not seeing this. Thanks for the review.

Comment 8 Dominik 'Rathann' Mierzejewski 2006-11-21 07:16:41 UTC
Imported and built for devel, FC6 and FC5 branches requested. Thanks again for
the review.

Comment 9 Dominik 'Rathann' Mierzejewski 2007-12-03 23:09:45 UTC
Package Change Request
======================
Package Name: libEMF
New Branches: EL-5


Comment 10 Kevin Fenzi 2007-12-04 01:04:20 UTC
cvs done. 

Comment 11 Dominik 'Rathann' Mierzejewski 2007-12-06 00:11:28 UTC
Package Change Request
======================
Package Name: libEMF
Updated EPEL Owners: rathann,rmyers

Rob Myers has offered to co-maintain for EPEL.

Comment 12 Kevin Fenzi 2007-12-06 20:53:18 UTC
cvs done.


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