Hub: Hub expands alias that git wouldn't

[aweibell]

The command

hub branch -d my_local_branch

results in this error:

fatal: cannot use -a with -d

I consider this a major bug. I did not use -a, but somehow that is added by hub!

Also, when running hub branch I only expect to see my local branches, but all the remote branches are listed as well. Looks like hub adds the -a option for all branch commands? Users should be allowed to add the -a option themselves when they find it applicable.

Plain old git branch works as expected in both situations mentioned above.

[mislav]

This is strange. Hub shouldn't ever add extra arguments, especially not to branch.

1. Which version of hub do you use?
2. Do you have any git aliases related to branch?
3. Do you get the same result with command git branch -d ..., or does that work properly?
4. What does HUB_VERBOSE=1 hub branch -d ... show?

[aweibell]

I agree that it is strange ;)

 1. I experienced the issue with both hub-linux-amd64-2.2.3.tgz and hub-linux-amd64-2.3.0-pre8.tgz
 2. I thought I had eliminated all aliases by testing without scm-breeze and oh-my-zsh, but it appears someone (a script I've tested or something, I assume) has set some git aliases without my knowledge: :open_mouth:

[alias]
br = branch
branch = branch -ra
bra = branch -ra

So that changes the situation, of course. BUT:

 3. That's exactly what made me look at it as a hub bug. The git command behaves different from the hub command here!

~> git branch
* master

~> hub branch
* master
  remotes/origin/master

 4. Hub seems to evaluate the alias:

HUB_VERBOSE=1 hub branch
$ git config alias.branch
$ git branch -ra
* master
  remotes/origin/master

~> HUB_VERBOSE=1 hub bra
$ git config alias.bra
$ git branch -ra
* master
  remotes/origin/master

~> HUB_VERBOSE=1 hub br
$ git config alias.br
$ git branch
* master

I did the same exercise with git, and it does not seem to bother about the alias!

~> GIT_TRACE=1 git branch
22:07:33.953967 git.c:344               trace: built-in: git 'branch'
* master

~> GIT_TRACE=1 git br
22:07:41.121808 git.c:561               trace: exec: 'git-br'
22:07:41.121840 run-command.c:334       trace: run_command: 'git-br'
22:07:41.122636 git.c:278               trace: alias expansion: br => 'branch'
22:07:41.123019 git.c:344               trace: built-in: git 'branch'
* master

~> GIT_TRACE=1 git bra
22:07:46.458184 git.c:561               trace: exec: 'git-bra'
22:07:46.458219 run-command.c:334       trace: run_command: 'git-bra'
22:07:46.458953 git.c:278               trace: alias expansion: bra => 'branch' '-ra'
22:07:46.459230 git.c:344               trace: built-in: git 'branch' '-ra'
* master
  remotes/origin/master

...and of course: removing the branch=branch -ra alias made hub behave like git on this one.

Thank you for pointing at git aliases, and sorry for blaming hub! :disappointed:

[mislav]

Huh, it seems that git won't expand an alias if it's named exactly like a built-in command. This might be a feature to prevent shooting yourself in the foot by defining a global alias that does something destructive?

Anyway, good catch for figuring out that hub acts different than git in this case. It's indeed a bug. I'm labeling it as such.

[akshatgoel]

I would like to take a stab at this.

Initial thoughts :
Hub tries to resolve alias for git only, not external ones. If no alias is found, it tries the command as-is. https://github.com/github/hub/blob/master/commands/runner.go#L93

This check for alias currently seems to be serving no purpose as it only handles Git aliases which can anyways be taken by git.

However, I don't know any historic reasons of expanding aliases before calling Git.

[mislav]

This check for alias currently seems to be serving no purpose as it only handles Git aliases which can anyways be taken by git.

You are almost correct. However, there is one subtle but important purpose that this alias resolution step solves and why it's necessary in hub: someone might have, for example, pr aliased to pull-request. In that case, it's important to resolve the aliases before passing the raw command to git, because git doesn't know anything about pull-request.

So, maybe the approach we should take is: resolve the command in case it's an alias for one of hub's custom commands (pull-request, fork, ci-status, etc). If the alias doesn't point to any of these commands, we can ignore the alias and pass the invocation to git in its original format.

Thanks for tackling this!

[akshatgoel]

I hear what you are saying, which I think is similar to git approach. However, I am wary of this approach for cases where hub and git share commands. I am working on the fix and will be testing for these cases. Any other cases, you would like me to keep in mind?

[mislav]

Hmm you bring up a valid point. I've searched and found why we added support for git aliases in the first place: https://github.com/github/hub/issues/54

So, we do need to expand aliases that point to either a custom hub command or a git command that hub extends. But, the alias itself must not correspond to any existing git-* command, coming from either hub, git core, or anywhere in PATH.

No matter how I look at this, I can't seem to find a way to avoid compiling a list of all existing git-* commands by looking inside git --exec-path and scanning PATH for git-* executables. Alternatively, we could scan the result of git help -a which already does this for us.

[akshatgoel]

I think parsing "hub help -a" is the best possible way right now (I am using this regex to list the commands : hub help -a | grep "^ [a-z]" | tr ' ' '\n' | grep -v "^$") . This will give list for both git and hub's in-built commands. We can then use logic similar to git's to skip in-built commands.

I was thinking of case where git parses aliases recursively but I don't think it does so. So, I guess a simple check for in-built commands we get from help should suffice.

What do you think? Sounds like good proposal?

[mislav]

@akshatgoel Yes, your logic checks out, except:

• We can't shell out to hub help -a. That would be calling hub recursively. Instead, let's shell out to git help -a instead.
• Your regex assumes that lines with command names are indented with a single space, but on my git they are indented with 2 spaces.
• We can get a list of hub's commands (both extended and custom) via:

go names := []string{} for name, _ := range CmdRunner.All() { names = append(names, name) }

[akshatgoel]

We can't shell out to hub help -a. That would be calling hub recursively. Instead, let's shell out to git help -a instead.

Didn't think about that. Thanks.

Your regex assumes that lines with command names are indented with a single space, but on my git they are indented with 2 spaces.

It assumes 2 space as indentation ("^ [a-z]" has two spaces after caret). Can you, please, try and run the same?

[mislav]

No I only see 1 space after caret. Also does hexdump:

$ pbpaste | hexdump -C
00000000  5e 20 5b 61 2d 7a 5d                              |^ [a-z]|

(Space is "20")

[akshatgoel]

Well... That's awkward. Looks like the one I am pasting is different from what we have in the comment. Original regex has two spaces, so I guess, nothing to worry about :)

Also, I am relying on Go's regex parsing and manipulation instead of grep and tr, which may not be available on Windows.

[mislav]

Ah sorry, you did write 2 spaces, but since it's in a <code> tag, the browser collapses all whitespace into a single space.