Package: sponsorship-requests
Severity: wishlist

Dear mentors,

I am looking for a sponsor for my[1] package "strawberry":

  * Package name    : strawberry
    Version         : 1.0.4-1
    Upstream Author : Jonas Kvinge <jonas@jkvinge.net>
  * URL             : https://www.strawberrymusicplayer.org/
  * License         : GPL-3+, LGPL-2.1+, CC0-1.0, BSD-2-clause, GPL-3, Apache-2.0, GPL-2+, Expat, MIT, BSD-3-clause
    Section         : sound

The source builds the following binary packages:

   strawberry - Audio player and music collection organizer

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

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

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

   dget -x https://mentors.debian.net/debian/pool/main/s/strawberry/strawberry_1.0.4-1.dsc

Changes for the initial release:

  strawberry (1.0.4-1) unstable; urgency=medium
  .
    * Initial release (Closes: #913079)
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=913079


Regards,

Control: tags -1 moreinfo

like pollo, I'm puzzled by the mention on the ITP bug of the package
being in NEW at some point, only to vanish into thin air? Would be
nice to know what happened to it, if only to avoid running into the
same problems. Maybe Thomas would like to chime in on this?


That said, I took a look anyway. Some comments and observations:
- There's an unused manpage in the debian dir, an apparent leftover
  from the earlier packaging effort
- Copyright:
  * missing copyright holder "Pascal Below" (for various
    scrobbler-related files)
  * missing info for 3rdparty/macdeployqt
  * wrong license for 3rdparty/SPMediaKeyTap
  * is upstream the sole contributor to the debian packaging?
  * MIT and Expat license definitions appear identical, please use
    Expat as the license name throughout and remove the duplicate
  * the content of the license paragraphs for GPL-3 and GPL-3+ is
    identical (but obviously shouldn't be)
  * be careful to exclude copyright claims, comments, etc. from the
    license paragraphs; i.e. make the definitions for the BSD-style
    licenses start at "Redistribution and use..." so they're generic
    and re-usable; everything else belongs in the Files paragraphs
- Control:
  * short description shouldn't start with caps
  * hardcoded libsqlite3-0 library dependency should be handled by
    ${shlibs:Depends} (libqt5sql5-sqlite is only recommended by the
    qt sql lib so that one might actually be justified)
  * a slightly newer standards-version out has come out recently
  * VCS: consider setting up a git repo on salsa.debian.org for your
    packaging work and enabling the CI there: it's a great quality
    control and collaboration tool, and a real timesaver for reviewers
    too
- Docs: upstream changelog installed as doc rather than as changelog
  (via dh_installchangelogs)
- Rules: better list those files in d/clean instead of using an
  override
- Upstream/metadata: is a github user page -even that of the lead
  developer- really the best place to contact the upstream project?
- Watch: unused dversionmangling

- Build: why -fpermissive?

- FHS: according to its manpage, the tagreader binary "is not meant to
  be run on its own"; is /usr/bin really where it should be installed?
  See https://www.debian.org/doc/packaging-manuals/fhs/ (libexec?)

- Lintian:
  * I: strawberry: desktop-entry-lacks-keywords-entry
    usr/share/applications/org.strawberrymusicplayer.strawberry.desktop

- Tests: upstream ships a testsuite; if possible, please run it on
  build and/or deploy it as an autopkgtest


Please remove the moreinfo tag (and CC me directly) once you have an
updated package ready.

Control: tags 1010663 - moreinfo


Hi Jeroen,

Thanks for your review.
I have implemented your suggestions (except testsuite & VCS) and uploaded to Mentors.

I cannot get the test suite to build. Also, as strawberry is an interactive GUI,
the scope for autopkgtest style tests seems restricted.
However, it would be helpful for me to be able to run formal tests locally,
so I intend to raise the issue upstream.

I don't have privilege to set up repositories on Salsa, but would like to have one to run CI.
Could someone create an empty repository for strawberry under debian group?


Cheers,
Peter B

I've just created https://salsa.debian.org/debian/strawberry and
granted your account maintainer level access.

Once you got that up and running with the CI I'll take another look
at the package.

Hi Jeroen,

Strawberry has passed all CI tests.
https://salsa.debian.org/debian/strawberry/-/jobs

Cheers,
Peter

I searched my records and found the rejection. Looks like it's fixable.
Quoted here and I've cc'd the ftpteam if anyone has questions

+----------------------+
|   REJECT reasoning   |
+----------------------+

A trainee points out:

ext\libstrawberry-common\core\scoped_nsautorelease_pool.mm has a different
license and copyright holders

src\engine\enginebase.cpp and others not accounted for by d/copyright.

src\widgets\qsearchfield_mac.mm and src\widgets\qsearchfield_nonmac.cpp not
accounted for by d/copyright.

+----------------------+
|    Other comments    |
+----------------------+

d/copyright does not account for ext\libstrawberry-common\core\logging.cpp.

src/dbus/*.xml are from a different project so presumably different
copyright
holders?

+----------------------+
|         N.B.         |
+----------------------+

This review may not be exhaustive.  Please check your source package
against your d/copyright and the ftpmaster REJECT-FAQ, throughly,
before uploading to NEW again.

Thank you for your time and contribution!



Cheers,
 paultag

Thanks for that Paul,

Some of these have been fixed already.
I've added missing files to the Chromium Authors paragraph
and added a src/dbus/*.xml paragraph.

Updated debian/copyright and uploaded to Mentors & Salsa

Cheers,
Peter

Regarding the latest Salsa CI fails;

Its not clear to me why changing the copyright file causes crossbuild & reprotest to fail,
https://salsa.debian.org/debian/strawberry/-/pipelines/382967

when they both passed on the first upload   ??
https://salsa.debian.org/debian/strawberry/-/pipelines/382861


reprotest passes when I run it locally

=======================
Reproduction successful
=======================
No differences in ./../*.deb
359c55b631de43ae319787bbacfb65fccafee48be9799b402505e08301fbb763 ./../strawberry-dbgsym_1.0.4-1_amd64.deb
3483b1d23b0d0f6ede8f1219c132a3de45afc9a649ef1712d09cce7387f6ad51 ./../strawberry_1.0.4-1_amd64.deb



Cheers,
Peter

Hi Jeroen and others,

I'm sorry I won't have time to retry a new packaging upload for Strawberry. I already have a lot to fix on other packages I maintain and I miss time.

So anyone who want to go on on this is welcome.

If I remember there where no big issues or blocking points left but all the copyrights need to be well rechecked. For some files it's also a bit tricky because some headers have been rewritten since the fork of Clementine (it's forked from amarok...). So the main job left should be to ensure the copyright file is really completed and accurate.

And as I'm not a DD I can't help for sponsoring so you will have to find one.

Thomas

On rerun, crossbuild passes, but reprotest now fails with
|   failed with status 22|
|ERROR: Job failed: exit code 1


||22 -> 'Device disabled'   ????

|

Paul, Thomas, thanks for digging up the old reject.

Peter, I did some more digging and found one unresolved copyright
issue from my previous comment as well as some fresh ones:
* copyright holder Pascal Below for various scrobbler-related files
  is still missing
* copyright holder Nick Lanham missing for savedgroupingmanager.cpp;
  note that copyright for both .cpp and .h is shared with Jonas Kvinge
* missing entry for src/core/lazy.h
* as the info for src/dbus/*.xml doesn't seem to exist anywhere in
  the sources, please add a comment field to that paragraph
  explaining what the entry is based on
* the license paragraph for GPL-3 doesn't restrict the version to
  v3 only (other than the shortname suggesting so) and lacks a link
  to the full text of the license on debian systems


I noticed you changed the build to qt6 and while that works fine, it
does seem that at the moment qt5 is very much the standard qt release
in the distribution. A quick comparison using `reverse-depends -b
qt6-tools-dev` vs the same for qttools5-dev showed 6 packages
build-depending on the former and around 300 on the latter. Is there
any particular advantage to using qt6 for strawberry?


Don't worry too much about the reprotest, sometimes these things throw
a tantrum.

Hi Jeroen,

Updated copyright and reverted to Qt5

Looks like there's an error in the dbus copyright entry though, given
that the file linked in the comment is under LGPL rather than GPL?

And more importantly: the comment only deals with one of the xml
files, but (as far as I can tell) these do not all originate from a
single source. For example, org.kde.KGlobalAccel.*.xml files appear
in the kglobalaccel package; the Udisks stuff might well be based on
[2]; mpris/MediaPlayer2 in turn appears in [3].

I suspect some of the org.freedesktop.*.xml files on the other hand
could be based directly on dbus specs [1] or similar generic interface
definitions for use with qdbusxml2cpp, possibly autogenerated by
qdbus along the lines of:
`QT_SELECT=5 qdbus org.freedesktop.Notifications /org/freedesktop/Notifications org.freedesktop.DBus.Introspectable.Introspect`


Aside from the dbus xml stuff: what exactly do you mean by the comment
in the GPL-3 license paragraph?


[1]https://dbus.freedesktop.org/doc/dbus-specification.html
[2]https://salsa.debian.org/utopia-team/udisks2/-/blob/debian/master/data/org.freedesktop.UDisks2.xml
[3]https://sources.debian.org/src/mpdris2/0.9.1-1/src/mpDris2.in.py/#L133

Also, I found that the Qt6 version on Debian had very a small font, with no obvious way of enlarging it.

If Strawberry is accepted into Debian, I'd consider updating the package to build both Qt5 & Qt6 versions.
With a Strawberry-common, maybe the Qt5 & Qt6 binaries could co-install?
My bad. Fixed.
I've created separate Files paragraphs for each group of files.

(I'm not sure what the impact of auto-generation is on the copyright.
I notice that in [3], the author is the upstream application author,
no reference to org.freedesktop)
I'm confused regarding GPL-3 & GPL-3+. We now have a License paragraph for GPL-3 that excludes later versions,
but the full text in
     /usr/share/common-licenses/GPL-3
     https://www.gnu.org/licenses/gpl-3.0.html
both include it!

I'm wondering whether it would be best to have a single GPL-3 licence, or at least to remove the "(version 3 only)".
ISTM that failing to explicitly state in a source file header that a later version can be used, does not necessarily
mean that right is excluded,
especially when its explicitly included in the linked full text....

Anyway, up to you, I have just removed the comment for now.
Thanks for the links. I've put [1] into the copyright file.


Cheers,
Peter

Uploaded to Salsa & Mentors.

Please change d/watch and d/copyright's Source field to https://files.strawberrymusicplayer.org and check if the
jkvinge.net files that you used before match.

This is the location that the homepage points to.

Hi Bastian,

Would it be OK if I change the homepage instead?
It would then become https://github.com/strawberrymusicplayer/strawberry
the same as the Source: field in debian/copyright

(Arch & Fedora official packages all use the source from GitHub.)


I'll still check if the two tarballs are the same. That's a good idea!


Cheers,
Peter

Am 04.06.22 um 17:11 schrieb Peter B:

No, the Homepage in d/control should be the place where users (not developers) go and learn about the program.

The GitHub Releases tar.xz matches the files provided on the other download URLs.
https://github.com/strawberrymusicplayer/strawberry is ambiguous as source (it could also be taken from the GitHub
generated tar.gz on tags or a git export), so I would really prefer the official download URL.

Nonetheless, d/watch and Source both have to match where you actually got that file and it is now only by chance that
they point to the same tarball.

Thanks for your input.

Updated on Mentors & Salsa.

Hi Peter,

Thank you for working on packaging Strawberry for Debian!  I was the one
who requested Clementine database import functionality (upstream) some
time ago, and have been using MPD+Cantata while waiting for Strawberry
to mature.  Clementine memory usage (and crashes) forced me to give it
up, and I'm looking forward to trying out Strawberry some day :)

Also, thank you for your willingness to learn Debian standards and
rationale; a lot of this stuff is nonintuitive, but eventually the
rationale guiding the mentorship decisions will become second-nature.

Thank you for your mentorship Bastian!

Regards,
Nicholas

paragraph 14, conditional on that being explicitly specified by the
program in question; the other as part of an example or how one could
apply the terms of the GPL. That example is explicitly outside of the
terms and conditions set by the license. So yes, GPL-version-X-only is
very much a thing.

Typically, output produced by a program isn't itself covered by the
license of said program. Producing a text on a GPL'ed editor doesn't
put that text under the GPL.

For org.freedesktop.Notifications.xml and DBus.ObjectManager, I don't
see much of a reason to assume it originates from some other project;
so those could reasonably fall directly under the default copyright
paragraph for strawberry (i.e., no separate entry needed).

The org.freedesktop.UDisk2.*.xml files should have their own
(collective) entry, as the original for all of the udisk ones seems to
be [1] with David Zeuthen as the sole copyright holder.

Something similar applies to the org.mpris.*.xml (currently missing
from d/copyright), except here the upstream seems to be [2] (debian
copyright file at [3]).

The org.kde.KGlobalAccel.*.xml appear to be based on files in the
kglobalaccel project [4] (debian copyright at [5]) rather than the kde
documentation, please update the entry to reflect that.


Other than that, the shortname for the LGPL-2 license should have a
plus symbol appended to reflect the 'any later version' option, in
line with the standard shortnames documented by [6].


[1]https://github.com/storaged-project/udisks/blob/master/data/org.freedesktop.UDisks2.xml
[2]https://github.com/eonpatapon/mpDris2/blob/5e5cdacea6e55544064f8b10e0b49bbe2aa044d9/src/mpDris2.in.py#L115
[3]https://salsa.debian.org/debian/mpdris2/-/blob/debian/latest/debian/copyright
[4]https://sources.debian.org/src/kglobalaccel/5.94.0-1/src/
[5]https://tracker.debian.org/media/packages/k/kglobalaccel/copyright-5.94.0-1
[6]https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/#license-short-name

Thanks for the clarification
Dropped the separate paragraph.
Done
I've used the same as in [3] which matches [2] and included
https://github.com/eonpatapon/mpDris2
in the comment.
The copyright in [5] has no specific paragraph for *.xml
However, I found the files here
https://invent.kde.org/frameworks/kglobalaccel/-/tree/master/src

Again, they have no copyright themselves, but the associated cpp & h files do,
so I have used that, with a comment to the link.
Done

Updated on Salsa & Mentors,
many thanks for all your help.


Cheers,
Peter B