ruby-git: Using one repo per thread results in errors

I’ll raise an issue to get rid of the chdirs. If #358 and #355 come along for free then that’s awesome 😃

On Sun, 1 Apr 2018, 14:16 Per Lundberg, notifications@github.com wrote:

Taking a look at the code in ruby-git it seems like Dir.chdir is used a lot, and so it makes sense we might get this warning.

This is bad practice on our behalf. We should try to get rid of the Dir.chdir dependencies in our code. Could also be one of the reasons why we’re seeing #358 https://github.com/ruby-git/ruby-git/issues/358; I didn’t see any chdir occurrences there but I might have not been looking carefully enough.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/ruby-git/ruby-git/issues/355#issuecomment-377786140, or mute the thread https://github.com/notifications/unsubscribe-auth/ADO2yZgnamA-6ggiK4CsbaDSUWWwsOTEks5tkNMYgaJpZM4Sk0g4 .

Thank you for the update @xdmx!

I am working on an update that I think will fix this. I plan on running the git subprocess with Process.spawn instead of using backticks. This makes collecting the process output more difficult, but it has the following advantages:

• The environment between different processes (and the process running the command) would be isolated.
• The current directly would be isolated between processes.
• stdout and stderr can be separated. Spurious stderr output currently breaks parsing because stdout and stderr are captured together.
• There are issues with command line escaping that cause git commands to fail (or worse). This can happen when filenames have weird characters.

I looked at less complex alternatives like those found in the Open3 module, but they didn’t offer flexibility currently used in this gem.

The one downside to this approach is that Process.spawn on JRuby is just different enough not to work in a few cases and I haven’t been able to figure out why. My approach is get a version working via MRI on Mac/Linux/Windows first and worry about JRuby later.

I hope to have something done by the end of the year (no promises – working in my free time). The changeset will have to touch a LOT of things so I will try to slow roll the update by having a patch release and major version bump.

For reference, discussion seems to be ongoing in https://github.com/ruby-git/ruby-git/pull/673#issuecomment-1782433049.

I’ve just got hit by this. I clone different repositories concurrently in Sidekiq jobs, when there is only one job running it works without any problem, but when there are 2+ jobs everything starts breaking down. Over the last 8 years there have been various attempts (#195, #372, #401, #383) to remove the use of chdir and make this gem thread safe. Is there any chance that will be done? 🙏 /cc @jcouball

If not, it might be worth writing a “red big warning” in the readme to prevent others hitting this limit

@taquitos File a GitHub issue at https://github.com/probot/stale 😉 I pinned this issue for now.