Skip to content

Replace all places that can call Dir.chdir in thread with passing chdir to Open3.popen2e #60

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.

Sign up for GitHub

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

Merged
gyfelton merged 8 commits into from
Sep 1, 2023
Merged

Replace all places that can call Dir.chdir in thread with passing chdir to Open3.popen2e #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
af62404
Replace two places that can call chdir in thread with `cd dir`
gyfelton Aug 31, 2023
89f0afe
rubocop
gyfelton Aug 31, 2023
565a0b3
fixup
gyfelton Aug 31, 2023
9e7c8bd
trial
gyfelton Aug 31, 2023
3e6a0a3
Revert "trial"
gyfelton Sep 1, 2023
52164f4
try
gyfelton Sep 1, 2023
87a300e
rubocop
gyfelton Sep 1, 2023
1f57f59
fixup
gyfelton Sep 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile.lock
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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