gmodena / nix-flatpak

Install flatpaks declaratively
Apache License 2.0
276 stars 10 forks source link

Add overrides management #24

Closed Tomaszal closed 7 months ago

Tomaszal commented 9 months ago

This PR builds on top of PR #23, as it relies on a similar strategy to separately manage the declarative and imperative states. If/when that PR is merged I'll rebase this PR to only include the relevant commit.

This wasn't a trivial task and was mainly accomplished using jq, the logic of which has been separated into the modules/overrides.jq file. The installer simply provides the needed values to it by looping over previously and currently managed overrides, then reading their active (the actual Flatpak overrides files), old and new states (the files introduced in #23) from the respective files.

The actual logic to determine a value for an overrides entry is quite simple:

Tomaszal commented 9 months ago

Here's an example usage of this from my configs:

{
  services.flatpak.overrides = {
    global = {
      # Force Wayland by default
      Context.sockets = ["wayland" "!x11" "!fallback-x11"];

      Environment = {
        # Fix un-themed cursor in some Wayland apps
        XCURSOR_PATH = "/run/host/user-share/icons:/run/host/share/icons";

        # Force correct theme for some GTK apps
        GTK_THEME = "Adwaita:dark";
      };
    };

    "com.visualstudio.code".Context = {
      filesystems = [
        "xdg-config/git:ro" # Expose user Git config
        "/run/current-system/sw/bin:ro" # Expose NixOS managed software
      ];
      sockets = [
        "gpg-agent" # Expose GPG agent
        "pcsc" # Expose smart cards (i.e. YubiKey)
      ];
    };

    "org.onlyoffice.desktopeditors".Context.sockets = ["x11"]; # No Wayland support
  };
}
gmodena commented 8 months ago

Thanks for another terrific piece of work @Tomaszal !

This PR builds on top of PR https://github.com/gmodena/nix-flatpak/pull/23, as it relies on a similar strategy to separately manage the declarative and imperative states. If/when that PR is merged I'll rebase this PR to only include the relevant commit.

Ack.

nikp123 commented 8 months ago

Technical question: Is the overrides directory supposed to be created on its own? Because I manually wiped /var/lib/flatpak before starting the module (to test how it'd handle a clean system). The result was that flatpak-managed-install.service fails to start without the directory present. Creating it fixes the problem.

Tomaszal commented 8 months ago

Technical question: Is the overrides directory supposed to be created on its own? Because I manually wiped /var/lib/flatpak before starting the module (to test how it'd handle a clean system). The result was that flatpak-managed-install.service fails to start without the directory present. Creating it fixes the problem.

Good catch! Yes, it should create the directory if it doesn't exist, I'll fix that.

Tomaszal commented 8 months ago

@gmodena in the spirit of https://github.com/gmodena/nix-flatpak/pull/23#issuecomment-1858771501, should another option be exposed that will do a similar thing with overrides? F.e. an option called removeUnmanagedOverrides. It would be simple to implement, as it could just delete the overrides folder and re-create it on activation, making sure only the overrides defined in the config are present. I personally don't really see why this would be needed, but it would be consistent with uninstallUnmanagedPackages.

gmodena commented 8 months ago

Hey @Tomaszal

I personally don't really see why this would be needed, but it would be consistent with uninstallUnmanagedPackages.

I like the idea but I share the quoted sentiment. I don't see an obvious use case for removeUnmanagedOverrides. I'd rather we leave the option out for now, and add it might the need arise. It's easier to add APIs than to deprecate them IMHO.

Tomaszal commented 7 months ago

I've rebased this onto the latest changes in #23 which now include some small changes that were previously here. I also added an mkdir command that should fix the issue mentioned in https://github.com/gmodena/nix-flatpak/pull/24#issuecomment-1866227059

Tomaszal commented 7 months ago

@gmodena I've resolved the merge conflict with main, you should be able to merge now.

I realise now that there's probably a bug similar to the one you fixed in #28 with this, where on new installations OLD_STATE is empty. However, we can't fix it with the same simple check here, as if it's empty we still need to generate the overrides file with the overrides from the new state. I think the best fix for this (and for uninstalls too) is to initialize the OLD_STATE to a correct structure instead of simply {}. I can submit that fix, but I think it's better as a separate PR, since it affects both this and the uninstalls.

gmodena commented 7 months ago

Hey @Tomaszal

I think the best fix for this (and for uninstalls too) is to initialize the OLD_STATE to a correct structure instead of simply {}. I can submit that fix, but I think it's better as a separate PR, since it affects both this and the uninstalls.

Agree. +1 to do this in a dedicated PR.