183 Commits

Author SHA1 Message Date
Jan Kundrát
9b68a44f38 Support usernames that contain '@' and ssh Git URLs
Our company-internal Gerrit uses federated logins (Shibboleth/SAML), and
the login names happen to include the at-signs. This needed a patch to
Gerrit ([1], released in 2.13.10) to allow them. Cloning using regular
git tools as well as through the gertty work, but `git-review --list`
complained because it couldn't parse these URLs.

[1] https://gerrit-review.googlesource.com/c/94914/

Change-Id: I6c1c75a585184ee3fb2847c1e6d30802e53f8b4c
2019-02-19 10:11:32 +00:00
Zuul
afa54af210 Merge "Fix wrong and misleading "using default: None" in --verbose mode" 2018-12-27 11:25:24 +00:00
Marc Herbert
d5f02d61cb Fix wrong and misleading "using default: None" in --verbose mode
When something is not found in git config, the --verbose option logs the
wrong value "None", example:

2018-12-12  Running: git config --get gitreview.branch
2018-12-12  using default: None
2018-12-12  Running: git config --get gitreview.scheme
2018-12-12  using default: None
2018-12-12  Running: git config --get remote.gerrit.pushurl
2018-12-12  using default: None
2018-12-12  Running: git config --get remote.gerrit.url
2018-12-12  result: ssh://marc@review.openstack.org:29418/openstack-infra/git-review

From a --verbose user perspective this is plain wrong for (at least) all the
options defined in the DEFAULTS list.

Change --verbose message to look like this instead:

2018-12-12  Running: git config --get gitreview.branch
2018-12-12  Config['branch'] = master
2018-12-12  Running: git config --get gitreview.scheme
2018-12-12  Config['scheme'] = ssh
2018-12-12  Running: git config --get remote.gerrit.pushurl
2018-12-12  Running: git config --get remote.gerrit.url
2018-12-12  ... remote.gerrit.url = ssh://marc@review.openstack.org:29418/openstack-infra/git-review

If git_config_get_value('new_option',... ) is ever invoked in the future
with a not None, default="fubar" parameter then --verbose will print these
lines:

2018-12-19  Running: git config --get gitreview.new_option
2018-12-19  ... nothing in git config, returning func parameter: fubar

This logging issue is especially misleading considering the many levels of
defaults and fallbacks: git config x3; .gitreview; DEFAULTS list,
git_config_get_value(default=...) parameter, options parser logic, etc.

Change-Id: I6cee46e88b90b8f11689be3875d64ec5e577f12f
2018-12-20 06:14:16 +00:00
Marc Herbert
11cf3bae58 Add hint to just "git remote rename origin gerrit" when no .gitreview
... because it Just Works with absolutely zero configuration when cloning
with ssh://

Change-Id: I7a413704c4a567e397b5bb6075677175b8997704
2018-12-18 15:36:08 -08:00
Zuul
a2bea9f57e Merge "test_uploads_with_nondefault_rebase: fix git screen scraping" 2018-12-14 16:21:56 +00:00
Zuul
d416d103f6 Merge "tests/__init__.py: ssh-keygen -m PEM for bouncycastle" 2018-12-10 00:17:47 +00:00
Marc Herbert
6f31931b87 test_uploads_with_nondefault_rebase: fix git screen scraping
Newer versions of git quote branch names in their output; at least git
2.16.3 does. This causes all test_uploads_with_nondefault_rebase tests to
fail like this:

testtools.matchers._impl.MismatchError:
 'Branch test_branch set up to track remote branch maint from origin.'
 not in
  u"Switched to a new branch 'test_branch'\n\
 Branch 'test_branch' set up to track remote branch 'maint' from 'origin'."

Add a testtools.matchers.MatchesRegex() to support both styles.

Change-Id: I9f1417c53de2f7d638e845f553df3bd426a4c750
2018-12-05 14:56:37 -08:00
Zuul
e9fede805a Merge "Avoid UnicodeEncodeError on python 2" 2018-12-05 11:15:49 +00:00
Marc Herbert
d41f5d7d0a tests/__init__.py: ssh-keygen -m PEM for bouncycastle
From: https://www.openssh.com/txt/release-7.8 change log:

 * ssh-keygen(1): write OpenSSH format private keys by default
   instead of using OpenSSL's PEM format. The OpenSSH format,
   supported in OpenSSH releases since 2014 and described in the
   PROTOCOL.key file in the source distribution, offers substantially
   better protection against offline password guessing and supports
   key comments in private keys. If necessary, it is possible to write
   old PEM-style keys by adding "-m PEM" to ssh-keygen's arguments
   when generating or updating a key.

This fixes all tests failing with this error:

[2018-12-04 17:46:24,130] WARN  org.apache.sshd.common.keyprovider.FileKeyPairProvider :
   Unable to read key /home/mherber2/pip/src/git-review/.gerrit/site-5123/etc/ssh_host_rsa_key
java.io.IOException: unrecognised object: OPENSSH PRIVATE KEY
	at org.bouncycastle.openssl.PEMParser.readObject(Unknown Source)
	at org.apache.sshd.common.keyprovider.FileKeyPairProvider.doLoadKey(FileKeyPairProvider.java:124)

Change-Id: Id10f9b5be928f2ba57847fa32814e9db979375f5
2018-12-04 18:12:06 -08:00
Sorin Sbarnea
d181f04353 Use six for cross python compatibility
- Fixes linting error related to undefined unicode on python3.
- Simplifies code

Change-Id: Ib40237e0fb7156b084dfdab47be69ccad09547fe
2018-11-26 19:17:28 +00:00
Sorin Sbarnea
138e9a25e3 Avoid UnicodeEncodeError on python 2
Python2 has default encoding as ascii which means
that is likely that some print() commands would
fails with UnicodeEncodeError.

This hack changes default encoding in order to
avoid such errors.

Change-Id: I4e21e6e32d4bb815693b7d6ce35efb6a5cca2fc2
2018-11-09 10:18:54 +00:00
Fabio Porcedda
c243d80af9 As suggested by pep8 don't compare boolean values or empty sequences
Refactoring to improve code style as suggested by pep8.

- "rc" and "status" variables cannot be None beacause the process is
  terminated after the "run_command_status"

- It's better to ignore an empty "username"

Change-Id: I8a34f4ab9e05d91786ce6d62e4db96811787b633
2018-11-07 12:16:58 +00:00
Ilya Etingof
d94e3a3431 Improve exit code implementation
Unnecessary dynamic GitReviewException.EXIT_CODE attribute
lookup refactored into static to benefit static analyzers.

Default process exit code changed from 127 (command not found error)
to 1 (general error)

Change-Id: I1fcb583a740bf32c4427a587e208d099712a7bc4
2018-11-06 12:26:11 +00:00
Stephen Finucane
af955c932e Don't set topic when submitting no-topic patches
Change-Id: I8f8880791ad7e46fb9e18623ab8bd295457424b2
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
2018-10-16 15:13:07 +01:00
Stephen Finucane
03768832c4 Remove auto-branch name
The feature to configure branch names based on the presence of "bug",
"lp", "blueprint" or "bp" in the commit message is overly elaborate and
very OpenStack specific. Even with this, it hasn't been updated to keep
up with the times as many projects have migrated to Storyboard, which
isn't handled here.

Given that it frequently does the wrong thing and likely doesn't apply
to anyone outside of the OpenStack ecosystem, the wisest thing it to
simply remove the feature. This is a break in behavior but it seems
better than adding yet another flag for something that many users will
want enabled by default.

Change-Id: I82ecc1719de5c87d59bbfe73a042917e6559da1e
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Story: 1130330
Task: 566
Story: 2001247
Task: 5777
2018-10-16 09:53:40 +01:00
Pete Zaitcev
56779ebe21 Clean up vestigal scripting in cmd.py
This was brought up by an RPM lint checker. It complaned about
a non-script being executable. Apparently the shebang was not
removed when we went all PBR.

Change-Id: I60098f334d136fe98d3492a3578516310e31296c
2018-09-19 17:39:18 +00:00
Zuul
185fb8da38 Merge "Fix compare_review's use of fetch_review" 2018-09-19 17:34:42 +00:00
Zuul
f6260701fa Merge "Use new %topic=XXXX syntax for topic pushes" 2018-09-19 17:09:43 +00:00
Clark Boylan
3c46c33ec0 Fix compare_review's use of fetch_review
fetch_review was updated to take project config information so that
projects hosted over http below some root (eg
https://gerrit.org/gerrit/here) could be properly fetched against. But
in adding this we broke the compare_review functionality which is the
other user of fetch_review.

Fix this by passing project config info into compare_review so that it
can pass that along to fetch_review.

Additionally we need to compare like types when looking at patchset
numbers so coerce them to ints when comparing.

Change-Id: I98fbf31821dc3a9162700725dec07bc7685ea5ca
2018-09-18 21:50:20 +01:00
Steve Kowalik
6587639df2 Always print failure case when testing remotes
Rather than requiring verbose to be specified when testing remote URLs,
always print the failure case to allow easier debugging, for example,
with unsigned CLAs.

Change-Id: I37e2d8b6b70a573a4a3f9091205d31100e3bf9f2
2018-09-14 15:28:43 -06:00
Han-Wen Nienhuys
dcbfb32b05 Use new %topic=XXXX syntax for topic pushes
The old syntax is undocumented and slated to be removed.

Uniformize handling of % options. Previously, combining options could
yield destination refs that look like

   refs/for/BRANCH%wip%private

which tries to switch on a non-existent option "wip%private".

Change-Id: Ia4e97eafbf685fcba78d95370dae08254a2c718b
2018-09-13 07:03:56 +00:00
Zuul
ea82a1598b Merge "Testing getting specific patchset" 2018-08-22 02:21:04 +00:00
Zuul
f561bf3ff4 Merge "work-in-progress and private workflow in Gerrit 2.15" 2018-08-20 18:10:20 +00:00
Zuul
59c771d7cb Merge "support review URLs as download arguments" 2018-08-20 15:31:29 +00:00
Zuul
211eb98a2a Merge "Fix git review -d M,N with later gerrit" 2018-08-20 15:26:01 +00:00
Derek Waldner
694f532ca8 Update default gerrit namespace
According to latest Gerrit documentation, 2.15.3, refs/for/'branch'
should be used when pushing changes to Gerrit. Change the default
behavior to refs/for/* instead of refs/publish/* which has been
deprecated. This removes a warning from Gerrit remotes running the
latest version when pushing changes without commandline options.

Change-Id: Ibaddfda96457a72c54ca9c91a8ad25d14b88c582
2018-08-16 13:21:55 +00:00
Doug Hellmann
7de9ffc933 support review URLs as download arguments
Rather than forcing users to copy only part of a URL to a review, allow
them to paste the whole URL and then parse it for them to find the
change id.

Change-Id: Ic012c86b2b477d17354bf8d119e1d4b698378dd7
Signed-off-by: Doug Hellmann <doug@doughellmann.com>
2018-06-13 22:09:10 +01:00
Doug Hellmann
ed3c79e452 expand help for --download option
Show the syntax for requesting a specific version of a patch.

Change-Id: Id72935c715a7b5c17722400dde6175b99e61860b
Signed-off-by: Doug Hellmann <doug@doughellmann.com>
2018-04-25 11:41:49 -04:00
Abbas Yazdanpanah
6f50b591db work-in-progress and private workflow in Gerrit 2.15
In Gerrit 2.15 two new workflow states(`work-in-progress` and
`private`) are introduced. This patch tries to implement them.

Gerrit documentation says:

To push a private change or to turn a change private on push the private
 option can be specified:

  git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/master%private

Omitting the private option when pushing updates to a private change
doesn’t make change non-private again. To remove the private flag from a
 change on push, explicitly specify the remove-private option:

  git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/master%remove-private

To push a wip change or to turn a change to wip the work-in-progress
(or wip) option can be specified:

  git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/master%wip

Omitting the wip option when pushing updates to a wip change doesn’t
make change ready again. To remove the wip flag from a change on push,
 explicitly specify the ready option:

  git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/master%ready

https://gerrit-documentation.storage.googleapis.com/Documentation/2.15/intro-user.html#private-changes
https://gerrit-documentation.storage.googleapis.com/Documentation/2.15/intro-user.html#wip

Change-Id: Ia093e4a691fa8eb17c798473a79a97694202fd03
2018-04-22 18:41:01 +04:30
Thomas Hisch
aeeff97487 Fix git review -d M,N with later gerrit
It seems the "number" field returned via gerrit JSON can be a string
(2.13) or a integer (~2.14).  Ensure we can handle both.

Change-Id: I3cbda6c07343332aba592bd96fd8545f08a2cbfe
2018-03-13 20:36:34 +11:00
Ian Wienand
244731bd46 Testing getting specific patchset
Add a test for "change,patchset" downloading

Change-Id: I0cc340b9dc24d7f4457e336176649d7fc7994624
2018-02-08 10:32:02 +11:00
Ian Wienand
bbea22f773 Fix output printing with python3
I'm not sure what print_safe_encoding is trying to do
(Ie834931a549175471af029a6ec4d5794543d8c92).

  print(str.encode(sys.stdout.encoding, 'replace'))

means that print() ends up getting a bytes-object encoded in the
output encoding.  Thus print() is then going to output this as raw
bytes... not only is unicode going to come out as "\x..\x.." etc, but
even newlines get squished and you end up with stuff like

  $ /tmp/gitreview3/bin/git-review
  b'remote: \rremote: Processing changes: updated: 1, refs: 1\rremote: Processing changes: updated: 1,

I think this problem stems from not ensuring that incoming data
(output of git) is encoded as a string.  By setting
universal_newlines=True in run_command_status() we automatically
ensure that any incoming data is a unicode str, not bytes.

The final section, with the message about rhbz#1058167, appears to be
related to running git under an alternative locale where it's output
is translated.  I1bf1124f0b09d6658a7b0703e3b9e74ed80f4eea changed
things to run the commands under C locale, so there shouldn't be
unicode in here I wouldn't think.  But, by ensuring it's just a
string, we can print() it safely anyway.

Change-Id: I596dcba317ecfbaf437cb72d9850580ed6765fce
2017-11-17 09:33:39 +11:00
Clark Boylan
f918bf76d2 Handle http queries below /
Previously git review could not properly query Gerrit servers via http
if the root of the Gerrit api was below /. This is because it was always
rooting the changes query api at /changes instead of /foo/changes if
hosted at /foo.

To fix this we read the project name from the config so that we can
remove the project name suffix from the urls then append changes/ to the
resulting url to get a properly rooted query url.

Note this was never a problem with ssh because ssh can't be hosted as
some subpath. Everything is rooted with ssh and gerrit.

Change-Id: I46e21dfdbbb7f60aa89a7b028501c0d953ae1d7f
2017-11-15 17:37:11 -08:00
Zuul
ef47e1f553 Merge "Support git 2.15 and newer" 2017-11-15 04:32:14 +00:00
Zuul
3686e0967c Merge "show the config value result after fetching it in verbose mode" 2017-11-15 03:58:08 +00:00
Clark Boylan
d3d66a4715 Support git 2.15 and newer
The 2.15 release of git drops support for `git branch --set-upstream`
and replaces it with `git branch --set-upstream-to` which was added in
git 1.8. Move from --set-upstream to --set-upstream-to in order to
support git 2.15 and beyond. Note that this specifically affects the
`git review -d` functionality as it is what needs to update the
upstream.

This does drop support for git 1.7 and older though as this option did
not exist then. Polling linux distros and OS X the oldest git I can find
is on CentOS 7 which has 1.8 so this should be fine.

Story: 2001266
Co-Authored-By: Harry Mallon <Harry.Mallon@codex.online>
Change-Id: I5ded1ab39bb2c76bdc38655299bac11b4584e138
2017-11-05 15:26:18 -08:00
Doug Hellmann
84511b1124 show the config value result after fetching it in verbose mode
We show the command used to retrieve the config value, but don't show
what we find. Fix that, and also show cases where we are going to use
the default setting.

Change-Id: I53ee9ea6e3af4e951587c0845c8333874238f0c2
Signed-off-by: Doug Hellmann <doug@doughellmann.com>
2017-10-19 18:27:19 -04:00
Daniel Lublin
54ca2e399c Actually output the warning
Change-Id: If47f0be53e4b80eae9f03e48acb299c686984d95
2017-08-10 09:12:18 +00:00
Christoph Settgast
b61f5b2ec9 Fix listing changes over SSH for 2.14
The SSH query API changed from 2.13 to 2.14 from

{"number":"1", ...}

to

{"number":1, ...}

so we need to stringify everything in order to do len, encode etc. on
it.

Change-Id: I06bdd1b4b1181d00a27ce1e76f97baed8cde1284
Story: 2001095
2017-07-25 08:53:48 +02:00
Jenkins
089e8e64b1 Merge "Refactor displaying of reviews" 2017-07-21 16:05:47 +00:00
Jenkins
b3e0cac1aa Merge "Added topic field to the list output" 2017-07-21 16:04:12 +00:00
Jenkins
54221b1075 Merge "Switch to string format to avoid interpolation issues" 2017-07-21 15:54:05 +00:00
Sorin Sbarnea
72175104dd Better username detection for add_remote()
Change-Id: Ia083ec9d5f19bff4944080559672b5947d4e43ee
2017-02-23 15:42:51 +00:00
David Caro
60f46a5a65 Refactor displaying of reviews
Split the displaying of reviews into a separate class to make it easier
to alter the output and accommodate different display options.

Change-Id: I6007259686fc15ae21ae6843faa4ff7ad545cef1
2016-11-16 15:13:49 +00:00
David Caro
ef4711d695 Added topic field to the list output
Now you can pass multiple '-l' options like '-ll' and when doing so, it
will show also the topic of the patches

Change-Id: Iabc1107d66a17fde751f90d7fa33f15fe54c7f58
Signed-off-by: David Caro <dcaroest@redhat.com>
Co-Authored-By: Darragh Bailey <dbailey@hpe.com>
2016-11-16 15:13:46 +00:00
Darragh Bailey
efb5126e67 Switch to string format to avoid interpolation issues
Convert to using python format to align and interpolate values to
ensure the correct output on python 2 & 3 when dealing with unicode and
str.

Interpolating strings using encode into '%s' and subsequently
interpolating the results into a second string results in the output
printing embedded byte strings on python 3.

 $ git-review -l
 b'226894'  b'master'  b'Add pem key parameter (WIP)'
 b'199225'  b'master'  b'support urls as change-id input

Which occurs because after converting a string to unicode bytes, it was
then interpolated using '%s'.

Switching to using '.format()' method takes care of converting the
output to the correct string format for python 2 & 3 automatically.

Change-Id: Ia962d18bda34e0244fb05636a7be263045ecb256
2016-11-16 15:10:18 +00:00
Jenkins
67dbbc2e59 Merge "Set a default EXIT_CODE for GitReviewException" 2016-08-30 14:33:40 +00:00
Jenkins
051f37c1c8 Merge "Support git without git credential" 2016-08-30 14:33:35 +00:00
Jenkins
59f87fe3d6 Merge "Fix no_git_dir UnboundLocalError in except block" 2016-08-30 14:26:37 +00:00
Jenkins
535202f579 Merge "fix encoding issue on Windows" 2016-08-30 14:25:30 +00:00