darktable-org / lua-scripts

157 stars 111 forks source link

shell command "which" does not work on macOS #93

Closed wejot closed 6 years ago

wejot commented 7 years ago

Hello, I'm trying to run gimp.lua on a Mac with macOS 10.12.5. After some fiddling with the placement of the different lua scripts, I came to a point where 'check_if_bin_exists' fails:

[add_job] 0 | lua: destroy storage param | queue: 4 | priority: 0 [run_job+] 04 207527788,441767 lua: destroy storage param | queue: 4 | priority: 0LUA ERROR : /Users/walter/.config/darktable/lua/gimp.lua:238: attempt to call field 'check_if_bin_exists' (a nil value) stack traceback: [C]: in function 'check_if_bin_exists' /Users/walter/.config/darktable/lua/gimp.lua:238: in function </Users/walter/.config/darktable/lua/gimp.lua:237>

[run_job-] 04 207527788,441807 [run_job-] 06 207527788,441810 lua: destroy storage param | queue: 4 | priority: 0export | queue: 3 | priority: 0

[add_job] 0 | lua: destroy storage param | queue: 4 | priority: 0 [run_job+] 06 207527788,441889 lua: destroy storage param | queue: 4 | priority: 0 [run_job-] 06 207527788,441894 lua: destroy storage param | queue: 4 | priority: 0

Looking into 'check_if_bin_exists', I find 'which' is used to check the presence of the Gimp binary. 'which' will only find binaries in the current path. However, Gimp (like all 'official' applications) is installed as an 'Application Package', and will not appear in the traditional paths.

Best regards, Walter Jansen

boucman commented 7 years ago

usually check_if_bin_exist(bin) is called to make sur we can call "bin" after that...

if gimp is not in the PATH, is it even possible to launch gimp from the command line ? how can we find/check if bin is installed ?

houz commented 7 years ago

From C we could use https://developer.apple.com/documentation/coreservices/1449588-lsfindapplicationforinfo?preferredLanguage=occ – I would assume that something is possible from the shell, too?

wejot commented 7 years ago

Hello, I have digged around a bit and found the "open" shell command. The following worked, for Gimp as well as Affinity Photo: ~ walter$ open /Applications/gimp.app and: ~ walter$ open /Applications/"affinity photo.app"

Of course, this does not help to fix the not working "which" command.

Best regards, Walter

wejot commented 7 years ago

Sorry, closed by accident... Walter

wejot commented 7 years ago

Once more... After reading the "open" man page, opening a picture works, too: ~ walter$ open /Applications/"affinity photo.app" --args /full/path/to/picture/picture123.jpg Affinity Photo starts and loads the pic.

wejot commented 7 years ago

And another one... Please have a look at "mdfind". Maybe this can replace "which" on macOS?

  ~ walter$ mdfind -onlyin /Applications gimp
  /Applications/GIMP.app

Have a nice weekend, Walter

boucman commented 7 years ago

This still means that we can't just use os.execute to run/detect an application in general. We need to always wrap these calls in order to treat them specifically for macos.

That's quite a bummer, i'm not quite sure how to deal with that cleanly in our lib/

wejot commented 7 years ago

Well, it seems that, at some point, you would have to tell a Linux from a macOS. I believe this could be done with the uname shell command. Trying uname on the LUA command line, it says

> os.execute("uname ")
Darwin
true    exit    0

On my Xubuntu system, os.execute("uname ") returns "Linux".

Now, you could have an if clause in _check_if_binexists() to match the OS you are running on. (I have no idea of LUA coding, so no example here).

In the macOS (Darwin) branch, you could use mdfind (here on the LUA command line):

> os.execute("mdfind -onlyin /Applications " .. "gimp")
/Applications/GIMP.app
true    exit    0
> 
> os.execute("mdfind -onlyin /Applications " .. "not-gimp")
true    exit    0
> 

Using the -count parameter, the result would be even more consistent:

> os.execute("mdfind -count -onlyin /Applications " .. "gimp")
1
true    exit    0
> os.execute("mdfind -count -onlyin /Applications " .. "not-gimp")
0
true    exit    0

Just my ideas...

supertobi commented 7 years ago

We should integrate something like this;

https://gist.github.com/soulik/82e9d02a818ce12498d1

Patches are welcome.

houz commented 7 years ago

Folks, please. We should make use of the fact that we know the darktable developers and just kindly ask them for a flag in the darktable object that tells what system it is. There is really no need to have lengthy Lua code for that.

