Bug#821171: RFS: growl-for-linux/0.8.1-2 [ITP]

[Mattia Rizzolo]

control: tag -1 moreinfo

On Sat, Apr 30, 2016 at 06:56:25PM +0900, HAYASHI Kentaro wrote:
> So, I've removed package from mentors.d.n. and I'll contact
> to upstream above issues to fix them.

When solved, please remove the moreinfo tag.

-- 
regards,
Mattia Rizzolo

GPG Key: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540  .''`.
more about me:  https://mapreri.org : :'  :
Launchpad user: https://launchpad.net/~mapreri  `. `'`
Debian QA page: https://qa.debian.org/developer.php?login=mattia  `-


signature.asc
Description: PGP signature

Bug#821171: RFS: growl-for-linux/0.8.1-2 [ITP]

[HAYASHI Kentaro]

On Sat, 23 Apr 2016 15:07:18 +0800 Paul Wise  wrote:
> On Sat, Apr 23, 2016 at 12:44 AM, HAYASHI Kentaro wrote:
> 
> > 0.8.2 has been released, so I've updated to it.
> >
> >   
> > http://mentors.debian.net/debian/pool/main/g/growl-for-linux/growl-for-linux_0.8.2-1.dsc
> 
> I don't intend to sponsor this, but here is a review.

Thanks. It's a very helpful review!
As you pointed out, there are license related concerns that I've not noticed 
yet.
So, I've removed package from mentors.d.n. and I'll contact
to upstream above issues to fix them.


pgp_cVa41cEvP.pgp
Description: PGP signature

Bug#821171: RFS: growl-for-linux/0.8.1-2 [ITP]

[Paul Wise]

On Sat, Apr 23, 2016 at 12:44 AM, HAYASHI Kentaro wrote:

> 0.8.2 has been released, so I've updated to it.
>
>   
> http://mentors.debian.net/debian/pool/main/g/growl-for-linux/growl-for-linux_0.8.2-1.dsc

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

Some things that I think block the upload of this package:

Some of the XPM/PNG images look like they were derived from
proprietary icons and interfaces on Windows/etc. I don't know what
they are used for (they look like screenshots) but I would encourage
you to replace them.

The Tux image is not under a fully DFSG-free license AFAICT and you
haven't complied with the license for it either. Please remove your
package from mentors.d.n until that is fixed. In addition the
differing license/copyright should be documented in debian/copyright.

https://en.wikipedia.org/wiki/Tux

Some other files have copyright/license info that is not mentioned in
debian/copyright.

Some things you might want to fix at some point:

Use DEP-3 headers for the patches:

http://dep.debian.net/deps/dep3/

Why do you enable all hardening, disable pie in debian/rules and then
enable pie in the upstream Makefile.am?

I would suggest upgrading to debhelper compat 10, which automatically
runs autoreconf.

The override_dh_install looks like it is done in the wrong order,
shouldn't the find commands come after the dh_install command?

override_dh_makeshlibs looks like something that should be handled by
debhelper, have you filed a bug?

The upstream NEWS file is empty, don't install it in the binary package.

The upstream README/README.mkd files are basically duplicates.
Upstream should remove one of them but definitely don't install both
into the binary package.

The Standards-Version is out of date:

http://www.debian.org/doc/debian-policy/upgrading-checklist

The Homepage field redirects to here:

https://mattn.github.io/growl-for-linux/

Please remove all the libraries hardcoded into the Depends,
${shlibs:Depends} will automatically calculate those for you.

I would encourage you to get the package description reviewed by
debian-l10n-english.

https://wiki.debian.org/I18n/SmithReviewProject

What are these files about?

DO_NOT_USE_THIS_MODULE

I would encourage you to include copyright/license info in all files:

http://lu.is/blog/2012/03/17/on-the-importance-of-per-file-license-information/

Some of the XPM/PNG images are duplicates of each other. You should
choose which is the source image and generate the other one at build
time using something like convert from imagemagick. I would also
generate icon_dnd.png, gol.ico and growl4linux.jpg from icon.png and a
Linux mascot image.

I would encourage you to use xz instead of bzip2 in the upstream
makedist.sh, it is superior in every way.

Please add some upstream metadata:

https://wiki.debian.org/UpstreamMetadata

Please add cryptographic verification of the upstream tarball:

https://wiki.debian.org/debian/watch#Cryptographic_signature_verification

You might want to fuzz-test the code using zzuf or afl.

I wonder if the upstream code needs copyright years bumped, you have
clearly worked on it since 2011:

https://github.com/mattn/growl-for-linux/commits/master

Automatic checks:

build

