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.

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

A fix for Issue #713 #780

Closed
wants to merge 2 commits into from
Closed

A fix for Issue #713 #780

wants to merge 2 commits into from

Conversation

mackoj
Copy link

@mackoj mackoj commented Feb 4, 2013

Hi,

I tried to fix the issue #713 however I'm not a ruby developer and I don't really know how to test it properly. The goal is just to give you a hint to help you fix it.

I just hope it helps.

@mackoj
Copy link
Author

mackoj commented Feb 5, 2013

I don't really understand what going on with Travis I will be glad to understand it in order try to fix my commit.

@mackoj
Copy link
Author

mackoj commented Feb 6, 2013

With this fix git rev-list seems to work as expected.

-> Installing AFNetworking (1.1.0)
  $ /usr/bin/git config core.bare
  true
 > Cloning git repo
   $ /usr/bin/git rev-list --max-count=1 --tags 1.1.0
   121ef7afa8ce1b1cefdc5312f7e9d75f5af65b8d
   $ /usr/bin/git init
   Initialized empty Git repository in /Users/jeffreymacko/projects/perso/test/Pods/AFNetworking/.git/
   $ /usr/bin/git remote add origin '/Users/jeffreymacko/Library/Caches/CocoaPods/Git/a7f129229d47b74a225e1e2ed0bc604f35d71ac7'
   $ /usr/bin/git fetch origin tags/1.1.0
   From /Users/jeffreymacko/Library/Caches/CocoaPods/Git/a7f129229d47b74a225e1e2ed0bc604f35d71ac7
    * tag               1.1.0      -> FETCH_HEAD
   $ /usr/bin/git reset --hard FETCH_HEAD
   HEAD is now at 121ef7a Bumping version to 1.1.0
   $ /usr/bin/git checkout -b activated-pod-commit
   Switched to a new branch 'activated-pod-commit'
 > Using existing documentation

@alloy
Copy link
Member

alloy commented Feb 6, 2013

@mackoj Thanks a lot, we’ll look into this ASAP. Rest assured, the issues with Travis atm are because we are switching to the currently in ‘beta’ mac workers, which are atm broken but should be fixed soon.

@alloy
Copy link
Member

alloy commented Feb 7, 2013

I tried to replicate this issue, but have been unable to do so.

My git version is:

/usr/bin/git --version
git version 1.7.10.2 (Apple Git-33)

Which git version do you have?

@mackoj
Copy link
Author

mackoj commented Feb 7, 2013

I have this version.

$>which git
/usr/bin/git
$>git --version
git version 1.7.12.4 (Apple Git-37)

@mackoj
Copy link
Author

mackoj commented Feb 8, 2013

Even if you couldn't reproduce this bug at least it disambiguates git rev-list.

The way cocoapods used to use it with tags.

git rev-list --max-count=1 1.1.0
git rev-list [ --max-count=<number> ] <commit>

The way it should be used with tags.

git rev-list --max-count=1 --tags 1.1.0
git rev-list [ --max-count=<number> ] [ --tags[=<pattern>] ]

I have to admit I cannot reproduce this bug either since friday until a couple days ago it's does not work and now its working I don't know why exactly.

By the way I don't have the same behavior on all of my computer I'm trying to find the difference between those two and maybe give a better explanation to this bug.

@alloy
Copy link
Member

alloy commented Feb 8, 2013

Ah you know what, I might need to update the command-line utilities, which I don’t think I did since the Xcode 4.6 update.

Even if you couldn't reproduce this bug at least it disambiguates git rev-list.

Yeah definitely. I don’t think there is no problem at all, I just want to be able to verify any fix and the details before I push something.

By the way I don't have the same behavior on all of my computer I'm trying to find the difference between those and maybe give a better explanation to this bug.

Awesome, thanks for taking the time.

@fabiopelosin
Copy link
Member

I encountered the issues as well, so I think that we should merge it asap.

$ git --version
git version 1.7.12.4 (Apple Git-37)

@mackoj
Copy link
Author

mackoj commented Feb 8, 2013

By the way I don't have the same behavior on all of my computer I'm trying to find the difference between those two and maybe give a better explanation to this bug.

Sorry but I have no luck on this point now it's working everywhere.

