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.

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

Fix bug of Git shallow cloning is not used when branch is set #117

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix bug of Git shallow cloning is not used when branch is set #117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f8df52f
Fix bug of Git shallow cloning is not used when branch is set
dimohamdy Aug 10, 2021
4cd3db4
Update change log
dimohamdy Aug 10, 2021
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
[xdkhan](https://github.com/xdkhan)
[Dimitris Koutsogiorgas](https://github.com/dnkoutso)
[#116](https://github.com/CocoaPods/cocoapods-downloader/pull/116)

* Fix bug of Git shallow cloning is not used when branch is set.
[Dimo Hamdy](https://github.com/dimohamdy)
[Pavel](https://github.com/paiv)
[#106](https://github.com/CocoaPods/cocoapods-downloader/issues/106)


## 1.4.0 (2020-07-17)
Expand Down
5 changes: 4 additions & 1 deletion lib/cocoapods-downloader/git.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ def self.preprocess_options(options)
return options if match.nil?

options[:commit] = match
options.delete(:branch)

options
end
Expand Down Expand Up @@ -134,6 +133,10 @@ def clone_arguments(force_head, shallow_clone)
command += %w(--single-branch --depth 1)
end

if shallow_clone && options[:commit] && options[:branch]
command += %w(--single-branch --depth 1)
end

unless force_head
if tag_or_branch = options[:tag] || options[:branch]
command += ['--branch', tag_or_branch]
Expand Down
24 changes: 24 additions & 0 deletions spec/git_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,30 @@ def ensure_only_one_ref(folder)
should.not.raise { downloader.download }
end
end

describe 'get remote default branch if not set branch with shallow clones' do
it 'with git <= 2.10.x' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this version enforced within the tests? I do not see it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only try to follow the same way of this unit-test

cocoapods-downloader/spec/git_spec.rb

Line 202 in bce1d26

describe 'will retry if a remote does not support shallow clones' do

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh interesting, had no idea....curious how this happens.....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dimohamdy I checked out your branch and it seems even without your change the tests here are still passing? Can you confirm we are testing the right thing and these tests fail without your change?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @dimohamdy for any updates here

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dnkoutso, Git shallow is working fine when branch is set. The real issue is caused by self.preprocess_option(options) method. This method is deleting the :branch option from options, and add :commit option into options and after that it causing to pass adding --depth 1 parameter when git clone shallow_clone && !options[:commit]. We should add test for preprocess_options

options = { :git => fixture_url('git-repo') }
downloader = Downloader.for_target(tmp_folder, options)
message = '/usr/local/bin/git clone URL directory --single-branch ' \
"--depth 1\nCloning into 'directory'...\n" \
'fatal: dumb http transport does not support --depth'
dumb_remote_error = Pod::Downloader::DownloaderError.new(message)
downloader.stubs(:git!).raises(dumb_remote_error).then.returns(true)
should.not.raise { downloader.download }
end

it 'with git >= 2.11.x' do
options = { :git => fixture_url('git-repo') }
downloader = Downloader.for_target(tmp_folder, options)
message = '/usr/local/bin/git clone URL directory --single-branch ' \
"--depth 1\nCloning into 'directory'...\n" \
'fatal: dumb http transport does not support shallow capabilities'
dumb_remote_error = Pod::Downloader::DownloaderError.new(message)
downloader.stubs(:git!).raises(dumb_remote_error).then.returns(true)
should.not.raise { downloader.download }
end
end
end

describe ':commit_from_ls_remote' do
Expand Down