Bug 210823

*** Bug 210822 has been marked as a duplicate of this bug. ***

Hi Mola, this is by no means a thorough review--just a quick list of 
observations:

needswork:
 - spec file contains a mix of spaces and tabs--please cleanup
 - please consider deleting "Prefix: %{_prefix}"
 - dir %{_includedir}/wxXML appears to be unowned
 - please consider deleting or excluding the static libs
 - why is %{_libdir}/libwxsvg.la both listed as a file and
     also as an %exclude item?
 - usually, all *.so files go in the -devel sub-package and the 
     *.so.* files go in the main package
 - please add "%defattr(-,root,root,-)" to the "%files devel" section
 - the license type needs to be more specific (it appears to be the 
     wxWindows Library Licence, Version 3)
 - the "%doc" section should, at a min, contain the COPYING file
 - neither of the two Summary:-ies should end in a "."

Please take a close look at:

  http://fedoraproject.org/wiki/Packaging/ReviewGuidelines

and run "rpmlint" against this package (both the SRPM and the various 
binary rpms) before posting an updated SRPM.

Update URl with new spec file 

You should also post link to an updated .src.rpm to facilitate
review.

update .
Spec URL: http://c100c.com/fedora/wxsvg.spec
SRPM URL: http://c100c.com/fedora/wxsvg-1.0b7-1.src.rpm

Hi Mola, the srpm in comment #5 does not build in mock.  It results 
in the error:

+ ./autogen.sh
./autogen.sh: line 3: aclocal: command not found
./autogen.sh: line 4: automake: command not found
./autogen.sh: line 5: autoconf: command not found
error: Bad exit status from /var/tmp/rpm-tmp.24188 (%build)

This is a missing BuildRequires for the autotools.  Please post the 
URL for a new SRPM -- one that builds in mock using:

  $ mock ${SRPM_NAME}

since you'll have to do that for the Extras build system to work.

Also, please make sure to comment on your changes in the changelog and 
bump the release number.

> $ rpmlint wxsvg-1.0b7-1.src.rpm 
> W: wxsvg summary-not-capitalized wxSVG is C++ library to create,
> manipulate and render SVG files.
> W: wxsvg summary-ended-with-dot wxSVG is C++ library to create,
> manipulate and render SVG files.

Better:

  Summary: C++ library to create, manipulate and render SVG files

Repeating the package name in the summary is often considered bad
style, because it either leads to poor summaries or raises questions
not answered in the summary. For a more detailed description, there
is the package %description field, where you can explain acronyms and
other things, too.

> E: wxsvg no-description-tag

It must not be empty.

> W: wxsvg invalid-license wxWindows Library Licence, Version 3

This needs a second look, since the project is named wxWidgets for
a long time.


Now a brief look at the spec file:

> Version:        1.0b7
> Release:        1%{?dist}

You should take a look at the pre-release versioning scheme, which
makes it possible that you can go from "1.0b7" to "1.0":

  Version:        1.0
  Release:        0.1.b7%{?dist}

> Requires: wxGTK

Not needed and ought to be deleted. Rely on rpmbuild's automatic
dependencies on the library SONAMEs your built files are linked
against.

> %package devel

Missing  "Requires: wxGTK-devel"  since the wxSVG headers include
wxGTK headers.

> Requires: %{name} = %{version}

Ought to be

  Release: %{name} = %{version}-%{release}

so your package-set is always in sync (even if in many cases,
%{version} may be sufficient actually). This strict requirement is
particularly useful when patches are applied. Assume what happens
when the main package is permitted to be older than the -devel package.
The -devel package mentions fixes in its %changelog, but the main
package is not up-to-date and suffers from problems at build-time
or run-time. Or vice versa.


> %build
> ./autogen.sh

Try to get the upstream developers to pregenerate these files, which
is what they ought to do for every release, regardless of whether it's
beta or not. Then you would not be missing automake and friends, which
might be wrong versions.

> %files
> %defattr(-,root,root,-)
> %doc

No %doc files? At least the license file ought to be included.
Possibly also a few of the available text files.

> %files devel
[...]
> %{_includedir}/wxSVG/*.h
> %{_includedir}/wxXML/*.h

The directories are not included. Use either:

  %{_includedir}/wxSVG/
  %{_includedir}/wxXML/

Or:

  %dir %{_includedir}/wxSVG/
  %{_includedir}/wxSVG/*.h
  %dir %{_includedir}/wxXML/
  %{_includedir}/wxXML/*.h

> %{_libdir}/libwxsvg.a
> %{_libdir}/libwxsvg.la

If these cannot be disabled, delete them in %install.

for license : http://www.wxwindows.org/about/newlicen.htm

for version and release field that i change it . the source0 field haven't
correct URL for source 

and for pregenerate i send mail to developer :)

on FC6 the compilation fails with the following error message:

g++ -DPACKAGE_NAME=\"wxsvgtest\" -DPACKAGE_TARNAME=\"wxsvgtest\"
-DPACKAGE_VERSION=\"1.0\" "-DPACKAGE_STRING=\"wxsvgtest 1.0\""
-DPACKAGE_BUGREPORT=\"wx-svg-users.net\"
-DPACKAGE=\"wxsvgtest\" -DVERSION=\"1.0\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1
-DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1
-DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1
-DHAVE_DLFCN_H=1 -I. -I. -I../../include -I../../include/wxSVG
-I/usr/include/freetype2 -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables -DUSE_RENDER_LIBART -I/usr/include/pango-1.0
-I/usr/include/freetype2 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include
-I/usr/include/libart-2.0 -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2
-fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386
-mtune=generic -fasynchronous-unwind-tables -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4
-m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -DUSE_RENDER_LIBART
-I/usr/lib/wx/include/gtk2-unicode-release-2.6 -I/usr/include/wx-2.6
-DGTK_NO_CHECK_CASTS -D__WXGTK__ -D_FILE_OFFSET_BITS=64 -D_LARGE_FILES
-DNO_GCC_PRAGMA -MT SVGCanvasTextFreetype.lo -MD -MP -MF
.deps/SVGCanvasTextFreetype.Tpo -c SVGCanvasTextFreetype.cpp  -fPIC -DPIC -o
.libs/SVGCanvasTextFreetype.o
/usr/include/wx-2.6/wx/hashmap.h: In member function
'wxLongToLongHashMap_wxImplementation_HashTable::Node**
wxLongToLongHashMap_wxImplementation_HashTable::GetNodePtr(const long int&) const':
/usr/include/wx-2.6/wx/hashmap.h:705: warning: dereferencing type-punned pointer
will break strict-aliasing rules
../../include/wxXML/xml.h: In member function
'wxXmlAttrHashBase_wxImplementation_HashTable::Node**
wxXmlAttrHashBase_wxImplementation_HashTable::GetNodePtr(const wxString&) const':
../../include/wxXML/xml.h:83: warning: dereferencing type-punned pointer will
break strict-aliasing rules
/usr/include/wx-2.6/wx/gdicmn.h: In member function
'wxStringToColourHashMap_wxImplementation_HashTable::Node**
wxStringToColourHashMap_wxImplementation_HashTable::GetNodePtr(const wxString&)
const':
/usr/include/wx-2.6/wx/gdicmn.h:476: warning: dereferencing type-punned pointer
will break strict-aliasing rules
../../include/wxSVG/CSSStyleDeclaration.h: In member function
'wxHashMapCSSValue_wxImplementation_HashTable::Node**
wxHashMapCSSValue_wxImplementation_HashTable::GetNodePtr(const wxCSS_PROPERTY&)
const':
../../include/wxSVG/CSSStyleDeclaration.h:81: warning: dereferencing type-punned
pointer will break strict-aliasing rules
/usr/include/wx-2.6/wx/image.h: In member function
'wxImageHistogramBase_wxImplementation_HashTable::Node**
wxImageHistogramBase_wxImplementation_HashTable::GetNodePtr(const long unsigned
int&) const':
/usr/include/wx-2.6/wx/image.h:123: warning: dereferencing type-punned pointer
will break strict-aliasing rules
SVGCanvasTextFreetype.cpp: At global scope:
SVGCanvasTextFreetype.cpp:231: error: invalid conversion from 'gint
(*)(FT_Vector*, void*)' to 'int (*)(const FT_Vector*, void*)'
SVGCanvasTextFreetype.cpp:231: error: invalid conversion from 'gint
(*)(FT_Vector*, void*)' to 'int (*)(const FT_Vector*, void*)'
SVGCanvasTextFreetype.cpp:231: error: invalid conversion from 'gint
(*)(FT_Vector*, FT_Vector*, void*)' to 'int (*)(const FT_Vector*, const
FT_Vector*, void*)'
SVGCanvasTextFreetype.cpp:231: error: invalid conversion from 'gint
(*)(FT_Vector*, FT_Vector*, FT_Vector*, void*)' to 'int (*)(const FT_Vector*,
const FT_Vector*, const FT_Vector*, void*)'
make[2]: *** [SVGCanvasTextFreetype.lo] Error 1
make[2]: Leaving directory `/usr/src/redhat/BUILD/wxsvg-1.0b7/src/freetype'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/usr/src/redhat/BUILD/wxsvg-1.0b7/src'
make: *** [all-recursive] Error 1
Fehler: Fehler-Status beim Beenden von /var/tmp/rpm-tmp.55997 (%build)

Are you building against rawhide wx or FC6?

What is the status of this bug?

Good question. I have a wxsvg package "in the pipe" too. If the initial reporter
doesn't show any interest in getting the package in anymore, I think I'll be
submitting mine directly.

Current version here :
http://ftp.es6.freshrpms.net/tmp/extras/wxsvg/wxsvg.spec
http://ftp.es6.freshrpms.net/tmp/extras/wxsvg/wxsvg-1.0-0.2.b7.src.rpm

I haven't looked at the packages from this bug, but the first major issue I do
see is that the version is wrong, as this "b7" is a "beta7", not some post 1.0
release, as Michael already pointed out. It builds fine on FC6 x86_64 for me.

If no response from initial reporter is received within
a week,

* Matthias, if you are still interested in being a maintainer
  of this package, we regard you as the submitter of this
  review request, okay?
* Otherwise I will close this as NOTABUG.

Yup, I'm still interested :-)

Declaring the switch of the submitter
=============================================

Well, then:
(In reply to comment #12)
> Current version here :
> http://ftp.es6.freshrpms.net/tmp/extras/wxsvg/wxsvg.spec
> http://ftp.es6.freshrpms.net/tmp/extras/wxsvg/wxsvg-1.0-0.2.b7.src.rpm

A quick check:
* For source,
  - For URL, please check:
    http://fedoraproject.org/wiki/Packaging/SourceURL
    (Now the traffic is very heavy!!)
  - By the way, the newest seems 1.0b7"_3"?? IMO release number
    of fedora rpm should contain "b7_3", not only "b7".

* For timestamp:
  - Please use (for this package):
-----------------------------------------------------
%{__make} install DESTDIR=%{buildroot} INSTALL="%{__install} -p"
-----------------------------------------------------
    to keep timestamps especially on header files.

= mockbuild okay for F-devel i386

ping?

> http://ftp.es6.freshrpms.net/tmp/extras/wxsvg/wxsvg.spec
> http://ftp.es6.freshrpms.net/tmp/extras/wxsvg/wxsvg-1.0-0.3.b7_3.src.rpm

* Wed Jun  6 2007 Matthias Saou <http://freshrpms.net/> 1.0-0.3.b7_3
- Update to 1.0b7_3.
- Pass -p to install.
- Remove no longer needed freetype patch, but...
- ...include our own ltmain.sh because it's missing...
- ...run ./autogen.sh since Makefile.in files are missing too.

BTW, what is wrong with the source URL? Oh, and the b7_3 tarball is pretty badly
"broken" as you will see...

Well, while I have not checked your latest spec/srpm:

(In reply to comment #17)
> BTW, what is wrong with the source URL? 
I just mention that the recommended URL uses downloads.*sourceforge*.net,
while I don't know the different between this and downloads.*sf*.net

> Oh, and the b7_3 tarball is pretty badly
> "broken" as you will see...
What is strange is that while the upstream only announces:
--------------------------------------------------
2006-10-12  wxSVG v1.0 beta 7 released.
--------------------------------------------------
why are there some b7 series tarball?? Anyway perhaps fedora
wants b7_3 because it is newer...

> I just mention that the recommended URL uses downloads.*sourceforge*.net,
> while I don't know the different between this and downloads.*sf*.net

They're the same AFAIK, so it shouldn't be a problem. It's the "download" vs.
"downloads" part that matters most, since without the "s" it can point to a web
page, whereas with the "s" it points to some kind of redirection scripts, which
sends to the file on a mirror which has it.

> why are there some b7 series tarball?? Anyway perhaps fedora
> wants b7_3 because it is newer...

I don't know. Let's try to have the latest go through the review. If you find
any problems with it, just let me know and I can revert to some previous version.

Okay. No problem.

-----------------------------------------------------
  This package (wxsvg) is APPROVED by me.
-----------------------------------------------------

New Package CVS Request
=======================
Package Name: wxsvg
Short Description: C++ library to create, manipulate and render SVG files
Owners: matthias
Branches: devel F-7 FC-6 EL-5 (only the most recent)
InitialCC: 

cvs done.

wxsvg-1.0-0.3.b7_3.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.

Please close this bug when rebuilding is done.

All built and pushed. Closing the report. Thanks for the review and for the CVS
creation!

wxsvg-1.0-0.3.b7_3.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.