ku1ik / git-dude

Git commit notifier
http://ku1ik.com/
GNU General Public License v3.0
948 stars 68 forks source link

Fails handling fetch results other than new branch/tag/commit #22

Closed digitaljhelms closed 12 years ago

digitaljhelms commented 12 years ago

Error occurring:

fatal: ambiguous argument '+': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions

Working example of a git fetch which indicates a force-pushed branch ref in it's output:

$ git fetch
remote: Counting objects: 78, done.
remote: Compressing objects: 100% (18/18), done.
remote: Total 48 (delta 28), reused 48 (delta 28)
Unpacking objects: 100% (48/48), done.
From github.com:Username/repo
 + 83e93db...b212eee branch-name -> origin/branch-name  (forced update)

Workflow:

The output of git fetch 2>&1 | grep -F -- '->' | sed -E 's/^\s+\+\s+//' (git-dude#L71) is passed to awk (via a case statement) multiple times, each of which assigns a column of the output to a variable used by the notifier.

Piped output from working example:

$ git fetch 2>&1 | grep -F -- '->' | sed -E 's/^\s+\+\s+//'
 + 83e93db...b212eee branch-name -> origin/branch-name  (forced update)

Cause of error:

The case statement has values for new branch (git-dude#L85), new tag (git-dude#L89), and new commits (git-dude#L79). However, because ".." is also included in the output for a force-pushed branch, but not as the first column in the output, it's matched by the "new commit" case and awk assigns an output flag to the commit_range variable. For instance, if the output from the git fetch above is sent to awk (git-dude#L80) it would result in the following:

$ echo " + 83e93db...b212eee branch-name -> origin/branch-name  (forced update)" | awk '{ print $1 }'
+

In the case of a force-pushed branch, a + symbol is the first column in the output, not the abbreviated SHA-1 commit range, and thus the error. Similar issues may occur for other scenarios, such as deleted refs returned in the git fetch output which are prefixed with a - symbol.

digitaljhelms commented 12 years ago

Output flags returned by git fetch (based on git version 1.7.8):

Also note that in fast-forwards, the output contains ".." (2 dots) but in a forced update the output contains "..." (3 dots).

https://gist.github.com/1480257

ku1ik commented 12 years ago

Thanks for deep investigation. However for me it looks like this:

$ echo " + 83e93db...b212eee branch-name -> origin/branch-name  (forced update)" | grep -F -- '->' | sed -E 's/^\s+\+\s+//'
83e93db...b212eee branch-name -> origin/branch-name  (forced update)

No "+" at the start of string. I wonder why sed is not stripping out "+" for you...

digitaljhelms commented 12 years ago

AFAIK, not all sed's are equal; linux uses GNU sed, the BSDs use their own, etc. In looking at the manpage for sed on OS X, it looks like the POSIX.2 is being used for regex, so I'm guessing the regex used to strip the + would be different on OS X -- just a shot in the dark...

Either way, I figured it was safe to open this as an issue with what I had uncovered so far, considering more than a + could cause this issue.

digitaljhelms commented 12 years ago

Considering awk is already in use, maybe the sed regex can be swapped for awk to avoid cross-platform issues?

ku1ik commented 12 years ago

I appreciate that and I'll keep that issue open for some time, maybe more facts will come up.

As for sed and output flags you listed I've made small fix: https://github.com/sickill/git-dude/pull/23 Can you try this fix on your machine?

ku1ik commented 12 years ago

Pull #23 fixed the issue so I'm closing this one.