Skip to content

Commit

Permalink
Replace all places that can call Dir.chdir in thread with passing `…
Browse files Browse the repository at this point in the history
…chdir` to `Open3.popen2e` (#60)
  • Loading branch information
gyfelton committed Sep 1, 2023
1 parent ec4fca8 commit d05d049
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 26 deletions.
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
git-fastclone (1.4.2)
git-fastclone (1.4.3)
colorize

GEM
Expand Down
31 changes: 13 additions & 18 deletions lib/git-fastclone.rb
Original file line number Diff line number Diff line change
Expand Up @@ -225,10 +225,9 @@ def clone(url, rev, src_dir, config)

# Only checkout if we're changing branches to a non-default branch
if rev
Dir.chdir(File.join(abs_clone_path, src_dir)) do
fail_on_error('git', 'checkout', '--quiet', rev.to_s, quiet: !verbose,
print_on_failure: print_git_errors)
end
fail_on_error('git', 'checkout', '--quiet', rev.to_s, quiet: !verbose,
print_on_failure: print_git_errors,
chdir: File.join(abs_clone_path, src_dir))
end

update_submodules(src_dir, url)
Expand All @@ -250,11 +249,9 @@ def update_submodules(pwd, url)

threads = []
submodule_url_list = []
output = ''
Dir.chdir(File.join(abs_clone_path, pwd).to_s) do
output = fail_on_error('git', 'submodule', 'init', quiet: !verbose,
print_on_failure: print_git_errors)
end
output = fail_on_error('git', 'submodule', 'init', quiet: !verbose,
print_on_failure: print_git_errors,
chdir: File.join(abs_clone_path, pwd))

output.split("\n").each do |line|
submodule_path, submodule_url = parse_update_info(line)
Expand All @@ -270,11 +267,10 @@ def update_submodules(pwd, url)
def thread_update_submodule(submodule_url, submodule_path, threads, pwd)
threads << Thread.new do
with_git_mirror(submodule_url) do |mirror, _|
Dir.chdir(File.join(abs_clone_path, pwd).to_s) do
cmd = ['git', 'submodule', verbose ? nil : '--quiet', 'update', '--reference', mirror.to_s,
submodule_path.to_s].compact
fail_on_error(*cmd, quiet: !verbose, print_on_failure: print_git_errors)
end
cmd = ['git', 'submodule',
verbose ? nil : '--quiet', 'update', '--reference', mirror.to_s, submodule_path.to_s].compact
fail_on_error(*cmd, quiet: !verbose, print_on_failure: print_git_errors,
chdir: File.join(abs_clone_path, pwd))
end

update_submodules(File.join(pwd, submodule_path), submodule_url)
Expand Down Expand Up @@ -350,10 +346,9 @@ def store_updated_repo(url, mirror, repo_name, fail_hard)
quiet: !verbose, print_on_failure: print_git_errors)
end

Dir.chdir(mirror) do
cmd = ['git', 'remote', verbose ? '--verbose' : nil, 'update', '--prune'].compact
fail_on_error(*cmd, quiet: !verbose, print_on_failure: print_git_errors)
end
cmd = ['git', 'remote', verbose ? '--verbose' : nil, 'update', '--prune'].compact
fail_on_error(*cmd, quiet: !verbose, print_on_failure: print_git_errors, chdir: mirror)

reference_updated[repo_name] = true
rescue RunnerExecutionRuntimeError => e
# To avoid corruption of the cache, if we failed to update or check out we remove
Expand Down
2 changes: 1 addition & 1 deletion lib/git-fastclone/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

# Version string for git-fastclone
module GitFastCloneVersion
VERSION = '1.4.2'
VERSION = '1.4.3'
end
10 changes: 4 additions & 6 deletions spec/git_fastclone_runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,14 @@ def create_lockfile_double
before(:each) do
allow(runner_execution_double).to receive(:fail_on_error) {}
allow(Dir).to receive(:pwd) { '/pwd' }
allow(Dir).to receive(:chdir).and_yield
allow(subject).to receive(:with_git_mirror).and_yield('/cache', 0)
expect(subject).to receive(:clear_clone_dest_if_needed).once {}
end

it 'should clone correctly' do
expect(subject).to receive(:fail_on_error).with(
'git', 'checkout', '--quiet', 'PH',
{ quiet: true, print_on_failure: false }
{ chdir: '/pwd/.', print_on_failure: false, quiet: true }
) { runner_execution_double }
expect(subject).to receive(:fail_on_error).with(
'git', 'clone', '--quiet', '--reference', '/cache', 'PH', '/pwd/.',
Expand All @@ -113,7 +112,7 @@ def create_lockfile_double
subject.verbose = true
expect(subject).to receive(:fail_on_error).with(
'git', 'checkout', '--quiet', 'PH',
{ quiet: false, print_on_failure: false }
{ chdir: '/pwd/.', print_on_failure: false, quiet: false }
) { runner_execution_double }
expect(subject).to receive(:fail_on_error).with(
'git', 'clone', '--verbose', '--reference', '/cache', 'PH', '/pwd/.',
Expand Down Expand Up @@ -338,7 +337,6 @@ def create_lockfile_double

it 'should correctly update the hash' do
allow(subject).to receive(:fail_on_error)
allow(Dir).to receive(:chdir) {}

subject.reference_updated = placeholder_hash
subject.store_updated_repo(placeholder_arg, placeholder_arg, placeholder_arg, false)
Expand Down Expand Up @@ -389,7 +387,6 @@ def try_with_git_mirror(responses, results)
expected_command = expected_commands.shift
expect(command).to eq(expected_command)
}
allow(Dir).to receive(:chdir).and_yield

allow(subject).to receive(:print_formatted_error) {}
allow(subject).to receive(:reference_repo_dir).and_return(test_reference_repo_dir)
Expand All @@ -404,7 +401,8 @@ def clone_cmds(verbose: false)
[
['git', 'clone', verbose ? '--verbose' : '--quiet', '--mirror', test_url_valid,
test_reference_repo_dir],
['git', 'remote', verbose ? '--verbose' : nil, 'update', '--prune'].compact
['git', 'remote', verbose ? '--verbose' : nil, 'update',
'--prune'].compact
]
end

Expand Down

0 comments on commit d05d049

Please sign in to comment.