Support for other remotes named gerrit

git-review expects a remote named "gerrit" which corresponds to the Gerrit CLI for your code review system. Something akin to:

    git remote add gerrit ssh://<username>@gerrit.wikimedia.org:29418/mediawiki/extensions/UploadWizard.git

By cloning with -o you're adding a remote named "gerrit" which corresponds to your Web mirror of the repo, and this is certain to create all manner of problems for git-review. Do you consider this a design flaw or lack of clarity in documentation?

the clone method i used in step 1 above was recommended in the tutorial http://www.mediawiki.org/wiki/Git/Tutorial#Download_MediaWiki_example_extension_using_Git

the use of -o gerrit was what was recommended here http://www.mediawiki.org/wiki/Git/Workflow#Clone_the_repository in order to avoid issues with git review.

i combined the two methods and found the issue, however, if i use the ssh:// version of the url, git-review has no problem with setup.

i don't understand the issues related to having gerrit as the sole remote for the repository; i imagine someone at wikimedia does. what are the issues?

in any case, the issue with git-review seems to be with this method, parse_git_show, and testing if port is not None on line 177. the method, parse_git_show, returns (hostname, username, str(port), project_name) and so, as far as i can tell, the test on line 177 of the code will pass when no port is derived from the remote url, because None and None as a string are different and thus the scp command tries to use -P None which throws the error.

It seems some of those workflow docs assume patterns which are incompatable with git-review while others suggest use of git-review (always a challenge with collaborative documentation). Based on the discussion pages for those articles I see Saper was trying to address some of that problem earlier this year in https://review.openstack.org/5720 (now abandoned). Some of the MW integration folks hang out with us in #openstack-ci so I'll see if they've got insight into this.

The way git-review is commonly used with OpenStack is documented in our wiki: http://wiki.openstack.org/GerritWorkflow

Oh, and as for the issues with str(port) when port is None, I think we may have one or more bugs related to default port handling and leaving out the -P option when no port is specified in the remote (to support users with .netrc and similar SSH configuration overrides). So we'll probably solve both in the same way. Unfortunately there are likely to be conflicting needs between wanting to let the SSH client pick the default--in which case it will try to use 22/tcp if it hasn't been overridden--and falling back to the default Gerrit SSH port instead.

thanks jeremy

Er, meant #openstack-infra (on freenode) earlier.

Revisiting the Wikimedia documentation, it was pointed out to me that the example in http://www.mediawiki.org/wiki/Git/Workflow#Clone_the_repository does indeed demonstrate cloning from an ssh:// URL for the remote rather than the https:// URL you used. It might perhaps help if that document were clearer on the point that this technique must use SSH for any remote named "gerrit" to be compatible with git-review.

The default port handling strangeness (or at least something which will require us to solve it) is already reported in bug 1075751, but in this case fixing that None == str(None) comparison would not have solved your issue anyway.

yes, it's true that the workflow documentation mentions the ssh:// version of the repo and using that url allows me to continue with using the wikimedia gerrit server with git-review, however, i still think there's a bug, different than bug 1075751.

forget about the documentation for a moment and follow the steps i mentioned. why is git-review trying to use port None?

looking at the code, lines 177-180, the port used in the scp command s/b "", but its not getting to that part of the if statement because 'None' != None; at least this is what i think is happening. anyway, i really don't know enough yet about gerrit or python so i probably don't understand this well enough yet.

the only thing that concerns me at the moment, is that you mentioned above that it's not good to check-out the repo with -o gerrit and thus have only the one git remote, gerrit. so i'm now a bit confused/concerned about why that's the recommended method in the workflow document ...

I didn't mean to imply that checking out the repo with -o (so that you can use a name other than "origin") was a bad idea. Mediawiki's Gerrit admins might suggest this so that they don't have to help users troubleshoot separate origin fetch problems, or because they might have branches for some projects which aren't updated on their Web mirrors in real time. The problem here is that git-review very much intends to use the "gerrit" remote from your local git configuration so that it can decide how to reach your Gerrit server's SSH interface to push changes. If you set it to an HTTP-based mirror of the repo, that likely won't be possible regardless.

As for the port=="None" thing... it appears you're getting this because urlparse.urlparse() tries to break down your "https://gerrit.wikimedia.org/r/p/mediawiki/extensions/UploadWizard.git" URL into a host and port, but the port isn't mentioned in that URL so an object of type None is returned for the port instead. The conditional you're seeing (if parsed_url.scheme == "ssh" and port is None) doesn't even apply in this case because your parsed_url.scheme is "https" anyway. I don't really see a lot in the way of sane alternatives... we could add a separate scheme check for "https" and default the port to 443 (and that likely needs to be done at some point), but it seems unlikely you'd be able to push changes to that. I would suggest confirming with the Gerrit admins for Mediawiki whether that is a recommended configuration.

I'll go ahead and resurrect this bug as a wishlist request for supporting additional "gerrit" remote protocols besides SSH, but unless you can get confirmation there are Gerrit servers accepting changes via other protocols this will likely remain on the back burner.

for me, what would have helped, would have been to see an error message along the lines of “git-review cannot work with the url used to clone this repository. make sure that the clone url is an ssh url or ???”. this would have given me an idea of how i might correct the issue git-review had with the repo.

thanks for all of your followup jeremy.

As of Friday's 1.22 release, it's possible to have an existing remote named "gerrit" used for other tasks, and choose the name of the remote with which you intend to interact with your Gerrit server. See the "defaultremote" option for details.