I never remember in the moment how to resolve the occasional "Missing
tree" error. Every time, I have to search and eventually rediscover
https://storyboard.openstack.org/#!/story/1332549 and --no-thin.
Now, on error check for the "Missing tree" message and suggest
--no-thin if it wasn't already enabled.
Change-Id: I7be68812b68f661219a6687fea54c48bc41adac8
Recent releases of Gerrit have evolved their use of the "topic"
functionality. Originally it was there merely for the convenince
of users. It could be used to make a group of related or similar
changes easy to query. In current releases of Gerrit, it serves
a much more deliberate purpose: if submitWholeTopic is enabled,
then merging one change in a topic will merge all of them
simultaneously.
Since this has very significant impacts to developer workflow,
git-review should no longer set a topic unless it is explicitly
requested by a user.
This change alters the behavior of git-review so that it will no
longer set a topic unless explicitly set by a user. It also
deprecates the 'notopic' and '-T' options. They are retained for
backwards compatability with existing scripts, but no longer have
any effect.
Change-Id: Ib83981a4952e1d6d68ebb8add11aabefbf3b2a4b
OpenDev is preparing to upgrade its Gerrit deployment to 3.9, so
make sure git-review is working with the latest 3.9 point release.
This also requires a newer JDK.
While we're at it, switch from testing with Python 3.11 to 3.12
since that's ending up in more user environments these days.
Also drop unit test jobs that exercise old Python with new Gerrit
and new Python with old Gerrit to simplify the test matrix, because
our Python 3.6 platform lacks new enough Java to support current
Gerrit releases.
And since the Python 3.12 testing uses a platform with newer tools,
remove an unnecessary Git command output assert where the output has
diverged significantly from version to version, trusting that Git
will do what it's asked to do because we're not interested in
testing it for regressions anyway.
Depends-On: https://review.opendev.org/920841
Change-Id: Idb1af841ba68a6f536117ef64f3bec011e905203
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
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
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
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
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
...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
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
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
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
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>
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