I don't understand why I can't reproduce it now, even if a couple days ago its wasn't working.
It might be due to a bug in github or something.

@alloy
Copy link
Member

alloy commented Feb 8, 2013

Yeah it works for me too with the latest…

$ /usr/bin/git --version
git version 1.7.12.4 (Apple Git-37)

But since @irrationalfab experienced it too, I’m sure there is a variation of factors out there that triggers this. I’m gonna see if I can figure out anything tomorrow, or just apply the required changes without knowing for sure we’ve fixed anything.

@mackoj
Copy link
Author

mackoj commented Feb 8, 2013

I’m sure there is a variation of factors out there that triggers this.

Me too, but I wasn't able to establish it. I will be thrilled to understand what's happening.

@dtorres
Copy link
Contributor

dtorres commented Feb 9, 2013

Please merge or fix this ASAP, I'm unable to test a new podspec to update a component.

@alloy
Copy link
Member

alloy commented Feb 9, 2013

@dtorres In order for us to merge a fix asap that we know will work, it would be great if you could find out why this happens in your situation. What Git version are you using? Is it reproducible? On multiple machines? Any info that will allow us to fully understand the issue would be much appreciated.

@fabiopelosin
Copy link
Member

I have tried to reproduce the issue as well without success. I observed it with AFNetworking 1.1.0. My theory is that rev-list is confused by 1.1.0 tag if it hasn't been fetched by the bare repo, and for this reason we cannot observe it anymore because it is know fetched in our machines. I tried deleting the cache repo and as expected the issue doesn't appears.

For these reasons I'm not sure if the proposed solution is the correct one. Apparently it can't hurt, but I'm not sure that it would fix the issue. Given that the reported error is the following, it appears to have been an issue with the remote.

 > Cloning git repo
   $ /usr/bin/git rev-list --max-count=1 1.1.0
   fatal: ambiguous argument '1.1.0': unknown revision or path not in the working tree.
   Use '--' to separate paths from revisions, like this:
   'git <command> [<revision>...] -- [<file>...]'
   [!] Failed: /usr/bin/git rev-list --max-count=1 1.1.0
   $ /usr/bin/git init
   Initialized empty Git repository in /private/tmp/CocoaPods/Lint/Pods/AFNetworking/.git/
   $ /usr/bin/git remote add origin '/Users/jeffreymacko/Library/Caches/CocoaPods/Git/a7f129229d47b74a225e1e2ed0bc604f35d71ac7'
   $ /usr/bin/git fetch origin tags/1.1.0
   fatal: Couldn't find remote ref tags/1.1.0
   fatal: The remote end hung up unexpectedly
[!] git fetch origin tags/1.1.0

Our implementation of git rev-list should work with tags because it has been battle tested for a while. I.e. git is confused only if the tag is not there and thus if fetches the remote, on the second check the tag will be there and the installation will proceed. Apparently even if we don't specify the --tags argument git looks in the tags as well. However if we would like make it more robust I would propose to:

change this:

$ /usr/bin/git rev-list --max-count=1 1.1.0

with:

$ /usr/bin/git rev-list --max-count=1 1.1.0 --

@drodriguez
Copy link
Contributor

I think this is not the right solution.

From git-rev-list man page:

