Paul Wise wrote:
On Fri, Jun 4, 2010 at 5:51 AM, Matteo Vescovi <[email protected]> wrote:I am looking for a sponsor/reviewer for my package "presage".Here is a review:
Many thanks for reviewing my package.I started off fixing the reported issues with debian (quilt) patches, but soon realized that many fixes would have benefitted the upstream package, so I wore my upstream developer hat and integrated those fixes upstream into presage-0.8.3, which this new upload of presage package is based on.
The new package can be found here: - URL: http://mentors.debian.net/debian/pool/main/p/presage- Source repository: deb-src http://mentors.debian.net/debian unstable main contrib non-free - dget http://mentors.debian.net/debian/pool/main/p/presage/presage_0.8.3-1.dsc
Since you are packaging a library, I presume you have readlibpkg-guide and its two bugs?
I had read libpkg-guide, but not the two bugs. Done now.
You should never install *.pyc files in the .deb, in Debian they are always generated by the maintainer scripts.
Done, removed .pyc file from python-presage.install.
libpresage1 contains a file that doesn't appear to change its path/name when the ABI is changed (/etc/presage.xml). This is against policy and the file needs to be moved elsewhere, probably to the -data package.
Fixed, moved file to -data package.
Installation/compilation instructions do not belong in binary packages, especially for non-Debian platforms. Please remove these files from the doc package: doc/INSTALL_Cygwin_dev_env.txt doc/INSTALL_MinGW_MSYS_dev_env.txt
Done, removed files from -doc package.
resources/la_coscienza_di_zeno.txt is under a non-free license. You need to remove it from the orig.tar.gz.
Done.
Your menu files don't list an icon. you might want to build-dep onimagemagick so you can convert the upstream PNG to an XPM.
Added icon and pointed .menu file to it.
It looks like the package includes an embedded code copy of scintilla. Usually Debian prefers different packages to be packaged separately. Is that needed? If it is, please notify the security team and give more details so they can record this information in their embedded-code-copies file. Same goes for tinyxml.
Right, I modified presage build system to use the system libtinyxml rather than the embedded copy.
As for Scintilla, an embedded copy is used because scintilla is currently not packaged for Debian.
You might want to look at using debhelper 7 instead of cdbs: https://penta.debconf.org/dc9_schedule/events/418.en.html http://kitenet.net/~joey/blog/entry/cdbs_killer___40__design_phase__41__/ Your watch file isn't specific enough: p...@chianamo:~/tmp/presage-0.8.2$ uscan presage: Newer version (extra-data-arpa-1.0) available on remote site: http://qa.debian.org/watch/sf.php/presage/presage-extra-data-arpa-1.0.tar.gz (local version is 0.8.2)
Fixed watch file.
Insert standard whine about package descriptions being duplicates of each other. Please make the binary package descriptions describe the binary packages, not the upstream project.
Fixed. Removed redundant duplicate text from descriptions.
Your Standards-Version is out of date, please read debian-policy upgrading.txt and update it.
Done.
Your Vcs-Browser URL should point to trunk too.
Done.
Do you really need to distribute the static library and .la file? In Debian we seem to be moving towards not shipping those.
Ok, removed them.
debian/README.Debian is not needed, splitting out libs/etc is standard practice in Debian.
Removed it too.
Is this warning something to be concerned about? configure: CMU-Cambridge SLM tools not found. ARPA ngram language model will not be built.
Nothing to be concerned about. This means that no ARPA ngram language models will be built from the training text corpus for the ARPAPredictor, which is an optional predictor and thus not required for libpresage to work.
The package fails to build, it looks like a race condition since it builds with debuild -j1: make[3]: Entering directory `/tmp/buildd/presage-0.8.2/bindings' Making all in python make[4]: Entering directory `/tmp/buildd/presage-0.8.2/bindings/python' /usr/bin/swig -c++ -python -I../../src/lib -o presage_wrap.cpp -outdir . ./../presage.i make[4]: *** No rule to make target `presage_wrap.h', needed by `all'. Stop. make[4]: *** Waiting for unfinished jobs.... make[4]: Leaving directory `/tmp/buildd/presage-0.8.2/bindings/python'
Fixed, tested building with -j2 on my 2-core machine.
gcc warnings: gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -g -O2 -g -Wall -O2 -fPIC -I../../src/lib -I/usr/include/python2.5 -c presage_wrap.cpp -o build/temp.linux-x86_64-2.5/presage_wrap.o cc1plus: warning: command line option "-Wstrict-prototypes" is valid for Ada/C/ObjC but not for C++
The -Wstrict-prototypes flag comes from Python distutils module, which is used to build presage python extension module. Not sure how to remove the warning. It is harmless though, as the flag is ignored.
g++ -DHAVE_CONFIG_H -I. -I../../.. -pthread -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -pthread -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I./scintilla/include -I./scintilla/src -DNDEBUG -DGTK -DSCI_LEXER -DG_THREADS_IMPL_NONE -g -O2 -g -Wall -O2 -c scintilla/src/Editor.cxx -fPIC -DPIC -o .libs/libscintilla_la-Editor.o scintilla/src/Editor.cxx: In member function 'void Editor::LayoutLine(int, Surface*, ViewStyle&, LineLayout*, int)': scintilla/src/Editor.cxx:2049: warning: array subscript has type 'char' scintilla/src/Editor.cxx:2052: warning: array subscript has type 'char' scintilla/src/Editor.cxx:2055: warning: array subscript has type 'char' scintilla/src/Editor.cxx: In member function 'virtual void Editor::NotifyStyleToNeeded(int)': scintilla/src/Editor.cxx:4090: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'void Editor::NotifyChar(int)': scintilla/src/Editor.cxx:4101: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'void Editor::NotifySavePoint(bool)': scintilla/src/Editor.cxx:4108: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'void Editor::NotifyModifyAttempt()': scintilla/src/Editor.cxx:4118: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'virtual void Editor::NotifyDoubleClick(Point, bool, bool, bool)': scintilla/src/Editor.cxx:4124: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'void Editor::NotifyHotSpotDoubleClicked(int, bool, bool, bool)': scintilla/src/Editor.cxx:4134: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'void Editor::NotifyHotSpotClicked(int, bool, bool, bool)': scintilla/src/Editor.cxx:4143: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'void Editor::NotifyUpdateUI()': scintilla/src/Editor.cxx:4152: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'void Editor::NotifyPainted()': scintilla/src/Editor.cxx:4158: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'void Editor::NotifyIndicatorClick(bool, int, bool, bool, bool)': scintilla/src/Editor.cxx:4166: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'bool Editor::NotifyMarginClick(Point, bool, bool, bool)': scintilla/src/Editor.cxx:4184: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'void Editor::NotifyNeedShown(int, int)': scintilla/src/Editor.cxx:4198: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'void Editor::NotifyDwelling(Point, bool)': scintilla/src/Editor.cxx:4206: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'void Editor::NotifyZoom()': scintilla/src/Editor.cxx:4215: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'virtual void Editor::NotifyModified(Document*, DocModification, void*)': scintilla/src/Editor.cxx:4388: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/Editor.cxx: In member function 'void Editor::NotifyMacroRecord(unsigned int, uptr_t, sptr_t)': scintilla/src/Editor.cxx:4523: warning: missing braces around initializer for 'Sci_NotifyHeader' g++ -DHAVE_CONFIG_H -I. -I../../.. -pthread -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -pthread -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I./scintilla/include -I./scintilla/src -DNDEBUG -DGTK -DSCI_LEXER -DG_THREADS_IMPL_NONE -g -O2 -g -Wall -O2 -c scintilla/src/ScintillaBase.cxx -fPIC -DPIC -o .libs/libscintilla_la-ScintillaBase.o scintilla/src/ScintillaBase.cxx: In member function 'void ScintillaBase::AutoCompleteCancel()': scintilla/src/ScintillaBase.cxx:296: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/ScintillaBase.cxx: In member function 'void ScintillaBase::AutoCompleteCharacterDeleted()': scintilla/src/ScintillaBase.cxx:337: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/ScintillaBase.cxx: In member function 'void ScintillaBase::AutoCompleteCompleted()': scintilla/src/ScintillaBase.cxx:357: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/src/ScintillaBase.cxx: In member function 'void ScintillaBase::CallTipClick()': scintilla/src/ScintillaBase.cxx:445: warning: missing braces around initializer for 'Sci_NotifyHeader' g++ -DHAVE_CONFIG_H -I. -I../../.. -pthread -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -pthread -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I./scintilla/include -I./scintilla/src -DNDEBUG -DGTK -DSCI_LEXER -DG_THREADS_IMPL_NONE -g -O2 -g -Wall -O2 -c scintilla/gtk/ScintillaGTK.cxx -fPIC -DPIC -o .libs/libscintilla_la-ScintillaGTK.o scintilla/gtk/ScintillaGTK.cxx: In member function 'void ScintillaGTK::NotifyKey(int, int)': scintilla/gtk/ScintillaGTK.cxx:1042: warning: missing braces around initializer for 'Sci_NotifyHeader' scintilla/gtk/ScintillaGTK.cxx: In member function 'void ScintillaGTK::NotifyURIDropped(const char*)': scintilla/gtk/ScintillaGTK.cxx:1051: warning: missing braces around initializer for 'Sci_NotifyHeader' g++ -DHAVE_CONFIG_H -I. -I../../.. -pthread -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -pthread -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I../../../src/lib -I./scintilla/include -g -O2 -g -Wall -O2 -c -o gprompter-gprompter.o `test -f 'gprompter.cpp' || echo './'`gprompter.cpp gprompter.cpp: In function 'void on_char_added(SCNotification*, ScintillaObject*, Presage*)': gprompter.cpp:175: warning: format '%d' expects type 'int', but argument 2 has type 'uptr_t' gprompter.cpp:183: warning: format '%d' expects type 'int', but argument 2 has type 'uptr_t' gprompter.cpp:184: warning: format '%c' expects type 'int', but argument 2 has type 'uptr_t' gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -g -O2 -g -Wall -O2 -fPIC -I./src/lib -I/usr/include/python2.5 -c bindings/python/presage_wrap.cpp -o /tmp/buildd/presage-0.8.2/./build/temp.linux-x86_64-2.5/bindings/python/presage_wrap.o cc1plus: warning: command line option "-Wstrict-prototypes" is valid for Ada/C/ObjC but not for C++ gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -g -O2 -g -Wall -O2 -fPIC -I./src/lib -I/usr/include/python2.6 -c bindings/python/presage_wrap.cpp -o /tmp/buildd/presage-0.8.2/./build/temp.linux-x86_64-2.6/bindings/python/presage_wrap.o cc1plus: warning: command line option "-Wstrict-prototypes" is valid for Ada/C/ObjC but not for C++
I examined the scintilla compilation warnings and they seem harmless. I think they might be false positives.
doxygen warnings: doxygen Warning: Tag `USE_WINDOWS_ENCODING' at line 66 of file Doxyfile has become obsolete. To avoid this warning please update your configuration file using "doxygen -u" Warning: Tag `DETAILS_AT_TOP' at line 158 of file Doxyfile has become obsolete. To avoid this warning please update your configuration file using "doxygen -u" Warning: Tag `MAX_DOT_GRAPH_WIDTH' at line 1199 of file Doxyfile has become obsolete. To avoid this warning please update your configuration file using "doxygen -u" Warning: Tag `MAX_DOT_GRAPH_HEIGHT' at line 1207 of file Doxyfile has become obsolete. To avoid this warning please update your configuration file using "doxygen -u"
Fixed.
Generating call graph for function PredictorRegistry::updat/tmp/buildd/presage-0.8.2/src/lib/core/profileManager.h:47: Warning: Unsupported xml/html tag <sysconfdir> found /tmp/buildd/presage-0.8.2/src/lib/predictors/recencyPredictor.h:46: Warning: Found unknown command `\frac' /tmp/buildd/presage-0.8.2/src/lib/predictors/recencyPredictor.h:46: Warning: Found unknown command `\lambda' /tmp/buildd/presage-0.8.2/src/lib/predictors/recencyPredictor.h:50: Warning: Found unknown command `\lambda' e
Fixed.
dpkg-shlibdeps warnings: dpkg-shlibdeps: warning: dependency on libdl.so.2 could be avoided if "debian/libpresage1/usr/lib/libpresage.so.1.1.1" were not uselessly linked against it (they use none of its symbols). dpkg-shlibdeps: warning: dependency on libz.so.1 could be avoided if "debian/presage-gprompter/usr/bin/gprompter" were not uselessly linked against it (they use none of its symbols). dpkg-shlibdeps: warning: dependency on libfontconfig.so.1 could be avoided if "debian/presage-gprompter/usr/bin/gprompter" were not uselessly linked against it (they use none of its symbols). dpkg-shlibdeps: warning: dependency on libatk-1.0.so.0 could be avoided if "debian/presage-gprompter/usr/bin/gprompter" were not uselessly linked against it (they use none of its symbols). dpkg-shlibdeps: warning: dependency on librt.so.1 could be avoided if "debian/presage-gprompter/usr/bin/gprompter" were not uselessly linked against it (they use none of its symbols). dpkg-shlibdeps: warning: dependency on libgio-2.0.so.0 could be avoided if "debian/presage-gprompter/usr/bin/gprompter" were not uselessly linked against it (they use none of its symbols). dpkg-shlibdeps: warning: dependency on libcairo.so.2 could be avoided if "debian/presage-gprompter/usr/bin/gprompter" were not uselessly linked against it (they use none of its symbols). dpkg-shlibdeps: warning: dependency on libgthread-2.0.so.0 could be avoided if "debian/presage-gprompter/usr/bin/gprompter" were not uselessly linked against it (they use none of its symbols). dpkg-shlibdeps: warning: dependency on libpangocairo-1.0.so.0 could be avoided if "debian/presage-gprompter/usr/bin/gprompter" were not uselessly linked against it (they use none of its symbols). dpkg-shlibdeps: warning: dependency on libfreetype.so.6 could be avoided if "debian/presage-gprompter/usr/bin/gprompter" were not uselessly linked against it (they use none of its symbols). dpkg-shlibdeps: warning: dependency on libpangoft2-1.0.so.0 could be avoided if "debian/presage-gprompter/usr/bin/gprompter" were not uselessly linked against it (they use none of its symbols).
Mmm.. I'm not sure how to resolve those warnings... Can you please point me in the right direction? Is it a matter of passing LDFLAGS="-Wl,--as-needed" to the configure line?
dpkg-gencontrol warning:
dpkg-gencontrol: warning: package python-presage: unused substitution
variable ${python:Versions}
I'm not sure how to resolve this... Any suggestions?
lintian complaints: W: presage source: ancient-standards-version 3.7.3 (current is 3.8.4) P: presage-doc: copyright-refers-to-symlink-license usr/share/common-licenses/GPL P: libpresage-data: copyright-refers-to-symlink-license usr/share/common-licenses/GPL I: presage: hyphen-used-as-minus-sign usr/share/man/man1/text2ngram.1.gz:7 P: presage: copyright-refers-to-symlink-license usr/share/common-licenses/GPL P: libpresage1: copyright-refers-to-symlink-license usr/share/common-licenses/GPL I: libpresage1: spelling-error-in-binary ./usr/lib/libpresage.so.1.1.1 Commiting Committing X: libpresage1: shlib-calls-exit usr/lib/libpresage.so.1.1.1 I: libpresage1: no-symbols-control-file usr/lib/libpresage.so.1.1.1 P: libpresage-dev: copyright-refers-to-symlink-license usr/share/common-licenses/GPL P: python-presage: copyright-refers-to-symlink-license usr/share/common-licenses/GPL P: presage-gprompter: copyright-refers-to-symlink-license usr/share/common-licenses/GPL P: presage-pyprompter: copyright-refers-to-symlink-licenseusr/share/common-licenses/GPL
Fixed copyright-refers-to-symlink-license lintian complaints and ancient-standards-version.
Cheers, - Matteo -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected] Archive: http://lists.debian.org/[email protected]
