-
Notifications
You must be signed in to change notification settings - Fork 533
Removed all Dir.chdir
calls from private use
#195
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
Conversation
Ty @mbishop-fiksu ! I've started that over the |
Note that I took the |
Hey @mbishop-fiksu can you please rebase your branch to master and There was a problem with dates and tests. Ty. |
I'd be happy to. I'll get to this after the thanksgiving break. Thank you for the heads up! |
We know this is safe because a- there is a bug where `build_list` is being called, but that's obviously a bug copied from `config_list`. Also, if `@git_dir` is non-nil, `#command` will already ensure the command is run within `@git_dir` as the working directory.
We know this is safe because if `@git_dir` is non-nil, `#command_lines` will already ensure the command is run within `@git_dir` as the working directory.
Ok, rebased and pushed. There is still a Ruby 1.8 incompatibility (I'm using |
I've re-added back 1.8-compatibility, but at the cost of re-adding the |
I'm not sure the failure in the test suite is the code. What do you think? |
It's a build problem, happens frequently for some ruby versions (not our fault). Just sent a re-run, hope that will work. If not we should probably wait and try again later. |
Bah, same thing. Ok, we'll wait I guess... |
@robertodecurnex Think it's worth hitting that test up again or should we wait a little more? Also, I can't remove all the |
Ruby 1.8 compatibility is definitely a show-stopper for the 1.x line. Since there are a bunch of Ruby 1.8 issues that can be easily solved with Ruby >= 1.9 I will be removing 1.8 support on git 2.0 (the next release after 1.2.9). I'll take your changes to the |
@robertodecurnex I think this could still be considered for the 1.2.9 release and here's my reasoning. The change improves thread-safety for rubies from 1.9 on while the behavior remains the same for rubies < 1.9. Additionally, no public interface is changed. It doesn't fail on ruby 1.8. It remains the same. What do you think? |
@mbishop-fiksu I just came across this when trying to use ruby-git multithreaded. With the current state of 0f78b67, can you still fetch/init/checkout in parallel without chdir-related problems? |
@jbohren since your problems would be just on the way that Mostly of the thread safety improvements will be also available under the next release (soon). |
Cool, I'll check that out. Thanks! |
3fc1c27
to
b224195
Compare
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
When writing command-line tools that use ruby-git to interface with git, there are often ways to achieve better performance by running git query commands in parallel. This is unworkable with
ruby-git
given it's extensive use of the non-thread-safeDir.chdir
call. With all implicit calls toDir.chdir
removed,ruby-git
is one step closer to being usable amongst threads.From the
Dir.chdir
documentation: