Commit Graph

254 Commits

Author SHA1 Message Date
Jeremy Stanley b65c3c201c It's patchset not patch set
The Gerrit interface seems to prefer patchset over patch set, so get
consistent and just use the former in help texts and documentation.

Change-Id: I03d4b3c8fcdc0624dbf2dc48fcf8f21ecdb87135
2024-03-12 23:16:20 +00:00
Zuul d8440649e2 Merge "Don't keep incomplete rebase state by default" 2024-03-06 18:49:48 +00:00
Zuul ae64617a5f Merge "Vendor a copy of Gerrit's commit-msg Git hook" 2024-03-06 18:06:37 +00:00
Zuul 252ddada6d Merge "Don't make hook script read-only" 2024-03-06 17:44:17 +00:00
Jeremy Stanley da1e8a05f4 Don't keep incomplete rebase state by default
Dumping the user into a dirtied working tree after a failed rebase
attempt can be confusing, no matter how much contextual explanation
we provide when doing so. By default, run `git rebase --abort`
automatically so as to clean up from a failed test rebase, and then
let the user rebase again on their own if that's the state they want
to be in. Add a -K/--keep-rebase option to get the old behavior, and
mention it when we automatically abort in case the user wants to
just have git-review redo the rebase for them and leave things in
that incomplete state.

Change-Id: I7d7bfca1623a71a9b4fe445360d94fd6b039f040
2024-03-05 23:00:09 +00:00
Zuul df920f9f9b Merge "Test that the --version option returns something" 2024-03-05 15:26:50 +00:00
Zuul 52f09eac53 Merge "Use importlib.metadata instead of pkg_resources" 2024-03-05 00:26:51 +00:00
Jeremy Stanley 47c4096f06 Test that the --version option returns something
With the transition from pkg_resources to importlib.metadata,
finding the version string has become more complicated. Test it so
we don't break it in the future.

Change-Id: I20526518ab75ac7b58869d4126c4b5b2bc0df541
2024-03-04 23:39:35 +00:00
Jeremy Stanley d633541ecc Vendor a copy of Gerrit's commit-msg Git hook
Gerrit wants each commit message to include a unique identifier
string in a special footer line, so provides a commit-msg hook to
randomly generate and insert one. Traditionally, this file is served
directly from each Gerrit server and users retrieve it via SCP or
HTTPS to install a local copy in their clone of every repository.

Retrieving this file over the network has historically presented a
number of challenges: modern OpenSSH has deprecated the SCP protocol
while the mina-sshd library Gerrit uses hasn't implemented
compatible SFTP support, authentication failures can shadow some
clearer error handling later in git-review's workflow leading to
confusing error messages, and then there are the security concerns
with needing to trust the Gerrit server to supply a script which
will end up running locally on the developer's machine.

In order to address these problems, making git-review more robust
and secure, we embed a copy of the Gerrit upstream project's
commit-msg hook in the client itself and write that to disk by
default rather than pulling a remote copy. This approach does mean
that the user will end up with a frozen version of the script
contemporary with the git-review release they've installed (but its
function is simple and the implementation has changed very
infrequently). It may also break workflows for sites which rely on
users retrieving a customized commit-msg hook. For those reasons, a
command-line option is provided to restore the prior behavior.

Change-Id: Ia26abc781a281817115cb1cafcd5e7b78b383e39
2024-03-04 23:14:16 +00:00
Jeremy Stanley 4fddad6558 Don't make hook script read-only
When a hook script is not executable, Git will ignore its presence
in the hooks dir. To work around that, git-review checks the
downloaded commit-message hook to see whether it's executable and
then adjusts its permissions accordingly. This behavior has been
included since the initial release, but its naive implementation
wiped all existing permissions and then added only read and execute
for the file's owner (0o500/r-x------), leaving it set read-only.
This is overly-restrictive and can lead to minor annoyances when
deleting directories or for atypical multi-user and group ownership
scenarios due to ignoring the umask set for the process.

It is expected that, at this time, the described behavior is not
widely observed outside workflows which rely on fetching the hook
script over HTTP, as the SCP protocol preserves filesystem
permission flags from the source system, but it will have a much
broader impact in the future if git-review's default workflow shifts
away from SCP.

Replace the naive chmod implementation with one which adds execute
for anyone who already has read permission, but does not remove any
existing permissions, for example:

    0o644/rw-r--r-- .. 0o755/rwx-r-xr-x
    0o640/rw-r----- .. 0o750/rwx-r-x---

This new behavior should be more intuitive and less surprising for
users.

