Bug 216299 - Review Request: libEMF - A library for generating Enhanced Metafiles
Review Request: libEMF - A library for generating Enhanced Metafiles
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT 216300
  Show dependency treegraph
 
Reported: 2006-11-18 20:41 EST by Dominik 'Rathann' Mierzejewski
Modified: 2007-12-06 15:53 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-11-21 02:16:41 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Dominik 'Rathann' Mierzejewski 2006-11-18 20:41:46 EST
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 05:21:22 EST
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 07:45:07 EST
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 09:21:55 EST
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 11:34:17 EST
(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 11:39:59 EST
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 12:31:37 EST
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 12:39:06 EST
Ah, sorry. I feel dumb now for not seeing this. Thanks for the review.
Comment 8 Dominik 'Rathann' Mierzejewski 2006-11-21 02:16:41 EST
Imported and built for devel, FC6 and FC5 branches requested. Thanks again for
the review.
Comment 9 Dominik 'Rathann' Mierzejewski 2007-12-03 18:09:45 EST
Package Change Request
======================
Package Name: libEMF
New Branches: EL-5
Comment 10 Kevin Fenzi 2007-12-03 20:04:20 EST
cvs done. 
Comment 11 Dominik 'Rathann' Mierzejewski 2007-12-05 19:11:28 EST
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 15:53:18 EST
cvs done.

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