D28677: Make PipeWire, GBM and Epoxy libs as required dependencies

[Jan Grulich]

This revision was automatically updated to reflect the committed changes.
Closed by commit R838:b8af0f136865: Make PipeWire, GBM and Epoxy libs as 
required dependencies (authored by jgrulich).

REPOSITORY
  R838 Flatpak Support: KDE Portal for XDG Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28677?vs=80001=80002

REVISION DETAIL
  https://phabricator.kde.org/D28677

AFFECTED FILES
  CMakeLists.txt
  data/CMakeLists.txt
  data/kde-no-pipewire.portal
  src/CMakeLists.txt
  src/desktopportal.cpp
  src/desktopportal.h
  src/waylandintegration.cpp
  src/waylandintegration.h
  src/waylandintegration_p.h

To: jgrulich, #plasma, apol
Cc: arfrever, apol, asturmlechner, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, ahiemstra, mart

D28677: Make PipeWire, GBM and Epoxy libs as required dependencies

[Jan Grulich]

jgrulich updated this revision to Diff 80001.
jgrulich marked 2 inline comments as done.
jgrulich added a comment.


  - Better naming for the cmake option disabling PipeWire

REPOSITORY
  R838 Flatpak Support: KDE Portal for XDG Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28677?vs=79948=80001

BRANCH
  pipewire-required

REVISION DETAIL
  https://phabricator.kde.org/D28677

AFFECTED FILES
  CMakeLists.txt
  data/CMakeLists.txt
  data/kde-no-pipewire.portal
  src/CMakeLists.txt
  src/desktopportal.cpp
  src/desktopportal.h
  src/waylandintegration.cpp
  src/waylandintegration.h
  src/waylandintegration_p.h

To: jgrulich, #plasma, apol
Cc: arfrever, apol, asturmlechner, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, ahiemstra, mart

D28677: Make PipeWire, GBM and Epoxy libs as required dependencies

[Arfrever Frehtes Taifersar Arahesis]

arfrever added inline comments.

INLINE COMMENTS