Change-Id: I48ac230df09bc802610cfef65bd9818c5b01673d
2024-03-04 23:14:11 +00:00
Zuul ae80cb6374 Merge "Add --wip as an alias to --work-in-progress" 2024-01-30 01:04:49 +00:00
Tim Burke 7b823c16e2 Use importlib.metadata instead of pkg_resources
...if available. It was added in Python 3.8, and marked no-longer-
provisional in Python 3.10.

Python 3.12 no longer pre-installs setuptools in virtual environments,
which means we can no longer rely on distutils, setuptools,
pkg_resources, and easy_install being available.

Fortunately, importlib.metadata covers the one use we have of
pkg_resources.

Change-Id: Iaa68282960a1c73569f916c3b00acf7f839b9807
2024-01-29 09:38:46 -08:00
Tim Burke b1de1c35f4 Fix flake8 issue
Change-Id: I9950da672d286de978e568d0fa6d2c350fd4ad7b
2024-01-29 09:09:47 -08:00
Antoine Musso 0502235d55 Add --wip as an alias to --work-in-progress
Since I am never sure what `-w` would do (maybe it is to raise a
warning?) and the long form `--work-in-progress` is a bit too long, add
a `--wip` long form option as an alias.

That seems to align nicely with Gerrit semantic and the `--ready`
option? I can then:
```
git-review --wip
git-review --wip
git-review --ready
```

Change-Id: I818b45d1186f8ff19ec4c77acd67839df525bc28
2024-01-24 12:12:22 +01:00
Zuul 3fbead8637 Merge "feat(cmd): add hashtag implementation" 2023-09-25 13:32:00 +00:00
Zuul ccaf223642 Merge "Warn rather than fail if HEAD already exists on the remote" 2023-09-25 13:04:31 +00:00
Jeremy Stanley d879220c10 Uncap test requirements
Newer stestr is needed to avoid failures on Python 3.11 like:

    nox > stestr run --color
    stestr: 'run --color' is not a stestr command.

Go ahead and uncap all the test dependencies. Also drop the pbr
addition for Python<=3.5 since we no longer test with anything older
than 3.6 anyway, and fix an exception that hacking seems to really
not want us to use.

Change-Id: I85585324c4e47d37b6de555110066fb8c29f293f
2023-09-20 15:05:25 +00:00
James Muir 702a1dcfb2 Warn rather than fail if HEAD already exists on the remote
assert_one_change() would fail if it detects that HEAD exists on the
remote (on any branch at all).  However, some gerrit projects are
configured to allow such reviews to be opened so long as HEAD does not
exist on the target branch.  Start warning rather than failing and let
the gerrit server do its own check.

Story: 2010887
Task: 48652
Change-Id: I5040aa24d78abec31054d7eeee9f6f27ce538988
2023-09-18 21:40:52 -04:00
Bartosz Dziewoński 21d9ceb5a5 Use GIT_SSH for the SSH executable
Git allows setting the SSH client via the GIT_SSH environmental variable.
Honor it in git-review as well.

Story: 1024054
Task: 537
Change-Id: I760335ebc8e45749227f4328aba9edbb52196d3b
Co-Authored-By: Dr. Jens Harbott <frickler@offenerstapel.de>
2023-08-16 12:58:41 +00:00
Clark Boylan 904d175d19 Test old and new Gerrit
This change updates the CI testing for git-review to test with Gerrit
3.4.4 and 3.7.1. This should give us good coverage of new and old Gerrit
behavior when making changes to git-review.

Note we leave 3.4.4 as the default in the test suite for local runs to
try and limit possibility of breaking old Gerrit when adding features to
git-review for new Gerrit.

Change-Id: I4a42eddec1e247cbb0af5e74b8f1cee0ad58a79a
2023-03-14 08:08:22 -07:00
Clark Boylan fcd9de135b Test Python bounds only
This bumps the newest python version tested to 3.11 and drops testing
for 3.7, 3.8, and 3.9. We only test the lower and upper python version
bounds.

We switch to an ed25519 ssh key instead of rsa because Ubuntu Jammy
defaults to rsa + sha2 and Gerrit 3.4.4 does not understand this. This
results in a golden site version bump.

Change-Id: I3989460575fa9c389233244683318a6cfded68fe
2023-03-14 08:07:27 -07:00
Jeremy Stanley f02b41df74 Simplify test output strings for new Gerrit
In adapting the integration tests to output from new Gerrit
versions, we observed possible nondeterministic ordering or content
in some of the fields. We don't need to be nearly so specific in the
strings we match anyway, so simplify them for robustness against
variable output from the service and improved future flexibility.

Change-Id: Icbd70d9c8c9da3637cfa5722a636d140597fe623
2023-03-08 20:42:38 +00:00
Zuul 0ecdd60a0a Merge "Upgrade testing to Gerrit 3.4.4" 2023-03-08 00:38:28 +00:00
Jeremy Stanley 4c6ab44916 Upgrade testing to Gerrit 3.4.4
An addition has been proposed to leverage Gerrit's "Cc" feature, but
in order to test this we need a newer version of the server.

Newer Gerrit versions require HTTP basic auth instead of digest for
the REST API, so switch our tests to use that when uploading SSH
keys.

A newer JDK (11) is required, but this version should still be
available on our configured test platforms.

We update the test Gerrit server config to not try to send email. This
prevents annoying tracebacks from ending up in the Gerrit logs on test
failures.

We update test_cloned_repo and test_multiple_changes to look for strings
that newer Gerrit emits on successful push. Git review passes these
through to users and the tests look for them to determine if pushes were
successful.

Because of an SSH host key negotiation regression in 3.4.5, stick
with 3.4.4 for now:

https://bugs.chromium.org/p/gerrit/issues/detail?id=16215

Finally we remove skipsdist from tox.ini so that `which git-review` can
find git-review installed to the test venv. Tox v4 won't install the
project into the venv if skipsdist is set.

Change-Id: I540950b93356b1efbc34bca976bfb3134f47a599
2023-03-07 15:56:59 -08:00
Tama McGlinn 52752c85d3 Add message option
Change-Id: I34741e0baf6dabb5cbf3ab648bf2e4c2a28a9399
2022-11-11 09:46:12 +00:00
Andy Ladjadj ce909662d7 feat(cmd): add hashtag implementation
Change-Id: I1b4ab28bea9a8b2bb7501b1de7e3a080c87fe46d
2022-08-25 17:14:07 +00:00
Jeremy Stanley 7009d7d03d Clarify that test rebases are not kept
There has been a long-standing misconception that git-review pushes
automatically rebased changes by default. It does not, but our
documentation and context help have been less than clear on that
point, contributing to this impression. Try to do a better job of
explaining that the default rebasing performed by git-review is
purely exploratory, and used only to notify users about possible
merge conflicts with their target branch before pushing a change.

Change-Id: I3c841af5ff9430a0de4d9dc9526dd3be6ab53ad2
2022-07-15 18:21:27 +00:00
Clark Boylan b39c812e01 Fix get_git_version on OS X
Apparently Apple's `git --version` provides different output than
Linux's. Improve the version parsing by splitting on all whitespace and
taking the exact element that should be the version out of that rather
than relying on the version we want being a suffix of the command
output.

Story: 2010002
Change-Id: I40356ee81b98c1210de348e51335a20be48bec1d
2022-04-19 08:44:45 -07:00
Zuul 82a2c8df2e Merge "Enforce minimum git version" 2022-04-18 18:45:18 +00:00
Zuul 75561d1a72 Merge "Fix submitting signed patches" 2022-04-18 18:36:34 +00:00
Ian Wienand a47f5afbfc Enforce minimum git version
Id4528209f1cd500afd06e2e61eb5689022251118 introduced a minimum git
version.  Abstract our existing check and setup a global with the
local git version for tests.  Add a minimum version check.

Change-Id: I9d1de11269758a453ecc8dde0a4c631d8e762a91
2022-04-12 16:30:12 +10:00
Zuul 85fee5c197 Merge "Fix git_review.cmd.CommandFailed: <exception str() failed>" 2022-04-11 17:48:35 +00:00
Dr. Jens Harbott 053b629472 Fix submitting signed patches
When a commit is signed and the git config contains the setting
log.ShowSignature=True, even the "--oneline" git log output for it will
include multiple lines (the output from gpg verifying the signature),
thus fooling us into assuming that multiple commits are to be submitted.
Override the option to make sure we always get one line per commit only.

Signed-off-by: Dr. Jens Harbott <harbott@osism.tech>
Change-Id: Id4528209f1cd500afd06e2e61eb5689022251118
2022-04-11 16:59:59 +00:00
Sunyeop Lee e328db38c0 Fix git_review.cmd.CommandFailed: <exception str() failed>
Neither the CommandFailed nor ChangeSetException classes have
docstrings, so self.__doc__ is initialized to None and can't be
trivially combined with other strings (nor would there be any point
in doing so). Just drop these unnecessary references.

Change-Id: I1f17325baa69522a4471f5bcf270a74038ad8642
2022-04-11 16:56:17 +00:00
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