aclocal: warning: couldn't open directory 'm4': No such file or directory
ar: `u' modifier ignored since `D' is the default (see `U')
dpkg-shlibdeps: warning:
debian/growl-for-linux/usr/lib/x86_64-linux-gnu/growl-for-linux/display/libnotify_gol.so.0.0.0
contains an unresolvable reference to symbol curl_easy_cleanup: it's
probably a plugin
dpkg-shlibdeps: warning: 5 other similar warnings have been skipped
(use -v to see them all)


lintian

P: growl-for-linux source: debian-watch-may-check-gpg-signature
W: growl-for-linux: binary-without-manpage usr/bin/gol

check-all-the-things

# Just style checks, feel free to ignore
$ find -type f \( -iname '*.sh' -o -iname '*.bash' \) -exec bashate
--ignore E002,E003 {} +
E010: Do not on same line as for: 'for distro in "${Ubuntus[@]}"'
 - ./release.sh : L16
1 bashate error(s) found

$ find -type f -iname '*.sh' -exec checkbashisms {} +
possible bashism in ./makedist.sh line 9 (brace expansion):
rm -rf INSTALL aclocal.m4 autom4te.cache/ compile config.{sub,guess}
configure depcomp install-sh ltmain.sh  m4/ missing

$ cme check dpkg
...
Warning in 'control source Standards-Version' value '3.9.7': Current
standards version is 3.9.8
Warning in 'patches:"disable-display-execstack.patch" Synopsis' value
: Empty synopsis (this can be fixed with 'cme fix' command)
Warning in 'patches:"add-pie-flags-for-gol.patch" Synopsis' value
: Empty synopsis (this can be fixed with 'cme fix' command)

$ codespell --quiet-level=3
./gol.c:1244: adn  ==> and
./gol.c:1247: adn  ==> and

$ cppcheck -j1 --quiet -f .
[plugins/memfile.c:16]: (error) Memory pointed to by 'mf' is freed twice.

$ find \( -name .git -o -name .svn -o -name .bzr -o -name CVS -o -name
.hg -o -name _darcs -o -name _FOSSIL_ -o -name .sgdrawer \) -prune -o

Bug#821171: RFS: growl-for-linux/0.8.1-2 [ITP]

[HAYASHI Kentaro]

On Fri, 22 Apr 2016 22:42:03 +0900 d...@debian.org wrote:
> > Okay, I'll fix it.
> 
> Thank you.  I am waiting for your uploading.

0.8.2 has been released, so I've updated to it.

  
http://mentors.debian.net/debian/pool/main/g/growl-for-linux/growl-for-linux_0.8.2-1.dsc


pgpUTgmWLStgQ.pgp
Description: PGP signature

Bug#821171: RFS: growl-for-linux/0.8.1-2 [ITP]

[dai]

On Fri, Apr 22, 2016 at 10:26:23PM +0900, HAYASHI Kentaro wrote:
> I've already contacted with upstream author in this issue,
> isn't good enough?
>   https://github.com/mattn/growl-for-linux/issues/56

You are more than wonderful.
And I am sorry to overlook this issue.

> Okay, I'll fix it.

Thank you.  I am waiting for your uploading.
-- 
Regards,
dai

GPG Fingerprint = 0B29 D88E 42E6 B765 B8D8 EA50 7839 619D D439 668E


signature.asc
Description: PGP signature

Bug#821171: RFS: growl-for-linux/0.8.1-2 [ITP]

[HAYASHI Kentaro]

On Fri, 22 Apr 2016 01:22:46 +0900 d...@debian.org wrote:
> OK, prior confirmation is not always needed.
> Generally, if others already have made package,
> it is nice to contact him/her to avoid redundant work.
> Now, since you have sent PR to upstream github to make packaging better,
> I hope you cooperate with him nicely from now on.

I've already contacted with upstream author in this issue,
isn't good enough?
  https://github.com/mattn/growl-for-linux/issues/56

> Confirmed. But debian/changelog.bak is contained. Please remove it.

Okay, I'll fix it.

Thanks!


pgpMF4B4uIDxn.pgp
Description: PGP signature

Bug#821171: RFS: growl-for-linux/0.8.1-2 [ITP]

[dai]

On Thu, Apr 21, 2016 at 10:43:45PM +0900, HAYASHI Kentaro wrote:
> No. As far as I know, growl-for-linux had never released in Ubuntu.
> upstream author had released it as PPA before.
>   
>   https://launchpad.net/~mattn/+archive/ubuntu/growl-for-linux
> 
> So, I'd like to put this into Debian.

OK, prior confirmation is not always needed.
Generally, if others already have made package,
it is nice to contact him/her to avoid redundant work.
Now, since you have sent PR to upstream github to make packaging better,
I hope you cooperate with him nicely from now on.

