Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved portability #467

Merged
merged 7 commits into from
Mar 9, 2021
Merged

Improved portability #467

merged 7 commits into from
Mar 9, 2021

Conversation

Hvassaa
Copy link
Contributor

@Hvassaa Hvassaa commented Mar 5, 2021

I tried to improve portability by removing hardcoded paths and configure files in CMake where needed.

This makes it possible to build and use on non-FHS distros such as NixOS (discussion here).

@lassebramer
Copy link

nice

installation/touchegg.service.in Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
@JoseExposito
Copy link
Owner

Hi @Hvassaa and @jtojnar

Thanks a lot for improving this, it is an area that definitely needed some attention.

I just discovered NixOS, but I guess applications get installed into its own prefix? Pretty cool concept by the way, is like using a Docker container for your desktop.

I need to review the changes with more detail once they are ready, but so far there are 2 thinks that are going to be problematic:

The first one is the service file. In Ubuntu 18 (and I believe this is the case for Debian) there is no /usr/lib/systemd/system folder, that's why it was hardcoded to /lib/systemd/system.

I have no idea about how NixOS works, so I might be asking something stupid, but, where is Touchégg default config (/usr/share/touchegg/touchegg.conf) going to be installed in NixOS?
Both Touché and GNOME X11 Gestures extension use that file to check that Touchégg is installed (and show a download dialog if not) and as default config.
So, if they get installed in different prefixes, they will fail 😕

@jtojnar
Copy link

jtojnar commented Mar 6, 2021

The first one is the service file. In Ubuntu 18 (and I believe this is the case for Debian) there is no /usr/lib/systemd/system folder, that's why it was hardcoded to /lib/systemd/system.

Good point, this one is more complicated thanks to the different rootprefixdir between Debian-based systems and other distros. The usual method is either getting the location using pkg-config --variable systemdsystemunitdir systemd (which will allow us to override it using PKG_CONFIG_SYSTEMD_SYSTEMDSYSTEMUNITDIR environment variable) or using a CMake option:

set(SYSTEMD_SYSTEM_UNIT_DIR "/lib/systemd/system" CACHE STRING "Path to systemd system unit dir")

I have no idea about how NixOS works, so I might be asking something stupid, but, where is Touchégg default config (/usr/share/touchegg/touchegg.conf) going to be installed in NixOS?

It would get installed to something like /nix/store/23cxwwlw7djwnrnscknc8qi2ppb02qrl-touchegg/share/touchegg/touchegg.conf making it unsuitable for detecting if touchegg is installed.

Would it be possible instead to detect if the systemd service is available? E.g. by checking the return code of systemctl cat touchegg.service as suggested here?

@JoseExposito
Copy link
Owner

The usual method is either getting the location using pkg-config --variable systemdsystemunitdir systemd

Cool, I didn't know about that. I will test it to make sure it works on Ubuntu/Fedora/Arch and if so we can go with this option. It is cleaner that adding an extra config flag and way cleaner than the current hardcoded value.

Would it be possible instead to detect if the systemd service is available? E.g. by checking the return code of systemctl cat touchegg.service as suggested here?

I don't think I can do that from a Flatpak container. Also, I need to read the file to use it as default configuration. Some distros patch it to ship their own gestures, so I can't fallback to upstream config.

Anyway, this is already broken in Touché/X11 Gestures on NixOS, so it shouldn't be an stopper for this pull request since every officially supported package installs on the /usr prefix.

I need to find a way to get the path of the default config file in the other 2 projects 🤔 Would it be possible to create a symbolic link on NixOS in /usr as part of the installation process?

@jtojnar
Copy link

jtojnar commented Mar 6, 2021

I need to find a way to get the path of the default config file in the other 2 projects thinking Would it be possible to create a symbolic link on NixOS in /usr as part of the installation process?

We do not have /usr, the best we can do is put it to /etc.

@jtojnar
Copy link

jtojnar commented Mar 6, 2021

I don't think I can do that from a Flatpak container. Also, I need to read the file to use it as default configuration. Some distros patch it to ship their own gestures, so I can't fallback to upstream config.

How does reading the config file work in Flatpak container? Or does it have full file system access?

Though, I would expect that at least a presence of a DBus service could be detected. For example, Piper seems to pass this to Flatpak:

https://github.com/libratbag/piper/blob/7cc0b61983c1026188d4a06630135e4fb101ca27/org.freedesktop.Piper.json#L15

I need to find a way to get the path of the default config file in the other 2 projects thinking Would it be possible to create a symbolic link on NixOS in /usr as part of the installation process?

Alternatives would be

  • having touchegg.pc from which the consumers could get the default config file path at build time
  • querying config values from touchegg binary/library/over a DBus

@JoseExposito
Copy link
Owner

How does reading the config file work in Flatpak container? Or does it have full file system access?

Yes, it has read access to system files.

