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

plasma/xfce/enlightenment: improve detection of application mime types for apps that use GIO (firefox) #44866

Closed
wants to merge 4 commits into from

Conversation

michaelpj
Copy link
Contributor

Motivation for this change

One the trail of #44849 (related #11355), I discovered that apps that use GIO use a slightly different (and possibly not standards compliant) way of finding mime types for applications.

One thing that came up was the presence of share/applications/mimeinfo.cache, and lo and behold, this is not present using my DE.

A bit of further tracking revealed that we do populate this using update-desktop-database, but only if it happens to be present on the path (see https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/config/system-path.nix#L147).

Adding this to environment.systemPackages for plasma results in firefox now having file associations! They're still wrong (it opens pdfs in evince rather than okular, which is what xdg-open says it should use), but that's for another day.

I added desktop-file-utils to the DEs that appear to be doing the same thing with shared-mime-info - I wasn't brave enough to touch the others.

Separately, I'd like to talk about whether system-path.nix is doing the right thing. It feels quite fragile to just be relying on the presence of these executables, and I don't know what would be lost by always running them (and referring to the packages directly). There's also the issue that mimeinfo.cache is missing from my profile's share/applications, and I'm not sure what should be responsible for building that.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@michaelpj
Copy link
Contributor Author

@vcunat perhaps, based on your comments on the issues?

@jtojnar
Copy link
Contributor

jtojnar commented Aug 12, 2018

Looks fine. In the long run we should probably have freedesktop module that should handle this and be depended on by the DEs.

Separately, I'd like to talk about whether system-path.nix is doing the right thing. It feels quite fragile to just be relying on the presence of these executables, and I don't know what would be lost by always running them (and referring to the packages directly).

I guess the late binding is used to prevent introducing those utilities on servers/more lightweight systems.

@vcunat
Copy link
Member

vcunat commented Aug 13, 2018

NixOS, i.e. systemPackages

There's that ugly fragile hack, and we might do the signalling with some internal option instead of relying on the presence of the executables, though I can't see much advantage in that switch. If we always ran the generation, we might be pulling those utilities into build-time closure even where it's not desired (headless containers etc.), though that could most likely be approximated by the existing environment.noXlibs or some similar general option.

User profiles, i.e. nix-env

Here we don't have even that hack AFAIK (yet), and looking at NixOS options certainly aren't an option. I expect the current hack is easy to copy to nix's env-builder, but there probably wouldn't be anything automatically putting those executables into the env (and injecting them from nix itself sounds like a bad idea).

@michaelpj
Copy link
Contributor Author

Apart from the future work discussion, I think this is good to go?

@michaelpj michaelpj mentioned this pull request Aug 15, 2018
9 tasks
@michaelpj
Copy link
Contributor Author

Superseded by #45058.

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

Successfully merging this pull request may close these issues.

None yet

4 participants