Navigation Menu

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

Jump to bottom

Fix thumbnails generation in nautilus #56188

Merged
merged 2 commits into from Feb 25, 2019
Merged

Fix thumbnails generation in nautilus #56188

merged 2 commits into from Feb 25, 2019

Conversation

layus
Copy link
Member

@layus layus commented Feb 22, 2019 •
edited

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
Copy link
Member

vcunat commented Feb 22, 2019 •
edited

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".

@GrahamcOfBorg GrahamcOfBorg added the 6.topic: GNOME GNOME desktop environment and its underlying platform label Feb 22, 2019
@layus
Copy link
Member 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
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
Copy link
Member 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
Copy link
Member

vcunat commented Feb 22, 2019 •
edited

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
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

hedning
hedning approved these changes Feb 23, 2019
View reviewed changes
Copy link
Contributor

@hedning hedning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@hedning
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 poc-fix-nautilus-thumbnails branch from 1ee5056 to a3e440c Compare February 25, 2019 09:43
@layus
Copy link
Member Author

layus commented Feb 25, 2019

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

@hedning
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
Copy link
Contributor

hedning commented Feb 25, 2019

@GrahamcOfBorg build gnome3.nautilus gnome3.gnome-desktop

@hedning
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
@layus
Copy link
Member Author

layus commented Feb 25, 2019

thanks !

@hedning
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
Reviewers

@hedning hedning hedning approved these changes

Assignees
No one assigned
Labels
6.topic: GNOME GNOME desktop environment and its underlying platform 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@layus @vcunat @jtojnar @hedning @GrahamcOfBorg