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 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.