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

Bug#766982: RFS: plowshare4/1.0.6-1



On Mon, Oct 27, 2014 at 7:45 PM, Carl Suster wrote:

> I am looking for a sponsor for my package "plowshare4"

I don't intend to sponsor this but here is a review.

This looks like the only thing that would block the upload:

Some of the copyright holders are missing from debian/copyright.

These things would be nice to fix:

Whoa that is a lot of shell. shellcheck says it is probably
buggy/insecure shell too. Personally Python seems a better choice for
writing this sort of software but I guess upstream wouldn't want to
rewrite everything...

Upstream might want to switch from their homebrew version stuff to autorevision:

https://packages.debian.org/sid/autorevision

Is there any reason for not using the upstream `make install` target?
It seems to be very much correct. The only issue might be the git
version stuff but you can override that with GIT_VERSION until
upstream switches to autorevision (or if they do not want to).

I would suggest running `wrap-and-sort -sa` so that diffs of the
source package are easier to read.

The README, plowup manual page and Makefile use of /tmp in various
examples. Using /tmp can cause vulnerabilities on multi-user systems
or systems where an attacker has a shell and is looking to escalate
that access. It would be better for the examples to use a path in the
home directory using ~/ or $HOME depending on the example.

Upstream has some tests but the build does not run them. Is that
because they require network to work? Network can't be used on the
buildds but you could run them using DEP-8:

http://dep.debian.net/deps/dep8/
http://ci.debian.net/

You might want to add some upstream metadata:

https://wiki.debian.org/UpstreamMetadata

You might want to add a watch file:

https://wiki.debian.org/debian/watch
https://qa.debian.org/cgi-bin/fakeupstream.cgi?upstream=vcs/git/google/plowshare

You might want to add some debtags:

http://debtags.debian.net/edit/plowshare4

You might want to add some screenshots of typical usage:

https://screenshots.debian.net/upload/plowshare4

Automated checks:

https://wiki.debian.org/HowToPackageForDebian#Check_points_for_any_package
https://anonscm.debian.org/cgit/collab-maint/check-all-the-things.git

$ lintian
I: plowshare4 source: debian-watch-file-is-missing
P: plowshare4: no-upstream-changelog

$ cme check dpkg
...
Warning in 'control binary:plowshare4 Depends:0' value 'bash (>=4.1)':
unnecessary versioned dependency: bash >= 4.1. Debian has squeeze ->
4.1-3; squeeze-lts -> 4.1-3+deb6u2; wheezy-security ->
4.2+dfsg-0.1+deb7u3; wheezy -> 4.2+dfsg-0.1+deb7u3; jessie -> 4.3-11;
sid -> 4.3-11;
Cannot find license text for BSD-3-clause
...

$ codespell --quiet-level=3
./src/probe.sh:25: HELPFULL  ==> HELPFUL
./src/probe.sh:279: HELPFULL  ==> HELPFUL
./src/download.sh:25: HELPFULL  ==> HELPFUL
./src/download.sh:758: HELPFULL  ==> HELPFUL
./src/download.sh:865: sucessive  ==> successive
./src/list.sh:25: HELPFULL  ==> HELPFUL
./src/list.sh:182: HELPFULL  ==> HELPFUL
./src/core.sh:2547: existance  ==> existence
./src/delete.sh:25: HELPFULL  ==> HELPFUL
./src/delete.sh:97: HELPFULL  ==> HELPFUL
./src/upload.sh:25: HELPFULL  ==> HELPFUL
./src/upload.sh:261: HELPFULL  ==> HELPFUL
./src/modules/freakshare.sh:136: cant  ==> can't
./src/modules/ryushare.sh:87: adminstrator  ==> administrator
./src/modules/ge_tt.sh:194: recieve  ==> receive
./src/modules/nowdownload_co.sh:215: transfered  ==> transferred
./src/modules/nowdownload_co.sh:216: transfered  ==> transferred
./src/modules/hipfile.sh:360: successfull  ==> successful
./src/modules/sendspace.sh:178: folowing  ==> following
./src/modules/filemonkey.sh:94: occured  ==> occurred
./src/modules/bitshare.sh:98: cant  ==> can't

$ perlcritic -1 .
<lots of stuff>

$ find -type f -iname '*.sh' -exec shellcheck {} +
<lots of stuff>

$ find -type f | xargs isutf8
./src/modules/dl_free_fr.sh: line 71, char 1, byte offset 29: invalid UTF-8 code

$ egrep -ri 'fixme|todo' .
<lots of stuff>

-- 
bye,
pabs

https://wiki.debian.org/PaulWise


Reply to: