Bug 216299

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.

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.

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.

(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

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.

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

Ah, sorry. I feel dumb now for not seeing this. Thanks for the review.

Imported and built for devel, FC6 and FC5 branches requested. Thanks again for
the review.

Package Change Request
======================
Package Name: libEMF
New Branches: EL-5

cvs done. 

Package Change Request
======================
Package Name: libEMF
Updated EPEL Owners: rathann,rmyers

Rob Myers has offered to co-maintain for EPEL.

cvs done.