Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

[SourcesManager] Ignore repos with unreachable sources on update #1595

Merged
merged 4 commits into from

6 participants

@Kapin
Collaborator

Fixes #1571. We now will ignore all repo's that have unreachable remotes when updating repos.

@coveralls

Coverage Status

Coverage remained the same when pulling 8123182 on Kapin:unreachable-repo-fix into 9f03ab9 on CocoaPods:master.

@coveralls

Coverage Status

Coverage remained the same when pulling 8123182 on Kapin:unreachable-repo-fix into 9f03ab9 on CocoaPods:master.

lib/cocoapods/sources_manager.rb
@@ -159,12 +159,24 @@ def update(source_name = nil, show_output = false)
end
end
+ # Returns whether a git repo's remote is reachable.
+ #
+ # @param [Pathname] dir
+ # The directory where the source is stored.
+ #
+ # @return [Bool] Whether the given source's remote is reachable.
+ #
+ def git_remote_reachable?(dir)
+ Dir.chdir(dir) { `git ls-remote` }
+ $?.exitstatus.zero?
@alloy Owner
alloy added a note

Not sure why the method below was using backticks, but this method and the one below should preferably be using the git! method, which also raises a proper error message if the command does not exist.

Finally, but just an FYI, for the exit status you can also use $?.success?.

@fabiopelosin Owner

$?.success?Ah I didn't know about this one!

@Kapin Collaborator
Kapin added a note

@alloy I'll convert all of them over then. Also good to know about the $?.success? thing

@Kapin Collaborator
Kapin added a note

So it turns out using git! will not throw a fatal with ls-remote if there is no source. Using backticks lets this filter through to the console and not explode. Similarly, using git! for the lower command doesn't seem to work.

@fabiopelosin Owner

FYI: The executable module implements some logic about this:

  • git! raises if the exit code is not 0 (it is intended to abort on unexpected failures for subprocesses).
  • git doesn't raise (intended to be used in cases where we are checking for a failure).
@Kapin Collaborator
Kapin added a note

Ahhh that makes a ton more sense. I'll try that again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@coveralls

Coverage Status

Coverage remained the same when pulling 0feb5f1 on Kapin:unreachable-repo-fix into f09bb13 on CocoaPods:master.

@fabiopelosin

That failure sometimes happens on Travis and I'm still not able to understand the cause. I have restarted the build which usually is enough to fix it.

@fabiopelosin

The patch looks ace!

@Lascorbe you might be interested in this implementation and taking parts of it. @Kapin approach is cleaner that the one that I suggested to check for git repos.

@Kapin Kapin merged commit 9e71add into from
@Lascorbe Lascorbe referenced this pull request from a commit in Lascorbe/CocoaPods
@Lascorbe Lascorbe List repo format changes d396f6c
@jollychang

@Kapin every time when pod install/update will check git_remote_reachable?

but git ls-remote is very slow, so can this feature be switch by options?

[jollychang@WilliamMBP CocoaPods]$ time git ls-remote
From git@github.com:CocoaPods/CocoaPods.git
real 0m10.546s

Owner

You can disable this (and performing any git pull's) with pod install --no-repo-update.

Collaborator

What @kylef said will work. I tried to figure out a faster way to do this at the time but I'm not a git master by any means. If you have a faster way I highly encourage you to make a pull request :smile:

pod install --no-repo-update is fine,Thanks

but why need to check git_remote_reachable or git ls-remote?
if what you need is update repo ,why not just git pull?

Collaborator

If I recall correctly the issue was when there was a custom firewalled specs repo, pod install broke. So we check if the repo is reachable before trying to get at it.

Owner

@Kapin Is this the same thing as kylef/CocoaPods@567b74c ?

That uses git remote which is quicker than git ls-remote.

I am not familiar with pod's logic, so why not check(throw out error) the remote/branch of repo by git itself?

jenkins@jolly1 /tmp $ git pull aaa
fatal: 'aaa' does not appear to be a git repository
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
jenkins@jolly1 /tmp $ echo $?
1
jenkins@jolly1 /tmp $ git pull origin tttt
fatal: Couldn't find remote ref tttt
Unexpected end of command stream
jenkins@jolly1 /tmp $ echo $?
1
Owner

@Kapin Thinking about this more, a safer solution would be to check that there is an upstream setup for the branch.

$ git config --get branch.$(git rev-parse --abbrev-ref HEAD).remote
origin

Since I could have a remote but not configure for the current branch. As shown in the following example:

$ git checkout -b testing_branch
Switched to a new branch 'testing_branch'
$ git push
fatal: The current branch testing_branch has no upstream branch.
To push the current branch and set the remote as upstream, use

    git push --set-upstream origin testing_branch
Collaborator

@kylef I found the original issue here #1571. The issue with that one was that pod install was exploding if a repo's remote was unreachable. I'm not sure if that's the same case, but that might provide a bit more insight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 14, 2013
  1. @Kapin
  2. @Kapin

    [CHANGELOG] Update

    Kapin authored
Commits on Nov 15, 2013
  1. @Kapin
  2. @Kapin

    Merge branch 'master'

    Kapin authored
    Conflicts:
    	CHANGELOG.md
This page is out of date. Refresh to see the latest.
View
7 CHANGELOG.md
@@ -12,6 +12,13 @@ To install or update CocoaPods see this [guide](http://docs.cocoapods.org/guides
[Kevin Wales](https://github.com/kwales)
[#1562](https://github.com/CocoaPods/pull/1562)
+* When updating the pod repos, repositories with unreachable remotes
+ are now ignored. This fixes an issue with certain private repositories.
+ [Joshua Kalpin](https://github.com/Kapin)
+ [#1595](https://github.com/CocoaPods/CocoaPods/pull/1595)
+ [#1571](https://github.com/CocoaPods/CocoaPods/issues/1571)
+
+
## 0.28.0
[CocoaPods](https://github.com/CocoaPods/CocoaPods/compare/0.27.1...0.28.0)
• [CocoaPods-core](https://github.com/CocoaPods/Core/compare/0.27.1...0.28.0)
View
20 lib/cocoapods/sources_manager.rb
@@ -145,7 +145,7 @@ def update(source_name = nil, show_output = false)
raise Informative, "The `#{source_name}` repo is not a git repo." unless git_repo?(specified_source.repo)
sources = [specified_source]
else
- sources = aggregate.all.select { |source| git_repo?(source.repo) }
+ sources = aggregate.all.select { |source| git_repo?(source.repo) && git_remote_reachable?(source.repo) }
end
sources.each do |source|
@@ -159,16 +159,28 @@ def update(source_name = nil, show_output = false)
end
end
+ # Returns whether a git repo's remote is reachable.
+ #
+ # @param [Pathname] dir
+ # The directory where the source is stored.
+ #
+ # @return [Bool] Whether the given source's remote is reachable.
+ #
+ def git_remote_reachable?(dir)
+ Dir.chdir(dir) { git('ls-remote') }
+ $?.success?
+ end
+
# Returns whether a source is a GIT repo.
#
# @param [Pathname] dir
# The directory where the source is stored.
#
- # @return [Bool] Wether the given source is a GIT repo.
+ # @return [Bool] Whether the given source is a GIT repo.
#
def git_repo?(dir)
- Dir.chdir(dir) { `git rev-parse >/dev/null 2>&1` }
- $?.exitstatus.zero?
+ Dir.chdir(dir) { git('rev-parse >/dev/null 2>&1') }
+ $?.success?
end
# Checks the version information of the source with the given directory.
View
5 spec/unit/sources_manager_spec.rb
@@ -95,6 +95,11 @@ module Pod
UI.output.should.match /Already up-to-date/
end
+ it 'returns whether a source has a reachable git remote' do
+ SourcesManager.git_remote_reachable?(repo_make('a_new_repo_that_is_new')).should.be.false
+ SourcesManager.git_remote_reachable?(SourcesManager.master_repo_dir).should.be.true
+ end
+
it "returns whether a source is backed by a git repo" do
SourcesManager.git_repo?(SourcesManager.master_repo_dir).should.be.true
SourcesManager.git_repo?(Pathname.new('/tmp')).should.be.false
Something went wrong with that request. Please try again.