boucman commented 7 years ago

+1 @houz

there is already an API in lua to export config constants from C, that's how API versionning works

i'm not sure how to find out the os at compile time, but if someone give me a hint, i'll add the API

houz commented 7 years ago

This might be a way (totally untested quasi pseudo code):

const char *foo =
#if defined _WIN32
  "WINDOWS"
#elif defined __linux__
  "LINUX"
#elif defined __APPLE__
  "APPLE"
#elif defined __FreeBSD__
...
#endif
;

I am sure there are better ways though.

Edit: Maybe passing ${CMAKE_SYSTEM_NAME} via src/config.cmake.h would be better, but I am not completely sure how that behaves for cross compiles.

wejot commented 7 years ago

Of course, a more general approach would make much more sense. Thanks for your work so far, and I'll wait until results show up. Regards, Walter

supertobi commented 7 years ago

@boucman addedd darktable.configuration.running_os to detect the OS DT is running on https://github.com/darktable-org/darktable/commit/b9cf154e8b2f70d1474bb6d4d936dd36a7859d75

I think we can use it in the next dt version.

boucman commented 7 years ago

the detailed semantic could change, please wait for the next release. We are still thinking what exactly this command should return...

supertobi commented 6 years ago

Now with the latest dev version we can do this:

file.lua

function dtutils_file.check_if_bin_exists(bin)
  local result
  if (dt.configuration.running_os == "windows") then
    local f=io.open(bin,"r")
    if f~=nil then 
      io.close(f) 
      return true 
    else
      return false 
    end  
  elseif (dt.configuration.running_os == "linux") then
    result = os.execute("which " .. bin)
  elseif (dt.configuration.running_os == "macos") then
    result = os.execute() -- ToDo: We need something here
  else
    result = false
  end
  if (not result or result == nil) then
    result = false
  end
  return result
end

I've added the code for Windows. We now need someone with MacOS to fix the ToDo. @wejot I'm looking at you here ;)

wejot commented 6 years ago

Hi all,

We now need someone with MacOS to fix the ToDo. @wejot I'm looking at you here ;)

Well, as I said earlier, I'm neither a LUA pro nor a macOS pro :-) In between, I have found a quick and dirty workaround which does the job for me. However, if I find some time I'll try the following... After some more experiments with mdfind, a simple approach seems to run a grep on the Applications directory, here for the Luminar application:

os.execute('ls /Applications | grep -i "luminar 2018.app"')
Luminar 2018.app
true    exit    0

os.execute('ls /Applications | grep -i "luminar.app"')
Luminar.app
true    exit    0 

Please note that the .app extension and the quotes are required, otherwise more than one result could be returned:

os.execute('ls /Applications | grep -i luminar')
/Applications/Luminar 2018.app
/Applications/Luminar.app
true    exit    0

So far for the 'check if binary exists' part. Running the external program will require a similar approach. For macOS, the open shell command would do the trick:

os.execute('open /Applications/"Luminar 2018.app" --args /full/path/to/pic123.JPG')
true    exit    0

I'll keep you informed.

Regards, Walter

wejot commented 6 years ago

Hi all, more fun... Seems my previous update was a bit early, and a solution for macOS will be more complicated. Looking at gimp.lua and file.lua, _dtutils_file.check_if_binexists is used to check for shell commands (cp, mv, ...) and for applications (Gimp in this case). This works fine on unix systems. Not so on macOS. Running os.execute("which cp") works on macOS for shell commands, but not for "apps" (like Gimp), as explained in the opening post.

Apparently, the macOS branch of _check_if_binexists needs to know if it checks a shell command or an application. Unfortunately, I'm running out of ideas on this issue. I guess I'll have to wait till somebody else jumps in with a better knowledge of Lua and/or macOS.

Thanks for your effort so far. Walter

houz commented 6 years ago

Neither knowing Lua nor OSX, can't you just try which first, and if that didn't find anything check if there is an app by that name? Or the other way round, whatever is the default search order on Macs.

wpferguson commented 6 years ago

Hi Walter,

Would you be willing to test possible solutions? There is a similar problem trying to get the scripts to run on windows, i.e. check_if_bin_exists can't find the binary, so maybe we can fix both the problems at once. I don't have access to anything with macOS, so I can't test a solution, but I can take a shot a coding one.

Thanks,

BIll

wejot commented 6 years ago

Hi all, sorry for coming back late... I have - finally - DT 2.4.1 running and managed to run the running_os.lua example. I had to add a dt.print_log() call, though, because I didn't see any output from the dt.print() call - maybe a diifferent story.

Here is the result:

wjMBP:MacOS walter$ ./darktable -d lua

(process:1220): GLib-GObject-CRITICAL **: g_object_set: assertion 'G_IS_OBJECT (object)' failed
[l10n] error: can't open directory `/usr/local/share/locale'

(darktable-bin:1220): GLib-GObject-WARNING **: invalid cast from 'GtkMenuBar' to 'GtkWindow'

(darktable-bin:1220): Gtk-CRITICAL **: gtk_window_add_accel_group: assertion 'GTK_IS_WINDOW (window)' failed
LUA You are running: macos

Regards, Walter

wpferguson commented 6 years ago

lua.zip

Walter,

Here is a set of lua scripts that should allow you to specify the executable for gimp. Install them, then add require "contrib/gimp" to your luarc file. When you select Edit with GIMP in the exporter, you should see a file selector. Click on that and find the gimp executable. The location will be saved for future runs and gimp should launch when you pick a file and click on export. Let me know if this works please.

Thanks,

Bill

wejot commented 6 years ago

Hi Bill, I've installed the contents of your lua.zip and tried to export a pic to GIMP, without success. I selected "Edit with GIMP", in the "Select GIMP executable" I selected GIMP.app, then clicked the "export" button. DT shows the "Exporting 1 picture to Edit with GIMP" message and goes to sleep. Nothing more happens, the text console tells why:


(process:891): GLib-GObject-CRITICAL **: g_object_set: assertion 'G_IS_OBJECT (object)' failed
[l10n] error: can't open directory `/usr/local/share/locale'

(darktable-bin:891): GLib-GObject-WARNING **: invalid cast from 'GtkMenuBar' to 'GtkWindow'

(darktable-bin:891): Gtk-CRITICAL **: gtk_window_add_accel_group: assertion 'GTK_IS_WINDOW (window)' failed

(darktable-bin:891): Gtk-WARNING **: Allocating size to main_window 0x7ff5efca2260 without calling gtk_widget_get_preferred_width/height(). How does the code know the size to allocate?

(darktable-bin:891): Gtk-WARNING **: Allocating size to main_window 0x7ff5efca2260 without calling gtk_widget_get_preferred_width/height(). How does the code know the size to allocate?

(darktable-bin:891): Gtk-WARNING **: Allocating size to main_window 0x7ff5efca2260 without calling gtk_widget_get_preferred_width/height(). How does the code know the size to allocate?
2018-03-05 14:46:38.335 darktable-bin[891:58209] Warning: -[NSSharingServicePicker showRelativeToRect: ofView: preferredEdge:] should not be called on mouseUp
Please configure the sender with -[NSControl sendActionOn:NSLeftMouseDownMask];

LUA ERROR "/Applications/GIMP.app" /var/folders/hw/15pt_7q151gbxslh2vs19hq80000gn/T/N3010852.jpg 
sh: /Applications/GIMP.app: is a directory
LUA ERROR moving /var/folders/hw/15pt_7q151gbxslh2vs19hq80000gn/T/N3010852.jpg to /Volumes/ExtHD2A/Fotos/2018/2018-03-01-E-M1 Alsumer Berg/JPG/N3010852.jpg
Terminated: 15
wjMBP:MacOS walter$ 

I have to kill DT to get back. Apparently, we still have the problem that "Applications" in MacOS are folders, with the real binary buried in folder structure. Please have a look at my comments from June 23 and 24, regarding the open and mdfind shell commands.

Please let me know if I can help with more testing. Walter

wpferguson commented 6 years ago

Hi Walter,

When you use the file selector can you go in the GIMP.app directory and select the executable? It should work correctly then.

You can also pull the latest copy of the lua scripts from github, which have all these changes incorporated.

Thanks,

Bill

wejot commented 6 years ago

Hi Bill, unfortunately, this does not work. This file selector does not allow to enter an APP package, probably because it's a macos file selector. The Finder is also aware of the special role of APP packages.

However, I tried DT's own file selector on an APP package, like in "Import image" or "export to file". Now it is possible to enter the APP package and step down to the real binary, i.e. /Applications/GIMP.app/Contents/MacOS/GIMP

