Packaging and License feedback

Mon Aug 20 21:54:17 BST 2018

Hello Aurélien,

On dimanche 19 août 2018 16:30:41 CEST Aurélien COUDERC wrote:
> Dear Elisa developers,
> 
> first a big thanks for your work on Elisa.
> I’ve been using it as my daily music player for some time now and I enjoy it
> a lot !

Thanks a lot for this.

> I’m currently packaging Elisa for Debian and you can find my work at [0].
> I have a couple of remarks and feedback from my packaging work you may be
> interested in.

Let me first say thanks you for taking care of debian packaging. I really 
appreciate that given I am a user of Debian for a long time.

> 1. License declaration issue
> The files created for Elisa are published under the "GNU Library General
> Public License as published by the Free Software Foundation; either version
> 3 of the License, or (at your option) any later version".
> Unfortunately this license doesn’t exist :) as it was renamed to GNU
> *Lesser* General Public license starting with version 2.1.
> 
> I see in the source history that at some point there was a s/2/3/ so I guess
> the intention was to move to LGPLv3. Still I think the source files require
> to be correct about the exact license to lift any doubt.
> 
> You may want to use the reference license notice from the GNU website [1]
> adding Lesser to the first sentence so that it reads "GNU Lesser General
> Public License".

You are correct. This is a mistake from me. Do you want to submit a patch for 
that to phabricator.kde.org ? If you cannot, I will do it myself as soon as I 
can.

> 2. Private library installed in common /usr/lib
> Elisa builds libelisaLib.so.0.1 and installs it directly into:
> /usr/lib/${multiarch_triplet}
> 
> For what I understand this library is intended for private use by the elisa
> binary only and thus should be installed in a subfolder like:
> /usr/lib/${multiarch_triplet}/elisa
> 
> You’ll find a patch with this change at [2].

Could you submit your patch to phabricator such that we can integrate it 
properly in the Elisa repository ?
We depend on this for our workflow.

> 3. Failing tests in 0.2.1
> I’m currently packaging the latest release 0.2.1 and after some iterations
> on the build configuration, still have the following couple of tests
> failing. I know it may be related to the way I did the packaging but the
> these 2 remaining seem like « real » failures to me so I’m requesting your
> assistance or comments here.
> 
>  8/15 Test  #6: mediaplaylistTest ................***Failed    1.27 sec
> ********* Start testing of MediaPlayListTest *********
> Config: Using QtTest library 5.11.1, Qt 5.11.1 (x86_64-little_endian-lp64
> shared (dynamic) release build; by GCC 8.1.0)
> …
> QWARN  : MediaPlayListTest::randomPlayList()
> QSqlDatabasePrivate::addDatabase: duplicate connection name
> 'testDbDirectContent', old connection removed. QDEBUG :
> MediaPlayListTest::randomPlayList() database open
> QDEBUG : MediaPlayListTest::randomPlayList() DatabaseInterface::init yes
> FAIL!  : MediaPlayListTest::randomPlayList() Compared values are not the
> same Loc:
> [/home/coucouf/hack/debian/kde/elisa.salsa/autotests/mediaplaylisttest.cpp(6
> 130)] QWARN  : MediaPlayListTest::randomAndContinuePlayList()
> QSqlDatabasePrivate::addDatabase: duplicate connection name
> 'testDbDirectContent', old connection removed.
> QDEBUG : MediaPlayListTest::randomAndContinuePlayList() database open
> QDEBUG : MediaPlayListTest::randomAndContinuePlayList()
> DatabaseInterface::init yes FAIL!  :
> MediaPlayListTest::randomAndContinuePlayList() Compared values are not the
> same
>    Loc:
> [/home/coucouf/hack/debian/kde/elisa.salsa/autotests/mediaplaylisttest.cpp(6
> 262)] …
> QWARN  : MediaPlayListTest::testSaveAndRestoreSettings()
> QSqlDatabasePrivate::addDatabase: duplicate connection name
> 'testDbDirectContent', old connection removed.
> QDEBUG : MediaPlayListTest::testSaveAndRestoreSettings() database open
> QDEBUG : MediaPlayListTest::testSaveAndRestoreSettings()
> DatabaseInterface::init yes
> FAIL!  : MediaPlayListTest::testSaveAndRestoreSettings() Compared values are
> not the same
>    Loc:
> [/home/coucouf/hack/debian/kde/elisa.salsa/autotests/mediaplaylisttest.cpp(6
> 620)] Totals: 47 passed, 3 failed, 0 skipped, 0 blacklisted, 1227ms
> ********* Finished testing of MediaPlayListTest *********
> 
> 13/15 Test  #5: manageaudioplayerTest ............***Failed    4.46 sec
> ********* Start testing of ManageAudioPlayerTest *********
> Config: Using QtTest library 5.11.1, Qt 5.11.1 (x86_64-little_endian-lp64
> shared (dynamic) release build; by GCC 8.1.0)
> …
> FAIL!  : ManageAudioPlayerTest::skipNextTrackWithRandomPlay() Compared
> values are not the same
>    Actual   (playerSourceChangedSpy.count()): 1
>    Expected (2)                             : 2
>    Loc:
> [/home/coucouf/hack/debian/kde/elisa.salsa/autotests/manageaudioplayertest.c
> pp(1490)] …
> Totals: 20 passed, 1 failed, 0 skipped, 0 blacklisted, 4396ms
> ********* Finished testing of ManageAudioPlayerTest *********

I will take care of those failing tests given that they are locally failing on 
my development laptop. They are not failing on the CI infrastructure from KDE. 
This may be related to the Qt 5.11 version.

> 4. Incorrect repo URL on the Elisa community page
> The Elisa page on the KDE Community website [3] refers to an outdated or
> incorrect repository, the URL if found to be correct is at [4].

I will have a look. The preferred address to pick the git repository is 
https://phabricator.kde.org/source/elisa/ . I will fix the wiki but if you 
have time, it is also very easy for you to fix it.

> Also if you have any feedback on the packaging for things that are incorrect
> or should be improved you’re very much welcome to contact me, or even
> propose Merge Requests in our GitLab repository at [0].
> 
> I’m not subscribed to this list due to the Google clickwall required for
> subscription so you may want to copy me in your responses.
> 
> 
> [0] https://salsa.debian.org/qt-kde-team/extras/elisa
> [1] https://www.gnu.org/licenses/gpl-howto.en.html
> [2]
> https://salsa.debian.org/qt-kde-team/extras/elisa/blob/master/debian/patches
> /move-private-lib-to-subfolder [3] https://community.kde.org/Elisa
> [4] https://cgit.kde.org/elisa.git/
> 
> 
> Thanks and cheers !
> --
> Aurélien

Thanks for your work. This is very appreciated. If you have more feedback, 
feel free to report any bug to the KDE bugtracker.

Best regards

--
Matthieu Gallien