220 Commits

Author SHA1 Message Date
Zuul
bf760f221f Merge "Force use of scp rather than sftp when possible" 2022-04-11 05:43:23 +00:00
Zuul
bb58ee198c Merge "Treat ^C as "no" at confirmation prompt" 2022-04-11 05:18:26 +00:00
Jonathan Rosser
5bfaa4a6f3 Force use of scp rather than sftp when possible
OpenSSH has deprecated its use of scp/rcp protocol in favor of SFTP,
which the embedded Apache mina-sshd in widely-deployed Gerrit
versions does not yet support. The default officially changed in
OpenSSH 9.0 (some distributions, such as Fedora and CentOS, switched
their default behavior to this as early as OpenSSH 8.7 or 8.8),
leading to a ``subsystem request failed on channel 0`` error during
commit-msg hook retrieval. Now git-review will attempt to detect
whether scp's -O option is available to force use of the legacy
scp/rcp protocol, and apply it if so.

Change-Id: Ib64c03c3e12a3a8390e38f6ca9393db3b3c2a9e3
2022-04-10 12:39:22 +00:00
Tim Burke
8cbd515e0c Treat ^C as "no" at confirmation prompt
The user intent is clear.

Change-Id: Ibdaa2f95e7417619f651d6f41fbf15a357839bf3
2022-04-07 16:39:41 -07:00
Steve Kowalik
1912e685ff Switch to unittest.mock
Since mock is a backport of the standard library, and we no longer
support Python 2, we can use the standard library, and drop one external
dependency.

Change-Id: I798c85f1581f4562908c10cd1b58134cdcb40281
2022-03-08 16:13:27 +10:00
Pierre Riteau
7182166ec0 Fix use of removed --preserve-merges option
The --preserve-merges (-p) option was replaced by --rebase-merges (-r).
This fixes the following error when using git version 2.34.0:

    Errors running git rebase -p -i remotes/gerrit/stable/xena
    fatal: --preserve-merges was replaced by --rebase-merges

In order to keep compatibility with git < 2.18.0 we detect the git
version and use the old --preserve-merges flag when the version is older
than 2.18.0.

Co-Authored-By: Clark Boylan <clark.boylan@gmail.com>
Change-Id: I04de3d0f20aa6bafcf746b7706d61dd9b9af296c
2021-11-19 08:25:34 -08:00
Jeremy Stanley
6c3f134ac3 Ignore unstaged/uncommitted submodule changes
When checking for unstaged or uncommitted changes to avoid the test
rebase (which could cause data loss for users of git.autostash),
it's still fine if there are unstaged or uncommitted changes in
submodules since those won't be rebased. Have the git diff
invocations explicitly ignore submodules, and also add regression
tests which demonstrate it's working.

This fixes a regression originally introduced by change
Iabb8387c9db59a7d02ebfd43b688e7bb93d3159f.

Change-Id: I20d602e86537b573ac1f9788221215047a594f83
2021-07-07 16:08:06 +00:00
Florian Haas
c40eb491e6 Support the Git "core.hooksPath" option when dealing with hook scripts
Previously, git-review would assume that the Git repository's hook
directory is .git/hooks, relative to the root of the checkout. This
assumption breaks if the user has set the core.hooksPath option on the
repository (or, for that matter, in ~/.gitconfig or /etc/gitconfig).

core.hooksPath can either be set to an absolute path, in which case it
is to be interpreted as-is, or to a relative path, in which case it
should be interpreted as relative to the root of the checkout.

Introduce a new convenience function to suss out the correct path, and
use it in places where the reference to .git/hooks was previously
hard-coded.

Reference:
https: //git-scm.com/docs/git-config#Documentation/git-config.txt-corehooksPath

Depends-on: I0f0f44e57a100420d8e6d2eaec7dbb5d77b654af
Change-Id: Id8a3ac464ff75e6d8207f198089f018cc790eca5
2021-06-21 16:24:11 +02:00
Clark Boylan
39cd763d5d Add option for disabling thin pushes
There is a long standing issue with C Git pushing to Gerrit and Jgit
where the occasional push will fail because the negotiated packs are
missing a tree object. This happens very occasionally but when it does
it would be nice to be able to point users at an easy workaround.
Pushing with --no-thin is that workaround.

