From cf417107693f15e171159697d572495854a4352e Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Mon, 19 Oct 2015 08:48:50 +0200 Subject: [PATCH 01/17] InitPlugins: Suggest to upgrade installed plugins per default So we had again Gerrit outage for wrong default plugin installation option during gerrit upgrade from 2.10.4 to 2.10.7: [1]. The reason for that outage was wrong default option for plugin installation or upgrade in gerrit installer: Install plugin reviewnotes version v2.10.2-18-gc6c5e0b [y/N]? Gerrit administrator relies on the installer suggesting the right thing to do per default. And always confirms the default option suggested by Gerrit installer. In this specific case, the installer made it too easy to shoot the admin in the foot, because JGit version had API changes, by removing the function release() in favor of close(), and by not updating the plugin(s) installer left the Gerrit site in inherently broken state. Note, that Gerrit admin neither must necessarily know if libraries that are used by plugins had incompatible changes in Gerrit core or not, nor Gerrit admin is supposed to study the Git history of all related libraries before upgrading Gerrit site. It's always the right thing to do to upgrade the plugins. [1] http://paste.openstack.org/show/476639 Test plan: * The plugins suggested to be upgraded per default on site upgrade: Installing plugins. Install plugin replication version v2.11.4-29-gefa3c6a [Y/n]? version v2.11.4-29-gefa3c6a is already installed, overwrite it [Y/n]? Install plugin singleusergroup version v2.11.3-3-gf6df712 [Y/n]? version v2.11.3-3-gf6df712 is already installed, overwrite it [Y/n]? Install plugin download-commands version v2.11.3-11-g86eb557 [Y/n]? version v2.11.3-11-g86eb557 is already installed, overwrite it [Y/n]? Install plugin reviewnotes version v2.11.3-8-g26f38c4 [Y/n]? version v2.11.3-8-g26f38c4 is already installed, overwrite it [Y/n]? Install plugin commit-message-length-validator version v2.11-4-g8d295ed [Y/n]? version v2.11-4-g8d295ed is already installed, overwrite it [Y/n]? Change-Id: I5d7f83f050144fda9c084fff3f163c120c642077 --- .../java/com/google/gerrit/pgm/init/InitPlugins.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitPlugins.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitPlugins.java index 72400a42b5..921f69e4f2 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitPlugins.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitPlugins.java @@ -111,17 +111,18 @@ public class InitPlugins implements InitStep { String pluginName = plugin.name; try { final File tmpPlugin = plugin.pluginFile; + File p = new File(site.plugins_dir, plugin.name + ".jar"); + boolean upgrade = p.exists(); - if (!(initFlags.installPlugins.contains(pluginName) || ui.yesno(false, + if (!(initFlags.installPlugins.contains(pluginName) || ui.yesno(upgrade, "Install plugin %s version %s", pluginName, plugin.version))) { tmpPlugin.delete(); continue; } - final File p = new File(site.plugins_dir, plugin.name + ".jar"); - if (p.exists()) { + if (upgrade) { final String installedPluginVersion = getVersion(p); - if (!ui.yesno(false, + if (!ui.yesno(upgrade, "version %s is already installed, overwrite it", installedPluginVersion)) { tmpPlugin.delete(); From 82a48b54d8440f75df4c489fedd3c7b0ccbeeea5 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 24 Nov 2015 17:15:04 +0900 Subject: [PATCH 02/17] Make InternalChangeQuery.query public so it can be used by plugins Change-Id: I83ca1ef41ab0ba7728ae60e075d6a7986cc91d50 --- .../google/gerrit/server/query/change/InternalChangeQuery.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java index e08847ad8a..008cd0f7a6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java @@ -123,7 +123,7 @@ public class InternalChangeQuery { return query(and(topic(topic), open())); } - private List query(Predicate p) throws OrmException { + public List query(Predicate p) throws OrmException { try { return qp.queryChanges(p).changes(); } catch (QueryParseException e) { From 72eff3121a10fb045965ad2e234fbb4abb1e532f Mon Sep 17 00:00:00 2001 From: Andrew Bonventre Date: Tue, 24 Nov 2015 13:52:31 -0500 Subject: [PATCH 03/17] Correct docs since --name is not a valid option for create-project Change-Id: Id0b3bdf0ca1b994f534cfe983be35fa84cbb1f36 --- Documentation/install-quick.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/install-quick.txt b/Documentation/install-quick.txt index ffd68e7169..26232565b7 100644 --- a/Documentation/install-quick.txt +++ b/Documentation/install-quick.txt @@ -121,7 +121,7 @@ time setup between client and server is easier. This is done via the SSH port: ---- - user@host:~$ ssh -p 29418 user@localhost gerrit create-project --empty-commit --name demo-project + user@host:~$ ssh -p 29418 user@localhost gerrit create-project demo-project --empty-commit user@host:~$ ---- @@ -134,7 +134,7 @@ want to try out Gerrit on. First you have to create the project. This is done via the SSH port: ---- - user@host:~$ ssh -p 29418 user@localhost gerrit create-project --name demo-project + user@host:~$ ssh -p 29418 user@localhost gerrit create-project demo-project user@host:~$ ---- From 7ec8bb3f601d79a56001aaae507053ff798abbc9 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Wed, 25 Nov 2015 11:42:19 +0000 Subject: [PATCH 04/17] Brings Gerrit docs back to life After the regression introduced by [1] the Gerrit static documentation was not available anymore in the standard Jetty embedded server scenario. This change is the ideal completion of the refactoring of the Jetty-controlled static resources serving legacy logic into the new Gerrit-driven ResourceServlet mechanism. [1] https://gerrit-review.googlesource.com/#/c/72085/ Change-Id: I554adc51766554e5dde039fc83d2d480525ab7af --- .../google/gerrit/httpd/raw/StaticModule.java | 26 ++++++++++++++ .../gerrit/httpd/raw/WarDocServlet.java | 36 +++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/WarDocServlet.java diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticModule.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticModule.java index 4cd0672725..ffb33c5203 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticModule.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticModule.java @@ -32,10 +32,15 @@ import java.io.IOException; import java.nio.file.FileSystem; import java.nio.file.Path; +import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; public class StaticModule extends ServletModule { private static final String GWT_UI_SERVLET = "GwtUiServlet"; + private static final String DOC_SERVLET = "DocServlet"; + static final String CACHE = "static_content"; private final FileSystem warFs; @@ -55,6 +60,8 @@ public class StaticModule extends ServletModule { @Override protected void configureServlets() { + serveRegex("^/Documentation/(.+)$").with( + Key.get(HttpServlet.class, Names.named(DOC_SERVLET))); serve("/static/*").with(SiteStaticDirectoryServlet.class); serveGwtUi(); install(new CacheModule() { @@ -75,6 +82,25 @@ public class StaticModule extends ServletModule { } } + @Provides + @Singleton + @Named(DOC_SERVLET) + HttpServlet getDocServlet(@Named(CACHE) Cache cache) { + if (warFs != null) { + return new WarDocServlet(cache, warFs); + } else { + return new HttpServlet() { + private static final long serialVersionUID = 1L; + + @Override + protected void service(HttpServletRequest req, + HttpServletResponse resp) throws IOException { + resp.sendError(HttpServletResponse.SC_NOT_FOUND); + } + }; + } + } + @Provides @Singleton @Named(GWT_UI_SERVLET) diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/WarDocServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/WarDocServlet.java new file mode 100644 index 0000000000..ad233140b4 --- /dev/null +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/WarDocServlet.java @@ -0,0 +1,36 @@ +// Copyright (C) 2015 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.httpd.raw; + +import com.google.common.cache.Cache; + +import java.nio.file.FileSystem; +import java.nio.file.Path; + +class WarDocServlet extends ResourceServlet { + private static final long serialVersionUID = 1L; + + private final FileSystem warFs; + + WarDocServlet(Cache cache, FileSystem warFs) { + super(cache, false); + this.warFs = warFs; + } + + @Override + protected Path getResourcePath(String pathInfo) { + return warFs.getPath("/Documentation/" + pathInfo); + } +} From c4dc9d661aa7cf8418df1613839dceef8c8bdbd0 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 26 Nov 2015 09:46:26 +0900 Subject: [PATCH 05/17] StaticModule: Remove unused import Change-Id: I41034774cb1ccdf7905bddecd7c55f91d9a85aa7 --- .../src/main/java/com/google/gerrit/httpd/raw/StaticModule.java | 1 - 1 file changed, 1 deletion(-) diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticModule.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticModule.java index ffb33c5203..ded81f188e 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticModule.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticModule.java @@ -32,7 +32,6 @@ import java.io.IOException; import java.nio.file.FileSystem; import java.nio.file.Path; -import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; From bedd4eaf845ce6a6f23eb6598771672978fd568c Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 26 Nov 2015 14:55:24 +0900 Subject: [PATCH 06/17] HttpPluginServlet: Use try-with-resource in readWholeEntry This was already done on master/stable-2.12 in change I94f481a33 which updated several places to use try-with-resource. Backporting this specific one so it's easier to apply Luca's fix for "short read of block" without conflict. Change-Id: I1ec84f15b09b860c3f416a5000ad4e950a2a3977 --- .../com/google/gerrit/httpd/plugins/HttpPluginServlet.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java index 64b754d164..73fbb3c24e 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java @@ -649,11 +649,8 @@ class HttpPluginServlet extends HttpServlet private static byte[] readWholeEntry(PluginContentScanner scanner, PluginEntry entry) throws IOException { byte[] data = new byte[entry.getSize().get().intValue()]; - InputStream in = scanner.getInputStream(entry); - try { + try (InputStream in = scanner.getInputStream(entry)) { IO.readFully(in, data, 0, data.length); - } finally { - in.close(); } return data; } From 4554d27f275a42583714be90abf14da398d088df Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Fri, 20 Nov 2015 13:53:53 +0000 Subject: [PATCH 07/17] HttpPluginServlet: Fix "short read of block" IO error on plugin docs When accessing plugins static resources from the Jar, treat the JarEntry size as a "hint" to the expected uncompressed size, rather than the exact size. Treating as "exact value" can result in an IO error whilst reading the resource: java.io.EOFException: Short read of block Change-Id: I6b488e58bb34620483d9ed7524ac3c23ce44071a --- .../com/google/gerrit/httpd/plugins/HttpPluginServlet.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java index 73fbb3c24e..4c9c59d95d 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java @@ -648,11 +648,9 @@ class HttpPluginServlet extends HttpServlet private static byte[] readWholeEntry(PluginContentScanner scanner, PluginEntry entry) throws IOException { - byte[] data = new byte[entry.getSize().get().intValue()]; try (InputStream in = scanner.getInputStream(entry)) { - IO.readFully(in, data, 0, data.length); + return IO.readWholeStream(in, entry.getSize().get().intValue()).array(); } - return data; } private static class PluginHolder { From ba85346117b850e143be9a7fe95052191f998de5 Mon Sep 17 00:00:00 2001 From: Michael Zhou Date: Wed, 25 Nov 2015 18:40:06 -0500 Subject: [PATCH 08/17] Add !important back to .cm-searching and .cm-trailingspace Those were removed in change I907068f45, causing search results and trailing whitespaces to not be highlighted in intraline diff chunks. Change-Id: Ica8f8611cecdd34f0013fa5d3a67929cb1cb139d --- gerrit-gwtui/src/main/java/net/codemirror/lib/style.css | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gerrit-gwtui/src/main/java/net/codemirror/lib/style.css b/gerrit-gwtui/src/main/java/net/codemirror/lib/style.css index c5f79d3c56..6ce70dbbe1 100644 --- a/gerrit-gwtui/src/main/java/net/codemirror/lib/style.css +++ b/gerrit-gwtui/src/main/java/net/codemirror/lib/style.css @@ -62,11 +62,11 @@ } .cm-searching { - background-color: #ffa; + background-color: #ffa !important; } .cm-trailingspace { - background-color: red; + background-color: red !important; } /* Line length margin displayed at NN columns to provide From 713551b09573d6f67916d562908f5618d6adee70 Mon Sep 17 00:00:00 2001 From: Khai Do Date: Thu, 12 Nov 2015 11:03:52 -0800 Subject: [PATCH 09/17] Fix query for changes using a label with a group operator. The group operator is being ignored when searching for changes with labels because the search index does not contain group information. This change will convert a group query into mutiple user queries. For example the query: 'label:Code-Review=+1,group=heros' Gets expanded to: 'label:Code-Review=+1,user=superman OR label:Code-Review=+1,user=batman' The latter is the actual query. Bug: issue 3018 Change-Id: Ief5408ee4774f9491fe713b0089e76aa4cae4403 --- .../query/change/BasicChangeRewrites.java | 2 +- .../query/change/ChangeQueryBuilder.java | 79 +++++++++++++++++-- .../gerrit/server/index/FakeQueryBuilder.java | 7 +- .../change/AbstractQueryChangesTest.java | 49 ++++++++++++ 4 files changed, 127 insertions(+), 10 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java index 1053d9214e..851bbcd8ed 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java @@ -29,7 +29,7 @@ public class BasicChangeRewrites extends QueryRewriter { new InvalidProvider(), new InvalidProvider(), null, null, null, null, null, null, null, null, null, null, null, - null, null, null, null, null, null, null, null)); + null, null, null, null, null, null, null, null, null, null, null)); private static final QueryRewriter.Definition mydef = new QueryRewriter.Definition<>(BasicChangeRewrites.class, BUILDER); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index 250b1f2954..eb71f881be 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -18,12 +18,18 @@ import static com.google.gerrit.server.query.change.ChangeData.asChanges; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.gerrit.common.Nullable; +import com.google.gerrit.common.data.GroupDetail; import com.google.gerrit.common.data.GroupReference; +import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.common.errors.NotSignedInException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; +import com.google.gerrit.reviewdb.client.AccountGroupById; +import com.google.gerrit.reviewdb.client.AccountGroupMember; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.server.ReviewDb; @@ -34,6 +40,8 @@ import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.account.CapabilityControl; import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupBackends; +import com.google.gerrit.server.account.GroupCache; +import com.google.gerrit.server.account.GroupDetailFactory; import com.google.gerrit.server.change.ChangeTriplet; import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.GerritServerConfig; @@ -43,6 +51,7 @@ import com.google.gerrit.server.git.strategy.SubmitStrategyFactory; import com.google.gerrit.server.index.ChangeIndex; import com.google.gerrit.server.index.FieldDef; import com.google.gerrit.server.index.IndexCollection; +import com.google.gerrit.server.index.IndexConfig; import com.google.gerrit.server.index.Schema; import com.google.gerrit.server.patch.PatchListCache; import com.google.gerrit.server.project.ChangeControl; @@ -131,6 +140,7 @@ public class ChangeQueryBuilder extends QueryBuilder { final Provider queryProvider; final Provider rewriter; final IdentifiedUser.GenericFactory userFactory; + final GroupDetailFactory.Factory groupDetailFactory; final CapabilityControl.Factory capabilityControlFactory; final ChangeControl.GenericFactory changeControlGenericFactory; final ChangeData.Factory changeDataFactory; @@ -142,12 +152,14 @@ public class ChangeQueryBuilder extends QueryBuilder { final PatchListCache patchListCache; final GitRepositoryManager repoManager; final ProjectCache projectCache; + final GroupCache groupCache; final Provider listChildProjects; final IndexCollection indexes; final SubmitStrategyFactory submitStrategyFactory; final ConflictsCache conflictsCache; final TrackingFooters trackingFooters; final boolean allowsDrafts; + final IndexConfig indexConfig; private final Provider self; @@ -159,6 +171,7 @@ public class ChangeQueryBuilder extends QueryBuilder { IdentifiedUser.GenericFactory userFactory, Provider self, CapabilityControl.Factory capabilityControlFactory, + GroupDetailFactory.Factory groupDetailFactory, ChangeControl.GenericFactory changeControlGenericFactory, ChangeData.Factory changeDataFactory, FieldDef.FillArgs fillArgs, @@ -169,18 +182,20 @@ public class ChangeQueryBuilder extends QueryBuilder { PatchListCache patchListCache, GitRepositoryManager repoManager, ProjectCache projectCache, + GroupCache groupCache, Provider listChildProjects, IndexCollection indexes, SubmitStrategyFactory submitStrategyFactory, ConflictsCache conflictsCache, TrackingFooters trackingFooters, + IndexConfig indexConfig, @GerritServerConfig Config cfg) { this(db, queryProvider, rewriter, userFactory, self, - capabilityControlFactory, changeControlGenericFactory, + capabilityControlFactory, groupDetailFactory, changeControlGenericFactory, changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend, allProjectsName, patchListCache, repoManager, projectCache, - listChildProjects, indexes, submitStrategyFactory, conflictsCache, - trackingFooters, + groupCache, listChildProjects, indexes, submitStrategyFactory, + conflictsCache, trackingFooters, indexConfig, cfg == null ? true : cfg.getBoolean("change", "allowDrafts", true)); } @@ -191,6 +206,7 @@ public class ChangeQueryBuilder extends QueryBuilder { IdentifiedUser.GenericFactory userFactory, Provider self, CapabilityControl.Factory capabilityControlFactory, + GroupDetailFactory.Factory groupDetailFactory, ChangeControl.GenericFactory changeControlGenericFactory, ChangeData.Factory changeDataFactory, FieldDef.FillArgs fillArgs, @@ -201,11 +217,13 @@ public class ChangeQueryBuilder extends QueryBuilder { PatchListCache patchListCache, GitRepositoryManager repoManager, ProjectCache projectCache, + GroupCache groupCache, Provider listChildProjects, IndexCollection indexes, SubmitStrategyFactory submitStrategyFactory, ConflictsCache conflictsCache, TrackingFooters trackingFooters, + IndexConfig indexConfig, boolean allowsDrafts) { this.db = db; this.queryProvider = queryProvider; @@ -213,6 +231,7 @@ public class ChangeQueryBuilder extends QueryBuilder { this.userFactory = userFactory; this.self = self; this.capabilityControlFactory = capabilityControlFactory; + this.groupDetailFactory = groupDetailFactory; this.changeControlGenericFactory = changeControlGenericFactory; this.changeDataFactory = changeDataFactory; this.fillArgs = fillArgs; @@ -223,22 +242,24 @@ public class ChangeQueryBuilder extends QueryBuilder { this.patchListCache = patchListCache; this.repoManager = repoManager; this.projectCache = projectCache; + this.groupCache = groupCache; this.listChildProjects = listChildProjects; this.indexes = indexes; this.submitStrategyFactory = submitStrategyFactory; this.conflictsCache = conflictsCache; this.trackingFooters = trackingFooters; this.allowsDrafts = allowsDrafts; + this.indexConfig = indexConfig; } Arguments asUser(CurrentUser otherUser) { return new Arguments(db, queryProvider, rewriter, userFactory, Providers.of(otherUser), - capabilityControlFactory, changeControlGenericFactory, + capabilityControlFactory, groupDetailFactory, changeControlGenericFactory, changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend, allProjectsName, patchListCache, repoManager, projectCache, - listChildProjects, indexes, submitStrategyFactory, conflictsCache, - trackingFooters, allowsDrafts); + groupCache, listChildProjects, indexes, submitStrategyFactory, conflictsCache, + trackingFooters, indexConfig, allowsDrafts); } Arguments asUser(Account.Id otherId) { @@ -547,6 +568,19 @@ public class ChangeQueryBuilder extends QueryBuilder { } } + // expand a group predicate into multiple user predicates + if (group != null) { + Set allMembers = getMemberIds( + group, new HashSet()); + int maxTerms = args.indexConfig.maxLimit(); + if (allMembers.size() > maxTerms) { + // limit the number of query terms otherwise Gerrit will barf + accounts = ImmutableSet.copyOf(Iterables.limit(allMembers, maxTerms)); + } else { + accounts = allMembers; + } + } + return new LabelPredicate(args.projectCache, args.changeControlGenericFactory, args.userFactory, args.db, name, accounts, group); @@ -808,6 +842,39 @@ public class ChangeQueryBuilder extends QueryBuilder { return g; } + private Set getMemberIds(AccountGroup.UUID groupUUID, + Set seenGroups) throws OrmException { + seenGroups.add(groupUUID); + + Set members = new HashSet<>(); + AccountGroup group = args.groupCache.get(groupUUID); + if (group != null) { + try { + GroupDetail groupDetail = + args.groupDetailFactory.create(group.getId()).call(); + if (groupDetail.members != null) { + for (AccountGroupMember m : groupDetail.members) { + if (!members.contains(m.getAccountId())) { + members.add(m.getAccountId()); + } + } + } + // Get members of subgroups + if (groupDetail.includes != null) { + for (AccountGroupById includedGroup : groupDetail.includes) { + if (!seenGroups.contains(includedGroup.getIncludeUUID())) { + members.addAll( + getMemberIds(includedGroup.getIncludeUUID(), seenGroups)); + } + } + } + } catch (NoSuchGroupException e) { + // The included group is not visible + } + } + return members; + } + private List parseChange(String value) throws OrmException, QueryParseException { if (PAT_LEGACY_ID.matcher(value).matches()) { diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java index c9a2056c87..04fed32f55 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java @@ -25,9 +25,10 @@ public class FakeQueryBuilder extends ChangeQueryBuilder { super( new FakeQueryBuilder.Definition<>( FakeQueryBuilder.class), - new ChangeQueryBuilder.Arguments(null, null, null, null, null, null, - null, null, null, null, null, null, null, null, null, null, null, - indexes, null, null, null, null)); + new ChangeQueryBuilder.Arguments(null, null, null, null, + null, null, null, null, null, null, null, null, null, + null, null, null, null, null, null, indexes, null, null, + null, null, null)); } @Operator diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 34588fab99..eea8e588ba 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -40,6 +40,7 @@ import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.TopLevelResource; import com.google.gerrit.lifecycle.LifecycleManager; import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Patch; @@ -49,6 +50,9 @@ import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountManager; import com.google.gerrit.server.account.AuthRequest; +import com.google.gerrit.server.account.CreateGroupArgs; +import com.google.gerrit.server.account.GroupCache; +import com.google.gerrit.server.account.PerformCreateGroup; import com.google.gerrit.server.change.ChangeInserter; import com.google.gerrit.server.change.ChangeTriplet; import com.google.gerrit.server.change.ChangesCollection; @@ -109,6 +113,8 @@ public abstract class AbstractQueryChangesTest { @Inject protected Provider queryProvider; @Inject protected SchemaCreator schemaCreator; @Inject protected ThreadLocalRequestContext requestContext; + @Inject protected GroupCache groupCache; + @Inject protected PerformCreateGroup.Factory performCreateGroupFactory; protected LifecycleManager lifecycle; protected ReviewDb db; @@ -536,6 +542,49 @@ public abstract class AbstractQueryChangesTest { assertResultEquals(change, queryOne("label:Code-Review=+1,group=Administrators")); } + private void createGroup(String name, AccountGroup.Id owner, Account.Id member) + throws Exception { + CreateGroupArgs args = new CreateGroupArgs(); + args.setGroupName(name); + args.ownerGroupId = owner; + args.initialMembers = ImmutableList.of(member); + performCreateGroupFactory.create(args).createGroup(); + } + + @Test + public void byLabelGroup() throws Exception { + Account.Id user1 = accountManager + .authenticate(AuthRequest.forUser("user1")).getAccountId(); + Account.Id user2 = accountManager + .authenticate(AuthRequest.forUser("user2")).getAccountId(); + TestRepository repo = createProject("repo"); + + // create group and add users + AccountGroup.Id adminGroup = + groupCache.get(new AccountGroup.NameKey("Administrators")).getId(); + createGroup("group1", adminGroup, user1); + createGroup("group2", adminGroup, user2); + + // create a change + ChangeInserter ins = newChange(repo, null, null, user1.get(), null); + Change change1 = ins.insert(); + + // post a review with user1 + requestContext.setContext(newRequestContext(user1)); + ReviewInput input = new ReviewInput(); + input.labels = ImmutableMap. of("Code-Review", (short) 1); + postReview.apply(new RevisionResource( + changes.parse(change1.getId()), ins.getPatchSet()), input); + + // verify that query with user1 will return results. + requestContext.setContext(newRequestContext(userId)); + assertResultEquals(change1, queryOne("label:Code-Review=+1,group1")); + assertResultEquals(change1, queryOne("label:Code-Review=+1,group=group1")); + assertResultEquals(change1, queryOne("label:Code-Review=+1,user=user1")); + assertThat(query("label:Code-Review=+1,user=user2")).isEmpty(); + assertThat(query("label:Code-Review=+1,group=group2")).isEmpty(); + } + @Test public void limit() throws Exception { TestRepository repo = createProject("repo"); From b13d99a523abbfde02e99c630b245291ba1bd0a0 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Thu, 26 Nov 2015 08:12:34 +0100 Subject: [PATCH 10/17] Update 2.12 release notes Change-Id: I6abc5a305da7aa560ea941bb3847e5392daa89aa --- ReleaseNotes/ReleaseNotes-2.12.txt | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/ReleaseNotes/ReleaseNotes-2.12.txt b/ReleaseNotes/ReleaseNotes-2.12.txt index 1dfba725a1..342cc0dba9 100644 --- a/ReleaseNotes/ReleaseNotes-2.12.txt +++ b/ReleaseNotes/ReleaseNotes-2.12.txt @@ -212,6 +212,27 @@ link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/conf * Add support for hmac-sha2-256 and hmac-sha2-512 as MACs. +Plugins +~~~~~~~ + +General +^^^^^^^ + +* Gerrit client can now pass JavaScriptObjects to extension panels +* New UI extension point for header bar in change screen +* New UI extension point to password screen +* New UI extension points to project info screen +* New UI extension point for pop down buttons on change screen +* New UI extension point for buttons in header bar on change screen +* New UI extension point at bottom of the user preferences screen +* Plugins can extend Gerrit screens with GWT controls +* Plugins can add custom settings screens +* Admins can now configure URL aliases +* User-specific URL aliases are also supported +* Referencing groups in `project.config` ++ +Plugins can refer to groups so that when they are renamed, the project +config will also be updated in this section. Other ~~~~~ From d4b57bea6a4205c95b7445f04e4226811509108c Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 19 Nov 2015 15:30:46 +0900 Subject: [PATCH 11/17] Update 2.12 release notes Change-Id: Ia409f7f22564d57a8b26864799ee0701ff66b9da --- ReleaseNotes/ReleaseNotes-2.12.txt | 212 ++++++++++++++++++++++++++--- 1 file changed, 190 insertions(+), 22 deletions(-) diff --git a/ReleaseNotes/ReleaseNotes-2.12.txt b/ReleaseNotes/ReleaseNotes-2.12.txt index 342cc0dba9..72ff9275fc 100644 --- a/ReleaseNotes/ReleaseNotes-2.12.txt +++ b/ReleaseNotes/ReleaseNotes-2.12.txt @@ -15,9 +15,10 @@ Important Notes java -jar gerrit.war init -d site_path ---- -*WARNING:* Upgrading to 2.12.x requires the server be first upgraded to 2.8 (or -2.9) and then to 2.12.x. If you are upgrading from 2.8.x or later, you may ignore -this warning and upgrade directly to 2.12.x. +*WARNING:* To use online reindexing when upgrading to 2.12.x the server must +first be upgraded to 2.8 (or 2.9) and then through 2.10 and 2.11 to 2.12.x. If +reindexing will be done offline, you may ignore this warning and upgrade directly +to 2.12.x. *WARNING:* When upgrading from version 2.8.4 or older with a site that uses Bouncy Castle Crypto, new versions of the libraries will be downloaded. The old @@ -69,7 +70,27 @@ enter the "Submitted, Merge Pending" state. GPG Keys and Signed Pushes ~~~~~~~~~~~~~~~~~~~~~~~~~~ -* TODO: Details +* Signed push can be enabled by setting +link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/config-gerrit.html#receive.enableSignedPush[ +`receive.enableSignedPush`] to true. ++ +When a client pushes with `git push --signed`, Gerrit ensures that the push +certificate is valid and signed with a valid public key stored in the +`refs/gpg-keys` branch of the `All-Users` repository. + +* When signed push is enabled, and +link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/config-gerrit.html#gerrit.editGpgKeys[ +`gerrit.editGpgKeys`] is set to true, users may upload their public GPG +key via the REST API or UI. ++ +If this setting is not enabled, GPG keys may only be added by administrators +with direct access to the `All-Users` repository. + +* Administrators may also configure +link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/config-gerrit.html#receive.certNonceSeed[ +`receive.certNonceSeed`] +link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/config-gerrit.html#receive.certNonceSlop[ +`receive.certNonceSlop`]. Secondary Index @@ -122,7 +143,18 @@ General Previously it was only possible to edit these preferences from the actual diff and edit screens. -* Add "Edits" to My dashboard menu. +* Add 'Edits' to the 'My' dashboard menu to list changes on which the user +has an unpublished edit revision. + +* Support for URL aliases. ++ +Administrators may define +link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/config-gerrit.html#urlAlias[ +URL aliases] to map plugin screens into the Gerrit URL namespace. ++ +Plugins may use user-specific URL aliases to replace certain screens for certain +users. + Project Screen ^^^^^^^^^^^^^^ @@ -149,6 +181,21 @@ Edit preferences are stored and loaded to/from the `All-Users` repository. Change Screen ^^^^^^^^^^^^^ +* link:http://code.google.com/p/gerrit/issues/detail?id=3318[Issue 3318]: +Highlight 'Reply' button if there are draft comments on any patch set. ++ +If any patch set of the change has a draft comment by the current user, +the 'Reply' button is highlighted. ++ +The icons depicting draft comments are removed from the revisions drop-down +list. + +* link:http://code.google.com/p/gerrit/issues/detail?id=1100[Issue 1100]: +Publish all draft comments when replying to a change. ++ +All draft comments, including those on older patch sets, are published when +replying to a change. + * Show file size increase/decrease for binary files. * Show uploader if different from change owner. @@ -170,6 +217,14 @@ Add syntax highlighting for Puppet. Add syntax highlighting for VHDL. +Group Screen +^^^^^^^^^^^^ + +* link:http://code.google.com/p/gerrit/issues/detail?id=1479[Issue 1479]: +The group screen now includes an 'Audit Log' panel showing member additions, +removals, and the user who made the change. + + API ~~~ @@ -193,12 +248,76 @@ REST API New REST API endpoints and new options on existing endpoints. -Tags -^^^^ -* Support filtering by substring and regex in the list tags endpoint. +Accounts +^^^^^^^^ -* Support pagination with `--start` and `--end` in the list tags endpoint. +* link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/rest-api-accounts.html#set-username[ +Set Username]: Set the username of an account. + +* link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/rest-api-accounts.html#get-detail[ +Get Account Details]: Get the details of an account. ++ +In addition to the +link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/rest-api-accounts.html#account-info[ +AccountInfo] fields returned by the existing + link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/rest-api-accounts.html#get-account[ +Get Account] endpoint, the new REST endpoint returns the registration date of +the account and the timestamp of when contact information was filed for this +account. + + +Changes +^^^^^^^ + +* link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/rest-api-changes.html#set-review[ +Set Review]: Add an option to omit duplicate comments. + +* link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/rest-api-changes.html#get-safe-content[ +Download Content]: Downloads the content of a file from a certain revision, in a +safe format that poses no risk for inadvertent execution of untrusted code. + +* link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/rest-api-changes.html#submitted-together[ +Get Submitted Together]: Get the list of all changes that will be submitted at +the same time as the change. + +* link:http://code.google.com/p/gerrit/issues/detail?id=1100[Issue 1100]: +link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/rest-api-changes.html#set-review[ +Set Review]: Add an option to publish draft comments on all revisions. + +Config +^^^^^^ + +* link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/rest-api-config.html#get-info[ +Get Server Info]: Returns information about the Gerrit server configuration. + +* link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/rest-api-config.html#confirm-email[ +Confirm Email]: Confirm that the user owns an email address. + + +Groups +^^^^^^ + +* link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/rest-api-groups.html#list-group[ +List Groups]: Add option to suggest groups. ++ +This allows group auto-completion to be used in a plugin's UI. + +* link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/rest-api-groups.html#get-audit-log[ +Get Audit Log]: Gets the audit log of a Gerrit internal group, showing member +additions, removals, and the user who made the change. + + +Projects +^^^^^^^^ + +* link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/rest-api-projects.html#run-gc[ +Run GC]: Add `aggressive` option to specify whether or not to run an aggressive +garbage collection. + +* link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/rest-api-projects.html#list-tags[ +List Tags]: Support filtering by substring and regex, and pagination with +`--start` and `--end`. SSH @@ -218,25 +337,45 @@ Plugins General ^^^^^^^ -* Gerrit client can now pass JavaScriptObjects to extension panels -* New UI extension point for header bar in change screen -* New UI extension point to password screen -* New UI extension points to project info screen -* New UI extension point for pop down buttons on change screen -* New UI extension point for buttons in header bar on change screen -* New UI extension point at bottom of the user preferences screen -* Plugins can extend Gerrit screens with GWT controls -* Plugins can add custom settings screens -* Admins can now configure URL aliases -* User-specific URL aliases are also supported -* Referencing groups in `project.config` +* Gerrit client can now pass JavaScriptObjects to extension panels. + +* New UI extension point for header bar in change screen. + +* New UI extension point to password screen. + +* New UI extension points to project info screen. + +* New UI extension point for pop down buttons on change screen. + +* New UI extension point for buttons in header bar on change screen. + +* New UI extension point at bottom of the user preferences screen. + +* New UI extension point for the 'Included In' drop-down panel. ++ +By implementing the +link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/dev-plugins.html#included-in[ +Included In interface], plugins may add entries to the 'Included In' dropdown +menu on the change screen. + +* Plugins can extend Gerrit screens with GWT controls. + +* Plugins can add custom settings screens. + +* Referencing groups in `project.config`. + Plugins can refer to groups so that when they are renamed, the project config will also be updated in this section. + Other ~~~~~ +* link:http://code.google.com/p/gerrit/issues/detail?id=3401[Issue 3401]: +Add option to +link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/config-gerrit.html#sendemail.allowRegisterNewEmail[ +disable registration of new email addresses]. + * link:http://code.google.com/p/gerrit/issues/detail?id=2061[Issue 2061] Add Support for `git-upload-archive`. + @@ -254,7 +393,8 @@ changes and automatically abandon them. link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/database-setup.html#createdb_db2[ DB2 database]. -* Add support for the +* link:http://code.google.com/p/gerrit/issues/detail?id=3441[Issue 3441]: +Add support for the link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/database-setup.html#createdb_derby[ Apache Derby database]. @@ -268,15 +408,33 @@ working. Setting `download.checkForHiddenChangeRefs` in the `gerrit.config` to true allows the download commands plugin to check for hidden change refs. +* Add a new 'Maintain Server' global capability. ++ +Members of a group with the 'Maintain Server' capability may view caches, tasks, +and queues, and invoke the index REST API on changes. + + Bug Fixes --------- +* link:http://code.google.com/p/gerrit/issues/detail?id=3499[Issue 3499]: +Fix syntax highlighting of raw string literals in go. + +* link:http://code.google.com/p/gerrit/issues/detail?id=3643[Issue 3643]: +Fix syntax highlighting of ES6 string templating using backticks. + * link:http://code.google.com/p/gerrit/issues/detail?id=3653[Issue 3653]: Correct timezone in sshd log after DST change. + When encountering a DST switch, the timezone wasn't updated until the server was reloaded. +* link:http://code.google.com/p/gerrit/issues/detail?id=3306[Issue 3306]: +Allow admins to read, push and create on `refs/users/default`. + +* link:http://code.google.com/p/gerrit/issues/detail?id=3212[Issue 3212]: +Fix failure to run `init` when `--site-path` option is not explicitly given. + * Make email validation case insensitive. + While link:https://tools.ietf.org/html/rfc5321#section-2.3.11[ @@ -294,6 +452,16 @@ intended to be pushed to Gerrit, and don't need a `Change-Id` line. This also prevents changes from being accidentally uploaded, at least for projects that have the 'Require Change-Id' configuration enabled. +* link:http://code.google.com/p/gerrit/issues/detail?id=3444[Issue 3444]: +download-commands plugin: Fix clone with commit-msg hook when project name +contains '/'. + +* Include full ref names in `ref-updated` events. ++ +The link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.12/json.html#refUpdate[ +refUpdate attribute] in `ref-updated` events did not include the full name +of the ref, i.e. `master` instead of `refs/heads/master`. + Upgrades -------- From 21bd07bc97016651482ec29b872ae6691f47060a Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 27 Nov 2015 00:15:42 +0900 Subject: [PATCH 12/17] Move sendemail.allowRegisterNewEmail to auth.allowRegisterNewEmail The sendemail section of the configuration is for settings related to how Gerrit sends emails to users. allowRegisterEmail controls whether or not users are allowed to register a new email address, which is not related to Gerrit's sending of email. Move the setting to the auth section, which is a more appropriate place for it. Change-Id: I06d9c4870060eb85bc4a05b536645d42df44e041 --- Documentation/config-gerrit.txt | 20 +++++++++---------- .../gerrit/server/account/DefaultRealm.java | 8 ++------ .../gerrit/server/auth/ldap/LdapRealm.java | 4 +--- .../gerrit/server/config/AuthConfig.java | 7 ++++++- .../gerrit/server/mail/EmailSettings.java | 2 -- 5 files changed, 19 insertions(+), 22 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 3992368756..45f7082c8f 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -482,6 +482,16 @@ editing gerrit.config and restarting the server. + Default is true. +[[auth.allowRegisterNewEmail]]auth.allowRegisterNewEmail:: ++ +Whether users are allowed to register new email addresses. ++ +In addition for the HTTP authentication type +link:#auth.httpemailheader[auth.httpemailheader] must *not* be set to +enable registration of new email addresses. ++ +By default, true. + [[cache]] === Section cache @@ -3154,16 +3164,6 @@ and all other properties of section sendemail are ignored. + By default, true, allowing notifications to be sent. -[[sendemail.allowRegisterNewEmail]]sendemail.allowRegisterNewEmail:: -+ -Whether users are allowed to register new email addresses. -+ -In addition for the HTTP authentication type -link:#auth.httpemailheader[auth.httpemailheader] must *not* be set to -enable registration of new email addresses. -+ -By default, true. - [[sendemail.connectTimeout]]sendemail.connectTimeout:: + The connection timeout of opening a socket connected to a diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/DefaultRealm.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/DefaultRealm.java index 7669cf96e8..5978703b42 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/DefaultRealm.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/DefaultRealm.java @@ -19,7 +19,6 @@ import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AuthType; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.config.AuthConfig; -import com.google.gerrit.server.mail.EmailSettings; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -28,17 +27,14 @@ import java.util.Set; @Singleton public class DefaultRealm extends AbstractRealm { private final EmailExpander emailExpander; - private final EmailSettings emailSettings; private final AccountByEmailCache byEmail; private final AuthConfig authConfig; @Inject DefaultRealm(EmailExpander emailExpander, - EmailSettings emailSettings, AccountByEmailCache byEmail, AuthConfig authConfig) { this.emailExpander = emailExpander; - this.emailSettings = emailSettings; this.byEmail = byEmail; this.authConfig = authConfig; } @@ -52,7 +48,7 @@ public class DefaultRealm extends AbstractRealm { case FULL_NAME: return Strings.emptyToNull(authConfig.getHttpDisplaynameHeader()) == null; case REGISTER_NEW_EMAIL: - return emailSettings.allowRegisterNewEmail + return authConfig.isAllowRegisterNewEmail() && Strings.emptyToNull(authConfig.getHttpEmailHeader()) == null; default: return true; @@ -60,7 +56,7 @@ public class DefaultRealm extends AbstractRealm { } else { switch (field) { case REGISTER_NEW_EMAIL: - return emailSettings.allowRegisterNewEmail; + return authConfig.isAllowRegisterNewEmail(); default: return true; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java index 9d7f7b7b58..cd1c4d3a50 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java @@ -33,7 +33,6 @@ import com.google.gerrit.server.account.EmailExpander; import com.google.gerrit.server.auth.AuthenticationUnavailableException; import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.GerritServerConfig; -import com.google.gerrit.server.mail.EmailSettings; import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -79,7 +78,6 @@ public class LdapRealm extends AbstractRealm { Helper helper, AuthConfig authConfig, EmailExpander emailExpander, - EmailSettings emailSettings, @Named(LdapModule.GROUP_CACHE) final LoadingCache> membershipCache, @Named(LdapModule.USERNAME_CACHE) final LoadingCache> usernameCache, @GerritServerConfig final Config config) { @@ -98,7 +96,7 @@ public class LdapRealm extends AbstractRealm { if (optdef(config, "accountSshUserName", "DEFAULT") != null) { readOnlyAccountFields.add(Account.FieldName.USER_NAME); } - if (!emailSettings.allowRegisterNewEmail) { + if (!authConfig.isAllowRegisterNewEmail()) { readOnlyAccountFields.add(Account.FieldName.REGISTER_NEW_EMAIL); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java index 6e6ed97ba5..c3bd519630 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java @@ -60,6 +60,7 @@ public class AuthConfig { private final String cookiePath; private final boolean cookieSecure; private final SignedToken emailReg; + private final boolean allowRegisterNewEmail; @Inject AuthConfig(@GerritServerConfig final Config cfg) @@ -90,7 +91,7 @@ public class AuthConfig { useContributorAgreements = cfg.getBoolean("auth", "contributoragreements", false); userNameToLowerCase = cfg.getBoolean("auth", "userNameToLowerCase", false); - + allowRegisterNewEmail = cfg.getBoolean("auth", "allowRegisterNewEmail", true); String key = cfg.getString("auth", null, "registerEmailPrivateKey"); if (key != null && !key.isEmpty()) { @@ -297,4 +298,8 @@ public class AuthConfig { return authType == AuthType.LDAP || authType == AuthType.LDAP_BIND; } + + public boolean isAllowRegisterNewEmail() { + return allowRegisterNewEmail; + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailSettings.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailSettings.java index 31135d0a2c..3c14f2f919 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailSettings.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailSettings.java @@ -22,13 +22,11 @@ import org.eclipse.jgit.lib.Config; @Singleton public class EmailSettings { - public final boolean allowRegisterNewEmail; public final boolean includeDiff; public final int maximumDiffSize; @Inject EmailSettings(@GerritServerConfig Config cfg) { - allowRegisterNewEmail = cfg.getBoolean("sendemail", "allowRegisterNewEmail", true); includeDiff = cfg.getBoolean("sendemail", "includeDiff", false); maximumDiffSize = cfg.getInt("sendemail", "maximumDiffSize", 256 << 10); } From 8e414350c8c4e4a38bacffdb05cfbf642a8bd9fd Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 27 Nov 2015 14:41:24 +0900 Subject: [PATCH 13/17] InitAdminUser: Don't assume the group ID of the Administrators group When using a database configuration where auto-increment column values are pre-allocated, it is possible that the "Administrators" group is created with an ID other than "1". In this case, the created admin user will not be added to the correct group, and will not have the correct admin permissions. Instead of hard-coding the group ID, get it from the database. Bug: Issue 3698 Change-Id: Ie9b3f4f9166d03e91d58b2ec584c5ce47aa2021c --- .../main/java/com/google/gerrit/pgm/init/InitAdminUser.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java index c1f00904cd..3a4d0f2daf 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java @@ -23,6 +23,7 @@ import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountExternalId; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroupMember; +import com.google.gerrit.reviewdb.client.AccountGroupName; import com.google.gerrit.reviewdb.client.AuthType; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gwtorm.server.SchemaFactory; @@ -94,9 +95,11 @@ public class InitAdminUser implements InitStep { a.setPreferredEmail(email); db.accounts().insert(Collections.singleton(a)); + AccountGroupName adminGroup = db.accountGroupNames().get( + new AccountGroup.NameKey("Administrators")); AccountGroupMember m = new AccountGroupMember(new AccountGroupMember.Key(id, - new AccountGroup.Id(1))); + adminGroup.getId())); db.accountGroupMembers().insert(Collections.singleton(m)); } } From 9af3b9faec72d1e08bb66de12da6b0a8f0c38c91 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Fri, 27 Nov 2015 10:11:37 +0100 Subject: [PATCH 14/17] Fix wrong date/time for commits in refs/users/default branch When the refs/users/default branch is modified using the SetPreferences REST endpoint (e.g. when changing the 'My' menu preferences) the commit date/time is wrong. Instead of the actual date/time the date/time of the last Gerrit server start is used. This is because MetaDataUpdate.User which gets the GerritPersonIdent injected is kept as member in the SetPreferences singleton and the date/time for commits in the refs/users/default branches is retrieved from that GerritPersonIdent instance which is only created once when the SetPreferences singleton is instantiated. The same bug was already fixed for the refs/users/XX/YYYY branches by change Idbf4618. Change-Id: Ia7ca0a373042e906a0c179b76be86ce3abf0806c Signed-off-by: Edwin Kempin --- .../com/google/gerrit/server/config/SetPreferences.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java index d97499cee5..519a4a4cbb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java @@ -23,6 +23,7 @@ import com.google.gerrit.server.account.SetPreferences.Input; import com.google.gerrit.server.account.VersionedAccountPreferences; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.inject.Inject; +import com.google.inject.Provider; import com.google.inject.Singleton; import org.eclipse.jgit.errors.ConfigInvalidException; @@ -32,11 +33,11 @@ import java.io.IOException; @RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER) @Singleton public class SetPreferences implements RestModifyView { - private final MetaDataUpdate.User metaDataUpdateFactory; + private final Provider metaDataUpdateFactory; private final AllUsersName allUsersName; @Inject - SetPreferences(MetaDataUpdate.User metaDataUpdateFactory, + SetPreferences(Provider metaDataUpdateFactory, AllUsersName allUsersName) { this.metaDataUpdateFactory = metaDataUpdateFactory; this.allUsersName = allUsersName; @@ -58,7 +59,7 @@ public class SetPreferences implements RestModifyView { } VersionedAccountPreferences p; - MetaDataUpdate md = metaDataUpdateFactory.create(allUsersName); + MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName); try { p = VersionedAccountPreferences.forDefault(); p.load(md); From 3aa6d030d3cfe394d209290e1e08cdd4dd378567 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Fri, 27 Nov 2015 11:02:39 +0100 Subject: [PATCH 15/17] Use REST implementation to list members for label with group operator ListMembers already implements the listing of group members and we should not implement the same logic in ChangeQueryBuilder once again. Change-Id: I5eb406355dc21ec88d329a2563fdd432610bcb45 Signed-off-by: Edwin Kempin --- .../gerrit/server/group/ListMembers.java | 3 +- .../query/change/BasicChangeRewrites.java | 2 +- .../query/change/ChangeQueryBuilder.java | 81 ++++++------------- .../gerrit/server/index/FakeQueryBuilder.java | 4 +- 4 files changed, 29 insertions(+), 61 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java index aab9efeafc..405cdf1c56 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java @@ -58,8 +58,9 @@ public class ListMembers implements RestReadView { this.accountLoader = accountLoaderFactory.create(true); } - public void setRecursive(boolean recursive) { + public ListMembers setRecursive(boolean recursive) { this.recursive = recursive; + return this; } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java index 851bbcd8ed..a9ef595a42 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java @@ -29,7 +29,7 @@ public class BasicChangeRewrites extends QueryRewriter { new InvalidProvider(), new InvalidProvider(), null, null, null, null, null, null, null, null, null, null, null, - null, null, null, null, null, null, null, null, null, null, null)); + null, null, null, null, null, null, null, null, null, null)); private static final QueryRewriter.Definition mydef = new QueryRewriter.Definition<>(BasicChangeRewrites.class, BUILDER); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index eb71f881be..34fdf5c19d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -17,19 +17,17 @@ package com.google.gerrit.server.query.change; import static com.google.gerrit.server.query.change.ChangeData.asChanges; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; import com.google.common.base.Optional; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.gerrit.common.Nullable; -import com.google.gerrit.common.data.GroupDetail; import com.google.gerrit.common.data.GroupReference; -import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.common.errors.NotSignedInException; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; -import com.google.gerrit.reviewdb.client.AccountGroupById; -import com.google.gerrit.reviewdb.client.AccountGroupMember; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.server.ReviewDb; @@ -40,14 +38,13 @@ import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.account.CapabilityControl; import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupBackends; -import com.google.gerrit.server.account.GroupCache; -import com.google.gerrit.server.account.GroupDetailFactory; import com.google.gerrit.server.change.ChangeTriplet; import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.TrackingFooters; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.strategy.SubmitStrategyFactory; +import com.google.gerrit.server.group.ListMembers; import com.google.gerrit.server.index.ChangeIndex; import com.google.gerrit.server.index.FieldDef; import com.google.gerrit.server.index.IndexCollection; @@ -140,7 +137,6 @@ public class ChangeQueryBuilder extends QueryBuilder { final Provider queryProvider; final Provider rewriter; final IdentifiedUser.GenericFactory userFactory; - final GroupDetailFactory.Factory groupDetailFactory; final CapabilityControl.Factory capabilityControlFactory; final ChangeControl.GenericFactory changeControlGenericFactory; final ChangeData.Factory changeDataFactory; @@ -152,14 +148,14 @@ public class ChangeQueryBuilder extends QueryBuilder { final PatchListCache patchListCache; final GitRepositoryManager repoManager; final ProjectCache projectCache; - final GroupCache groupCache; final Provider listChildProjects; final IndexCollection indexes; final SubmitStrategyFactory submitStrategyFactory; final ConflictsCache conflictsCache; final TrackingFooters trackingFooters; - final boolean allowsDrafts; final IndexConfig indexConfig; + final Provider listMembers; + final boolean allowsDrafts; private final Provider self; @@ -171,7 +167,6 @@ public class ChangeQueryBuilder extends QueryBuilder { IdentifiedUser.GenericFactory userFactory, Provider self, CapabilityControl.Factory capabilityControlFactory, - GroupDetailFactory.Factory groupDetailFactory, ChangeControl.GenericFactory changeControlGenericFactory, ChangeData.Factory changeDataFactory, FieldDef.FillArgs fillArgs, @@ -182,20 +177,20 @@ public class ChangeQueryBuilder extends QueryBuilder { PatchListCache patchListCache, GitRepositoryManager repoManager, ProjectCache projectCache, - GroupCache groupCache, Provider listChildProjects, IndexCollection indexes, SubmitStrategyFactory submitStrategyFactory, ConflictsCache conflictsCache, TrackingFooters trackingFooters, IndexConfig indexConfig, + Provider listMembers, @GerritServerConfig Config cfg) { this(db, queryProvider, rewriter, userFactory, self, - capabilityControlFactory, groupDetailFactory, changeControlGenericFactory, + capabilityControlFactory, changeControlGenericFactory, changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend, allProjectsName, patchListCache, repoManager, projectCache, - groupCache, listChildProjects, indexes, submitStrategyFactory, - conflictsCache, trackingFooters, indexConfig, + listChildProjects, indexes, submitStrategyFactory, + conflictsCache, trackingFooters, indexConfig, listMembers, cfg == null ? true : cfg.getBoolean("change", "allowDrafts", true)); } @@ -206,7 +201,6 @@ public class ChangeQueryBuilder extends QueryBuilder { IdentifiedUser.GenericFactory userFactory, Provider self, CapabilityControl.Factory capabilityControlFactory, - GroupDetailFactory.Factory groupDetailFactory, ChangeControl.GenericFactory changeControlGenericFactory, ChangeData.Factory changeDataFactory, FieldDef.FillArgs fillArgs, @@ -217,13 +211,13 @@ public class ChangeQueryBuilder extends QueryBuilder { PatchListCache patchListCache, GitRepositoryManager repoManager, ProjectCache projectCache, - GroupCache groupCache, Provider listChildProjects, IndexCollection indexes, SubmitStrategyFactory submitStrategyFactory, ConflictsCache conflictsCache, TrackingFooters trackingFooters, IndexConfig indexConfig, + Provider listMembers, boolean allowsDrafts) { this.db = db; this.queryProvider = queryProvider; @@ -231,7 +225,6 @@ public class ChangeQueryBuilder extends QueryBuilder { this.userFactory = userFactory; this.self = self; this.capabilityControlFactory = capabilityControlFactory; - this.groupDetailFactory = groupDetailFactory; this.changeControlGenericFactory = changeControlGenericFactory; this.changeDataFactory = changeDataFactory; this.fillArgs = fillArgs; @@ -242,24 +235,24 @@ public class ChangeQueryBuilder extends QueryBuilder { this.patchListCache = patchListCache; this.repoManager = repoManager; this.projectCache = projectCache; - this.groupCache = groupCache; this.listChildProjects = listChildProjects; this.indexes = indexes; this.submitStrategyFactory = submitStrategyFactory; this.conflictsCache = conflictsCache; this.trackingFooters = trackingFooters; - this.allowsDrafts = allowsDrafts; this.indexConfig = indexConfig; + this.listMembers = listMembers; + this.allowsDrafts = allowsDrafts; } Arguments asUser(CurrentUser otherUser) { return new Arguments(db, queryProvider, rewriter, userFactory, Providers.of(otherUser), - capabilityControlFactory, groupDetailFactory, changeControlGenericFactory, + capabilityControlFactory, changeControlGenericFactory, changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend, allProjectsName, patchListCache, repoManager, projectCache, - groupCache, listChildProjects, indexes, submitStrategyFactory, conflictsCache, - trackingFooters, indexConfig, allowsDrafts); + listChildProjects, indexes, submitStrategyFactory, conflictsCache, + trackingFooters, indexConfig, listMembers, allowsDrafts); } Arguments asUser(Account.Id otherId) { @@ -570,8 +563,15 @@ public class ChangeQueryBuilder extends QueryBuilder { // expand a group predicate into multiple user predicates if (group != null) { - Set allMembers = getMemberIds( - group, new HashSet()); + Set allMembers = + new HashSet<>(Lists.transform( + args.listMembers.get().setRecursive(true).apply(group), + new Function() { + @Override + public Account.Id apply(AccountInfo accountInfo) { + return new Account.Id(accountInfo._accountId); + } + })); int maxTerms = args.indexConfig.maxLimit(); if (allMembers.size() > maxTerms) { // limit the number of query terms otherwise Gerrit will barf @@ -842,39 +842,6 @@ public class ChangeQueryBuilder extends QueryBuilder { return g; } - private Set getMemberIds(AccountGroup.UUID groupUUID, - Set seenGroups) throws OrmException { - seenGroups.add(groupUUID); - - Set members = new HashSet<>(); - AccountGroup group = args.groupCache.get(groupUUID); - if (group != null) { - try { - GroupDetail groupDetail = - args.groupDetailFactory.create(group.getId()).call(); - if (groupDetail.members != null) { - for (AccountGroupMember m : groupDetail.members) { - if (!members.contains(m.getAccountId())) { - members.add(m.getAccountId()); - } - } - } - // Get members of subgroups - if (groupDetail.includes != null) { - for (AccountGroupById includedGroup : groupDetail.includes) { - if (!seenGroups.contains(includedGroup.getIncludeUUID())) { - members.addAll( - getMemberIds(includedGroup.getIncludeUUID(), seenGroups)); - } - } - } - } catch (NoSuchGroupException e) { - // The included group is not visible - } - } - return members; - } - private List parseChange(String value) throws OrmException, QueryParseException { if (PAT_LEGACY_ID.matcher(value).matches()) { diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java index 04fed32f55..2430815814 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java @@ -27,8 +27,8 @@ public class FakeQueryBuilder extends ChangeQueryBuilder { FakeQueryBuilder.class), new ChangeQueryBuilder.Arguments(null, null, null, null, null, null, null, null, null, null, null, null, null, - null, null, null, null, null, null, indexes, null, null, - null, null, null)); + null, null, null, null, indexes, null, null, null, null, + null, null)); } @Operator From 9bf41e4ecb8eaae0ea15e92a184f1ac57ab6378c Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sat, 28 Nov 2015 23:04:10 +0100 Subject: [PATCH 16/17] CurrentSchemaVersion: Allow to use it in plugins To implement dedicated databases in plugin based on GWTORM, current version plays important role in upgrading schema version. Change the modifiers for the attributes and the constructor to allow this. Gerrit CI plugin needs this to support schema upgrade in init plugin step. Change-Id: I372be503cff79a48d1320244e625bc7c5635eca5 --- .../google/gerrit/reviewdb/client/CurrentSchemaVersion.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/CurrentSchemaVersion.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/CurrentSchemaVersion.java index 183bb1ef42..cba5d41968 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/CurrentSchemaVersion.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/CurrentSchemaVersion.java @@ -26,7 +26,7 @@ public final class CurrentSchemaVersion { private static final String VALUE = "X"; @Column(id = 1, length = 1) - protected String one = VALUE; + public String one = VALUE; public Key() { } @@ -50,12 +50,12 @@ public final class CurrentSchemaVersion { } @Column(id = 1) - protected Key singleton; + public Key singleton; /** Current version number of the schema. */ @Column(id = 2) public transient int versionNbr; - protected CurrentSchemaVersion() { + public CurrentSchemaVersion() { } } From 455ed9cd27a16bf6991f04dcc57ef575dc4d5e75 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Tue, 1 Dec 2015 19:31:02 +0100 Subject: [PATCH 17/17] Buck: Remove non working local_jar rule and documentation Since If336ea8697 and since: [1] local_jar() is broken and cannot be fixed. It was always a hack that relied on the ability to define two different targets with the same output artifact name. Since: [1] it cannot be done any more. This rule was not used, but was provided by the build toolchain to allow to switch from maven_jar() to local_jar(). * [1] https://github.com/facebook/buck/commit/c92ef212b53fff08a8452649b4d4faadc6b89b11 Change-Id: I0900d7f17bb1fcf6c91c13d3d845293f805cc5ca --- Documentation/dev-buck.txt | 56 -------------------------------------- lib/local.defs | 33 ---------------------- lib/maven.defs | 2 -- 3 files changed, 91 deletions(-) delete mode 100644 lib/local.defs diff --git a/Documentation/dev-buck.txt b/Documentation/dev-buck.txt index 10f331949a..ec8515fa3c 100644 --- a/Documentation/dev-buck.txt +++ b/Documentation/dev-buck.txt @@ -371,62 +371,6 @@ that artifact: ) ---- -== Building against unpublished JARs, that change frequently - -If a dependent Gerrit library is undergoing active development it must be -recompiled and the change must be reflected in the Buck build process. For -example testing Gerrit against changed JGit snapshot version. After building -JGit library, the artifacts are created in local Maven build directory, e. g.: - ----- - mvn package - /home//projects/jgit/org.eclipse.jgit/target/org.eclipse.jgit-3.3.0-SNAPSHOT.jar - /home//projects/jgit/org.eclipse.jgit/target/org.eclipse.jgit-3.3.0-SNAPSHOT-sources.jar ----- - -If as usual, installation of the build artifacts takes place in local maven -repository, then the Buck build must fetch them from there with normal -`download_file.py` process. Disadvantage of this approach is that Buck cache -invalidation must occur to refresh the artifacts after next -change-compile-install round trip. - -To shorten that workflow and take the installation of the artifacts to the -local Maven repository and fetching it again from there out of the picture, -`local_jar()` method is used instead of `maven_jar()`: - -[source,python] ----- - local_jar( - name = 'jgit', - jar = '/home//projects/jgit/org.eclipse.jgit/target/org.eclipse.jgit-3.3.0-SNAPSHOT.jar', - src = '/home//projects/jgit/org.eclipse.jgit/target/org.eclipse.jgit-3.3.0-SNAPSHOT-sources.jar', - deps = [':ewah'] - ) ----- - -This creates a symlink to the Buck targets direct against artifacts in -another project's Maven target directory: - ----- - buck-out/gen/lib/jgit/jgit.jar -> - /home//projects/jgit/org.eclipse.jgit/target/org.eclipse.jgit-3.3.0-SNAPSHOT.jar ----- - -After `buck clean` and `buck build lib/jgit:jgit` the symbolic link that was -created the first time is lost due to Buck's caching mechanism. This means that -when a new version of the local artifact is deployed (by running `mvn package` -in the JGit project in the example above), Buck is not aware of it, because it -still has a stale version of it in its cache. - -To solve this problem and re-create the symbolic link, you don't need to wipe out -the entire Buck cache. Just rebuilding the target with the `--no-cache` option -does the job: - ----- - buck clean - buck build --no-cache lib/jgit:jgit ----- - == Building against artifacts from custom Maven repositories To build against custom Maven repositories, two modes of operations are diff --git a/lib/local.defs b/lib/local.defs deleted file mode 100644 index 6eec581a0d..0000000000 --- a/lib/local.defs +++ /dev/null @@ -1,33 +0,0 @@ -def local_jar( - name, - jar, - src = None, - deps = [], - visibility = ['PUBLIC']): - binjar = name + '.jar' - srcjar = name + '-src.jar' - genrule( - name = '%s__local_bin' % name, - cmd = 'ln -s %s $OUT' % jar, - out = binjar) - if src: - genrule( - name = '%s__local_src' % name, - cmd = 'ln -s %s $OUT' % src, - out = srcjar) - prebuilt_jar( - name = '%s_src' % name, - binary_jar = ':%s__local_src' % name, - visibility = visibility, - ) - else: - srcjar = None - - prebuilt_jar( - name = name, - deps = deps, - binary_jar = ':%s__local_bin' % name, - source_jar = ':%s__local_src' % name if srcjar else None, - visibility = visibility, - ) - diff --git a/lib/maven.defs b/lib/maven.defs index a16fda1281..5c29beda36 100644 --- a/lib/maven.defs +++ b/lib/maven.defs @@ -12,8 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -include_defs('//lib/local.defs') - GERRIT = 'GERRIT:' GERRIT_API = 'GERRIT_API:' MAVEN_CENTRAL = 'MAVEN_CENTRAL:'