deepin.deepin-screenshot: init at 4.1.8

[romildo]

Motivation for this change

Add deepin-screenshot, a easy-to-use screenshot tool for Deepin Desktop Environment.

About packaging deepin for NixOS: #59023

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 nix-review --run "nix-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)
• Assured whether relevant documentation is up to date
• Fits CONTRIBUTING.md.

[romildo]

Notice that dde-file-manager may be needed. I have started packaging it, but has not finished it yet. I will make a PR for it too.

[worldofpeace]

Notice that dde-file-manager may be needed. I have started packaging it, but has not finished it yet. I will make a PR for it too.

👍 Looked like it was optional functionality

[worldofpeace]

There's /usr/bin/deepin-screenshot in the dbus service and I think we should hardcode
everything in the .desktop file because it wants to use deepin-turbo-invoker.

Exec=deepin-turbo-invoker --type=dtkwidget deepin-screenshot --icon

Note: the desktop file has multiple actions

[worldofpeace]

Also appears to fallback to xdg-open if it can't find the path to the dde-file-manager

• https://github.com/linuxdeepin/deepin-screenshot/blob/9f1df8504065bc1eaee8a4199d5372837c655000/src/mainwindow.cpp#L1012

I'm thinking about patching away it trying to use dde-file-manager completely.
I feel the behavior of xdg-open just makes more sense.

[romildo]

@GrahamcOfBorg build deepin-screenshot

[flokli]

@GrahamcOfBorg build deepin.deepin-screenshot

[flokli]

did you already figure out whether dde-file-manager is needed or not?
Maybe add a small comment here…

[worldofpeace]

See #59242 (comment)

[flokli]

Right…

It might make sense it opens in a deepin desktop environment to open with dde-file-manager, but not in general. A hardcoded /usr/bin/dde-file-manager is ugly, for sure ;-)

I'm not sure how this is handled for example in gnome-shell, to make sure folders are opened with nautilus. Is that also just an xdg-open thing?

If that's the case, I'd be inclined to solely rely on xdg-open for deepin-screenshot, and even send the patch upstream (given it should work with xdg-open for them, too)

[worldofpeace]

I'm not sure how this is handled for example in gnome-shell, to make sure folders are opened with nautilus. Is that also just an xdg-open thing?

It's just mime type associations. So the default file manager will be used, and in a deepin environment it would be dde-file-manager.

If that's the case, I'd be inclined to solely rely on xdg-open for deepin-screenshot, and even send the patch upstream (given it should work with xdg-open for them, too)

I'd say that's reasonable, though there's still room for improvement.

I'll put the patch here for now, and pr it to deepin 👍

[worldofpeace]

• patch here f3c5992
• deepin pr Only use xdg-open to launch file-manager martyr-deepin/deepin-screenshot#52

[romildo]

From the commit message:

deepin.deepin-screenshot: only use xdg-open

This doesn't hardcode or put it into PATH
as I wasn't sure what romildo would have preferred.

To have xdg-open in PATH is it enough to add it to the buildInputs list, or some sort of wrapping is also needed? If not needed, that is the easier choice, in my opinion.

[romildo]

From the commit message:

deepin.deepin-screenshot: only use xdg-open

This doesn't hardcode or put it into PATH
as I wasn't sure what romildo would have preferred.

To have xdg-open in PATH is it enough to add it to the buildInputs list, or some sort of wrapping is also needed? If not needed, that is the easier choice, in my opinion.

[worldofpeace]

Usually hardcoding, like you've been doing, is preferred.

@worldofpeace then it is my choice too. Can you do that, please?

[flokli]

Pushed another version that uses fetchpatch instead of adding the patch to nixpkgs, removed debug statements and hardcoded xdg-open. I don't really like we effectively patch twice now, but alas :-D

I looked into other places in nixpkgs on how it's done, it seems for GTK applications where wrapGappsHook is used, we just add it to the wrapper already present anyhow.

For QT application, we currently don't have a wrapper (but might get one soon: #54525), so I think hardcoding this for now is fine.

@worldofpeace good to merge?

[worldofpeace]

Just had to add

fixPath $out/bin/deepin-screenshot /usr/bin/deepin-screenshot src/dbusservice/com.deepin.Screenshot.service

Looks good, merging 👍

I looked into other places in nixpkgs on how it's done, it seems for GTK applications where wrapGappsHook is used, we just add it to the wrapper already present anyhow.

We just suffixes PATH with a wrapper if you don't want to hardcode

[romildo]

Pushed another version that [...] removed debug statements

I would keep them because they help a lot spotting FHS hard coded paths in the source. There is the possibility that some of them may have unseen, or not handled yet, or introduced in new versions.

[romildo]

Just had to add

fixPath $out/bin/deepin-screenshot /usr/bin/deepin-screenshot src/dbusservice/com.deepin.Screenshot.service

That should be:

    fixPath $out /usr/bin/deepin-screenshot src/dbusservice/com.deepin.Screenshot.service

Fixed in #59429