Packaging review for the Elisa music player

[Aurélien COUDERC]

Le 15/01/2019 à 09:08, Pino Toscano a écrit :
> In data lunedì 14 gennaio 2019 17:07:20 CET, Aurélien COUDERC ha scritto:
>> Hi Pino,
>>
>> thanks for the additional review.
> 
> Thanks for the fixes!
> 
>> If nothing more pops up I’ll push it through NEW.
> 
> I tried a build of elisa, and found more things to fix:
> 
> - the dh_auto_clean override removes .qmlc files in the sources
> directory; I see none of them in the sources, are they generated at
> build time, outside of the build directory? If not, then most probably
> this extra work for dh_auto_clean is not needed

They are generated at build time, so for now I would keep the override:

Untracked files:
  (use "git add <file>..." to include in what will be committed)

        autotests/qmltests/tst_GridBrowserDelegate.qmlc
        autotests/qmltests/tst_NavigationActionBar.qmlc
        autotests/qmltests/tst_PlayListEntry.qmlc
        debian/.debhelper/
        debian/.rules.swp
        debian/debhelper-build-stamp
        debian/elisa.substvars
        debian/elisa/
        debian/files
        obj-x86_64-linux-gnu/
        src/qml/GridBrowserDelegate.qmlc
        src/qml/LabelWithToolTip.qmlc
        src/qml/MediaTrackMetadataView.qmlc
        src/qml/NavigationActionBar.qmlc
        src/qml/PlayListEntry.qmlc
        src/qml/RatingStar.qmlc


> - the .so symlink of the elisalib library is shipped in the binary
> package; as the library is private, just drop the .so file -- see for
> example the dh_auto_install override in krita

It’s fixed upstream here:
https://phabricator.kde.org/differential/changeset/?ref=352877

So I’ll leave it like that for now and wait for the upstream change to
reach us.


> - xvfb and xauth are not specified as build dependencies; without them,
> the xvfb-run invocation in the dh_auto_test override does not work
> (see below though)

Right, added.


> - the syntax used in the dh_auto_test override (i.e. "cmd1; cmd2; cmd3")
> basically ignores the failures of the commands, currently the fact that
> xvfb-run is missing; one possible idea is to use a simple directory for
> XDG_RUNTIME_DIRS like debian/tests.home, cleaning it before and after
> the real dh_auto_test run -- see the dh_auto_test override in tellico,
> for example

OK fixed.
I had done that so the build would reach 100% even with some failing
tests but it was supposed to be temporary.

> - other than xvfb and xauth, I had to add more build dependencies to
> make the some of the tests work: libqt5sql5-sqlite, qml-module-qttest,
> qml-module-qtqml-models2, qml-module-qtquick-layouts,
> qml-module-qtquick-controls, qml-module-qtgraphicaleffects
> 
> - with all the fixes above, there are still 3 tests failing for me:
> mediaplaylistTest, localfilelistingtest, elisaqmltests
> 
> - as effect of making some of the tests run, I noticed that most
> probably libqt5sql5-sqlite and qml-module-qtgraphicaleffects are
> missing as runtime dependencies

I’ve done a little more dependency poking and I’m down to only 2 tests
failing : mediaplaylistTest and localfilelistingtest.
Strangely enough they do pass with a local debuild and not in sbuild but
I couldn’t find ou what could be missing and if it’s configuration or
dependencies related. Advice welcome.
I just disabled the 2 failing tests for now.

Also I’ve added a couple of runtime dependencies to ensure it works when
installed on top of a default Gnome install.
Some icons are missing but the music detection and playback is functional.


I’ll upload now as time is running out. :)


Cheers,
--
Aurélien