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.

Sign up for GitHub

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

zathura: symlinkJoin all plugins #49141

Closed
wants to merge 1 commit into from
Closed

zathura: symlinkJoin all plugins #49141

wants to merge 1 commit into from

Conversation

teto
Copy link
Member

@teto teto commented Oct 26, 2018 •
edited

Motivation for this change

Zathura is not registered as a pdf reader, for instance nautilus doesn't propose it as a pdf reader.
It's because the default .desktop file doesn't contain any MimeType, these are provided by the plugins see https://git.pwmt.org/pwmt/zathura/issues/45#note_416.

Instead of copying the .desktop entries, I relied on the already present symlinkJoin.

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.

symphorien
symphorien reviewed Oct 26, 2018
View reviewed changes
pkgs/applications/misc/zathura/wrapper.nix

buildInputs = [ makeWrapper ];

postBuild = ''
wrapProgram $out/bin/zathura --add-flags --plugins-dir=${pluginsPath}
wrapProgram $out/bin/zathura --add-flags --plugins-dir="''${out}/lib/zathura"
Copy link
Member

Choose a reason for hiding this comment

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

I don't know anything about zathura, but is the wrapper still needed now that plugins are in an "usual" location ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to remove it but without it, zathura tries to load plugins from zathura-core store, while symlinkJoin creates a new folder zathura-with-plugins.

@GrahamcOfBorg
Copy link

Timed out, unknown build status on x86_64-linux (full log)

Attempted: zathura

Partial log (click to expand)

make[3]: Entering directory '/build/rustc-1.29.1-src/build/x86_64-unknown-linux-gnu/llvm/build'
[ 71%] Building CXX object lib/Target/ARM/Utils/CMakeFiles/LLVMARMUtils.dir/ARMBaseInfo.cpp.o
[ 71%] Linking CXX static library ../../../libLLVMARMUtils.a
building of '/nix/store/6c39p0vv61147bvjkirr78m3smm5qv3w-rustc-1.29.1.drv' timed out after 1800 seconds
cannot build derivation '/nix/store/d2wgz3ka33z5fhlwy1s7ddib94mhx7ms-cargo-1.29.1.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/drlif3f2j65p0yzs64mq3ajvgc1qp5s2-librsvg-2.42.4.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/gd9lsz6v4jgwdr8l1hjzrzd2h05a4ini-djvulibre-3.5.27.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/5bnxbhyki8kzi9bmzz91n8a8203hc2zn-zathura-djvu-0.2.8.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/yk1479rh9d8xva4fccgxfxczjarsf8yl-zathura-with-plugins-0.4.1.drv': 1 dependencies couldn't be built
error: build of '/nix/store/yk1479rh9d8xva4fccgxfxczjarsf8yl-zathura-with-plugins-0.4.1.drv' failed

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: zathura

Partial log (click to expand)

glibPreFixupPhase
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/q84q4fsg62k1psccdyvl5mvl30bcyj1g-zathura-djvu-0.2.8
shrinking /nix/store/q84q4fsg62k1psccdyvl5mvl30bcyj1g-zathura-djvu-0.2.8/lib/zathura/libdjvu.so
strip is /nix/store/p9akxn2sfy4wkhqdqa3li97pc6jaz3r1-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/q84q4fsg62k1psccdyvl5mvl30bcyj1g-zathura-djvu-0.2.8/lib
patching script interpreter paths in /nix/store/q84q4fsg62k1psccdyvl5mvl30bcyj1g-zathura-djvu-0.2.8
checking for references to /build in /nix/store/q84q4fsg62k1psccdyvl5mvl30bcyj1g-zathura-djvu-0.2.8...
building '/nix/store/dm2g71dq29n3grhbp9mk9jsxwyir7l5j-zathura-with-plugins-0.4.1.drv'...
/nix/store/7c2ww9diw9rbxswj1p4z15f6qg7niblg-zathura-with-plugins-0.4.1

@teto teto changed the title zathura: symlinkJoin all of plugins zathura: symlinkJoin all plugins Oct 26, 2018
ryantm
ryantm requested changes Feb 26, 2019
View reviewed changes
Copy link
Member

@ryantm ryantm left a comment

Choose a reason for hiding this comment

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

Please resolve merge conflicts

@wmertens
Copy link
Contributor

wmertens commented Nov 4, 2019

Triage: Is this PR still relevant?

@teto
Copy link
Member Author

teto commented Nov 4, 2019

without it, I don't see the zathura desktop. I can't solve the conflict for now but will do.

@doronbehar
Copy link
Contributor

without it, I don't see the zathura desktop. I can't solve the conflict for now but will do.

I see zathura in nautilus:

$ gio mime application/pdf
Default application for “application/pdf”: org.pwmt.zathura.desktop
Registered applications:
	krop.desktop
	firefox.desktop
	chromium-browser.desktop
	com.github.xournalpp.xournalpp.desktop
	pdfsam-basic.desktop
Recommended applications:
	krop.desktop
	firefox.desktop
	chromium-browser.desktop
	com.github.xournalpp.xournalpp.desktop
	pdfsam-basic.desktop

Though it is possible this is because I've once added:

[Default Applications]
application/pdf=org.pwmt.zathura.desktop

To ~/.config/mimeapps.list.

@TethysSvensson TethysSvensson mentioned this pull request Aug 29, 2020
Merged
10 tasks
@TethysSvensson
Copy link
Contributor

I believe this PR is still relevant, as zathura will still not contain any .desktop files registering itself as a pdf viewer unless something like this is implemented.

I have rebased this PR in #96630, but I can close the other PR if @teto wants revive this one instead.

@teto
Copy link
Member Author

teto commented Aug 29, 2020

no problem ! really happy to see someone else take a shot at this. I've carried the patch in my fork since then but wondered if it was still relavant. Apparently yes :)

@teto teto closed this Aug 29, 2020
@teto teto deleted the zathura branch September 5, 2021 22:56
@teto teto restored the zathura branch September 5, 2021 22:56
@teto teto deleted the zathura branch September 5, 2021 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@symphorien symphorien symphorien left review comments

@ryantm ryantm ryantm requested changes

Assignees
No one assigned
Labels
2.status: merge conflict 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@teto @GrahamcOfBorg @wmertens @doronbehar @TethysSvensson @ryantm @symphorien