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

Fix thumbnails generation in nautilus #56188

Merged
merged 2 commits into from Feb 25, 2019

Conversation

@layus
Copy link
Contributor

layus commented Feb 22, 2019

Motivation for this change

PDF thumbnails (and possibly others) are not generated in nautilus
because the thumbnailers execution is sandboxed.
/run/current-system is not mounted in the sandbox, and system binaries
that rely on $PATH are not resolvable.

It turns out that .desktop files sometimes contain an absolute path, but
I think it better to just mount /run/current-system in the sandbox
rather than enforcing all the .desktop files to contain an absolute
path, especially since some (like firefox) break when an absolute path
is used there.

$ rg -L Exec= /run/current-system/sw/share/thumbnailers                     
/run/current-system/sw/share/thumbnailers/gnome-font-viewer.thumbnailer
2:TryExec=gnome-thumbnail-font
3:Exec=gnome-thumbnail-font --size %s %u %o

/run/current-system/sw/share/thumbnailers/evince.thumbnailer
2:TryExec=evince-thumbnailer
3:Exec=evince-thumbnailer -s %s %u %o

/run/current-system/sw/share/thumbnailers/totem.thumbnailer
2:TryExec=/nix/store/rdiavbwkm520l12c3shc0rjbxaqd8glz-totem-3.30.0/bin/totem-video-thumbnailer
3:Exec=/nix/store/rdiavbwkm520l12c3shc0rjbxaqd8glz-totem-3.30.0/bin/totem-video-thumbnailer -s %s %u %o
Things done

@vcunat

This comment has been minimized.

Copy link
Member

vcunat commented Feb 22, 2019

Instead of /run/current-system it would be nicer to have e.g. a buildEnv of the required tools or something like that, unless the set of tools is "hard to determine".

@layus

This comment has been minimized.

Copy link
Contributor Author

layus commented Feb 22, 2019

This would add a dependency from nautilus to all the potential thumbnailers. I quite like the flexibility in here. But /run/current-system/bin may be a better pick. In vanilla nautilus they mount

"--ro-bind", "/usr", "/usr",
"--ro-bind", "/lib", "/lib",
"--ro-bind", "/lib64", "/lib64",
"--symlink", "usr/bin", "/bin",
@vcunat

This comment has been minimized.

Copy link
Member

vcunat commented Feb 22, 2019

Ah, OK. If I understand it right, the best approach might be to build this tiny tool separately make it depend on the set of thumbnailers, but that would probably be too complex and I don't see a significant issue in /run/current-system/bin; the worst are probably around restrictions of direct running without installation.

@layus

This comment has been minimized.

Copy link
Contributor Author

layus commented Feb 22, 2019

Now, given that the sandbox aims to protect about programs writing in strange locations when they only need to write one output file (like nix sandboxes), it does not hurt to provide access to the whole /run/current-system as it is a symlink to the store and the whole store is already mounted.

@vcunat

This comment has been minimized.

Copy link
Member

vcunat commented Feb 22, 2019

Yes, if that's the (main) purpose of this sandbox. I just thought you also need the thumbnailers to reside in the sandbox, and you won't have them there (in /run/current-system) unless you "install" them (i.e. presence in /nix/store isn't enough).

@jtojnar

This comment has been minimized.

Copy link
Contributor

jtojnar commented Feb 22, 2019

We might want to do the same in gnome-desktop package since this is it vendored. cc @hedning

Copy link
Contributor

hedning left a comment

Looks sensible to me at least (as @jtojnar says we should do the same for gnome-desktop).

@hedning

This comment has been minimized.

Copy link
Contributor

hedning commented Feb 24, 2019

Ah, sorry, should've mentioned that we just merged an update to gnome-desktop where the thumbnailer code changed.

@layus layus force-pushed the layus:poc-fix-nautilus-thumbnails branch from 1ee5056 to a3e440c Feb 25, 2019
@layus

This comment has been minimized.

Copy link
Contributor Author

layus commented Feb 25, 2019

@hedning This was a mess, so I rebased my changes.

@hedning

This comment has been minimized.

Copy link
Contributor

hedning commented Feb 25, 2019

Cheers, I'll merge after ofborg is done so we'll get this into 19.03

@hedning

This comment has been minimized.

Copy link
Contributor

hedning commented Feb 25, 2019

@GrahamcOfBorg build gnome3.nautilus gnome3.gnome-desktop

@hedning

This comment has been minimized.

Copy link
Contributor

hedning commented Feb 25, 2019

Tested nautilus and it produces pdf thumbnails. Couldn't get gnome-documents to work, but I think that's an issue with the gnome-desktop version bump (eog also fails to produce thumbnails on master now), I'll look into it.

@hedning hedning merged commit c3694e1 into NixOS:master Feb 25, 2019
13 checks passed
13 checks passed
gnome3.gnome-desktop, gnome3.nautilus on x86_64-darwin No attempt
Details
gnome3.gnome-desktop, gnome3.nautilus on aarch64-linux Success
Details
gnome3.gnome-desktop, gnome3.nautilus on x86_64-linux Success
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@layus

This comment has been minimized.

Copy link
Contributor Author

layus commented Feb 25, 2019

thanks !

@hedning

This comment has been minimized.

Copy link
Contributor

hedning commented Feb 25, 2019

Thanks for tracking down and fixing the issue :)

Solved the gnome-desktop problem in #56346

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

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.