I still think that opening an macos application should use the way(s) provided by the OS, but for a start you might probably use DT's file selector to select the binary.

If you want to have something tested, please let me know.

Regards, Walter

wpferguson commented 6 years ago

Hi Walter,

I'll do some more research and take another crack at it

Thanks,

Bill

wpferguson commented 6 years ago

Hi Walter,

I have a question. If you open a shell and type "open gimp", does gimp start?

Thanks,

Bill

wpferguson commented 6 years ago

lua.zip

Hi Walter,

Here's another lua.zip to test. This time I changed check_if_bin_exists to prepend "open " to the path. If you have already selected the GIMP.app path, then this should just work.

Let me know,

Thanks,

Bill

wejot commented 6 years ago

Hi Bill,

success! I had to make some changes to the way result is concatenated, as well as adding two flags to the open command.

-- old result = "\"" .. "open " .. path .. "\""
-- new
        result = "open -W -a " .. "\"" .. path .. "\"" .. " "

The additional quotes are necessary for my other external editor, which can now be called as well.

Here is a debug output (original path shortened):

wjMBP:~ walter$ /Applications/darktable.app/Contents/MacOS/darktable -d lua

(process:1056): GLib-GObject-CRITICAL **: g_object_set: assertion 'G_IS_OBJECT (object)' failed
[l10n] error: can't open directory `/usr/local/share/locale'

(darktable-bin:1056): GLib-GObject-WARNING **: invalid cast from 'GtkMenuBar' to 'GtkWindow'

(darktable-bin:1056): Gtk-CRITICAL **: gtk_window_add_accel_group: assertion 'GTK_IS_WINDOW (window)' failed
LUA open -W -a "/Applications/GIMP.app"  /var/folders/hw/15pt_7q151gbxslh2vs19hq80000gn/T/QB149414.jpg 
LUA moving /var/folders/hw/15pt_7q151gbxslh2vs19hq80000gn/T/QB149414.jpg to /V...n/JPG/QB149414.jpg
LUA importing file
LUA QB149414.JPG is a member
LUA QB149414.jpg is a member
LUA Already in group
LUA attaching tag

(darktable-bin:1056): GLib-GObject-CRITICAL **: g_object_unref: assertion 'G_IS_OBJECT (object)' failed
wjMBP:~ walter$ 

One more thought: it might be useful if the externally edited file gets a 'darktable' tag, like 'darktable|extedit' or similar, maybe even with a shortened app name...

Thanks for your effort and let me know, if you want me to test some final, refined or whatever other version!

Regards, Walter

wpferguson commented 6 years ago

Thanks Walter,

I'll finalize the changes and push them today.

Bill

On Sat, Mar 10, 2018 at 10:30 AM, wejot notifications@github.com wrote:

Hi Bill,

success! I had to make some changes to the way result is concatenated, as well as adding two flags to the open command.

-- old result = "\"" .. "open " .. path .. "\"" -- new result = "open -W -a " .. "\"" .. path .. "\"" .. " "

The additional quotes are necessary for my other external editor, which can now be called as well.

Here is a debug output (original path shortened):

wjMBP:~ walter$ /Applications/darktable.app/Contents/MacOS/darktable -d lua

(process:1056): GLib-GObject-CRITICAL **: g_object_set: assertion 'G_IS_OBJECT (object)' failed [l10n] error: can't open directory `/usr/local/share/locale'

(darktable-bin:1056): GLib-GObject-WARNING **: invalid cast from 'GtkMenuBar' to 'GtkWindow'

(darktable-bin:1056): Gtk-CRITICAL **: gtk_window_add_accel_group: assertion 'GTK_IS_WINDOW (window)' failed LUA open -W -a "/Applications/GIMP.app" /var/folders/hw/15pt_7q151gbxslh2vs19hq80000gn/T/QB149414.jpg LUA moving /var/folders/hw/15pt_7q151gbxslh2vs19hq80000gn/T/QB149414.jpg to /V...n/JPG/QB149414.jpg LUA importing file LUA QB149414.JPG is a member LUA QB149414.jpg is a member LUA Already in group LUA attaching tag

(darktable-bin:1056): GLib-GObject-CRITICAL **: g_object_unref: assertion 'G_IS_OBJECT (object)' failed wjMBP:~ walter$

One more thought: it might be useful if the externally edited file gets a 'darktable' tag, like 'darktable|extedit' or similar, maybe even with a shortened app name...

Thanks for your effort and let me know, if you want me to test some final, refined or whatever other version!

Regards, Walter

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/darktable-org/lua-scripts/issues/93#issuecomment-372038401, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjyS6vK_5n0McpTg6FATb2Osv85ZIYcks5tc_GhgaJpZM4OCS6e .

supertobi commented 6 years ago

Merged, can we close this issue?

wejot commented 6 years ago

For me, it's fixed - yes, can be closed.

Walter

semperit89 commented 6 years ago

Is this change already in the master branch? I´ve tried the lua script to start gimp with darktable on os x, but it´s not working. Lua tells me "gimp not found"

wpferguson commented 6 years ago

The change should be in the master branch. When you selected Edit with GIMP in the exporter, did you specify the location of the executable using the file chooser? My understanding is that if you select the gimp.app folder in applications that should work, otherwise you can drill down into the contents and actually select the binary.

Just checked and the changes are in the master branch.

Bill

On Tue, Mar 13, 2018 at 5:01 PM, semperit89 notifications@github.com wrote:

Is this change already in the master branch? I´ve tried the lua script to start gimp with darktable on os x, but it´s not working. Lua tells me "gimp not found"

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/darktable-org/lua-scripts/issues/93#issuecomment-372816912, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjySy5FGihen5NBkSFmJf7ZOt41kHiCks5teDOkgaJpZM4OCS6e .

semperit89 commented 6 years ago

Hey Bill,

Thank you for the fast reply. I could select the gimp.app from my application folder, but that is not working.

In the selector which opens in darktable it is not possible to get into the .app folder and to choose the bin there.

David

wejot commented 6 years ago

Bill, semeperit89 is right - something must have changed in the current version vs. the version I have tested. Just d'loaded the complete package from git, applied it and "edit with Gimp" does'nt work any longer. Don't yet know where the difference is.

wpferguson commented 6 years ago

I'm in the process of setting up a mac vm, which should hopefully be done today. As soon as I get it running, I'll see what's going on with this.

I'll also compare the version I sent you with the current. I did make some changes, so maybe I broke something

Bill

On Wed, Mar 14, 2018 at 11:19 AM, wejot notifications@github.com wrote:

Bill, semeperit89 is right - something must have changed in the current version vs. the version I have tested. Just d'loaded the complete package from git, applied it and "edit with Gimp" does'nt work any longer. Don't yet know where the difference is.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/darktable-org/lua-scripts/issues/93#issuecomment-373059543, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjySyc5ladhTYXPteTovTqAOkTD6Rq4ks5teTUGgaJpZM4OCS6e .

wejot commented 6 years ago

Bill, the extra quotes around filepath in dtutils_file.check_if_file_exists() seem to break io.open(), with the quotes added, io.open() does not return a valid result. For a test, I commented out the filepath = "\"" .. filepath .. "\"" statement, and io.open() worked as expected. Second, the parameter -a for open must be followed by the application to open, so the correct sequence is open -W -a <appname> <more>.

Walter

wpferguson commented 6 years ago

Hi Walter,

I appear to have broken something rather well. I did some testing on io.open to see what it would take and what it wouldn't. It doesn't like quotes or escape sequences, but it will take spaces in path and file names and work correctly. I took out the path quoting, fixed the -W -a and tried it but it still didn't work. I have a mac test bed now, plus a windows test bed, and I'm running on linux, so I should be able to test everything cross platform before I push it and verify that it works as intended. I'm going to retrieve what I sent you and start from there again. Hopefully I'll have a fix tonight.

Bill

wpferguson commented 6 years ago

HI Walter,

I fixed the quoting issues, and pushed the fix, but not before I tested it on macos, windows, and linux.

Bill

semperit89 commented 6 years ago

Hey Bill,

just tested the fix, it is working for me too! Tried also to open affinity, which works like a charm :) Thank you for your efforts!

David

wpferguson commented 6 years ago

You're welcome

On Fri, Mar 16, 2018 at 7:15 PM, semperit89 notifications@github.com wrote:

Hey Bill,

just tested the fix, it is working for me too! Tried also to open affinity, which works like a charm :) Thank you for your efforts!

David

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/darktable-org/lua-scripts/issues/93#issuecomment-373869548, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjySy8PA9upIsxgGkqXbowqbqWLlOLYks5tfEeogaJpZM4OCS6e .

wejot commented 6 years ago

Hi Bill, everything fine now! Thanks for your work. Walter