Note that --no-thin is much less efficient so shouldn't be used by
default.

This old bug, https://bugs.launchpad.net/git-review/+bug/1332549, has
details but it seems to affect current C git and Gerrit+Jgit.

Change-Id: Id6ba52a656a14c921acab1b14ef668e6251245da
2021-04-12 11:38:23 -07:00
Zuul
281af6b879 Merge "Remove comments for unstaged/uncommitted tests" 2021-03-03 00:46:34 +00:00
Zuul
c5a83977e5 Merge "Overhaul Python package metadata and OpenDev URLs" 2021-03-02 17:53:06 +00:00
Zuul
e14c72ab7a Merge "Don't test rebasing with unstaged changes" 2021-03-02 17:16:24 +00:00
Zuul
8aeb461d5d Merge "Add test helpers for unstaged/uncommitted changes" 2021-03-01 19:36:12 +00:00
Jeremy Stanley
006b0ee3d7 Remove comments for unstaged/uncommitted tests
The tests for pushing with unstaged or uncommitted edits had some
left-over code comments from an earlier revision, which were no
longer accurate but also entirely unnecessary. Remove them.

Change-Id: Icfc98d426fa994cce6f1ec290acfa5c0d55123bf
2021-03-01 19:28:07 +00:00
Jeremy Stanley
7e2f99b2d0 Increase test timeout to 5 minutes
A 2-minute global test timeout was added to the base test class back
in 2014, to prevent tests from hanging until the job timeout is hit.
This has served us well, but with updates to Gerrit and increasing
complexity of our tests, we often get a slew of test timeouts on
some CI nodes (ad on my workstation for that matter!). Increase this
to 5 minutes, which should still serve the intended purpose while
providing more time for tests to complete on less-performant
systems.

Change-Id: I9673ce3cf18f9e4c96fd7e9f96f5ddce2ef6d957
2021-02-26 23:11:46 +00:00
Jeremy Stanley
319953d5ab Overhaul Python package metadata and OpenDev URLs
Modernize our package metadata in the following ways:

* switch from description-file to long_description with the file
  attribute, and specify an explicit content type and encoding

* replace the home-page parameter with the newer general url one

* add specific labelled project links for improved navigation from
  PyPI's summary sidebar

* add commandline keyword to help folks searching

* use the specific license metadata in addition to the corresponding
  trove classifier for it

* make sure wheels when built also incorporate the LICENSE and
  AUTHORS files so that we're not distributing them without a copy
  of the license text

* stop flagging wheels as "universal" now that git-review no longer
  supports Python 2.7

* drop the old Sphinx integration config for PBR now that it's no
  longer needed

https://setuptools.readthedocs.io/en/latest/userguide/declarative_config.html

Also update old openstack.org URLs throughout contributor docs and
examples/comments to newer opendev.org counterparts. Remove the old
redundant HACKING.rst file as well as a lingering MANIFEST.in from
the times before PBR was a thing. Replace the CONTRIBUTING.rst with
a shorter one cribbed from bindep. Add the test profile to the one
entry in bindep.txt to make it more apparent that's not a runtime
dependency of git-review. Adjust some old "OpenStack, LLC."
copyrights as indicated by the foundation's "Legal Issues FAQ."

Change-Id: Ie45d4d73ba7b5a860f09cc4f1d849587761d846c
2021-02-26 20:45:24 +00:00
Jeremy Stanley
d83d99cadc Don't test rebasing with unstaged changes
For safety, attempts to push a commit with unstaged or uncommitted
changes in the worktree will be caught and an error reported, rather
than leaving it up to ``git rebase`` to spot them. This addresses a
situation where users enabling "rebase.autostash" would otherwise
experience data loss when the test rebase is subsequently reset.

Change-Id: Iabb8387c9db59a7d02ebfd43b688e7bb93d3159f
Task: #38921
2021-02-26 16:20:49 +00:00
Jeremy Stanley
e8d5404320 Add test helpers for unstaged/uncommitted changes
In order to better test detection of unstaged and uncommitted
changes, add base test methods to create changes and not stage or
commit them, ensuring they can result in an unstaged diff or staged
but uncommitted edits. To ensure that these work, use them within
the simple change creation method to perform the actual file edit.

Change-Id: Ib698d0057a404f073490d1683a8eef8d0c143122
2021-02-26 16:20:41 +00:00
Jeremy Stanley
95df6748c8 Test with Gerrit 2.13
More recent OpenSSH/OpenSSL versions no longer want to communicate
with the mina-sshd in Gerrit 2.11. In order to pave the way for
testing on distros new enough to supply Python 3.9 packages, upgrade
the version of Gerrit used in the functional testsuite.

Because 2.12 and later moved user SSH keys into Git, we can no
longer rely on the same gsql bootstrapping method to inject them
into the golden site. However, if we init the server in --dev mode,
we can use the default admin/secret test administrator account
instead of creating a new administrator account ourselves (we just
need to post an SSH public key for it to the REST API at runtime).

Also stop precreating host keys and running a reindex, partially
reverting I1c5db09c53a56ffd856da24decf29566e86f9d87, which we did to
keep 2.11 working on some older (but then newer) platforms. Keep the
reindexing in the golden site though, as this remains necessary and
likely also provides some performance benefits.

Add/improve some code comments around relevant bits, so further
updates or rewrites will be less of a struggle for maintainers.

Change-Id: I1245519d912a385c4b9bd09da1728d9b7933802b
2021-02-24 00:58:53 +00:00
Jeremy Stanley
deb6d08645 Create test projects with positional argument
In the years since our test framework was created, the
create-project CLI syntax has changed and the --name parameter is
deprecated in favor of using a positional argument. Switching to
this newer syntax becomes necessary for newer Gerrit versions which
no longer recognize the --name option.

Change-Id: I342de827d6a7d7af418f4b1eac54f5dce7c672ed
2021-02-23 23:30:00 +00:00
Sorin Sbarnea
b461d8ff1d Remove test site dirs before doing copytree
Avoids errors when leftovers from old tests prevent starting new
tests.

Change-Id: If7541de708ec6b610f378219fb249cdd09918ed3
2021-01-27 15:19:53 +00:00
Daniel Lublin
a0963a1b51 Allow choosing field for author in named branch
This adds a setting for choosing which of {name, email, username} to use
as "author" when constructing the name for the branch to where a change
is downloaded.

The rationale is that sometimes the given "name" is just long and
unwieldy (when displaying the branch in the shell prompt, for example),
and one may be already used to the "username".

Change-Id: Ieed465f69ed0c0864979a92f609bd8f58cd8e883
2021-01-22 14:22:35 +00:00
Zuul
23980a8b9d Merge "Fix bug in git_credentials()" 2021-01-21 16:34:17 +00:00
Zuul
84264b9c88 Merge "Fix "git-review -d" erases work directory if on the same branch as the change downloaded" 2021-01-20 18:41:02 +00:00
cornelius
9b585e00d8 Fix "git-review -d" erases work directory if on the same branch as the change downloaded
Story: 1096057
Task: 562

Change-Id: I3fed1bc02bc29da5295e436edeaedc42c3293589
2021-01-20 17:44:41 +00:00
Alexander Szakaly
b0bf084d66 Fix bug in git_credentials()
git_credentials() was converting the stdin argument to bytes. However
according to the Python3 documentation subprocess.communicate() expects
a string when universal_newlines=True is passed to Popen.

The symptom was that git review would fail in p.communicate(stdin) on
line 156 with the message "'bytes' object has no attribute 'encode'".
This was observed with Python version 3.6.9 when running git-review
against a gerrit repo over https, where the repo requires username and
password to authenticate.

Change-Id: I0c0314c3f7b0eb631e72e4ac187a9d443a2bc82b
2021-01-20 17:44:14 +00:00
Mattias Jernberg
96cfd92c89 Allow the default of notopic to be configurable
In our setup branch names are not designed to contain topics and are in
certain cases also dictated by other tools. To avoid having git-review
constantly overwrite the topics it is useful to set --no-topic to be the
default state when running git-review.

Change-Id: I1e77e062d73d47f1cceeb34b3418c074d06c9005
2021-01-20 17:43:55 +00:00
Sorin Sbarnea
3757503895 Drop support for py27
Officially removes support for py27, which was already broken for
3+ months (unmaintained test tools).

Change-Id: I38c966d4e1680592f5cd94695051710695d41f71
Related: https://github.com/testing-cabal/subunit/pull/32
2021-01-13 14:10:31 +00:00
Sorin Sbarnea
f4e0e70bb0 Bring zuul configuration in-tree
Change-Id: I42dd5b0e1a5ac5a83f4370790ada9617f1122f6b
Depends-On: https://review.opendev.org/c/openstack/project-config/+/763808
2021-01-13 13:16:27 +00:00
Zuul
1a58ace6cb Merge "Make it possible to specify who is notified" 2020-02-05 18:03:31 +00:00
Miklos Vajna
c37c73a73e Make it possible to specify who is notified
This is documented at
<https://gerrit-review.googlesource.com/Documentation/user-upload.html#push_options>.

Change-Id: Ifbc0ec1225052cb804fcf537f5a071cad29e8328
2020-01-28 10:25:36 +01:00
Zuul
3366196601 Merge "Install commit hook into submodules" 2020-01-17 11:25:13 +00:00
David Ostrovsky
02491ca845 Discontinue support for draft workflow
As of gerrit 2.15 and later, draft workflow is replaced with
work-in-progress and private workflow. See this CL: [1] and this
issue upstream: [2].

Even though support for draft worklfow was removed, the drafts magic
draft option was preserved, and is mapped to creation of the private
change when pushed first time, or creation of change edit on subsequent
pushes. These behaviour was alaways controvesial, but was kept in place
because 2 major Gerrit clients: repo and git-review were still
referencing drafts magic branch. In upcoming gerrit releases the support
for drafts magic branch option is discontinued, and thus removed from
both repo and git-review: [3].

[1] https://gerrit-review.googlesource.com/c/gerrit/+/97230
[2] https://crbug.com/gerrit/6880
[3] https://gerrit-review.googlesource.com/c/gerrit/+/238898

Sem-Ver: api-break
Change-Id: I08a590d42e1ebaa230da960cd192c0b1df528332
2020-01-17 16:36:41 +09:00
Stephen Finucane
ddbd0ba1d9 trivial: Update to hacking 2.x
Fixes an issue we're seeing in the python3-based gate plus some other
random things.

Change-Id: I417c0a7669090ee3419c406024f6f3e3289b4c4b
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
2020-01-16 15:12:58 +00:00
Monty Taylor
f9340df184 Install commit hook into submodules
If there are submodules, the commit hook should be installed into them,
according to https://gerrit-review.googlesource.com/Documentation/dev-crafting-changes.html#git-commit-settings

git submodule foreach is a no-op if there are no submodules, so just run
it directly.

Change-Id: I559e2786c84be9975cc082bce80b32c8e47a9fb5
2019-08-26 09:47:57 +02:00
Hannu Hartikainen
98cc896bc2 Push with --no-follow-tags
When the git configuration value push.followTags is set and a repo has
tags, git-review pushes are rejected:

    ! [remote rejected]   TAG -> TAG
       (cannot combine normal pushes and magic pushes)

This change ensures that git-review never pushes with followTags.

Change-Id: Ifbd13284b16bad1165e73d25b99f17344180d423
2019-03-29 12:28:06 +02:00
Monty Taylor
853f3bffb3 Use remote_url instead of remote for download
If usepushurl is set, then it's likely that the pushurl is pointing
to the code review system while the normal url is pointing to a read
only mirror. If the read-only mirror in question doesn't contain
the gerrit refs (like opendev.org currently) then git fetch origin
won't work properly. However, we already calculate the correct
URL at the top of the function for use in query_reviews, and fetch
will happily work with a full url rather than a named remote.

Update the function to use the remote_url so that usepushurl works
even if the mirror url does not contain refs/changes.

Change-Id: Ib72afe97e65cb1dcaf95e28450dfe6e7d5f88965
2019-03-18 14:37:27 +00:00
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