--tags[=<pattern>]
    Pretend as if all the refs in refs/tags are listed on the command line as <commit>. If <pattern> is given, limit tags to ones matching given shell glob. If pattern lacks ?, *, or [, /* at the end is implied.

So --tags by itself (without the equals and the pattern) will simply list all the tags in refs/tags as if commits where on the command line.

Example: in the current CocoaPods repository:

$ git tag -l
[...snip...]
0.16.1
0.16.2
[...snip...]

$ git show 0.16.2
tag 0.16.2
Tagger: Eloy Durán <eloy.de.enige@gmail.com>
Date:   Sat Feb 2 21:18:25 2013 +0100

Release 0.16.2

commit 0be241d8dfb496c37b4a4b82cc8cda39748ef998
[...snip...]

$ git show 0.16.1
tag 0.16.1
Tagger: Eloy Durán <eloy.de.enige@gmail.com>
Date:   Sun Jan 13 22:43:22 2013 +0100

Release 0.16.1

commit fbb58e4fa812da799383e76de6d538007542d7d7
[...snip...]

$ git rev-list --max-count 1 --tags 0.16.2
0be241d8dfb496c37b4a4b82cc8cda39748ef998

$ git rev-list --max-count 1 --tags 0.16.1
0be241d8dfb496c37b4a4b82cc8cda39748ef998

$ git rev-list --max-count 1 0.16.1
fbb58e4fa812da799383e76de6d538007542d7d7

$ git rev-list --max-count 1 0.16.2
0be241d8dfb496c37b4a4b82cc8cda39748ef998

As you can see, with --tags, in both cases I get the 0.16.2 commit, not the tagged commit (the --tags-less version works ok for me, git version 1.8.1.1 from Homebrew).

I have tried to do git rev-list --max-count 1 1.1.0 in a bare clone of AFNetworking in several versions of Git I have around and no one fails me with the same error as OP (1.7.0.4, 1.7.5.1, 1.7.11.3, 1.7.12, 1.8.1.1).

Anyway, I tried another way of specifying we want a tag and this seems to work and maybe avoids the “ambiguous argument” Git talks about:

$ git rev-list --max-count 1 refs/tags/0.16.2
0be241d8dfb496c37b4a4b82cc8cda39748ef998

$ git rev-list --max-count 1 refs/tags/0.16.1
fbb58e4fa812da799383e76de6d538007542d7d7

What do you think?

@mackoj
Copy link
Author

mackoj commented Feb 9, 2013

Cocoapods is now working and has proven to be working for a long time.

However if this bug has multiple root causes has we suspect. You should at least consider implementing git rev-list as it is described in git documentation.

In doing so it should exclude one probable bug cause.

And I don't think it would or could cause any harm to Cocoapods.

@subdigital
Copy link
Contributor

I just ran into this as well. I was trying to fix the SocketRocket & PonyDebugger podspecs because they don't currently work.

Here's my Podfile:

platform :ios, "6.0"

pod 'RestKit'
pod 'SVProgressHUD'
pod 'SocketRocket', :git => 'https://github.com/square/SocketRocket.git'
pod 'PonyDebugger', :git => 'https://github.com/square/PonyDebugger.git'

It gives me the error after pre-downloading SocketRocket.

Malformed version number string 0.3.1-beta2

CocoaPods version: 0.16.2
Ruby version: 1.9.3p327
Git version: 1.7.10.2

Hopefully this helps repro it, @alloy.

@alloy
Copy link
Member

alloy commented Feb 10, 2013

@subdigital That one is unrelated, because the version is in fact ‘malformed’. This is only because RubyGems’ Version class isn’t actually compatible with SemVer for these types of version. This will be fixed after 0.17 when we vendor all these sources from RubyGems and we can make changes to them.

This one is related #293, but I’m also sure that @blakewatters had a ticket and was working on it, but I’m unable to find that ticket right now.

@alloy
Copy link
Member

alloy commented Feb 10, 2013

Back on topic, as commented here #713 (comment), this issue doesn't seem to be related to the actual problem of the OP in #713.

This leads me to think that we should not try to fix this right now when we don’t know the actual problem and the net result is that we’re only making this code more complex.

@alloy
Copy link
Member

alloy commented Feb 10, 2013

@subdigital Btw, for now you can fix the spec by replacing the dash by a dot.

@blakewatters
Copy link

I had submitted a fix and tests that made the version class borrowed from RubyGems conform to the SemVer spec several months back. The original issue is captured on #583 and my pull request was on #584, which was closed by @irrationalfab.

I had meant to resubmit the PR against CocoaPods/Core, but it slipped by mind. I'll go ahead and fire that off now.

@rynaardb
Copy link

Hi

I'm currently experiencing the same issue.

Are there any working solutions at this stage?

@fabiopelosin
Copy link
Member

The issue appears to be related to the git remotes not being not available. We are looking for a definitive explanation so if somebody does encounter it please report the output of pod install --verbose.

Regarding the pull request I agree with @drodriguez that the original pull request is not correctly interpreting the git documentation and therefore I'm closing it.

jzapater pushed a commit to jzapater/CocoaPods that referenced this pull request Sep 17, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants