Bug#888807: RFS: qstardict/1.3-1

[Boyuan Yang]

Control: tags -1 - moreinfo

在 2018年2月2日星期五 CST 下午11:25:11，Tobias Frost 写道：
> But two things escaped your eyes:
> - Left-tover "Comments:" on line 69
> - license-reconsile finds that in the libqxt is BSD-licensed (eg.
>   qxt/qxtglobal.cpp)

OK. Fixed now.

> > > - don't install README.md -- it does not have extra information beyond
> > > 
> > >   a package  description and compilation instructions (which are useless
> > >   for the users of the binary package)
> > >   There is also a slight bug in it: The URLs at the bottom seems
> > >   outdated, they will forward to the github project from the watch file.
> > >   Maybe at least report that to upstream.)
> > 
> > Done. The typo was forwarded upstream and got fixed in trunk code.
> 
> Ok. You should reflect this in the dep3 header though.
> Forwarded: no is not what you want, the measning for this field is
> when it is a Debian specific patch (value not-needed) or if you did not
> bother to forward it (yet) -- then it is "no",
> Here You Want(tm) "Applied-Upstream"
> followed by either the commit-id or the URL pointing to it.
> (see the dep3 spec for details))
> 
> (This is also valid for the other patches you mentioned below)

I updated those patches and replaced the Forwarded: field with Applied-
Upstream: field as decribed in dep3.

> > > - The embedded libqxt -- can you use the Debian packaged version?
> > 
> > Sorry but nope -- If we take a look into libqxt in Debian, #875027 says
> > that libqxt is unmaintained upstream and will be removed from Debian
> > archive soon. Upstream git repository also suggested that all projects
> > previously using libqxt should either migrate away from libqxt or embed
> > part of its code to fit their own need. [1] That is exactly what
> > qstardict
> > upstream is doing,
> > see also the GitHub issue [2].
> > 
> > [1] https://bitbucket.org/libqxt/libqxt/wiki/Home
> > [2] https://github.com/a-rodin/qstardict/issues/16
> 
> Well, this is not exactly how we deal with embedded code copies.
> When a library is gonna be removed from Debian this is not a valid excuse to
> have an embedded code copy of the same in another package. So the right
> thing is (as you've done already) to bring it to upstreams' attention to
> get that fixed before QT4 will be removed within this development cycle.
> In this case the effort is probably not required to patch the buildsystem to
> use the packaged version, as long as available, but when you follow the
> instructions here: https://wiki.debian.org/EmbeddedCodeCopies
> Keep me CC in the mail you send the notice to the security team.

After some investigation, I found that embedded libqxt is becoming a general 
problem thus here the post is:

https://lists.debian.org/debian-security-tracker/2018/02/msg00019.html


> OK, round 2 done :)
> Its almost good, let me know when done!

I've updated the git repository on Salsa as well as its source package on 
mentors.debian.net .

--
Thanks,
Boyuan Yang



signature.asc
Description: This is a digitally signed message part.

Bug#888807: RFS: qstardict/1.3-1

[Tobias Frost]

Control: tags -1 moreinfo


Hi Boyuan,

On Fri, Feb 02, 2018 at 10:09:12PM +0800, Boyuan Yang wrote:
> X-Debbugs-CC: t...@debian.org
> Control: tag -1 - moreinfo
> 
> Hi tobi,
> 
> Thanks for your review! In fact I didn't receive your reply before
> (don't know why) and I just noticed it via BTS web interface. Anyway
> here's the updated status:
> 
> > - small typo in d/copyright: Alexander had maintained the package in
> >   2007 and 2008. Also it should be "Comment:" (singular)
> 
> Done.
> 
> > - Please review d/copyright. I found at least one file which is not
> >   properly recorded (wrong license and wrong copyright holder)
> 
> Done. I looked into every source files in the repository this time.
> 
Thanks for going over it!
But two things escaped your eyes:
- Left-tover "Comments:" on line 69
- license-reconsile finds that in the libqxt is BSD-licensed (eg.
  qxt/qxtglobal.cpp)

> > - don't install README.md -- it does not have extra information beyond
> >   a package  description and compilation instructions (which are useless
> >   for the users of the binary package)
> >   There is also a slight bug in it: The URLs at the bottom seems
> >   outdated, they will forward to the github project from the watch file.
> >   Maybe at least report that to upstream.)
> 
> Done. The typo was forwarded upstream and got fixed in trunk code.

Ok. You should reflect this in the dep3 header though.
Forwarded: no is not what you want, the measning for this field is
when it is a Debian specific patch (value not-needed) or if you did not
bother to forward it (yet) -- then it is "no",
Here You Want(tm) "Applied-Upstream"
followed by either the commit-id or the URL pointing to it.
(see the dep3 spec for details))

(This is also valid for the other patches you mentioned below)

> > - Please upstream the manpage (Alexander as upstream should include it
> >   there so that other distributions will also benefit from it)
> 
> I've filed an issue on upstream GitHub project.
> https://github.com/a-rodin/qstardict/issues/19

+1!
 
> > - The embedded libqxt -- can you use the Debian packaged version?
> 
> Sorry but nope -- If we take a look into libqxt in Debian, #875027 says that
> libqxt is unmaintained upstream and will be removed from Debian archive
> soon. Upstream git repository also suggested that all projects previously
> using libqxt should either migrate away from libqxt or embed part of its
> code to fit their own need. [1] That is exactly what qstardict
> upstream is doing,
> see also the GitHub issue [2].
> 
> [1] https://bitbucket.org/libqxt/libqxt/wiki/Home
> [2] https://github.com/a-rodin/qstardict/issues/16

Well, this is not exactly how we deal with embedded code copies.
When a library is gonna be removed from Debian this is not a valid excuse to
have an embedded code copy of the same in another package. So the right thing
is (as you've done already) to bring it to upstreams' attention to get that
fixed before QT4 will be removed within this development cycle.
In this case the effort is probably not required to patch the buildsystem to
use the packaged version, as long as available, but when you follow the
instructions here: https://wiki.debian.org/EmbeddedCodeCopies
Keep me CC in the mail you send the notice to the security team.

> > - Some lintian stuff:
> > N: Processing binary package qstardict (version 1.3-1, arch amd64) ...
> > I: qstardict: spelling-error-in-binary usr/bin/qstardict writen written
> > I: qstardict: spelling-error-in-binary
> > usr/lib/qstardict/plugins/libstardict.so wil will
> > I: qstardict: spelling-error-in-binary
> > usr/lib/qstardict/plugins/libstardict.so formated formatted
> > I: qstardict: desktop-entry-lacks-keywords-entry
> > usr/share/applications/qstardict.desktop
> > (spelling errors should be at least sent upstream, but they
> > are quite easy to fix and then a patch can be sent upstream :))
> > note that the spelling errors might also needs fixing in the
> > translation templates.
> 
> Fixes are submitted upstream and got merged. Patches are also cherry-picked
> in debian/patches directory.
> 
> > - check-all-the-things also found a bit of stuff.
> 
> I took a look and forwarded the information from cppcheck and flawfinder
> to upstream.
> 
> https://github.com/a-rodin/qstardict/issues/17
> 
> - The watch file is not working.
> 
> Fixed.
> 
> > Future homework (optional -- bonus points area ;-))
> 
> I decided not to do them this time -- will come back to them after I get to
> know qstardict better with a period of app using experience.
> 
> The new version is now uploaded onto mentors.debian.net and
> salsa.debian.org/debian/qstardict repository.

> --
> Thanks,
> Boyuan Yang

OK, round 2 done :)
Its almost good, let me know when done!

--
tobi

Bug#888807: RFS: qstardict/1.3-1

[Boyuan Yang]

X-Debbugs-CC: t...@debian.org
Control: tag -1 - moreinfo

Hi tobi,

Thanks for your review! In fact I didn't receive your reply before
(don't know why) and I just noticed it via BTS web interface. Anyway
here's the updated status:

> - small typo in d/copyright: Alexander had maintained the package in
>   2007 and 2008. Also it should be "Comment:" (singular)

Done.

> - Please review d/copyright. I found at least one file which is not
>   properly recorded (wrong license and wrong copyright holder)

Done. I looked into every source files in the repository this time.

> - don't install README.md -- it does not have extra information beyond
>   a package  description and compilation instructions (which are useless
>   for the users of the binary package)
>   There is also a slight bug in it: The URLs at the bottom seems
>   outdated, they will forward to the github project from the watch file.
>   Maybe at least report that to upstream.)

Done. The typo was forwarded upstream and got fixed in trunk code.

> - Please upstream the manpage (Alexander as upstream should include it
>   there so that other distributions will also benefit from it)

I've filed an issue on upstream GitHub project.
https://github.com/a-rodin/qstardict/issues/19

> - The embedded libqxt -- can you use the Debian packaged version?

