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
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
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
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>
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
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
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
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
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
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
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
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
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
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>
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
