[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#739056: RFS: cwm/5.1-1 [ITP] -- Lightweight and efficient window manager for X11



On 17 February 2014 23:11, Jakub Wilk <jwilk@debian.org> wrote:
>
> * James McDonald <james@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


Reply to: