On Fri, 2019-09-13 at 13:55 +0000, Jeremy Stanley wrote:
On 2019-09-13 10:53:16 +0000 (+0000), VanCutsem, Geoffroy wrote: [...]
I suspect this is a problem at the git-review Python code, not a problem with Gerrit itself. I took a (very) quick look at the code and noticed it uses the 'argparse' module to parse all arguments. This is the issue I believe we're hitting: https://bugs.python.org/issue22433 ... unfortunately it's been sitting there, opened for 5 years now.
Note: enclosing the string in quotes should help but I've seen reports that it would only work when using '--topic='.
@Dominig, if you can spare a couple of minutes, I'd be curious to know if the following command works for you: git review --topic="initial creation Doc page Navigate Git Code story/2005184"
It's a good guess, but no I tried all manner of passing the option and also more robust sorts of quoting directly in the parameter handling routines in git-review's source code. Where I'm stuck is that the serialization Gerrit uses for "push options" to communicate a change topic during a git push operation seems to have no means of encoding spaces. These do not work to set a topic of "topic spaces":
git push gerrit HEAD:refs/for/master%topic=topic spaces
git push gerrit HEAD:refs/for/master%topic='topic spaces'
git push gerrit HEAD:refs/for/master%topic="topic spaces"
git push gerrit HEAD:refs/for/master%"topic=topic spaces"
git push gerrit HEAD:"refs/for/master%topic=topic spaces"
git push gerrit "HEAD:refs/for/master%topic=topic spaces"
...and so on. As best I can tell, Git refspecs simply cannot contain spaces. There is a converse problem as well, that git review -d to retrieve a change from Gerrit wants to create a local branch incorporating the change topic string, and Git doesn't allow spaces in branch names either. So I think we're going to just need to have git-review check for spaces when pushing and either transliterate or reject them, but also have it check for them when downloading changes and perform substitutions in that direction as well.
Makes sense to me. Thanks for the additional investigation. Geoffroy