> Thanks, I've fixed typos.
> 
>   * Drop -> Dropped in debian/changelog
>   * Rewrited -> Rewrote in debian/changelog
>   * ballon -> balloon in debian/control

Confirmed. But debian/changelog.bak is contained. Please remove it.
-- 
Regards,
dai

GPG Fingerprint = 0B29 D88E 42E6 B765 B8D8 EA50 7839 619D D439 668E


signature.asc
Description: PGP signature

Bug#821171: RFS: growl-for-linux/0.8.1-2 [ITP]

[HAYASHI Kentaro]

> It seems that upstream author already uploads this package into Ubuntu,
> Do you talk him about uploading this package into Debian?

No. As far as I know, growl-for-linux had never released in Ubuntu.
upstream author had released it as PPA before.
  
  https://launchpad.net/~mattn/+archive/ubuntu/growl-for-linux

So, I'd like to put this into Debian.

> You can use aspell or ispell for spell check.
> 
> https://www.debian.org/doc/manuals/developers-reference/best-pkging-practices.html#bpp-pkg-desc

Thanks, I've fixed typos.

  * Drop -> Dropped in debian/changelog
  * Rewrited -> Rewrote in debian/changelog
  * ballon -> balloon in debian/control


pgpPg6InqtzwD.pgp
Description: PGP signature

Bug#821171: RFS: growl-for-linux/0.8.1-2 [ITP]

[dai]

On Sat, Apr 16, 2016 at 06:05:03PM +0900, HAYASHI Kentaro wrote:
> * Package name: growl-for-linux
>   Version : 0.8.1-2

It seems that upstream author already uploads this package into Ubuntu,
Do you talk him about uploading this package into Debian?

>   * debian/copyright
> - Rewrited to machine-readable debian/copyright

typo: Rewrited 

You can use aspell or ispell for spell check.

https://www.debian.org/doc/manuals/developers-reference/best-pkging-practices.html#bpp-pkg-desc

>   * debian/rules
> - Fixed non-empty-dependency_libs-in-la-file lintian warning
> - Added hardening flags (+all,-pie)
> - Drop deprecated libtweets subscriber

"Drop" is only present tense. Others are past tense.

debian/control:

> Growl For Linux provides four kind of display styles - ballon, fog,

typo: ballon
-- 
Regards,
dai

GPG Fingerprint = 0B29 D88E 42E6 B765 B8D8 EA50 7839 619D D439 668E


signature.asc
Description: PGP signature

Bug#821171: RFS: growl-for-linux/0.8.1-2 [ITP]

[HAYASHI Kentaro]

Package: sponsorship-requests
Severity: normal

Dear mentors,

I am looking for a sponsor for my package "growl-for-linux"

* Package name: growl-for-linux
  Version : 0.8.1-2
  Upstream Author : Yasuhiro Matsumoto 
* URL : http://mattn.github.com/growl-for-linux
* License : BSD-2-clause
  Section : gnome

It builds those binary packages:

  growl-for-linux - Notification system which is extendable by a plugin

To access further information about this package, please visit the following 
URL:

http://mentors.debian.net/package/growl-for-linux

Alternatively, one can download the package with dget using this command:

  dget -x 
http://mentors.debian.net/debian/pool/main/g/growl-for-linux/growl-for-linux_0.8.1-2.dsc

More information about hello can be obtained from 
http://mattn.github.com/growl-for-linux

Changes since the last upload:

  * Initial upload to Debian (Closes: #816514)
  * debian/control
- Fixed package-depends-on-hardcoded-libc lintian warning
- Upgraded to debhelper 9
- Upgraded to standards-version 3.9.7
- Added missing TLS dependency
- Updated package description about growl-for-linux
  * debian/copyright
- Rewrited to machine-readable debian/copyright
  * debian/changelog
- Fixed wrong email address
  * debian/rules
- Fixed non-empty-dependency_libs-in-la-file lintian warning
- Added hardening flags (+all,-pie)
- Drop deprecated libtweets subscriber
- Fixed not to call needless ldconfig
  * debian/patches/add-missing-desktop-keyword.patch
- Added missing "Keywords=" field
  * debian/patches/add-pie-flags-for-gol.patch
- Added hardening flags (-pie,-fPIE) for gol
  * debian/patches/disable-display-execstack.patch
- Fixed needless executable stack lintian warning
  * debian/patches/fix-a-typo-about-recieved.patch
- Fixed a typo in SQLite schema
  * debian/patches/fix-typo-about-faild.patch
- Fixed a typo in error message

Regards,
 HAYASHI Kentaro