Thank you very much for the detailed review. However, some of your comments are not entirely clear to me, so I have a couple of questions. I am preparing a new upstream version.
On 03/11/2013 07:54 AM, Paul Wise wrote: > In future, you might want to X-Debbugs-CC your earlier sponsors on the > RFS bug submission. I will do that. > I would encourage you to improve the watch file: > > http://wiki.debian.org/debian/watch#Common_mistakes I improved the watch file so that more extensions are found: http://sf.net/rrep/rrep-(.+)\.(?:zip|tgz|tbz|txz|(?:tar\.(?:gz|bz2|xz))) Were you referring to that? > I personally prefer --with autoreconf (using dh-autoreconf) to --with > autotools_dev. This ensures that when Debian QA folks do their usual > regular archive rebuilds, the build system gets checked for > rebuildability too. Done. > Please add --parallel to your invocation of dh. Done. > I would suggest that the upstream build system should install > doc/rrep.info, rather than dh_installinfo. Why would you suggest that? The dh_installinfo way seems to be the standard way that is described in the Maintainers Guide. I would prefer to stick to that. > It doesn't appear to be nessecary to distribute the upstream README in > the binary package. The package description covers the second part and > the rest isn't particular.y useful. README will not be distributed anymore. > The copyright/license info appears to be insufficient. The README is > not GPL-3+ and various files are copyright by the FSF. Various *.m4 > files are also not GPL. I added copyright/license texts for the FSF files to debian/copyright. > Why do you have ${shlibs:Depends} in Pre-Depends instead of Depends? > That looks like a mistake to me. That was a mistake. Corrected. > You don't appear to have any translations but you do have i18n, I > would suggest registering rrep with transifex to get in contact with > translators: For now I have boldquot and quot. I registered the project at transifex. > Please ensure that doc/rrep.info is regenerated at build time. I added texinfo to Build-Depends. This ensures that makeinfo is available during build time. > Please update the debtags information for this package: Done. > rrep contains something with a command-line interface, it would be > nice to create a screenshot or two showing typical usage. Done. > configure: > > configure: WARNING: libacl development library was not found or not usable. > configure: WARNING: rrep will be built without ACL support. I added libacl1-dev to Build-Depends and libacl1 to Suggests. No warnings anymore. > gcc: > > copy-file.c: In function 'qcopy_file_preserving': > copy-file.c:143:9: warning: ignoring return value of 'chown', declared > with attribute warn_unused_result [-Wunused-result] copy-file.c is part of gnulib. I tried the Debian version of gnulib and I tried the latest upstream gnulib. I get the warning in all cases. I am reluctant to change any gnulib files. > blhc: > > Lots of this: CFLAGS missing (-fPIE) .... > One of this: LDFLAGS missing (-fPIE -pie -Wl,-z,now) .... I added export DEB_BUILD_MAINT_OPTIONS = hardening=+all to rules. No flags missing anymore. > cppcheck: > > [lib/rpmatch.c:113]: (error) Memory leak: safe_pattern > [src/rrep.c:573] -> [src/rrep.c:575]: (error) Possible null pointer > dereference: next_path - otherwise it is redundant to check it against > null. > [lib/regcomp.c:2848]: (error) Uninitialized variable: symb_table > [lib/regcomp.c:2847]: (error) Uninitialized variable: table_size > [lib/regcomp.c:2850]: (error) Uninitialized variable: table_size I removed the redundant check in rrep.c. rpmatch.c and regcomp.c are part of gnulib. See response to copy-file.c. > msgfmt: > > msgfmt: ./po/rrep.pot: warning: source file contains fuzzy translation rrep.pot is meant as a template for potential translators. Should I not distribute it? > lacheck: > > lots of warnings on build-aux/texinfo.tex This is the standard texinfo file. I am reluctant to change anything here. > clang compiler: > > Build failure: > http://clang.debian.net/logs/2013-01-28/rrep_1.3.3-2_unstable_clang.log Sorry, I have no clue about the linker error of the clang compiler. This should work with any sane compiler. > ports: > > Build failure on x32 port: > http://buildd.debian-ports.org/status/package.php?p=rrep Works for rrep >= 1.3.4. Thanks, Arno -- To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/5145f305.3090...@asnelt.org