having touchegg.pc from which the consumers could get the default config file path at build time

I don't think this will work building for Flathub, as it is executed in their servers in a clean environment... But it is definitely not going to work with the GNOME Shell extension, as it is not even built, it is just a zip file with JavaScript that gets extracted in a directory.

querying config values from touchegg binary/library/over a DBus

I already export a D-Bus interface over Unix socket, so I think this is proper way of doing it. However, there are some cons:

  • A config GUI requesting network permissions is kind of suspicious
  • It is not backwards compatible, so I'll have to keep the filesystem=host:ro permission and support both methods in code
  • I have to implement it in 3 places (yes, I know, lazy)

We do not have /usr, the best we can do is put it to /etc.

This might be the simplest solution and I don't find any cons. It doesn't require any extra Flatpak permissions and a very little change in code.
We can go with this solution. I'll let you know when the changes in Touché/X11 Gestures are ready.

CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
src/utils/paths.cpp Outdated Show resolved Hide resolved
@jtojnar
Copy link

jtojnar commented Mar 6, 2021

Yes, it has read access to system files.

https://github.com/JoseExposito/touche/blob/c927a2ec32fb1da1d79e03793973a30cccac0312/com.github.joseexposito.touche.yml#L11

having touchegg.pc from which the consumers could get the default config file path at build time

I don't think this will work building for Flathub, as it is executed in their servers in a clean environment... But it is definitely not going to work with the GNOME Shell extension, as it is not even built, it is just a zip file with JavaScript that gets extracted in a directory.

Yeah, it would have to be built and installed in the flatpak manifest, which sounds like too much work for this.

querying config values from touchegg binary/library/over a DBus

I already export a D-Bus interface over Unix socket, so I think this is proper way of doing it. However, there are some cons:

Is there any benefit to using separate unix socket over the system bus? Either way, the presence of the socket could be used instead of availability of DBus service for checking if the service is available.

* A config GUI requesting network permissions is kind of suspicious

I do not see why it would need any different permission than it already uses for the communication with the socket.

* It is not backwards compatible, so I'll have to keep the `filesystem=host:ro` permission and support both methods in code

Yeah, that sounds annoying.

* I have to implement it in 3 places (yes, I know, lazy)

It could allow centralizing the decisions in the service, which could end up with less code after the transitory period. But yeah, it would be an extra work.

We do not have /usr, the best we can do is put it to /etc.

This might be the simplest solution and I don't find any cons. It doesn't require any extra Flatpak permissions and a very little change in code.
We can go with this solution. I'll let you know when the changes in Touché/X11 Gestures are ready.

A disadvantage is that it would somewhat distort the usual semantics. Typically, $prefix/share is used for default config, /etc is used for config by machine administrator and ~/.config for user config. If we use /etc for the default config, machine administrators would not have place to put their config, or NixOS would have to have a code to merge the machine administrator code with the default one.

We could also add configure/CMake flag to touché so we could adjust the default config location when building the package, and for the extension, just patch the code. That would leave /etc free. But if the programs use the presence of the config file as proxy for the presence of the dbus service they would get false positive when e.g. the extension is globally installed but not touchegg – installing the extension would pull touchegg into the Nix store so the config file would exist but it would not register the systemd service in systemd.

CMakeLists.txt Show resolved Hide resolved
@JoseExposito
Copy link
Owner

We could also add configure/CMake flag to touché so we could adjust the default config location when building the package, and for the extension, just patch the code

Here you go JoseExposito/touche#15
You can adjust this value with -Dsystem-config-file-path=.... The Flathub package will still fail though, as it is built with a different path.

But if the programs use the presence of the config file as proxy for the presence of the dbus service they would get false positive [...]

Not really, Touché doesn't use the D-Bus service at all, it just needs a system level config file to fallback when the config in home is not present.

@Hvassaa the PR look solid, I'm going to generate and test packages for a couple of distros and I'll let you know how it goes 😄

installation/touchegg.service.in Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@Hvassaa
Copy link
Contributor Author

Hvassaa commented Mar 8, 2021

@Hvassaa the PR look solid, I'm going to generate and test packages for a couple of distros and I'll let you know how it goes smile

Sounds great! Crossing my fingers 😎

@JoseExposito
Copy link
Owner

Thanks a lot @jtojnar and @Hvassaa for your work on this PR 🚀

It is working like a charm. Let me know when you finish packaging Touchégg for NixOS so I can add the install instructions to the README.md file.

@JoseExposito JoseExposito merged commit ffcd644 into JoseExposito:master Mar 9, 2021
@Hvassaa
Copy link
Contributor Author

Hvassaa commented Mar 10, 2021

It is working like a charm. Let me know when you finish packaging Touchégg for NixOS so I can add the install instructions to the README.md file.

Perfect, good to hear. And yes, I will let you know 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants