flathub / org.darktable.Darktable

https://flathub.org/apps/details/org.darktable.Darktable
4 stars 14 forks source link

Host programms not runnable from flatpak darktable #86

Open hcw70 opened 2 years ago

hcw70 commented 2 years ago

Main reason for me to use lua scripts is to be able to

However, when trying any of these lua scripts, they are blocked by not able to find the associated executable in the flatpak sandbox, however the host apps are installed and work flawlessly.

Recently i came across a solution based on

https://github.com/flathub/org.gnu.emacs/issues/14

where the app can be started by flatpak-run. This now works for me for HDRMerge, since i modified the file HDRMerge.lua like this:

local function BuildExecuteCmd(prog_table) --creates a program command using elements of the passed in program table
  local result = 'flatpak-spawn --host '..CleanSpaces(prog_table.bin)..' '..CleanSpaces(prog_table.arg_string)..' '..CleanSpaces(prog_table.images_string)
  return result
end

and added the dbus permission using flatseal to darktable.

This works perfectly, but needs to be done for every script which calls a host executable.

However it would be impractical to modify all scripts to also work in flatpak also.

Proposal

Extend function 'dtutils.system.external_command() to prepend the string "'flatpak-spawn --host " to the exec command when running in flatpak environments.

Or add a new command "external_host_command()" which has these semantics, when the former should not be modified....

hcw70 commented 1 year ago

Anyone from the flatpacker / darktable team had a look at this?

hcw70 commented 1 year ago

Maybe @turbogit ?

This would help alot to let most of the currently non-functional flatpak plugins be fixed...

paperdigits commented 1 year ago

The lua plugins are their own git repo, the community can fork them and do this work.

Part of the problem is that not everyone is running all flakpaks. Many have a mixed environment of flatpaks and non flatpaks, so just converting everything to call flatpaks or not won't solve it for all users.

On October 31, 2022 7:02:42 AM PDT, Hauke Wintjen @.***> wrote:

Maybe @turbogit ?

This would help alot to let most of the currently non-functional flatpak plugins be fixed...

-- Reply to this email directly or view it on GitHub: https://github.com/flathub/org.darktable.Darktable/issues/86#issuecomment-1297139904 You are receiving this because you are subscribed to this thread.

Message ID: @.***>

TurboGit commented 1 year ago

No sorry, I'm not the good guy for this because I don't really use Lua nor I do use flatpak.

rrenomeron commented 1 year ago

What's needed in the flatpak is to allow the trick that enables running an executable from the host from the sandboxed darktable. I've confirmed that if you run darktable like this:

flatpak run --talk-name=org.freedesktop.Flatpak org.darktable.Darktable 

And I update my lua script to run flatpak-spawn --host exiftool instead of exiftool, then I can run exiftool successfully in the script.

There's work to do on the lua side, yes, but that can't be done until the flatpak is updated to allow it. Since it (appears to be) a one-liner, I'll put together a quickie PR for more discussion.

hfiguiere commented 1 year ago

The linter has spoken anyway.

hcw70 commented 11 months ago

@rrenomeron Thanks for your try, but unfortunately it was closed. So my problem remains unsolved. I have no concerns running host tools from within darktable flatpak, since i am using darktable flatpak to easier stay up to date.

But not letting it call host tools breaks most lua scripts.

And adding all these host tools into darktable flatpak is not an option IMHO, since that will make the flatpak bigger without need (since most people dont need all scripts working). (See https://github.com/flathub/org.darktable.Darktable/pull/98 where including exiftool increases the image by about 60MB).

paperdigits commented 11 months ago

You'd need to edit the lua scripts to use flatpak spawn, since there seems to be no appetite to package the lua scripts: https://man7.org/linux/man-pages/man1/flatpak-spawn.1.html

On August 14, 2023 2:26:34 AM PDT, Hauke Wintjen @.***> wrote:

@rrenomeron Thanks for your try, but unfortunately it was closed. So my problem remains unsolved. I have no concerns running host tools from within darktable flatpak, since i am using darktable flatpak to easier stay up to date.

But not letting it call host tools breaks most lua scripts.

And adding all these host tools into darktable flatpak is not an option IMHO, since that will make the flatpak bigger without need (since most people dont need all scripts working). (See https://github.com/flathub/org.darktable.Darktable/pull/98 where including exiftool increases the image by about 60MB).

-- Reply to this email directly or view it on GitHub: https://github.com/flathub/org.darktable.Darktable/issues/86#issuecomment-1676989968 You are receiving this because you commented.

Message ID: @.***>

hcw70 commented 11 months ago

You'd need to edit the lua scripts to use flatpak spawn, since there seems to be no appetite to package the lua scripts: https://man7.org/linux/man-pages/man1/flatpak-spawn.1.html

Yes, that is exactly what i did (at least for the scripts what i use). But to make life not harder as needed for others, providing a solution by the flatpackers is what is needed IMHO.

paperdigits commented 11 months ago

That sounds like an excellent contribution you could make!

On August 15, 2023 5:22:54 AM PDT, Hauke Wintjen @.***> wrote:

You'd need to edit the lua scripts to use flatpak spawn, since there seems to be no appetite to package the lua scripts: https://man7.org/linux/man-pages/man1/flatpak-spawn.1.html

Yes, that is exactly what i did (at least for the scripts what i use). But to make life not harder as needed for others, providing a solution by the flatpackers is what is needed IMHO.

-- Reply to this email directly or view it on GitHub: https://github.com/flathub/org.darktable.Darktable/issues/86#issuecomment-1678843600 You are receiving this because you commented.

Message ID: @.***>

hcw70 commented 11 months ago

@paperdigits But what is needed is an extension in darktable, or the lua api of that, to detect if we are in a flatpak. As stated in my proposal. I am unable to add that extension, so this issue was raised...

paperdigits commented 11 months ago

You don't need that at all. You can package the lua scripts and include a patch that modifies the code as necessary.

On August 15, 2023 7:35:42 AM PDT, Hauke Wintjen @.***> wrote:

@paperdigits But what is needed is an extension in darktable, or the lua api of that, to detect if we are in a flatpak. As stated in my proposal. I am unable to add that extension, so this issue was raised...

-- Reply to this email directly or view it on GitHub: https://github.com/flathub/org.darktable.Darktable/issues/86#issuecomment-1679037880 You are receiving this because you were mentioned.

Message ID: @.***>

hcw70 commented 8 months ago

You don't need that at all. You can package the lua scripts and include a patch that modifies the code as necessary.

But if i modify the lua scripts to include the flatpak-spawn command, that will break on non-flatpak builds (debian packages, windows, MacOs ...). So my proposal (see above) was to include the handling into 'dtutils.system.external_command()' (see above) which may detect if it is inside a flatpak, then do flatpak-spawn, else do a exec().

So all scripts would be runnable without modifications on all platforms, and there is a single-place of implementation for this.

Otherwise (as it is currently) all lua scripts which exec an external command will fail in flatpak.