Bug 386659

Summary: plasma-desktop/kcms/phonon/ broken: undefined reference to `pa_stream_is_suspended'
Product: [Frameworks and Libraries] extra-cmake-modules Reporter: Johannes Hirte <johannes.hirte>
Component: generalAssignee: ecm-bugs-null <ecm-bugs-null>
Status: RESOLVED FIXED    
Severity: normal CC: mvourlakos, myriam, rikmills, simonandric5
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Gentoo Packages   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:
Attachments: fix build with FindPulseAudio.cmake from extra-cmake-modules
Renamed compat vars

Description Johannes Hirte 2017-11-08 19:59:10 UTC
Created attachment 108753 [details]
fix build with FindPulseAudio.cmake from extra-cmake-modules

Not sure if phonon is the right component here, cause it's plasma-desktop that fails. Linking of kcms/phonon/kcm_phonon.so fails, because pulseaudio is not linked in. This is caused by changes in extra-cmake-modules:

commits ee5b036c1df4776a258c0d8067fd2eaf18875829 and 7574022825804db2274bba992d6fc4a4817c1536 changed PULSEAUDIO_LIBRARY and PULSEAUDIO_MAINLOOP_LIBRARY into PulseAudio_LIBRARIES and PulseAudio_MAINLOOP_LIBRARY. Attached patch fixes this for kcm_phonon.
Comment 1 Luca Beltrame 2017-11-09 06:45:39 UTC
Git commit 5db70d0e6017a127aa05cd6c7cc20552a54d5a5f by Luca Beltrame, on behalf of Johannes Hirte.
Committed on 09/11/2017 at 06:44.
Pushed by lbeltrame into branch 'master'.

Fix build with extra-cmake-modules since commit
 7574022825804db2274bba992d6fc4a4817c1536

commits ee5b036c1df4776a258c0d8067fd2eaf18875829 and
7574022825804db2274bba992d6fc4a4817c1536 changed PULSEAUDIO_LIBRARY and
PULSEAUDIO_MAINLOOP_LIBRARY into PulseAudio_LIBRARIES and
PulseAudio_MAINLOOP_LIBRARY.

M  +1    -1    kcms/phonon/CMakeLists.txt

https://commits.kde.org/plasma-desktop/5db70d0e6017a127aa05cd6c7cc20552a54d5a5f
Comment 2 Luca Beltrame 2017-11-09 06:46:39 UTC
FYI, I think kmix needs a similar change as well.
Comment 3 Harald Sitter 2017-11-09 11:08:25 UTC
For future reference please use phabricator to post patches https://community.kde.org/Get_Involved/development#Submitting_your_first_patch

Luca, I do wonder if this is correct.
The way I see it the new FindPulseAudio in ECM is breaking source compatibility with the FindPulseAudio from KDELibs4Support (which I think is the one we used here). I'd imagine the way to fix this should be to make the ECM one backwards compatible with existing code, no?
Comment 4 Rik Mills 2017-11-11 12:57:34 UTC
5.11 branch still fails to build because of this issue:

https://build.neon.kde.org/job/xenial_stable_plasma_plasma-desktop/

https://kci.pangea.pub/job/bionic_stable_plasma-desktop/
Comment 5 Michail Vourlakos 2017-11-11 22:16:41 UTC
in Neon Stable edition plasma-desktop cant be built yet...

reverting commmit: https://cgit.kde.org/plasma-desktop.git/commit/?id=5db70d0e6017a127aa05cd6c7cc20552a54d5a5f

fixes the build...

shouldnt this be reviewed first before commiting through phabricator?
Comment 6 Rik Mills 2017-11-12 18:25:41 UTC
(In reply to Michail Vourlakos from comment #5)
> in Neon Stable edition plasma-desktop cant be built yet...
> 
> reverting commmit:
> https://cgit.kde.org/plasma-desktop.git/commit/
> ?id=5db70d0e6017a127aa05cd6c7cc20552a54d5a5f

Backport to 5.11 branch I assume you mean? Reverting that commit in master would fix nothing, unless accompanied by a change in ECM to fix more generally.

I also note that plasma-pa: 

https://build.neon.kde.org/job/xenial_stable_plasma_plasma-pa_bin_amd64/100/consoleFull
https://build.kde.org/job/Plasma%20plasma-pa%20kf5-qt5%20SUSEQt5.9/19/consoleFull

also now fails to build in a similar manner.
Comment 7 Michail Vourlakos 2017-11-12 18:39:29 UTC
(In reply to Rik Mills from comment #6)
> (In reply to Michail Vourlakos from comment #5)
> > in Neon Stable edition plasma-desktop cant be built yet...
> > 
> > reverting commmit:
> > https://cgit.kde.org/plasma-desktop.git/commit/
> > ?id=5db70d0e6017a127aa05cd6c7cc20552a54d5a5f
> 

what I mean is that I reverted that specific commit in a local branch in my system and plasma-desktop [master] was built correctly. I think that having a broken master for that long is top priority and must be fixed as soonish...

Also I think that this specific commit didnt pass a review through phabricator so this increases also a bit the frustration.
but I will just wait for the top plasma developers to get into the discussion for this.

I am just a developer reporting a big issue, not a plasma maintainer.
Comment 8 Johannes Hirte 2017-11-12 18:48:17 UTC
(In reply to Michail Vourlakos from comment #7)
> (In reply to Rik Mills from comment #6)
> > (In reply to Michail Vourlakos from comment #5)
> > > in Neon Stable edition plasma-desktop cant be built yet...
> > > 
> > > reverting commmit:
> > > https://cgit.kde.org/plasma-desktop.git/commit/
> > > ?id=5db70d0e6017a127aa05cd6c7cc20552a54d5a5f
> > 
> 
> what I mean is that I reverted that specific commit in a local branch in my
> system and plasma-desktop [master] was built correctly. I think that having
> a broken master for that long is top priority and must be fixed as soonish...

In this case your extra-cmake-modules isn't up-to-date. With current git master this patch is needed for building plasma-desktop. Or extra-cmake-modules get changed to the old behaviour, like Harald has proposed.
Comment 9 Michail Vourlakos 2017-11-12 18:59:44 UTC
(In reply to Johannes Hirte from comment #8)
> (In reply to Michail Vourlakos from comment #7)
> In this case your extra-cmake-modules isn't up-to-date. With current git
> master this patch is needed for building plasma-desktop.

correct. it wasnt a up-to-date git issue. I had to forcefully remove the build directory of
extra-cmake-modules and rebuild afterwards. Now plasma-desktop builds correctly...

thanks
Comment 10 Rik Mills 2017-11-12 19:32:55 UTC
As a packager, +1 for fixing in ECM in a compatible way if possible. 

If not, we are going to have stable plasma 5.11 that won't build with new FW 5.41 when it's released, or if the fix is backported, that won't then build with a previous version.
Comment 11 Christophe Marin 2017-11-12 20:42:39 UTC
Created attachment 108812 [details]
Renamed compat vars

We can rename the backward compatibility variables in FindPulseAudio.cmake that will be shipped with ECM 5.41.

What you need is :
- PULSEAUDIO_FOUND
- PULSEAUDIO_INCLUDE_DIR
- PULSEAUDIO_LIBRARY
- PULSEAUDIO_MAINLOOP_LIBRARY

The attached patch should make everyone happy.
Comment 12 Johannes Hirte 2017-11-12 21:00:35 UTC
see https://phabricator.kde.org/D8777
Comment 13 Christophe Marin 2017-11-14 08:01:50 UTC
Fixed with https://commits.kde.org/extra-cmake-modules/c02178fa3