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

Bug#777651: RFS: syncterm/1.0+dfsg-1 [ITP]



control: tags -1 moreinf

Hi,

>    * Initial release (Closes: #739035)
lets try a review:

1) std-version is 3.9.8 now

2) debhelper (>= 9), libncurses5-dev (>= 5.9),
 unzip (>= 6.0), libsdl2-dev (>= 2.0.2), libsdl1.2-dev (>= 1.2.15),
 gcc (>= 4:4.9)


do you really need both sdl1.2 and sdl2?
do you really need the version constraints for each dependency?
why gcc is listed here?

please  drop versions when already satisfied in jessie, or wheezy in case you want to try
a backport-sloppy (I would really avoid that)

3)
Architecture: i386 amd64
Depends: ${shlibs:Depends}, ${misc:Depends}, libncurses5 (>= 5.9),
 libsdl2-2.0-0 (>= 2.0.2), libsdl1.2debian (>= 1.2.15)

why only two architectures? why aren't the runtime dependencies picked up with shlibs:Depends?
ldd debian/syncterm/usr/bin/syncterm 
	linux-vdso.so.1 =>  (0x00007ffeca745000)
	libutil.so.1 => /lib/x86_64-linux-gnu/libutil.so.1 (0x00007efc98d79000)
	libncurses.so.5 => /lib/x86_64-linux-gnu/libncurses.so.5 (0x00007efc98b57000)
	libtinfo.so.5 => /lib/x86_64-linux-gnu/libtinfo.so.5 (0x00007efc9892d000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007efc98729000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007efc98420000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007efc98202000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007efc97e39000)
	/lib64/ld-linux-x86-64.so.2 (0x0000560b91261000)


at least they seems to be mostly not linked at runtime.

4) rules: to understand the platform you are building, I suggest to use dpkg-architecture
dpkg-architecture -qDEB_TARGET_*

5) override_dh_strip:
        dh_strip --dbg-package=syncterm-dbg


please avoid dbg packages, they are auto generated now


6) ls src/
build  comio  conio  sbbs3  smblib  syncterm  uifc  xpdev

some (most) of them looks like embedded libraries

7) disabled_cryptlib needs to end in .patch (also in series file you should change it)
---
The information above should follow the Patch Tagging Guidelines, please
checkout http://dep.debian.net/deps/dep3/ to learn about the format. Here
are templates for supplementary fields that you might want to add:

this seems useless

8) the license is non dfsg

 The files sbbs3/zmodem.h and sbbs3/zmodem.c are derived from the
 zmtx/zmrx package available at
 ftp://ftp.netsw.org/net/modem/protocols/zmodem/zmtx-zmrx/
 .
 The licence contained in the archive is:
 .
 MCS allows you to use and copy/modify this source under the following
 conditions:
 .
  - MCS or Jacques Mattheij shall not be liable for any damages arising
    from the use of this code
  - the archive must be distributed as a whole leaving version numbers intact.
    please do not distribute modifications; mail them back to us for inclusion
    in the next release which should follow each other fairly quickly in
    the beginning
  - you will not use this software for commercial purposes.
    (commercial licenses are available contact us for info)
 .
 As such, this program may not be redistributable.  YOU HAVE BEEN WARNED!
 .
 If anyone can put me (shurd@sasktel.net) in contact with the authours,
 that would be greatly appreciated.


9) LGPL with no versioning is wrong.

10) many missing licenses: e.g. BSD-4-clause
11) many missing copyrights, e.g.
grep copyright . -Ri

stopping here the review, because of 8, that needs to be fixed upstream I think


automatic checks from check-all-the-things:

$ env PERL5OPT=-m-lib=. cme check dpkg
[lots]
$ codespell --quiet-level=3
[lots]
$ cppcheck -j1 --quiet -f .
[lots]
$ find -type f -iname '*.desktop' -exec desktop-file-validate {} \;
[some]
$ fdupes -q -r . | grep -vE '/(\.(git|svn|bzr|hg|sgdrawer)|_(darcs|FOSSIL_)|CVS)(/|$)' | cat -s
[lots]
$ flawfinder -Q -c .
[some]

and so on
$ suspicious-source
./src/uifc/uifc.c
./src/syncterm/ooii.c

# Prevents reproducible builds: https://reproducible-builds.org/
$ grep -rE ' __DATE__|__TIME__|__TIMESTAMP__' .
./src/xpdev/xptime.c:	printf("Rev %s Built " __DATE__ " " __TIME__ " with %s\n\n", revision, str);

$ grep -r '/tmp/' .
./src/xpdev/dirwrap.h:	#define _PATH_TMP			"/tmp/"

$ grep -riE 'fixme|todo|hack|xxx+|broken' .
./.pc/disabled_cryptlib/src/build/Common.gmake:  CFLAGS    += -D_THREAD_SUID_BROKEN
./3rdp/build/js_src_jsnativestack_cpp.patch:      * FIXME: this function is non-portable;
./src/xpdev/sdlfuncs.c:		 * This ugly hack attempts to prevent this... of course, remote X11
./src/xpdev/SDL_win32_main.c:/* Special Dynamic/Static hackery */
./src/xpdev/SDL_win32_main.c:	   keep them open.  This is a hack.. hopefully it will be fixed 
./src/xpdev/sockwrap.h:// Borland C++ builder 6 comes with a broken ws2tcpip.h header for GCC.
./src/xpdev/sockwrap.h:#define sendsocket		write		/* FreeBSD send() is broken */
./src/xpdev/xpdatetime.c:/* TODO: adjust times in 24:xx:xx format */
./src/xpdev/xpprintf.c:					 * ToDo this is a signed type of same size
./src/xpdev/xpprintf.c:					 * ToDo this is an unsigned type of same size
...

$ test -d ./debian &&
! grep -s -q native debian/source/format &&
! test -e debian/upstream/metadata &&
echo 'Please add some upstream metadata: https://wiki.debian.org/UpstreamMetadata'
Please add some upstream metadata: https://wiki.debian.org/UpstreamMetadata

$ env PERL5OPT=-m-lib=. uscan --report-status --no-verbose
uscan: Newest version of syncterm on remote site is 1.0, local version is 1.0+dfsg


I think that's all for a first review.

G.

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: