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.

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

Fork app commit process hangs with lolcommits enabled #390

Open
donut opened this issue Apr 26, 2019 · 12 comments
Open

Fork app commit process hangs with lolcommits enabled #390

donut opened this issue Apr 26, 2019 · 12 comments

Comments

@donut
Copy link

donut commented Apr 26, 2019

I recently moved to a new computer with a fresh install of macOS Mojave and since then Fork hangs on commit/amend when lolcommits is enabled. The commit happens and the photo taken, but that commit process just hangs. This didn't happen on my old MacBook with the same version of Mojave (10.14.4).

I can't figure out what's going on. Adding the --fork and/or --stealth flags does nothing.

Screen Shot 2019-04-24 at 09 43 57

Video of the issue: https://www.rightthisminute.com/tools/jw/videos/EQtYHKgA/play?force_hq=1

(In the video you see this happening on an iMac. I was using that while I waited for a new computer. I'm also experiencing this on a 2018 15" MBP).

lolcommits 0.13.0
fork 1.0.76
git 2.21.0
@matthutchinson
Copy link
Member

For more info, can you visit the repo on your machine in a console window and paste the output of this?

lolcommits -c --debug

Also, can you paste the output of this git hook file in the repo?

your-repo/.git/hooks/post-commit

I did release a new version of the gem recently that modified the PATH set in the git hook, so that may have something to do with it.

Finally, how (and where) is Ruby installed on your machine? RVM, rbenv, chruby, system ?

@donut
Copy link
Author

donut commented Apr 28, 2019

This isn't for the same repo that's in the video, but it experiences the exact same problem.

[I] ~/RTM/OVPSync (master $)
↪  lolcommits -c --debug                                                                                 0@18:14:21
Outputting at DEBUG verbosity
Checking for valid vcs repo
Walking up dir tree
GitInfo: parsed the following values from commit:
GitInfo: 	build: fix “not found” error in production image
GitInfo: 	9c1e81827c1
GitInfo: 	/Users/donut/RTM/OVPSync/.git
GitInfo: 	RightThisMinute/OVPSync
GitInfo: 	master
GitInfo: 	2019-04-26 22:33:11 UTC
GitInfo: 	Donovan Mueller
GitInfo: 	zotobi@gmail.com
Lolcommits::Runner: running all enabled plugin hooks for pre_capture
*** Preserving this moment in history.
Capturer: initializing new instance #<Lolcommits::CaptureMac:0x00007fa75a873760>
Capturer: making system call for /Library/Ruby/Gems/2.3.0/gems/lolcommits-0.13.0/lib/lolcommits/../../vendor/ext/imagesnap/imagesnap -q "/Users/donut/.lolcommits/OVPSync/tmp_snapshot.jpg" -w 0
Runner: resizing snapshot
Runner: writing resized image to /Users/donut/.lolcommits/OVPSync/tmp_snapshot.jpg
Runner: copying resized image to /Users/donut/.lolcommits/OVPSync/9c1e81827c1.jpg
Lolcommits::Runner: running all enabled plugin hooks for post_capture
Lolcommits::Plugin::Loltext: Annotating image via MiniMagick
Lolcommits::Plugin::Loltext: annotating message to image with build: fix “not found” error in
production image
Lolcommits::Plugin::Loltext: annotating sha to image with 9c1e81827c1
Lolcommits::Plugin::Loltext: Writing changed file to /Users/donut/.lolcommits/OVPSync/9c1e81827c1.jpg
Lolcommits::Runner: running all enabled plugin hooks for capture_ready
Runner: running cleanup
[I] ~/RTM/OVPSync (master $)
↪  cat .git/hooks/post-commit                                                                            0@18:14:30
#!/bin/sh
        ### lolcommits hook (begin) ###
        LANG="en_US.UTF-8" && PATH="$PATH:/usr/bin:/usr/local/bin" && if [ ! -d "$GIT_DIR/rebase-merge" ] && [ "$LOLCOMMITS_CAPTURE_DISABLED" != "true" ]; then lolcommits --capture --device 'FaceTime HD Camera'; fi
        ###  lolcommits hook (end)  ###

System version of ruby:

[I] ~/RTM/OVPSync (master $)
↪  which ruby                                                                                                    0@18:14:56
/usr/bin/ruby
[I] ~/RTM/OVPSync (master $)
↪  ruby --version                                                                                                0@18:15:13
ruby 2.3.7p456 (2018-03-28 revision 63024) [universal.x86_64-darwin18]

@donut
Copy link
Author

donut commented Apr 29, 2019

Stumbled upon another hint: when committing through Fork, the imagesnap process never quits. After trying out several commits with Fork + lolcommits, I had seven imagesnap processes show up in Activity Monitor. If I tell those processes to quit via Activity Monitor, Fork stops hanging on those commits and behaves as expected.

When committing via CLI, the process quits as expected.

@donut
Copy link
Author

donut commented Apr 30, 2019

I was wrong before, it's not always taking a photo when in hangs in Fork. At least I'm not seeing it happen anymore.

@donut
Copy link
Author

donut commented May 1, 2019

Alright, so I switched to using GitUp and that worked fine. However, on first commit it brought up the system dialog for requesting camera access. Fork has not done this, but both GitUp and git on the command line have. I'm guessing this is hanging in Fork because, for whatever reason, that permission dialog isn't getting activated.

@matthutchinson
Copy link
Member

Hi @donut, thanks for reporting this and sharing your video.

Sorry for the super late reply, I started looking into this a few days ago and have been following along with your comments. I originally suspected that Mojave's new privacy permissions might have something to do with this.

I was able to replicate the issue with Fork. It fails to capture anything and hangs on the imagesnap process. The videosnap process hangs too when capturing an animated gif.

When capturing with git on the command line (in iTerm or Terminal) you would have been presented with a dialog (shown below) to consent to imagesnap (or videosnap) accessing the camera.

Screenshot 2019-05-01 at 14 21 50

Screenshot 2019-05-01 at 14 45 45

They get added to this app list in System Preferences -> Security & Privacy -> Privacy -> Camera

Screenshot 2019-05-01 at 14 46 03

Unfortunately for some reason - due to the way Fork launches the installed git hook at your-repo/.git/hooks/post-commit - this dialog is not offered, but the process continues to run without permission. I suspect Fork is somehow wrapping the execution of the hook in a way that the OS fails to detect an initial attempt to access the camera.

The normal sequence of a lolcommit capture process is;

git-hook (/bin/sh) -> lolcommit (ruby) -> launches a child process with Open4 to -> imagesnap/videosnap (ObjC executable binary)

It is interesting that GitUp works OK - I'll try GitHub Desktop and see how that goes. Maybe I can inspect the running processes and compare to what Fork is doing.


I can offer one 'hacky' workaround I cobbled together to get Fork to lolcommit. again.

On StackOverflow, I noticed that people have been using Automator or AppleScript to wrap a bash script and get around the permissions dialog not showing. So this approach works (although there is a significant delay between committing and capturing due to the layers of indirection).

Create a new Automator 'application', with the "Run Shell Script" action having the following code:

source ~/.bash_profile
cd /path/to/your/repo
lolcommits --capture --device 'FaceTime HD Camera'
# use whatever lolcommit ags are set in `your-repo/.git/hooks/post-commit`

Save this Automator app (or export it) to your ~/Applications folder; give it a name you'll remember.

Then edit your-repo/.git/hooks/post-commit and swap the lolcommits command to:

osascript -e 'launch the application "name-of-app-you-remember"'

The proper way to solve this problem is to update both imagesnap and videosnap to use the latest Apple APIs for Camera permissions. I maintain videosnap, imagesnap is quite old and not really maintained.

If we start to get more reports of this issue with other Desktop clients, I'll prioritise this; and I'll try out GitHub Desktop.

@matthutchinson
Copy link
Member

So GitUp works fine, GitHub desktop does nothing, I can't get it to execute any hooks at all (pre or post) ¯\_(ツ)_/¯

@matthutchinson
Copy link
Member

So this also fails with Git Tower having the same issue; the hacky workaround I suggested works, but it's not ideal. I'll see if/when I can prioritise a fix for ImageSnap and VideoSnap.

@donut
Copy link
Author

donut commented May 3, 2019

Thanks for the work-around, @matthutchinson! I modified it a bit to make it more generic and wrote a little script to make enabling easier.

Updated Automater bash script:

cd "$1"
/usr/local/bin/lolcommits --capture --device 'FaceTime HD Camera'

Make sure to set "Shell" to "/bin/bash" and "Pass input" to "as arguments".

Then put this shell script somewhere in your path with execute permissions:

#!/usr/bin/env bash

if [[ ! -d .git ]]; then
  echo "No .git directory"
  exit 1
fi

if [[ "$(which lolcommits)" = "" ]]; then
  echo "lolcommits not installed"
  exit 1
fi

pch=.git/hooks/post-commit

if [[ ! -f ${pch} ]]; then
  echo "#!/bin/sh" > ${pch}
  chmod +x ${pch}
else
  echo "Disabling lolcommits just in case."
  lolcommits --disable
fi

echo -n "Adding hook..."

echo "### lolcommits hook (begin) ###" >> ${pch}
echo "open /Applications/lolcommits-capture.app --args '$(pwd)'" >> ${pch}
echo "### lolcommits hook (end) ###" >> ${pch}

echo "done."

Just need to update the path to the Automater app. With this, you can disable it with lolcommits --disable and it will also automatically remove any existing lolcommits hooks. A bit hacky, but should make things just as easy as before after the initial setup.

@matthutchinson
Copy link
Member

matthutchinson commented Oct 21, 2019

@donut after this fix, I released lolcommits 0.16.0 which has new binaries for imagenap and videosnap.

Can you try this with fork and see what happens? (without the Automator approach). If all is OK I'll close this issue.

@donut
Copy link
Author

donut commented Oct 21, 2019

@matthutchinson This still isn't working, but it also isn't hanging any more. Works fine on command line.

With the --fork flag, a lolcommits.pid is created in the directory where the images go, but is never replace with an image. Also, no "preserving this moment" message is recorded in the log. Without the --fork flag, that "preserving" message is there but no image is saved. In both cases, the camera is not activated.

Screen Shot 2019-10-21 at 1149 11

macOS 10.14.6 (18G103)
Fork.app 1.0.85.2
lolcommits 0.16.0

Automator approach still works.

@matthutchinson
Copy link
Member

Ok, thanks for trying and letting me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants