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
Summary: | Review Request: libEMF - A library for generating Enhanced Metafiles | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Dominik 'Rathann' Mierzejewski <dominik> |
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | mtasaka, pertusus |
Target Milestone: | --- | Flags: | kevin:
fedora-cvs+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-11-21 07:16:41 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779, 216300 |
Description
Dominik 'Rathann' Mierzejewski
2006-11-19 01:41:46 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. 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. |