Re: [PATCH] gitk: macOS: ignore osascript errors

From: Stefan Haller <lists@haller-berlin.de>
Date: Oct 26, 2020, 17:21 UTC
Message-ID: <c4ca1c3b-0563-c35b-5bc0-5accddd0ad3a@haller-berlin.de>
In-Reply-To: <20201025175149.11853-1-dev+git@drbeat.li>

On 25.10.20 18:51, Beat Bolli wrote:

Show 8 quoted lines

> Starting gitk on a macOS 10.14.6 (Mojave) system fails with the error
> 
>      Error in startup script: 2020-10-25 17:16:44.568 osascript[36810:18758270]
>      Error loading /Library/QuickTime/EyeTV MPEG Support.component/Contents/MacOS/EyeTV MPEG Support:
>      dlopen(/Library/QuickTime/EyeTV MPEG Support.component/Contents/MacOS/EyeTV MPEG Support, 0x0106):
>      code signature in (/Library/QuickTime/EyeTV MPEG Support.component/Contents/MacOS/EyeTV MPEG Support)
>      not valid for use in process: mapping process is a platform binary, but mapped file is not
>      [[this same message repeated dozens of times]]

While the code change itself makes sense to me, the justification is a bit strange. This error message suggests that something is messed up on your system. Your commit message makes it sound as if all people on macOS 10.14.6 get this error, which is not the case.

> Ignore errors from the osascript invocation, especially because this macOS
> version seems to correctly place the gitk window in the foreground.

Whether gitk comes to the foreground on start depends on the Tcl/Tk version, not the macOS version. With Tk 8.6 it does, so it might actually be nice to add a version check here. (I'm not requesting that you actually do that as part of this patch; just saying.)

Show 21 quoted lines

> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
> ---
>   gitk | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gitk b/gitk
> index 23d9dd1..8551711 100755
> --- a/gitk
> +++ b/gitk
> @@ -12290,11 +12290,11 @@ if {[catch {package require Tk 8.4} err]} {
>   
>   # on OSX bring the current Wish process window to front
>   if {[tk windowingsystem] eq "aqua"} {
> -    exec osascript -e [format {
> +    catch { exec osascript -e [format {
>           tell application "System Events"
>               set frontmost of processes whose unix id is %d to true
>           end tell
> -    } [pid] ]
> +    } [pid] ] }
>   }

Like I said, the change itself looks good to me, especially since the corresponding code in git gui is also guarded by a catch.

Best, Stefan