parse_git_show fails on non-english output from git

Bug #1177429 reported by Daniel Kinzler on 2013-05-07
26
This bug affects 5 people
Affects Status Importance Assigned to Milestone
git-review
Fix Released
High
Tim Landscheidt

Bug Description

When using git version 1.8.1.2 with LANG=de_DE.UTF-8, git review -d fails with a non-helpful error message like this:

> git review -d 62474
Cannot query patchset information
The following command failed with exit code 255
    "ssh -x -p None gerrit query --format=JSON --current-patch-set change:62474"
-----------------------
Bad port ' None'
-----------------------

That's because parse_git_show fails to find the remote URL. The output of "git remote show" with LANG=de_DE.UTF-8 is something like:

> git remote show -n gerrit
* externes Projektarchiv gerrit
  URL zum Abholen: ssh://<email address hidden>:29418/mediawiki/extensions/Wikibase.git
  URL zum Versenden: ssh://<email address hidden>:29418/mediawiki/extensions/Wikibase.git

But parse_git_show is checking for startswith( "Push" ), and finds nothing. And it doesn't check, but just goes on to try and parse an empty URL, which eventually leads to the port being "None" (not None!).

To fix this, git's output language should be forced to english. Or shelling out should be avoided alltoigether. And there should be error handling for the case that parsing fails.

I have attached a very basic fix that forces LANG=C in the environment in run_command_status. This will only work on linux, and isn't very pretty even there, but it should serve to save a lot of people a lot of pain...

Daniel Kinzler (brightbyte) wrote :
Tim Landscheidt (scfc) wrote :

Submitted https://review.openstack.org/#/c/64307 to directly access the Git configuration.

Error messages are really not helpful, e.g.

... [master]$ git review -s
Problems encountered installing commit-msg hook
cp: der Aufruf von stat für »:hooks/commit-msg“ ist nicht möglich: Datei oder Verzeichnis nicht gefunden

git-review 1.12 and 1.23 have this.

Jeremy Stanley (fungi) on 2014-07-01
Changed in git-review:
status: New → Fix Committed
importance: Undecided → High
assignee: nobody → Tim Landscheidt (scfc)
Jeremy Stanley (fungi) on 2014-07-03
Changed in git-review:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers