git-review no longer honor url.insteadof

[JC Delay]

git-review no longer uses the URL replacement patterns defined in .gitconfig.

For example, let's assume the following config in ~/.gitconfig:

  [url "ssh://review.example.org:29418/"]
      insteadof = review:

Cloning a repository hosted on that remote works as expected (git clone actually expands "review:" in "ssh://review.example.org:29418/"):

  $ git clone review:project
  Cloning into 'project'...
  [..snip..] done.

However, using git-review will fail because it no longer expands "review:".

This change was introduced by commit cc28fb696 fixing bug #1177429:

  https://git.openstack.org/cgit/openstack-infra/git-review/commit/?id=cc28fb6969d8fd8dfe90c16429016b29a822b820

Previous "git-remote show" command was replaced by a call to git_config_get_value which in turn calls "git config --get".
Unfortunately, while "git-remote show" correctly expands the remote url:

  $ git remote show -n origin
  * remote origin
    Fetch URL: ssh://review.example.org:29418/project
    Push URL: ssh://review.example.org:29418/project
    [..snip..]

"git-config" doesn't:

  $ git config --get remote.origin.url
  review:project

This is only affecting the latest release of git-review, version 1.24, AFAICT.

An easy workaround it to rely on SSH config instead of Git config: remove the "insteadof" section in ~/.gitconfig and update ~/.ssh/config with something along the lines of:

  Host review
    Hostname review.example.org
    Port 29418

BTW, this is the recommended configuration as per Gerrit Code Review documentation:

  https://gerrit-documentation.storage.googleapis.com/Documentation/2.9/user-upload.html#push_create

And it allows the exact same syntax as the equivalent in Git configuration.
However, this potentially breaks users setup and is less consistent than the previous behavior IMHO.

Thank you,
JcD

[Jeremy Stanley (fungi)]

(Amusingly?) I wasn't even aware we had this feature in git-review. Given that we've introduced a regression, when this is fixed it should be accompanied by a regression test ensuring it doesn't happen again.

Thanks for the heads up!

[JC Delay (jcd-delay)]

Ping!

Not sure what to do next, but FYI I have a fix and reviews seem OK: https://review.openstack.org/#/c/109851
We're only waiting for the final approval.

Let me know if I can do anything else to help!

[JC Delay (jcd-delay)]

Fix was merged (https://review.openstack.org/#/c/109851).

[Anders Kaseorg (andersk)]

It should also honor url.*.pushInsteadOf:

https://review.openstack.org/#/c/160152