flathub / net.pcsx2.PCSX2

https://flathub.org/apps/details/net.pcsx2.PCSX2
12 stars 11 forks source link

pcsx2-flatpak no longer compiles #16

Closed Shoegzer closed 5 years ago

Shoegzer commented 5 years ago

pcsx2-flatpak builds after this upstream commit fail to compile.

$ flatpak-builder builddir --arch=i386 --default-branch=stable --force-clean --install-deps-from=flathub net.pcsx2.PCSX2.json --user --install
...
CMake Error at cmake/SelectPcsx2Plugins.cmake:21 (message):
Skip build of dev9ghzdrk: missing dependencies:check these libraries ->
gtk2, pcap, libxml2
Call Stack (most recent call first):
cmake/SelectPcsx2Plugins.cmake:130 (print_dep)
CMakeLists.txt:28 (include)

-- Configuring incomplete, errors occurred!
See also "/run/build/pcsx2/CMakeFiles/CMakeOutput.log".
See also "/run/build/pcsx2/CMakeFiles/CMakeError.log".
Error: module pcsx2: Child process exited with code 1

CMakeOutput.log CMakeError.log

Following a suggestion from @TingPing we attempted to turn off the dev9ghzdrk plugin by adding "-Ddev9ghzdrk=OFF", to the pcsx2 config-opts section of the net.pcsx2.PCSX2.json file, but the same errors are returned.

@arcum42 can you provide any thoughts or suggestions here? Thanks.

Test system is uisng linux kernel 4.18 x64 with gcc-9.1.0.

TingPing commented 5 years ago

Hmm, print_dep() is meant to be fatal in package mode, but I don't see a way to disable that feature.

Shoegzer commented 5 years ago

@TingPing thanks for the fix. Although, shouldn't there be a better way than not compiling the dev9 plugin? It's not a terrible essential feature right now, but it could be in the future. Then again this is likely to be more an upstream issue to fix.

TingPing commented 5 years ago

I don't know what that plugin does but looking at the build system it can never work in the sandbox as it requires elevated permissions.

TingPing commented 5 years ago

So the upstream issue is just that there wasn't a build flag to disable this plugin. (Or maybe I missed it)

Shoegzer commented 5 years ago

Interesting. Not sure why it would require elevated perms but perhaps that's an issue that can be fixed upstream. Also, I would think ideally it should be left enabled and the deps issue fixed upstream, though yes, at the very least there should be an option to disable. @arcum42 your thoughts here?

TingPing commented 5 years ago

Not sure why it would require elevated perms but perhaps that's an issue that can be fixed upstream.

It requires the capability of caputring network traffic: sudo setcap 'CAP_NET_RAW+eip CAP_NET_ADMIN+eip

I presume for debugging which isn't relevant for us.

Also, I would think ideally it should be left enabled and the deps issue fixed upstream

That makes no sense. Its a feature that doesn't work and there is no deps issue.

arcum42 commented 5 years ago

Most of the linux side of that plugin was done by @lowlyocean back in #2586, so he'd know more about what can be done about the permissions.

Most of what the commit referenced did was to make it print dependency messages if it didn't compile (which I didn't realise would cause packaging to fail, as when compiling normally, it just skips the plugin), and to skip the setcap command if not building the plugin.

Unfortunately, that plugin does fail to open the network adapters without it.

I had thought about moving it to extra plugins, which aren't compiled by packages by default. I could certainly add a build option to disable dev9ghzdrk. Unless you are going to be trying to play ps2 games online with pcsx2, the plugin isn't really needed.

lowlyocean commented 5 years ago

The plugin is for network gameplay and it does work- the elevated capability for capturing network traffic is not for debugging. The original error in this thread is not a bug- your build machine needs gtk2, lxml2, and pcap libraries to build the plugin. The Debian package is configured to add those libs and only require elevation to setcap when end-user is installing, not during compilation. Can't the same be done in this packaging system?

TingPing commented 5 years ago

Flatpak is a sandboxed format, it can never elevate permissions. The fact that is required for a feature is clearly not ideal... Those capabilities are historically used by development or maybe administrative tools, regular user software should in no way use them.

lowlyocean commented 5 years ago

I was curious if Wireshark (regular user software) was offered through Flatpak as it would seem to run into the same trouble. Sure enough https://github.com/flathub/org.wireshark.Wireshark/issues/4

TingPing commented 5 years ago

Indeed. Wireshark obviously has a valid reason to capture all network traffic on a system at least.

Shoegzer commented 5 years ago

Update: this recent upstream commit adds the option not to build the plugin. Can such an option be added here as well vs. ignoring the compile-time errors?

TingPing commented 5 years ago

Update: this recent upstream commit adds the option not to build the plugin. Can such an option be added here as well vs. ignoring the compile-time errors?

Done, thanks!