opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-3.2'

Browse Source 
* stable-3.2:
  Upgrade auto-value to 1.7.4
  Respect container.replica option in gerrit.sh script
  Fix plugin builds on OSX
  Upgrade metrics-core to 4.1.10.1
  Add guidelines on choosing a branch for contributions
  For errors on `refs/publish/...`, warn about too old git-review
  Fix an outdated test
  Fix selection on diff with range comments
  Restore keyboard shortcut for expand all diff context
  Fix the position of the hovercard
  In plugin list, add placeholders for missing versions
  Add Api version to plugin list in PolyGerrit UI
  Add Api version to `plugin ls` Ssh command text format
  Expose Api version through PluginInfo
  When writing temporary plugin files, ensure the directory exists
  Update git submodules
  Update git submodules

Change-Id: Id20934171f5fb8051858824758c50953f20948ef
This commit is contained in:
Marco Miller
2020-07-13 11:31:44 -04:00
parent bd8e734f64 21eba93b39
commit b3a71ccef8
16 changed files with 192 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
Documentation/dev-crafting-changes.txt
View File

@@ -10,6 +10,37 @@ problems are often highlighted and we find it hard to look
beyond simple spacing issues. Blame it on our short attention
spans, we really do want your code.
[[branch]]
== Branch
Gerrit provides support for more than one version, which naturally
raises the question of which branch you should start your contribution
on. There are no hard and fast rules, but below we try to outline some
guidelines:
* Genuinely new and/or disruptive features, should generally start on
`master`. Also consider submitting a
link:dev-design-docs.html[design doc] beforehand to allow discussion
by the ESC and the community.
* Improvements of existing features should also generally go into
`master`. But we understand that if you cannot run `master`, it
might take a while until you could benefit from it. In that case,
start on the newest `stable-*` branch that you can run.
* Bug-fixes should generally at least cover the oldest affected and
still supported version. If you're affected and run an even older
version, you're welcome to upload to that older version, even if
it is no longer officially supported, bearing in mind that
verification and release may happen only once merged upstream.
Regardless of the above, changes might get moved to a different branch
before being submitted or might get cherry-picked/re-merged to a
different branch even after they've landed.
For each of the above items, you'll find ad-hoc exceptions. The point
is: We'd much rather see your code and fixes than not see them.
[[commit-message]]
== Commit Message

14
Documentation/rest-api-plugins.txt
View File

@@ -48,6 +48,7 @@ by plugin ID.
"id": "delete-project",
"index_url": "plugins/delete-project/",
"filename": "delete-project.jar",
"api_version": "2.9.3-SNAPSHOT",
"version": "2.9-SNAPSHOT"
}
}
@@ -455,12 +456,13 @@ The `PluginInfo` entity describes a plugin.
[options="header",cols="1,^2,4"]
|=======================
|Field Name ||Description
|`id` ||The ID of the plugin.
|`version` ||The version of the plugin.
|`index_url`|optional|URL of the plugin's default page.
|`filename` |optional|The plugin's filename.
|`disabled` |not set if `false`|Whether the plugin is disabled.
|Field Name ||Description
|`id` ||The ID of the plugin.
|`version` ||The version of the plugin.
|`api_version`|optional|The version of the Gerrit Api used by the plugin.
|`index_url` |optional|URL of the plugin's default page.
|`filename` |optional|The plugin's filename.
|`disabled` |not set if `false`|Whether the plugin is disabled.
|=======================
[[plugin-input]]

6
WORKSPACE
View File

@@ -626,18 +626,18 @@ maven_jar(
sha1 = "a3ae34e57fa8a4040e28247291d0cc3d6b8c7bcf",
)
AUTO_VALUE_VERSION = "1.7.3"
AUTO_VALUE_VERSION = "1.7.4"
maven_jar(
name = "auto-value",
artifact = "com.google.auto.value:auto-value:" + AUTO_VALUE_VERSION,
sha1 = "cbd30873f839545c7c9264bed61d500bf85bd33e",
sha1 = "6b126cb218af768339e4d6e95a9b0ae41f74e73d",
)
maven_jar(
name = "auto-value-annotations",
artifact = "com.google.auto.value:auto-value-annotations:" + AUTO_VALUE_VERSION,
sha1 = "59ce5ee6aea918f674229f1147da95fdf7f31ce6",
sha1 = "eff48ed53995db2dadf0456426cc1f8700136f86",
)
declare_nongoogle_deps()

10
java/com/google/gerrit/extensions/common/PluginInfo.java
View File

@@ -17,13 +17,21 @@ package com.google.gerrit.extensions.common;
public class PluginInfo {
public final String id;
public final String version;
public final String apiVersion;
public final String indexUrl;
public final String filename;
public final Boolean disabled;
public PluginInfo(String id, String version, String indexUrl, String filename, Boolean disabled) {
public PluginInfo(
String id,
String version,
String apiVersion,
String indexUrl,
String filename,
Boolean disabled) {
this.id = id;
this.version = version;
this.apiVersion = apiVersion;
this.indexUrl = indexUrl;
this.filename = filename;
this.disabled = disabled;

10
java/com/google/gerrit/server/git/receive/ReceiveCommits.java
View File

@@ -1005,7 +1005,15 @@ class ReceiveCommits {
private String buildError(String error, List<String> branches) {
StringBuilder sb = new StringBuilder();
if (branches.size() == 1) {
sb.append("branch ").append(branches.get(0)).append(":\n");
String branch = branches.get(0);
sb.append("branch ").append(branch).append(":\n");
// As of 2020, there are still many git-review <1.27 installations in the wild.
// These users will see failures as their old git-review assumes that
// `refs/publish/...` is still magic, which it isn't. As Gerrit's default error messages are
// misleading for these users, we hint them at upgrading their git-review.
if (branch.startsWith("refs/publish/")) {
sb.append("If you are using git-review, update to at least git-review 1.27. Otherwise:\n");
}
sb.append(error);
return sb.toString();
}

4
java/com/google/gerrit/server/plugins/ListPlugins.java
View File

@@ -146,12 +146,14 @@ public class ListPlugins implements RestReadView<TopLevelResource> {
public static PluginInfo toPluginInfo(Plugin p) {
String id;
String version;
String apiVersion;
String indexUrl;
String filename;
Boolean disabled;
id = Url.encode(p.getName());
version = p.getVersion();
apiVersion = p.getApiVersion();
disabled = p.isDisabled() ? true : null;
if (p.getSrcFile() != null) {
indexUrl = String.format("plugins/%s/", p.getName());
@@ -161,6 +163,6 @@ public class ListPlugins implements RestReadView<TopLevelResource> {
filename = null;
}
return new PluginInfo(id, version, indexUrl, filename, disabled);
return new PluginInfo(id, version, apiVersion, indexUrl, filename, disabled);
}
}

5
java/com/google/gerrit/server/plugins/Plugin.java
View File

@@ -116,6 +116,11 @@ public abstract class Plugin {
return apiType;
}
@Nullable
public String getApiVersion() {
return null;
}
public Plugin.CacheKey getCacheKey() {
return cacheKey;
}

4
java/com/google/gerrit/server/plugins/PluginUtil.java
View File

@@ -53,7 +53,9 @@ public class PluginUtil {
}
static Path asTemp(InputStream in, String prefix, String suffix, Path dir) throws IOException {
Files.createDirectories(dir);
if (!Files.exists(dir)) {
Files.createDirectories(dir);
}
Path tmp = Files.createTempFile(dir, prefix, suffix);
boolean keep = false;
try (OutputStream out = Files.newOutputStream(tmp)) {

7
java/com/google/gerrit/server/plugins/ServerPlugin.java
View File

@@ -153,6 +153,13 @@ public class ServerPlugin extends Plugin {
return main.getValue(Attributes.Name.IMPLEMENTATION_VERSION);
}
@Override
@Nullable
public String getApiVersion() {
Attributes main = manifest.getMainAttributes();
return main.getValue("Gerrit-ApiVersion");
}
@Override
protected boolean canReload() {
Attributes main = manifest.getMainAttributes();

6
java/com/google/gerrit/sshd/commands/PluginLsCommand.java
View File

@@ -49,15 +49,17 @@ public class PluginLsCommand extends SshCommand {
.toJson(output, new TypeToken<Map<String, PluginInfo>>() {}.getType(), stdout);
stdout.print('\n');
} else {
stdout.format("%-30s %-10s %-8s %s\n", "Name", "Version", "Status", "File");
String template = "%-30s %-10s %-16s %-8s %s\n";
stdout.format(template, "Name", "Version", "Api-Version", "Status", "File");
stdout.print(
"-------------------------------------------------------------------------------\n");
for (Map.Entry<String, PluginInfo> p : output.entrySet()) {
PluginInfo info = p.getValue();
stdout.format(
"%-30s %-10s %-8s %s\n",
template,
p.getKey(),
Strings.nullToEmpty(info.version),
Strings.nullToEmpty(info.apiVersion),
status(info.disabled),
Strings.nullToEmpty(info.filename));
}

57
javatests/com/google/gerrit/acceptance/api/plugin/PluginIT.java
View File

@@ -37,7 +37,12 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.plugins.MandatoryPluginsCollection;
import com.google.inject.Inject;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.List;
import java.util.jar.Attributes;
import java.util.jar.JarOutputStream;
import java.util.jar.Manifest;
import org.junit.Test;
@NoHttpd
@@ -51,7 +56,14 @@ public class PluginIT extends AbstractDaemonTest {
private static final ImmutableList<String> PLUGINS =
ImmutableList.of(
"plugin-a.js", "plugin-b.html", "plugin-c.js", "plugin-d.html", "plugin_e.js");
"plugin-a.js",
"plugin-b.html",
"plugin-c.js",
"plugin-d.html",
"plugin-normal.jar",
"plugin-empty.jar",
"plugin-unset.jar",
"plugin_e.js");
@Inject private RequestScopeOperations requestScopeOperations;
@Inject private MandatoryPluginsCollection mandatoryPluginsCollection;
@@ -67,13 +79,14 @@ public class PluginIT extends AbstractDaemonTest {
// Install all the plugins
InstallPluginInput input = new InstallPluginInput();
for (String plugin : PLUGINS) {
input.raw = plugin.endsWith(".js") ? JS_PLUGIN_CONTENT : HTML_PLUGIN_CONTENT;
input.raw = pluginContent(plugin);
api = gApi.plugins().install(plugin, input);
assertThat(api).isNotNull();
PluginInfo info = api.get();
String name = pluginName(plugin);
assertThat(info.id).isEqualTo(name);
assertThat(info.version).isEqualTo(pluginVersion(plugin));
assertThat(info.apiVersion).isEqualTo(pluginApiVersion(plugin));
assertThat(info.indexUrl).isEqualTo(String.format("plugins/%s/", name));
assertThat(info.filename).isEqualTo(plugin);
assertThat(info.disabled).isNull();
@@ -168,12 +181,52 @@ public class PluginIT extends AbstractDaemonTest {
return plugin.substring(0, dot);
}
private RawInput pluginJarContent(String plugin) throws IOException {
ByteArrayOutputStream arrayStream = new ByteArrayOutputStream();
Manifest manifest = new Manifest();
Attributes attributes = manifest.getMainAttributes();
attributes.put(Attributes.Name.MANIFEST_VERSION, "1.0");
if (!plugin.endsWith("-unset.jar")) {
attributes.put(Attributes.Name.IMPLEMENTATION_VERSION, pluginVersion(plugin));
attributes.put(new Attributes.Name("Gerrit-ApiVersion"), pluginApiVersion(plugin));
}
try (JarOutputStream jarStream = new JarOutputStream(arrayStream, manifest)) {}
return RawInputUtil.create(arrayStream.toByteArray());
}
private RawInput pluginContent(String plugin) throws IOException {
if (plugin.endsWith(".js")) {
return JS_PLUGIN_CONTENT;
}
if (plugin.endsWith(".html")) {
return HTML_PLUGIN_CONTENT;
}
assertThat(plugin).endsWith(".jar");
return pluginJarContent(plugin);
}
private String pluginVersion(String plugin) {
String name = pluginName(plugin);
if (name.endsWith("empty")) {
return "";
}
if (name.endsWith("unset")) {
return null;
}
int dash = name.lastIndexOf("-");
return dash > 0 ? name.substring(dash + 1) : "";
}
private String pluginApiVersion(String plugin) {
if (plugin.endsWith("normal.jar")) {
return "2.16.19-SNAPSHOT";
}
if (plugin.endsWith("empty.jar")) {
return "";
}
return null;
}
private void assertBadRequest(ListRequest req) throws Exception {
assertThrows(BadRequestException.class, () -> req.get());
}

22
polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list_html.js
View File

@@ -21,7 +21,9 @@ export const htmlTemplate = html`
/* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
</style>
<style include="gr-table-styles">
/* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
.placeholder {
color: var(--deemphasized-text-color);
}
</style>
<gr-list-view
filter="[[_filter]]"
@@ -36,6 +38,7 @@ export const htmlTemplate = html`
<tr class="headerRow">
<th class="name topHeader">Plugin Name</th>
<th class="version topHeader">Version</th>
<th class="apiVersion topHeader">API Version</th>
<th class="status topHeader">Status</th>
</tr>
<tr id="loading" class$="loadingMsg [[computeLoadingClass(_loading)]]">
@@ -53,7 +56,22 @@ export const htmlTemplate = html`
[[item.id]]
</template>
</td>
<td class="version">[[item.version]]</td>
<td class="version">
<template is="dom-if" if="[[item.version]]">
[[item.version]]
</template>
<template is="dom-if" if="[[!item.version]]">
<span class="placeholder">--</span>
</template>
</td>
<td class="apiVersion">
<template is="dom-if" if="[[item.api_version]]">
[[item.api_version]]
</template>
<template is="dom-if" if="[[!item.api_version]]">
<span class="placeholder">--</span>
</template>
</td>
<td class="status">[[_status(item)]]</td>
</tr>
</template>

35
polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list_test.js
View File

@@ -25,13 +25,18 @@ let counter;
const pluginGenerator = () => {
const plugin = {
id: `test${++counter}`,
version: '3.0-SNAPSHOT',
disabled: false,
};
if (counter !== 2) {
plugin.index_url = `plugins/test${counter}/`;
}
if (counter !== 3) {
plugin.version = `version-${counter}`;
}
if (counter !== 4) {
plugin.api_version = `api-version-${counter}`;
}
return plugin;
};
@@ -61,10 +66,11 @@ suite('gr-plugin-list tests', () => {
test('plugin in the list is formatted correctly', done => {
flush(() => {
assert.equal(element._plugins[2].id, 'test3');
assert.equal(element._plugins[2].index_url, 'plugins/test3/');
assert.equal(element._plugins[2].version, '3.0-SNAPSHOT');
assert.equal(element._plugins[2].disabled, false);
assert.equal(element._plugins[4].id, 'test5');
assert.equal(element._plugins[4].index_url, 'plugins/test5/');
assert.equal(element._plugins[4].version, 'version-5');
assert.equal(element._plugins[4].api_version, 'api-version-5');
assert.equal(element._plugins[4].disabled, false);
done();
});
});
@@ -80,6 +86,25 @@ suite('gr-plugin-list tests', () => {
});
});
test('versions', done => {
flush(() => {
const versions = Polymer.dom(element.root).querySelectorAll('.version');
assert.equal(versions[2].innerText, 'version-2');
assert.equal(versions[3].innerText, '--');
done();
});
});
test('api versions', done => {
flush(() => {
const apiVersions = Polymer.dom(element.root).querySelectorAll(
'.apiVersion');
assert.equal(apiVersions[3].innerText, 'api-version-3');
assert.equal(apiVersions[4].innerText, '--');
done();
});
});
test('_shownPlugins', () => {
assert.equal(element._shownPlugins.length, 25);
});

3
resources/com/google/gerrit/pgm/init/gerrit.sh
View File

@@ -345,7 +345,8 @@ fi
test -z "$GERRIT_USER" && GERRIT_USER=`whoami`
RUN_ARGS="-jar $GERRIT_WAR daemon -d $GERRIT_SITE"
if test "`get_config --bool container.slave`" = "true" ; then
if test "`get_config --bool container.slave`" = "true" || \
test "`get_config --bool container.replica`" = "true"; then
RUN_ARGS="$RUN_ARGS --replica --enable-httpd --headless"
fi
DAEMON_OPTS=`get_config --get-all container.daemonOpt`

2
tools/bzl/plugin.bzl
View File

@@ -68,7 +68,7 @@ def gerrit_plugin(
"export TZ",
"GEN_VERSION=$$(cat bazel-out/stable-status.txt | grep -w STABLE_BUILD_%s_LABEL | cut -d ' ' -f 2)" % dir_name.upper(),
"cd $$TMP",
"unzip -q $$ROOT/$<",
"unzip -qo $$ROOT/$<",
"echo \"Implementation-Version: $$GEN_VERSION\n$$(cat META-INF/MANIFEST.MF)\" > META-INF/MANIFEST.MF",
"find . -exec touch '{}' ';'",
"zip -Xqr $$ROOT/$@ .",

4
tools/nongoogle.bzl
View File

@@ -23,8 +23,8 @@ def declare_nongoogle_deps():
maven_jar(
name = "dropwizard-core",
artifact = "io.dropwizard.metrics:metrics-core:4.1.9",
sha1 = "dd76a62b007ffea9e6aba10f64c04173ef65f895",
artifact = "io.dropwizard.metrics:metrics-core:4.1.10.1",
sha1 = "e55d1e4de0ccec6f404dbf775c62626d8b9f79a4",
)
SSHD_VERS = "2.4.0"