> CMakeLists.txt:23
>  
> -find_package(PipeWire)
> -set_package_properties(PipeWire PROPERTIES
> -TYPE OPTIONAL
> -PURPOSE "Required for screencast portal"
> -)
> -
> -find_package(GBM)
> -set_package_properties(GBM PROPERTIES
> -TYPE OPTIONAL
> -PURPOSE "Required for screencast portal"
> -)
> -
> -find_package(Epoxy)
> -set_package_properties(Epoxy PROPERTIES DESCRIPTION "libepoxy"
> -URL "https://github.com/anholt/libepoxy;
> -TYPE OPTIONAL
> -PURPOSE "Required for screencast portal"
> -)
> -
> -if (PipeWire_FOUND AND GBM_FOUND AND Epoxy_FOUND)
> -set (SCREENCAST_ENABLED true)
> +option(DISABLE_PIPEWIRE_SUPPORT "Disable PipeWire support. PipeWire is 
> needed for screen sharing and remote desktop" OFF)
> +if(DISABLE_PIPEWIRE_SUPPORT)

Could this option be renamed to positively sounding option enabled by default? 
(And word `SUPPORT` seems redundant.)

  option(ENABLE_PIPEWIRE "Enable PipeWire support. PipeWire is needed for 
screen sharing and remote desktop" ON)

REPOSITORY
  R838 Flatpak Support: KDE Portal for XDG Desktop

BRANCH
  pipewire-required

REVISION DETAIL
  https://phabricator.kde.org/D28677

To: jgrulich, #plasma, apol
Cc: arfrever, apol, asturmlechner, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, ahiemstra, mart

D28677: Make PipeWire, GBM and Epoxy libs as required dependencies

[Aleix Pol Gonzalez]

apol accepted this revision.
apol added a comment.
This revision is now accepted and ready to land.


  CMake bits look good.
  
  It's a bit of a shame that this whole complexity needs to be added, but oh 
well.

REPOSITORY
  R838 Flatpak Support: KDE Portal for XDG Desktop

BRANCH
  pipewire-required

REVISION DETAIL
  https://phabricator.kde.org/D28677

To: jgrulich, #plasma, apol
Cc: apol, asturmlechner, davidedmundson, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, 
ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
ahiemstra, mart

D28677: Make PipeWire, GBM and Epoxy libs as required dependencies

[Jan Grulich]

jgrulich marked 2 inline comments as done.
jgrulich added inline comments.

INLINE COMMENTS

> asturmlechner wrote in CMakeLists.txt:23-24
> maybe make it a proper cmake option?

I made it a cmake option. I have never used it before so please check if it's 
correct.

> asturmlechner wrote in CMakeLists.txt:51-52
> This is odd, is it a leftover or did you forget to add an actual KF5 
> dependency here?

It was a leftover, removed now.

REPOSITORY
  R838 Flatpak Support: KDE Portal for XDG Desktop

REVISION DETAIL
  https://phabricator.kde.org/D28677

To: jgrulich, #plasma
Cc: asturmlechner, davidedmundson, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, 
ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D28677: Make PipeWire, GBM and Epoxy libs as required dependencies

[Jan Grulich]

jgrulich updated this revision to Diff 79948.
jgrulich added a comment.


  Make DISABLE_PIPEWIRE_SUPPORT a cmake option and remove a leftover from 
previous change

REPOSITORY
  R838 Flatpak Support: KDE Portal for XDG Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28677?vs=79684=79948

BRANCH
  pipewire-required

REVISION DETAIL
  https://phabricator.kde.org/D28677

AFFECTED FILES
  CMakeLists.txt
  data/CMakeLists.txt
  data/kde-no-pipewire.portal
  src/CMakeLists.txt
  src/desktopportal.cpp
  src/desktopportal.h
  src/waylandintegration.cpp
  src/waylandintegration.h
  src/waylandintegration_p.h

To: jgrulich, #plasma
Cc: asturmlechner, davidedmundson, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, 
ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D28677: Make PipeWire, GBM and Epoxy libs as required dependencies

[Andreas Sturmlechner]

asturmlechner added a comment.


  Thanks for keeping pipewire optional, this is in line with how we have been 
packaging xdg-desktop-portal-kde in Gentoo so far, we had just been 
conditionally disabling the cmake finding of these 3 libraries to do it, and 
will switch over to using the new option then.

INLINE COMMENTS

> CMakeLists.txt:23-24
>  
> -find_package(PipeWire)
> -set_package_properties(PipeWire PROPERTIES
> -TYPE OPTIONAL
> -PURPOSE "Required for screencast portal"
> -)
> -
> -find_package(GBM)
> -set_package_properties(GBM PROPERTIES
> -TYPE OPTIONAL
> -PURPOSE "Required for screencast portal"
> -)
> -
> -find_package(Epoxy)
> -set_package_properties(Epoxy PROPERTIES DESCRIPTION "libepoxy"
> -URL "https://github.com/anholt/libepoxy;
> -TYPE OPTIONAL
> -PURPOSE "Required for screencast portal"
> -)
> -
> -if (PipeWire_FOUND AND GBM_FOUND AND Epoxy_FOUND)
> -set (SCREENCAST_ENABLED true)
> +if(DISABLE_PIPEWIRE_SUPPORT)
> +message(STATUS "Disabling PipeWire support")
> +set(HAVE_PIPEWIRE_SUPPORT 0)

maybe make it a proper cmake option?

> CMakeLists.txt:51-52
> +
> +find_package(KF5 ${KF5_MIN_VERSION} REQUIRED
> +)
>  endif()

This is odd, is it a leftover or did you forget to add an actual KF5 dependency 
here?

REPOSITORY
  R838 Flatpak Support: KDE Portal for XDG Desktop

REVISION DETAIL
  https://phabricator.kde.org/D28677

To: jgrulich, #plasma
Cc: asturmlechner, davidedmundson, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, 
ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D28677: Make PipeWire, GBM and Epoxy libs as required dependencies

[Jan Grulich]

jgrulich added a comment.


  In D28677#644196 , @davidedmundson 
wrote:
  
  > > Also allow to completely disable Wayland support in case some 
distribution don't ship PipeWire yet.
  >
  > I don't follow, why don't we just disable the screencasting thing if we 
don't have pipewire. I don't see why we would affect the rest.
  
  
  At the end it didn't hurt to make it this way :). It should now build with 
Wayland support for the background portal, but with DISABLE_PIPEWIRE_SUPPORT, 
it will drop everything relevant to PipeWire, screen sharing and remote desktop.

REPOSITORY
  R838 Flatpak Support: KDE Portal for XDG Desktop

REVISION DETAIL
  https://phabricator.kde.org/D28677

To: jgrulich, #plasma
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28677: Make PipeWire, GBM and Epoxy libs as required dependencies

[Jan Grulich]

jgrulich updated this revision to Diff 79684.
jgrulich edited the summary of this revision.
jgrulich added a comment.


  Allow to disable only PipeWire, while keeping rest of Wayland support

REPOSITORY
  R838 Flatpak Support: KDE Portal for XDG Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28677?vs=79632=79684

BRANCH
  pipewire-required

REVISION DETAIL
  https://phabricator.kde.org/D28677

AFFECTED FILES
  CMakeLists.txt
  data/CMakeLists.txt
  data/kde-no-pipewire.portal
  src/CMakeLists.txt
  src/desktopportal.cpp
  src/desktopportal.h
  src/waylandintegration.cpp
  src/waylandintegration.h
  src/waylandintegration_p.h

To: jgrulich, #plasma
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28677: Make PipeWire, GBM and Epoxy libs as required dependencies

[Jan Grulich]

jgrulich added a comment.


  I asked on kde-distribution-packagers, so far I got only response from Slack 
and the response was they don't care about Wayland, if they would, they would 
have included PipeWire. I think it should be either don't support Wayland at 
all if distribution doesn't care or support it fully and let them get PipeWire 
included.

REPOSITORY
  R838 Flatpak Support: KDE Portal for XDG Desktop

REVISION DETAIL
  https://phabricator.kde.org/D28677

To: jgrulich, #plasma
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28677: Make PipeWire, GBM and Epoxy libs as required dependencies

[Jan Grulich]

jgrulich added a comment.


  In D28677#644196 , @davidedmundson 
wrote:
  
  > > Also allow to completely disable Wayland support in case some 
distribution don't ship PipeWire yet.
  >
  > I don't follow, why don't we just disable the screencasting thing if we 
don't have pipewire. I don't see why we would affect the rest.
  
  
  I wanted to avoid adding tons of ifdefs to the wayland integration part, 
which I currently use only for screencasting and background portal. Before it 
was used only for screencasting, which was fine and I just avoided building it, 
but it will be used more and more.

REPOSITORY
  R838 Flatpak Support: KDE Portal for XDG Desktop

REVISION DETAIL
  https://phabricator.kde.org/D28677

To: jgrulich, #plasma
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28677: Make PipeWire, GBM and Epoxy libs as required dependencies

[David Edmundson]

davidedmundson added a comment.


  > Also allow to completely disable Wayland support in case some distribution 
don't ship PipeWire yet.
  
  I don't follow, why don't we just disable the screencasting thing if we don't 
have pipewire. I don't see why we would affect the rest.

REPOSITORY
  R838 Flatpak Support: KDE Portal for XDG Desktop

REVISION DETAIL
  https://phabricator.kde.org/D28677

To: jgrulich, #plasma
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28677: Make PipeWire, GBM and Epoxy libs as required dependencies

[Jan Grulich]

jgrulich created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
jgrulich requested review of this revision.

REVISION SUMMARY
  Also allow to completely disable Wayland support in case some distribution 
don't ship PipeWire yet

REPOSITORY
  R838 Flatpak Support: KDE Portal for XDG Desktop

BRANCH
  pipewire-required

REVISION DETAIL
  https://phabricator.kde.org/D28677

AFFECTED FILES
  CMakeLists.txt
  src/CMakeLists.txt
  src/desktopportal.cpp
  src/desktopportal.h
  src/waylandintegration.cpp

To: jgrulich
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart