Merge "Send hover events to context buttons"
diff --git a/Documentation/dev-processes.txt b/Documentation/dev-processes.txt
index 68e56ba..e43e021 100644
--- a/Documentation/dev-processes.txt
+++ b/Documentation/dev-processes.txt
@@ -296,14 +296,32 @@
test that verifies that the security vulnerability is no longer present.
+
Review and approval of the security fixes must be done by the Gerrit
-maintainers. Verifications must be done manually since the Gerrit CI doesn't
-build and test changes of the `gerrit-security-fixes` repository (and it
-shouldn't because everything on the CI server is public which would break
-the embargo).
+maintainers.
+
Once a security fix is ready and submitted, it should be cherry-picked to all
branches that should be fixed.
+. CI validation of the security fix:
++
+The validation of the security fixes does not happen on the regular Gerrit CI,
+because it would compromise the confidentiality of the fix and therefore break
+the embargo.
++
+The release manager maintains a private branch on the
+link:https://gerrit-review.googlesource.com/admin/repos/gerrit-ci-scripts[gerrit-ci-scripts,role=external,window=_blank] repository
+which contains a special build pipeline with special visibility restrictions.
++
+The validation process provides feedback, in terms of Code-Style, Verification
+and Checks, to the incoming security changes. The links associated
+with the build logs are exposed over the Internet but their access limited
+to only those who are actively participating in the development and review of
+the security fix.
++
+The maintainers that are willing to access the links to the CI logs need
+to request a time-limited (maximum 30 days) nominal X.509 certificate from a
+CI maintainer, which allows to access the build logs and analyze failures.
+The release manager may help obtaining that certificate from CI maintainers.
+
. Creation of fixed releases and announcement of the security vulnerability:
+
A release manager should create new bug fix releases for all fixed branches.
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-change-info-last-update.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-last-update.png
deleted file mode 100644
index 93c296a..0000000
--- a/Documentation/images/gwt-user-review-ui-change-screen-change-info-last-update.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-change-info-owner.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-owner.png
deleted file mode 100644
index 3d73ef7..0000000
--- a/Documentation/images/gwt-user-review-ui-change-screen-change-info-owner.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-quick-approve.png b/Documentation/images/gwt-user-review-ui-change-screen-quick-approve.png
deleted file mode 100644
index 638fc2f..0000000
--- a/Documentation/images/gwt-user-review-ui-change-screen-quick-approve.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-column.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-column.png
deleted file mode 100644
index b599f6d..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-column.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png
deleted file mode 100644
index c041311..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png
deleted file mode 100644
index ea14a21..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png
deleted file mode 100644
index 8406ce8..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png
deleted file mode 100644
index 1fd2033..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png
deleted file mode 100644
index 043c1ff..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png
deleted file mode 100644
index 7373b2f..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png
deleted file mode 100644
index f817d66..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png
deleted file mode 100644
index c767452..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png
deleted file mode 100644
index cbadd26..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-search.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-search.png
deleted file mode 100644
index e69bb0d..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-search.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png
deleted file mode 100644
index a4b019a..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-quick-approve.png b/Documentation/images/user-review-ui-change-screen-quick-approve.png
new file mode 100644
index 0000000..f692d07
--- /dev/null
+++ b/Documentation/images/user-review-ui-change-screen-quick-approve.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png b/Documentation/images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png
new file mode 100644
index 0000000..1eb7665
--- /dev/null
+++ b/Documentation/images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-file-level-comment.png b/Documentation/images/user-review-ui-side-by-side-diff-screen-file-level-comment.png
new file mode 100644
index 0000000..66c46b7
--- /dev/null
+++ b/Documentation/images/user-review-ui-side-by-side-diff-screen-file-level-comment.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-preferences.png b/Documentation/images/user-review-ui-side-by-side-diff-screen-preferences.png
new file mode 100644
index 0000000..e008f2b
--- /dev/null
+++ b/Documentation/images/user-review-ui-side-by-side-diff-screen-preferences.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-reviewed.png b/Documentation/images/user-review-ui-side-by-side-diff-screen-reviewed.png
new file mode 100644
index 0000000..e2a7957
--- /dev/null
+++ b/Documentation/images/user-review-ui-side-by-side-diff-screen-reviewed.png
Binary files differ
diff --git a/Documentation/index.txt b/Documentation/index.txt
index e56e7ca..dc94b14 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -75,6 +75,7 @@
. link:config-reverseproxy.html[Reverse Proxy]
. link:config-auto-site-initialization.html[Automatic Site Initialization on Startup]
. link:pgm-index.html[Server Side Administrative Tools]
+. link:repository-maintenance.html[Repository Maintenance]
. link:user-request-tracing.html[Request Tracing]
. link:note-db.html[NoteDb]
. link:config-accounts.html[Accounts on NoteDb]
diff --git a/Documentation/repository-maintenance.txt b/Documentation/repository-maintenance.txt
new file mode 100644
index 0000000..1672436
--- /dev/null
+++ b/Documentation/repository-maintenance.txt
@@ -0,0 +1,116 @@
+= Gerrit Code Review - Repository Maintenance
+
+== Description
+
+Each project in Gerrit is stored in a bare Git repository. Gerrit uses
+the JGit library to access (read and write to) these Git repositories.
+As modifications are made to a project, Git repository maintenance will
+be needed or performance will eventually suffer. When using the Git
+command line tool to operate on a Git repository, it will run `git gc`
+every now and then on the repository to ensure that Git garbage
+collection is performed. However regular maintenance does not happen as
+a result of normal Gerrit operations, so this is something that Gerrit
+administrators need to plan for.
+
+Gerrit has a built-in feature which allows it to run Git garbage
+collection on repositories. This can be
+link:config-gerrit.html#gc[configured] to run on a regular basis, and/or
+this can be run manually with the link:cmd-gc.html[gerrit gc] ssh
+command, or with the link:rest-api-projects.html#run-gc[run-gc] REST API.
+Some administrators will opt to run `git gc` or `jgit gc` outside of
+Gerrit instead. There are many reasons this might be done, the main one
+likely being that when it is run in Gerrit it can be very resource
+intensive and scheduling an external job to run Git garbage collection
+allows administrators to finely tune the approach and resource usage of
+this maintenance.
+
+== Git Garbage Collection Impacts
+
+Unlike a typical server database, access to Git repositories is not
+marshalled through a single process or a set of inter communicating
+processes. Unfortuntatlely the design of the on-disk layout of a Git
+repository does not allow for 100% race free operations when accessed by
+multiple actors concurrently. These design shortcomings are more likely
+to impact the operations of busy repositories since racy conditions are
+more likely to occur when there are more concurrent operations. Since
+most Gerrit servers are expected to run without interruptions, Git
+garbage collection likely needs to be run during normal operational hours.
+When it runs, it adds to the concurrency of the overall accesses. Given
+that many of the operations in garbage collection involve deleting files
+and directories, it has a higher chance of impacting other ongoing
+operations than most other operations.
+
+=== Interrupted Operations
+
+When Git garbage collection deletes a file or directory that is
+currently in use by an ongoing operation, it can cause that operation to
+fail. These sorts of failures are often single shot failures, i.e. the
+operation will succeed if tried again. An example of such a failure is
+when a pack file is deleted while Gerrit is sending an object in the
+file over the network to a user performing a clone or fetch. Usually
+pack files are only deleted when the referenced objects in them have
+been repacked and thus copied to a new pack file. So performing the same
+operation again after the fetch will likely send the same object from
+the new pack instead of the deleted one, and the operation will succeed.
+
+=== Data Loss
+
+It is possible for data loss to occur when Git garbage collection runs.
+This is very rare, but it can happen. This can happen when an object is
+believed to be unreferenced when object repacking is running, and then
+garbage collection deletes it. This can happen because even though an
+object may indeed be unreferenced when object repacking begins and
+reachability of all objects is determined, it can become referenced by
+another concurrent operation after this unreferenced determination but
+before it gets deleted. When this happens, a new reference can be
+created which points to a now missing object, and this will result in a
+loss.
+
+== Reducing Git Garbage Collection Impacts
+
+JGit has a `preserved` directory feature which is intended to reduce
+some of the impacts of Git garbage collection, and Gerrit can take
+advantage of the feature too. The `preserved` directory is a
+subdirectory of a repository's `objects/pack` directory where JGit will
+move pack files that it would normally delete when `jgit gc` is invoked
+with the `--preserve-oldpacks` option. It will later delete these files
+the next time that `jgit gc` is run if it is invoked with the
+`--prune-preserved` option. Using these flags together on every `jgit gc`
+invocation means that packfiles will get an extended lifetime by one
+full garbage collection cycle. Since an atomic move is used to move these
+files, any open references to them will continue to work, even on NFS. On
+a busy repository, preserving pack files can make operations much more
+reliable, and interrupted operations should almost entirely disappear.
+
+Moving files to the `preserved` directory also has the ability to reduce
+data loss. If JGit cannot find an object it needs in its current object
+DB, it will look into the `preserved` directory as a last resort. If it
+finds the object in a pack file there, it will restore the
+slated-to-be-deleted pack file back to the original `objects/pack`
+directory effectively "undeleting" it and making all the objects in it
+available again. When this happens, data loss is prevented.
+
+One advantage of restoring preserved packfiles in this way when an
+object is referenced in them, is that it makes loosening unreferenced
+objects during Git garbage collection, which is a potentially expensive,
+wasteful, and performance impacting operation, no longer desirable. It
+is recommended that if you use Git for garbage collection, that you use
+the `-a` option to `git repack` instead of the `-A` option to no longer
+perform this loosening.
+
+When Git is used for garbage collection instead of JGit, it is fairly
+easy to wrap `git gc` or `git repack` with a small script which has a
+`--prune-preserved` option which behaves as mentioned above by deleting
+any pack files currently in the preserved directory, and also has a
+`--preserve-oldpacks` option which then hardlinks all the currently
+existing pack files from the `objects/pack` directory into the
+`preserved` directory right before calling the real Git command. This
+approach will then behave similarly to `jgit gc` with respect to
+preserving pack files.
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
+
+SEARCHBOX
+---------
diff --git a/Documentation/user-review-ui.txt b/Documentation/user-review-ui.txt
index 98ec22d..1ea8bd8 100644
--- a/Documentation/user-review-ui.txt
+++ b/Documentation/user-review-ui.txt
@@ -93,13 +93,6 @@
image::images/gwt-user-review-ui-change-screen-change-info.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info.png"]
-- [[change-owner]]Change Owner:
-+
-The owner of the change is displayed as a link to a list of the owner's
-changes that have the same status as the currently viewed change.
-+
-image::images/gwt-user-review-ui-change-screen-change-info-owner.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info-owner.png"]
-
- [[reviewers]]Reviewers:
+
The reviewers of the change are displayed as chip tokens.
@@ -163,10 +156,6 @@
+
image::images/gwt-user-review-ui-change-screen-change-info-cannot-merge.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info-cannot-merge.png"]
-- [[update-time]]Time of Last Update:
-+
-image::images/gwt-user-review-ui-change-screen-change-info-last-update.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info-last-update.png"]
-
- [[actions]]Actions:
+
Depending on the change state and the permissions of the user, different
@@ -665,7 +654,7 @@
comments; a summary comment is only added if the reply popup panel is
open when the quick approve button is clicked.
-image::images/gwt-user-review-ui-change-screen-quick-approve.png[width=800, link="images/gwt-user-review-ui-change-screen-quick-approve.png"]
+image::images/user-review-ui-change-screen-quick-approve.png[width=800, link="images/gwt-user-review-ui-change-screen-quick-approve.png"]
[[history]]
=== History
@@ -746,28 +735,12 @@
image::images/gwt-user-review-ui-side-by-side-diff-screen-project-and-file.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-project-and-file.png"]
[[side-by-side-mark-reviewed]]
-The checkbox in front of the project name and the file name allows the
+The checkbox in front of the file name allows the
patch to be marked as reviewed. The link:#mark-reviewed[Mark Reviewed]
diff preference allows to control whether the files should be
automatically marked as reviewed when they are viewed.
-image::images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png"]
-
-[[scrollbar]]
-The scrollbar shows patch diffs and inline comments as annotations.
-This provides a good overview of the lines in the patch that are
-relevant for reviewing. By clicking on an annotation one can quickly
-navigate to the corresponding line in the patch.
-
-image::images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png"]
-
-[[gaps]]
-A gap between lines in the file content that is caused by aligning the
-left and right side or by displaying inline comments is shown as a
-vertical red bar in the line number column. This prevents a gap from
-being mistaken for blank lines in the file
-
-image::images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png"]
+image::images/user-review-ui-side-by-side-diff-screen-reviewed.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png"]
[[patch-set-selection]]
In the header, on each side, the list of patch sets is shown. Clicking
@@ -926,52 +899,10 @@
[[file-level-comments]]
=== File Level Comments
-Comments that apply to a whole file can be added on file level.
+File level comments are added by clicking the 'File' header at the top
+of the file.
-File level comments are added by clicking on the comment icon in the
-header above the file.
-
-image::images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png"]
-
-Clicking on the comment icon opens a comment box for typing the file
-level comment.
-
-image::images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png"]
-
-[[search]]
-=== Search
-
-For searching within a patch file, a Vim-like search is supported.
-Typing `/` opens the search box. Typing in the search box immediately
-highlights matches in the patch file with a yellow background. Using
-JavaScript regular expressions in the search term is supported. The
-search is case insensitive. After confirming the search by `ENTER` one
-can navigate between the matches by `n` / `N` to go to the next /
-previous match. Skipped lines are automatically expanded if they
-contain a match and one navigates to it.
-
-For additional possibilities to search please check the
-link:http://www.vim.org/docs.php[Vim documentation,role=external,window=_blank]. There are other
-useful ways to search, e.g. while the cursor is on a word, pressing `*`
-or `#` searches for the next or previous occurrence of the word.
-
-Searching by `Ctrl-F` finds matches only in the visible area of the
-screen unless the link:#render[Render] diff preference is set to `Slow`.
-
-image::images/gwt-user-review-ui-side-by-side-diff-screen-search.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-search.png"]
-
-[[key-navigation]]
-=== Key Navigation
-
-Vim-like commands can be used to navigate within a patch file:
-
-- `h` / `j` / `k` / `l` moves the cursor left / down / up / right
-- `0` / `$` moves the cursor to the start / end of the line
-- `gg` / `G` moves to cursor to the start / end of the file
-- `Ctrl-D` / `Ctrl-U` scrolls downwards / upwards
-
-Please check the link:http://www.vim.org/docs.php[Vim documentation,role=external,window=_blank]
-for further information.
+image::images/user-review-ui-side-by-side-diff-screen-file-level-comment.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png"]
[[diff-preferences]]
=== Diff Preferences
@@ -981,27 +912,10 @@
preferences. The diff preferences can be accessed by clicking on the
settings icon in the screen header.
-image::images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png"]
-
-The diff preferences popup allows to change the diff preferences.
-By clicking on the `Save` button changes to the diff preferences are
-saved permanently. Clicking on the `Apply` button applies the new
-diff preferences to the current screen, but they are discarded when the
-screen is refreshed. The `Save` button is only available if the user is
-signed in.
-
-image::images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png"]
+image::images/user-review-ui-side-by-side-diff-screen-preferences.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png"]
The following diff preferences can be configured:
-- [[theme]]`Theme`:
-+
-Controls the theme that is used to render the file content.
-+
-E.g. users could choose to work with a dark theme.
-+
-image::images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png"]
-
- [[ignore-whitespace]]`Ignore Whitespace`:
+
Controls whether differences in whitespace should be ignored or not.
@@ -1010,11 +924,11 @@
+
All differences in whitespace are highlighted.
+
-** `At Line End`:
+** `Trailing`:
+
Whitespace differences at the end of lines are ignored.
+
-** `Leading, At Line End`:
+** `Leading, Trailing`:
+
Whitespace differences at the beginning and end of lines are ignored.
+
@@ -1028,11 +942,7 @@
- [[columns]]`Columns`:
+
-Sets the preferred line length. At this position a vertical dashed line
-is displayed so that one can easily detect lines the exceed the
-preferred line length.
-+
-image::images/gwt-user-review-ui-side-by-side-diff-screen-column.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-column.png"]
+Sets the preferred line length. At this position, lines are wrapped.
- [[lines-of-context]]`Lines Of Context`:
+
@@ -1049,84 +959,28 @@
If many lines are skipped there are additional links to expand the
context by ten lines before and after the skipped block.
+
-image::images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png"]
+image::images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png"]
- [[syntax-highlighting]]`Syntax Highlighting`:
+
Controls whether syntax highlighting should be enabled.
+
The language for the syntax highlighting is automatically detected from
-the file extension. The language can also be set manually by selecting
-it from the `Language` drop-down list.
-+
-image::images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png"]
+the file extension.
-- [[whitespace-errors]]`Whitespace Errors`:
+- [[whitespace-errors]]`Show trailing whitespace`:
+
-Controls whether whitespace errors are highlighted.
+Controls whether trailing whitespace is highlighted.
- [[show-tabs]]`Show Tabs`:
+
Controls whether tabs are highlighted.
-- [[line-numbers]]`Line Numbers`:
-+
-Controls whether line numbers are shown.
-
-- [[empty-pane]]`Empty Pane`:
-+
-Controls whether empty panes are shown or not. The Left pane is empty when a
-file was added; the right pane is empty when a file was deleted.
-
-- [[left-side]]`Left Side`:
-+
-Controls whether the left side is shown. This preference is not
-persistent and is ignored by the `Save` button. Every time a
-patch diff is opened, this preference is reset to `Show`.
-
-- [[top-menu]]`Top Menu`:
-+
-Controls whether the top menu is shown.
-
-- [[auto-hide-diff-table-header]]`Auto Hide Diff Table Header`:
-+
-Controls whether the diff table header should be automatically hidden
-when scrolling down more than half of a page.
-
- [[mark-reviewed]]`Mark Reviewed`:
+
Controls whether the files of the patch set should be automatically
marked as reviewed when they are viewed.
-- [[expand-all-comments]]`Expand All Comments`:
-+
-Controls whether all comments should be automatically expanded.
-
-- [[render]]`Render`:
-+
-Controls how patch files that exceed the screen size are rendered.
-+
-If `Fast` is selected file contents which are outside of the visible
-area are not attached to the browser's DOM tree. This makes the
-rendering fast, but searching by `Ctrl+F` only finds content which is
-in the visible area.
-+
-If `Slow` is selected all file contents are attached to the browser's
-DOM tree, which makes the rendering slow for large files. The advantage
-of this setting is that `Ctrl+F` can be used to search in the complete
-file.
-+
-Large files that exceed 4000 lines will not be fully rendered.
-
-- [[line-wrapping]]`Line Wrapping`:
-+
-Controls whether to enable line wrapping or not.
-+
-If `false` is selected then line wrapping is disabled.
-This is the default option.
-+
-If `true` is selected then line wrapping is enabled.
-
[[keyboard-shortcuts]]
== Keyboard Shortcuts
diff --git a/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java b/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
index be975c5..b685011 100644
--- a/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
+++ b/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
@@ -477,8 +477,9 @@
final StringBuilder rdr = new StringBuilder();
rdr.append(urlProvider.get(req));
String nextToken = Url.decode(token);
- if (isNew && !token.startsWith(PageLinks.REGISTER + "/")) {
- rdr.append('#' + PageLinks.REGISTER);
+ String registerUri = PageLinks.REGISTER + "/";
+ if (isNew && !token.startsWith(registerUri)) {
+ rdr.append('#' + registerUri);
if (nextToken.startsWith("#")) {
// Need to strip the leading # off the token to fix registration page redirect
nextToken = nextToken.substring(1);
diff --git a/java/com/google/gerrit/server/patch/FilePathAdapter.java b/java/com/google/gerrit/server/patch/FilePathAdapter.java
index 7f34cf1..ccd1466 100644
--- a/java/com/google/gerrit/server/patch/FilePathAdapter.java
+++ b/java/com/google/gerrit/server/patch/FilePathAdapter.java
@@ -1,3 +1,17 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
package com.google.gerrit.server.patch;
import com.google.gerrit.entities.Patch.ChangeType;
diff --git a/java/com/google/gerrit/server/restapi/change/Submit.java b/java/com/google/gerrit/server/restapi/change/Submit.java
index 263d1e7..876c92c 100644
--- a/java/com/google/gerrit/server/restapi/change/Submit.java
+++ b/java/com/google/gerrit/server/restapi/change/Submit.java
@@ -104,7 +104,6 @@
private final GitRepositoryManager repoManager;
private final PermissionBackend permissionBackend;
- private final ChangeData.Factory changeDataFactory;
private final Provider<MergeOp> mergeOpProvider;
private final Provider<MergeSuperSet> mergeSuperSet;
private final AccountResolver accountResolver;
@@ -124,7 +123,6 @@
Submit(
GitRepositoryManager repoManager,
PermissionBackend permissionBackend,
- ChangeData.Factory changeDataFactory,
Provider<MergeOp> mergeOpProvider,
Provider<MergeSuperSet> mergeSuperSet,
AccountResolver accountResolver,
@@ -135,7 +133,6 @@
ChangeJson.Factory json) {
this.repoManager = repoManager;
this.permissionBackend = permissionBackend;
- this.changeDataFactory = changeDataFactory;
this.mergeOpProvider = mergeOpProvider;
this.mergeSuperSet = mergeSuperSet;
this.accountResolver = accountResolver;
diff --git a/java/com/google/gerrit/sshd/LogMaxConnectionsPerUserExceeded.java b/java/com/google/gerrit/sshd/LogMaxConnectionsPerUserExceeded.java
new file mode 100644
index 0000000..6f568b1
--- /dev/null
+++ b/java/com/google/gerrit/sshd/LogMaxConnectionsPerUserExceeded.java
@@ -0,0 +1,42 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.sshd;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import org.apache.sshd.common.Service;
+import org.apache.sshd.common.session.Session;
+import org.apache.sshd.common.session.SessionDisconnectHandler;
+
+@Singleton
+public class LogMaxConnectionsPerUserExceeded implements SessionDisconnectHandler {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ @Override
+ public boolean handleSessionsCountDisconnectReason(
+ Session session,
+ Service service,
+ String username,
+ int currentSessionCount,
+ int maxSessionCount)
+ throws IOException {
+ logger.atWarning().log(
+ "Max connection count for user %s exceeded, rejecting new connection."
+ + " currentSessionCount = %d, maxSessionCount = %d",
+ username, currentSessionCount, maxSessionCount);
+ return false;
+ }
+}
diff --git a/java/com/google/gerrit/sshd/NoShell.java b/java/com/google/gerrit/sshd/NoShell.java
index dd31e4c..e3f654b 100644
--- a/java/com/google/gerrit/sshd/NoShell.java
+++ b/java/com/google/gerrit/sshd/NoShell.java
@@ -27,10 +27,14 @@
import java.io.OutputStream;
import java.net.MalformedURLException;
import java.net.URL;
+import org.apache.sshd.common.io.IoInputStream;
+import org.apache.sshd.common.io.IoOutputStream;
+import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
import org.apache.sshd.server.Environment;
import org.apache.sshd.server.ExitCallback;
import org.apache.sshd.server.SessionAware;
import org.apache.sshd.server.channel.ChannelSession;
+import org.apache.sshd.server.command.AsyncCommand;
import org.apache.sshd.server.command.Command;
import org.apache.sshd.server.session.ServerSession;
import org.apache.sshd.server.shell.ShellFactory;
@@ -56,13 +60,19 @@
return shell.get();
}
- static class SendMessage implements Command, SessionAware {
+ /**
+ * When AsyncCommand is implemented by a command as below, the usual blocking streams aren't set.
+ *
+ * @see org.apache.sshd.server.command.AsyncCommand
+ */
+ static class SendMessage implements AsyncCommand, SessionAware {
private final Provider<MessageFactory> messageFactory;
private final SshScope sshScope;
- private InputStream in;
- private OutputStream out;
- private OutputStream err;
+ private IoInputStream in;
+ private IoOutputStream out;
+ private IoOutputStream err;
+
private ExitCallback exit;
private Context context;
@@ -73,21 +83,36 @@
}
@Override
- public void setInputStream(InputStream in) {
+ public void setIoInputStream(IoInputStream in) {
this.in = in;
}
@Override
- public void setOutputStream(OutputStream out) {
+ public void setIoOutputStream(IoOutputStream out) {
this.out = out;
}
@Override
- public void setErrorStream(OutputStream err) {
+ public void setIoErrorStream(IoOutputStream err) {
this.err = err;
}
@Override
+ public void setInputStream(InputStream in) {
+ // ignored
+ }
+
+ @Override
+ public void setOutputStream(OutputStream out) {
+ // ignore
+ }
+
+ @Override
+ public void setErrorStream(OutputStream err) {
+ // ignore
+ }
+
+ @Override
public void setExitCallback(ExitCallback callback) {
this.exit = callback;
}
@@ -107,8 +132,7 @@
} finally {
sshScope.set(old);
}
- err.write(Constants.encode(message));
- err.flush();
+ err.writeBuffer(new ByteArrayBuffer(Constants.encode(message)));
in.close();
out.close();
diff --git a/java/com/google/gerrit/sshd/SshDaemon.java b/java/com/google/gerrit/sshd/SshDaemon.java
index 9ae8660..fa20b9c 100644
--- a/java/com/google/gerrit/sshd/SshDaemon.java
+++ b/java/com/google/gerrit/sshd/SshDaemon.java
@@ -162,7 +162,8 @@
SshLog sshLog,
@SshListenAddresses List<SocketAddress> listen,
@SshAdvertisedAddresses List<String> advertised,
- MetricMaker metricMaker) {
+ MetricMaker metricMaker,
+ LogMaxConnectionsPerUserExceeded logMaxConnectionsPerUserExceeded) {
setPort(IANA_SSH_PORT /* never used */);
this.cfg = cfg;
@@ -235,6 +236,7 @@
setKeyPairProvider(hostKeyProvider);
setCommandFactory(commandFactory);
setShellFactory(noShell);
+ setSessionDisconnectHandler(logMaxConnectionsPerUserExceeded);
final AtomicInteger connected = new AtomicInteger();
metricMaker.newCallbackMetric(
diff --git a/javatests/com/google/gerrit/integration/ssh/BUILD b/javatests/com/google/gerrit/integration/ssh/BUILD
index dc8e68c..412aad8 100644
--- a/javatests/com/google/gerrit/integration/ssh/BUILD
+++ b/javatests/com/google/gerrit/integration/ssh/BUILD
@@ -5,3 +5,9 @@
group = "peer-keys-auth",
labels = ["ssh"],
)
+
+acceptance_tests(
+ srcs = ["NoShellIT.java"],
+ group = "no-shell",
+ labels = ["ssh"],
+)
diff --git a/javatests/com/google/gerrit/integration/ssh/NoShellIT.java b/javatests/com/google/gerrit/integration/ssh/NoShellIT.java
new file mode 100644
index 0000000..ccaf085
--- /dev/null
+++ b/javatests/com/google/gerrit/integration/ssh/NoShellIT.java
@@ -0,0 +1,96 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.integration.ssh;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.acceptance.GerritServer.TestSshServerAddress;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.StandaloneSiteTest;
+import com.google.gerrit.acceptance.UseSsh;
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.inject.Inject;
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import org.junit.Test;
+
+@NoHttpd
+@UseSsh
+public class NoShellIT extends StandaloneSiteTest {
+ private static final String[] SSH_KEYGEN_CMD =
+ new String[] {"ssh-keygen", "-t", "rsa", "-q", "-P", "", "-f"};
+
+ @Inject private GerritApi gApi;
+ @Inject private @TestSshServerAddress InetSocketAddress sshAddress;
+
+ private String identityPath;
+
+ @Test(timeout = 30000)
+ public void verifyCommandsIsClosed() throws Exception {
+ try (ServerContext ctx = startServer()) {
+ setUpTestHarness(ctx);
+
+ IOException thrown = assertThrows(IOException.class, () -> execute(cmd()));
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("Hi Administrator, you have successfully connected over SSH.");
+ }
+ }
+
+ private void setUpTestHarness(ServerContext ctx) throws Exception {
+ ctx.getInjector().injectMembers(this);
+ setUpAuthentication();
+ identityPath = sitePaths.data_dir.resolve(String.format("id_rsa_%s", "admin")).toString();
+ }
+
+ private void setUpAuthentication() throws Exception {
+ execute(
+ ImmutableList.<String>builder()
+ .add(SSH_KEYGEN_CMD)
+ .add(String.format("id_rsa_%s", "admin"))
+ .build());
+ gApi.accounts()
+ .id("admin")
+ .addSshKey(
+ new String(
+ java.nio.file.Files.readAllBytes(
+ sitePaths.data_dir.resolve(String.format("id_rsa_%s.pub", "admin"))),
+ UTF_8));
+ }
+
+ private ImmutableList<String> cmd() {
+ return ImmutableList.<String>builder()
+ .add("ssh")
+ .add("-tt")
+ .add("-o")
+ .add("StrictHostKeyChecking=no")
+ .add("-o")
+ .add("UserKnownHostsFile=/dev/null")
+ .add("-p")
+ .add(String.valueOf(sshAddress.getPort()))
+ .add("admin@" + sshAddress.getHostName())
+ .add("-i")
+ .add(identityPath)
+ .build();
+ }
+
+ private String execute(ImmutableList<String> cmd) throws Exception {
+ return execute(cmd, sitePaths.data_dir.toFile(), ImmutableMap.of());
+ }
+}
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
index 931579b..ca68926 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
@@ -153,7 +153,14 @@
<section
class$="[[_computeDisplayState(_showAllSections, change, _SECTION.UPDATED)]]"
>
- <span class="title">Updated</span>
+ <span class="title">
+ <gr-tooltip-content
+ has-tooltip=""
+ title="Last update of (meta)data for this change."
+ >
+ Updated
+ </gr-tooltip-content>
+ </span>
<span class="value">
<gr-date-formatter
has-tooltip=""
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 6f35c49..71b8f33 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -165,8 +165,8 @@
tr.collapsed td .summary-cell .actions {
display: none;
}
- tr.collapsed:hover .summary-cell .tags,
- tr.collapsed:hover .summary-cell .label {
+ tr.collapsed:hover .summary-cell .hoverHide.tags,
+ tr.collapsed:hover .summary-cell .hoverHide.label {
display: none;
}
td .summary-cell .tags .tag {
@@ -264,7 +264,7 @@
<div class="message" @click="${this.toggleExpanded}">
${this.isExpanded ? '' : this.result.message}
</div>
- <div class="tags">
+ <div class="tags ${this.hasLinksOrActions() ? 'hoverHide' : ''}">
${(this.result.tags ?? []).map(t => this.renderTag(t))}
</div>
${this.renderLabel()} ${this.renderLinks()} ${this.renderActions()}
@@ -294,6 +294,13 @@
`;
}
+ private hasLinksOrActions() {
+ const linkCount = this.result?.links?.length ?? 0;
+ const actionCount = this.result?.actions?.length ?? 0;
+ // The primary link is rendered somewhere else, so it does not count here.
+ return linkCount > 1 || actionCount > 0;
+ }
+
private renderExpanded() {
if (!this.isExpanded) return;
return html`<gr-result-expanded
@@ -323,7 +330,11 @@
renderLabel() {
const label = this.result?.labelName;
if (!label) return;
- return html`<div class="label">${label}</div>`;
+ return html`
+ <div class="label ${this.hasLinksOrActions() ? 'hoverHide' : ''}">
+ ${label}
+ </div>
+ `;
}
renderLinks() {
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
index 738abc0..3763fbf 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
@@ -72,7 +72,7 @@
_serverConfig?: ServerInfo;
@property({
- computed: '_computeUsernameMutable(_serverConfig, _account.username)',
+ computed: '_computeUsernameMutable(_account.username)',
type: Boolean,
})
_usernameMutable = false;
@@ -121,17 +121,14 @@
(this._account.username || '') !== (this._username || '');
}
- _computeUsernameMutable(config?: ServerInfo, username?: string) {
- // Polymer 2: check for undefined
- if (config === undefined) {
- return false;
- }
-
+ _computeUsernameMutable(username?: string) {
// Username may not be changed once it is set.
- return (
- config.auth.editable_account_fields.includes(
- EditableAccountField.USER_NAME
- ) && !username
+ return !username;
+ }
+
+ _computeUsernameEditable(config?: ServerInfo) {
+ return !!config?.auth.editable_account_fields.includes(
+ EditableAccountField.USER_NAME
);
}
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts
index 0e31bc9..4b86709 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts
@@ -84,20 +84,24 @@
</iron-input>
</span>
</section>
- <section>
- <span class="title">Username</span>
- <span hidden$="[[_usernameMutable]]" class="value">[[_username]]</span>
- <span hidden$="[[!_usernameMutable]]" class="value">
- <iron-input bind-value="{{_username}}">
- <input
- is="iron-input"
- id="username"
- bind-value="{{_username}}"
- disabled="[[_saving]]"
- />
- </iron-input>
- </span>
- </section>
+ <template is="dom-if" if="[[_computeUsernameEditable(_serverConfig)]]">
+ <section>
+ <span class="title">Username</span>
+ <span hidden$="[[_usernameMutable]]" class="value"
+ >[[_username]]</span
+ >
+ <span hidden$="[[!_usernameMutable]]" class="value">
+ <iron-input bind-value="{{_username}}">
+ <input
+ is="iron-input"
+ id="username"
+ bind-value="{{_username}}"
+ disabled="[[_saving]]"
+ />
+ </iron-input>
+ </span>
+ </section>
+ </template>
<hr />
<p>
More configuration options for Gerrit may be found in the
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
index 22f21e1..6c21e58 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
@@ -137,53 +137,28 @@
});
test('_computeUsernameMutable', () => {
+ assert.isTrue(element._computeUsernameMutable(undefined));
+ assert.isFalse(element._computeUsernameMutable('abc'));
+ });
+
+ test('_computeUsernameEditable', () => {
assert.isTrue(
- element._computeUsernameMutable(
- {
- ...createServerInfo(),
- auth: {
- auth_type: AuthType.HTTP,
- editable_account_fields: [EditableAccountField.USER_NAME],
- },
+ element._computeUsernameEditable({
+ ...createServerInfo(),
+ auth: {
+ auth_type: AuthType.HTTP,
+ editable_account_fields: [EditableAccountField.USER_NAME],
},
- undefined
- )
+ })
);
assert.isFalse(
- element._computeUsernameMutable(
- {
- ...createServerInfo(),
- auth: {
- auth_type: AuthType.HTTP,
- editable_account_fields: [EditableAccountField.USER_NAME],
- },
+ element._computeUsernameEditable({
+ ...createServerInfo(),
+ auth: {
+ auth_type: AuthType.HTTP,
+ editable_account_fields: [],
},
- 'abc'
- )
- );
- assert.isFalse(
- element._computeUsernameMutable(
- {
- ...createServerInfo(),
- auth: {
- auth_type: AuthType.HTTP,
- editable_account_fields: [],
- },
- },
- undefined
- )
- );
- assert.isFalse(
- element._computeUsernameMutable(
- {
- ...createServerInfo(),
- auth: {
- auth_type: AuthType.HTTP,
- editable_account_fields: [],
- },
- },
- 'abc'
- )
+ })
);
});
});
