On 17 February 2014 23:11, Jakub Wilk <jw...@debian.org> wrote: > > * James McDonald <ja...@jamesmcdonald.com>, 2014-02-15, 14:46: >> >> http://mentors.debian.net/debian/pool/main/c/cwm/cwm_5.1-1.dsc
Thanks for taking the time to review this package. I've attempted to fix some of these problems, but I also have some questions. I have pushed the latest package to mentors. > blhc says that at least some parts of the package were built without > hardening: > >> CFLAGS missing (-fstack-protector --param=ssp-buffer-size=4 -Wformat >> -Werror=format-security): cc -c -Wall -O2 -g -D_FORTIFY_SOURCE=2 `pkg-config >> --cflags fontconfig x11 xft xinerama xrandr` calmwm.c I have modified debian/rules to override CFLAGS and include these additions. > The compiler warns about use of a deprecated function: > >> menu.c:469:2: warning: 'XKeycodeToKeysym' is deprecated (declared at >> /usr/include/X11/Xlib.h:1699) [-Wdeprecated-declarations] > > > ...and about implicit function declaration: > >> parse.y:92:4: warning: implicit declaration of function 'asprintf' >> [-Wimplicit-function-declaration] These problems have been fixed in the upstream HEAD, but not yet in a tagged release. I have spoken to the maintainers, and they expect a release including these changes in about a month. I will apply my packaging to that new version, which should fix these issues. > Upstream PGP-signed his tarball, so you may want to enable signature checking > in d/watch. Done. The lintian on mentors doesn't seem to like the new debian/upstream/signing-key.asc yet, so I've put the key in debian/upstream-signing-key.pgp for the moment. > Any reason add-changelog is not included in d/patches/series? PEBKAC. Fixed. > I'd rather not patch upstream makefile to change PREFIX, but override it in > debian/rules instead. I have made this change. That does make more sense. > fix-man-hyphens is not complete. There are more places where hyphen is used > as minus sign, although likely Lintian is not smart enough to detect them. I'm not sure exactly which of them to fix. Should I just mark up the hyphens in the 'bind' and 'mousebind' sections of the description, or should all the hyphens in the example configuration also count as minus signs? > Typo in the package description: > "etc" -> "etc." > > The description is oddly wrapped. The line ending with "virtual desktop" > could be two words longer. Both fixed. > Enumerated lists in d/copyright are not formatted correctly. Please see bug > #700970. Good point. I've added spaces as per your example. > Upstream embeds a few BSD-specific functions (fgetln, strlcat, strlcpy, > strtonum). It would be nice if Debian package could link to libbsd instead of > using these embedded copies. I have not included this patch, but I am now running it on my desktop. The upstream porter wasn't keen to add a dependency on libbsd as it doesn't seem to be used a lot. It might affect portability to some Linux distributions or potentially compatibility with the OpenBSD original. > Typo in client.c: > cant -> can't Added a patch to fix this. As regards the name /usr/bin/cwm, is there a reference for the correct or recommended way to rename files in the event of such collisions? Cheers, James -- 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/CAC5159=fa_spsgub4yrs+xa7j5xwsknaotw00-woupdnnzu...@mail.gmail.com