Sorry but nope -- If we take a look into libqxt in Debian, #875027 says that
libqxt is unmaintained upstream and will be removed from Debian archive
soon. Upstream git repository also suggested that all projects previously
using libqxt should either migrate away from libqxt or embed part of its
code to fit their own need. [1] That is exactly what qstardict
upstream is doing,
see also the GitHub issue [2].

[1] https://bitbucket.org/libqxt/libqxt/wiki/Home
[2] https://github.com/a-rodin/qstardict/issues/16

> - Some lintian stuff:
> N: Processing binary package qstardict (version 1.3-1, arch amd64) ...
> I: qstardict: spelling-error-in-binary usr/bin/qstardict writen written
> I: qstardict: spelling-error-in-binary
> usr/lib/qstardict/plugins/libstardict.so wil will
> I: qstardict: spelling-error-in-binary
> usr/lib/qstardict/plugins/libstardict.so formated formatted
> I: qstardict: desktop-entry-lacks-keywords-entry
> usr/share/applications/qstardict.desktop
> (spelling errors should be at least sent upstream, but they
> are quite easy to fix and then a patch can be sent upstream :))
> note that the spelling errors might also needs fixing in the
> translation templates.

Fixes are submitted upstream and got merged. Patches are also cherry-picked
in debian/patches directory.

> - check-all-the-things also found a bit of stuff.

I took a look and forwarded the information from cppcheck and flawfinder
to upstream.

https://github.com/a-rodin/qstardict/issues/17

- The watch file is not working.

Fixed.

> Future homework (optional -- bonus points area ;-))

I decided not to do them this time -- will come back to them after I get to
know qstardict better with a period of app using experience.

The new version is now uploaded onto mentors.debian.net and
salsa.debian.org/debian/qstardict repository.

--
Thanks,
Boyuan Yang

Bug#888807: RFS: qstardict/1.3-1

[Tobias Frost]

Control: tags -1 moreinfo

Hi Boyuan,

here's your review:

- small typo in d/copyright: Alexander had maintained the package in
  2007 and 2008. Also it should be "Comment:" (singular)
- Please review d/copyright. I found at least one file which is not
  properly recorded (wrong license and wrong copyright holder)
- don't install README.md -- it does not have extra information beyond
  a package  description and compilation instructions (which are useless
  for the users of the binary package)
  There is also a slight bug in it: The URLs at the bottom seems
  outdated, they will forward to the github project from the watch file.
  Maybe at least report that to upstream.)
- Please upstream the manpage (Alexander as upstream should include it
  there so that other distributions will also benefit from it)
- The embedded libqxt -- can you use the Debian packaged version?
- Some lintian stuff:
N: Processing binary package qstardict (version 1.3-1, arch amd64) ...
I: qstardict: spelling-error-in-binary usr/bin/qstardict writen written
I: qstardict: spelling-error-in-binary
usr/lib/qstardict/plugins/libstardict.so wil will
I: qstardict: spelling-error-in-binary
usr/lib/qstardict/plugins/libstardict.so formated formatted
I: qstardict: desktop-entry-lacks-keywords-entry
usr/share/applications/qstardict.desktop
(spelling errors should be at least sent upstream, but they
are quite easy to fix and then a patch can be sent upstream :))
note that the spelling errors might also needs fixing in the
translation templates.
- check-all-the-things also found a bit of stuff.
- The watch file is not working.

Future homework (optional -- bonus points area ;-))
- There are many bugs on the BTS that needs bug triaging, e.g
  #494701, #611106, #699940 and maybe others too.  It is possible that
  those bugs are not valid anymore, as they are quite old already.
  Please document the findings in the BTS, file them upstream and/or
  link them to upstream bugs if relevant :)
- Please check with upstream if they have the source for the pngs
  which where obviously created with inkscape (check all the things
  told so). They should be included in the source...
# Check with upstream where the Inkscape SVG source files are.
$ find . -type f \( -iname '*.png' -o -iname '*.gif' -o -iname '*.jpg'
-o -iname '*.jpeg' \) -exec grep -nHiF inkscape {} +
Binary file ./qstardict/pixmaps/system-search.png matches
Binary file ./qstardict/pixmaps/download.png matches
Binary file ./qstardict/pixmaps/plugin.png matches

Please fix the points above, and then remove the moreinfo tag to
signal that it is ready for another round.

Many thanks!

-- 
tobi


signature.asc
Description: PGP signature

Bug#888807: RFS: qstardict/1.3-1

[Tobias Frost]

Control: owner -1 !

Hi Boyuan,

I will take a look a the package likely tonight.

One thing you can already prepare is to put yourself
in as maintainer; as you saw in private conversation,
Alexander won't have time for maintaining the pacakge in
the near future and agreed to get his name dropped (and maybe
later readded when this changes)

Tobi

On Tue, 30 Jan 2018 13:47:49 +0800 Boyuan Yang <073p...@gmail.com>
wrote:
> Package: sponsorship-requests
> Severity: normal
> X-Debbugs-CC: rodin.alexan...@gmail.com
> 
> Dear mentors,
> 
> I am looking for a sponsor for the package "qstardict".
> 
> I am neither this package's maintainer nor uploader (yet), but I will
> be in the uploaders list (co-maintainer)
> with the original maintainer's (who's also acting as qstardict's
> upstream) approval in a private mail.
> 
>  * Package name: qstardict
>Version : 1.3-1
>Upstream Author : Alexander Rodin 
>  * URL : https://github.com/a-rodin/qstardict
>  * License : GPL-2+
>Section : x11
> 
>   It builds those binary packages:
> 
> qstardict  - International dictionary written using Qt
> 
>   To access further information about this package, please visit the
> following URL:
> 
>   https://mentors.debian.net/package/qstardict
> 
> 
>   Alternatively, one can download the package with dget using this
command:
> 
> dget -x https://mentors.debian.net/debian/pool/main/q/qstardict/q
stardict_1.3-1.dsc
> 
>   Git packaging repository on Salsa:
> 
>   https://salsa.debian.org/debian/qstardict
> 
>   Changes since the last upload:
> 
> qstardict (1.3-1) unstable; urgency=medium
> 
>   * Sponsored upload with original maintainer's approval.
>   * Add myself into uploaders list.
>   * New upstream release. (2018-01-24) Closes: #528257
> + Ported to Qt5. Closes: #875145
>   * Switch to 3.0 (quilt) packaging format.
>   * Bump debhelper compat level 5 -> 11.
> + Use dh sequencer.
>   * Bump Standards-Version 3.8.0 -> 4.1.3.
>   * Drop d/menu and d/qstardict.xpm files in favor of .desktop file.
>   * Point d/watch file to GitHub releases.
>   * Refresh d/qstardict.1 manpage and install it properly.
>   * Rewrite d/copyright with machine-readable format.
> 
>  -- Boyuan Yang <073p...@gmail.com>  Tue, 30 Jan 2018 12:53:37 +0800
> 
> Also CC-ing the original maintainer here.
> 
> 

Bug#888807: RFS: qstardict/1.3-1

[Boyuan Yang]

Package: sponsorship-requests
Severity: normal
X-Debbugs-CC: rodin.alexan...@gmail.com

Dear mentors,

I am looking for a sponsor for the package "qstardict".

I am neither this package's maintainer nor uploader (yet), but I will
be in the uploaders list (co-maintainer)
with the original maintainer's (who's also acting as qstardict's
upstream) approval in a private mail.

 * Package name: qstardict
   Version : 1.3-1
   Upstream Author : Alexander Rodin 
 * URL : https://github.com/a-rodin/qstardict
 * License : GPL-2+
   Section : x11

  It builds those binary packages:

qstardict  - International dictionary written using Qt

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

  https://mentors.debian.net/package/qstardict


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

dget -x 
https://mentors.debian.net/debian/pool/main/q/qstardict/qstardict_1.3-1.dsc

  Git packaging repository on Salsa:

  https://salsa.debian.org/debian/qstardict

  Changes since the last upload:

qstardict (1.3-1) unstable; urgency=medium

  * Sponsored upload with original maintainer's approval.
  * Add myself into uploaders list.
  * New upstream release. (2018-01-24) Closes: #528257
+ Ported to Qt5. Closes: #875145
  * Switch to 3.0 (quilt) packaging format.
  * Bump debhelper compat level 5 -> 11.
+ Use dh sequencer.
  * Bump Standards-Version 3.8.0 -> 4.1.3.
  * Drop d/menu and d/qstardict.xpm files in favor of .desktop file.
  * Point d/watch file to GitHub releases.
  * Refresh d/qstardict.1 manpage and install it properly.
  * Rewrite d/copyright with machine-readable format.

 -- Boyuan Yang <073p...@gmail.com>  Tue, 30 Jan 2018 12:53:37 +0800

Also CC-ing the original maintainer here.


Regards,
Boyuan Yang