Merge "Allow to schedule a task to run gc for all projects"
diff --git a/.buckconfig b/.buckconfig
index f0422c9..43bfa5e 100644
--- a/.buckconfig
+++ b/.buckconfig
@@ -24,4 +24,4 @@
[cache]
mode = dir
- dir = buck-out/cache
+ dir = ~/.gerritcodereview/buck-cache/cache
diff --git a/.buckversion b/.buckversion
index 4462205..7a0d3ff 100644
--- a/.buckversion
+++ b/.buckversion
@@ -1 +1 @@
-75000f4273a399d2cd2768512639da80748877b7
+0c2056b8151692e12b9cb5e03242d1a1af2bd1a1
diff --git a/Documentation/BUCK b/Documentation/BUCK
index 9c2aea8..f070d7e 100644
--- a/Documentation/BUCK
+++ b/Documentation/BUCK
@@ -10,33 +10,23 @@
name = 'html',
cmd = 'cd $TMP;' +
'mkdir -p %s/images;' % DOC_DIR +
- 'unzip -q $SRCDIR/only_html.zip -d %s/;' % DOC_DIR +
+ 'unzip -q $(location %s) -d %s/;'
+ % (':generate_html', DOC_DIR) +
'for s in $SRCS;do ln -s $s %s;done;' % DOC_DIR +
'mv %s/*.{jpg,png} %s/images;' % (DOC_DIR, DOC_DIR) +
- 'rm %s/only_html.zip;' % DOC_DIR +
- 'rm %s/licenses.txt;' % DOC_DIR +
- 'cp $SRCDIR/licenses.txt LICENSES.txt;' +
+ 'cp $(location %s) LICENSES.txt;' % ':licenses.txt' +
'zip -qr $OUT *',
srcs = glob([
'images/*.jpg',
'images/*.png',
- ]) + [
- 'doc.css',
- genfile('licenses.txt'),
- genfile('only_html.zip'),
- ],
- deps = [
- ':generate_html',
- ':licenses.txt',
- ],
+ ]) + ['doc.css'],
out = 'html.zip',
visibility = ['PUBLIC'],
)
genasciidoc(
name = 'generate_html',
- srcs = SRCS + [genfile('licenses.txt')],
- deps = [':licenses.txt'],
+ srcs = SRCS + [':licenses.txt'],
attributes = documentation_attributes(git_describe()),
backend = 'html5',
out = 'only_html.zip',
@@ -66,18 +56,14 @@
'--prefix "%s/" ' % DOC_DIR +
'--in-ext ".txt" ' +
'--out-ext ".html" ' +
- '$SRCS',
- srcs = SRCS + [genfile('licenses.txt')],
- deps = [
- ':licenses.txt',
- '//lib/asciidoctor:doc_indexer',
- ],
+ '$SRCS ' +
+ '$(location :licenses.txt)',
+ srcs = SRCS,
out = 'index.jar',
)
prebuilt_jar(
name = 'index_lib',
- binary_jar = genfile('index.jar'),
- deps = [':index'],
+ binary_jar = ':index',
visibility = ['PUBLIC'],
)
diff --git a/Documentation/asciidoc.defs b/Documentation/asciidoc.defs
index b361d48..7adf265 100644
--- a/Documentation/asciidoc.defs
+++ b/Documentation/asciidoc.defs
@@ -16,7 +16,6 @@
name,
out,
srcs = [],
- deps = [],
attributes = [],
backend = None,
visibility = []):
@@ -35,33 +34,35 @@
asciidoc.extend(['-a', attribute])
asciidoc.append('$SRCS')
newsrcs = ["doc.css"]
- newdeps = deps + ['//lib/asciidoctor:asciidoc']
-
for src in srcs:
- tx = []
fn = src
- if fn.startswith('BUCKGEN:') :
- fn = src[8:]
- tx = [':' + fn]
+ # We have two cases: regular source files and generated files.
+ # Generated files are passed as targets ':foo', and ':' is removed.
+ # 1. regular files: cmd = '-s foo', srcs = ['foo']
+ # 2. generated files: cmd = '-s $(location :foo)', srcs = []
+ srcs = [src]
+ passed_src = fn
+ if fn.startswith(':') :
+ fn = src[1:]
+ srcs = []
+ passed_src = '$(location :%s)' % fn
ex = fn + EXPN
genrule(
name = ex,
cmd = '$(exe :replace_macros) --suffix=' + EXPN +
- ' -s %s' % fn +
+ ' -s ' + passed_src +
' -o $OUT',
- srcs = [src],
- deps = tx + [':replace_macros'],
+ srcs = srcs,
out = ex,
)
- newdeps.append(':' + ex)
- newsrcs.append(genfile(ex))
+
+ asciidoc.append('$(location :%s)' % ex)
genrule(
name = name,
cmd = ' '.join(asciidoc),
srcs = newsrcs,
- deps = newdeps,
out = out,
visibility = visibility,
)
diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt
index 0fe12ca..aaeb834 100644
--- a/Documentation/config-labels.txt
+++ b/Documentation/config-labels.txt
@@ -183,14 +183,6 @@
box will be either the lowest or highest score in the permissible range.
-[[label_abbreviation]]
-=== `label.Label-Name.abbreviation`
-
-An abbreviated name for a label shown as a compact column header, for
-example on project dashboards. Defaults to all the uppercase characters
-in the label name, e.g. `Label-Name` is abbreviated by default as `LN`.
-
-
[[label_function]]
=== `label.Label-Name.function`
diff --git a/Documentation/config-reverseproxy.txt b/Documentation/config-reverseproxy.txt
index 0c52be6..dde2661 100644
--- a/Documentation/config-reverseproxy.txt
+++ b/Documentation/config-reverseproxy.txt
@@ -135,6 +135,14 @@
Make sure to use a 'proxy_pass' URL without any path (esp. no trailing
'/' after the 'host:port').
+If you are using Apache httpd server with mod_jk and AJP connector, add
+the following option to your httpd.conf directly or included from another
+file:
+
+----
+JkOptions +ForwardURICompatUnparsed
+----
+
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/Documentation/dev-buck.txt b/Documentation/dev-buck.txt
index 2674398..492bfb3 100644
--- a/Documentation/dev-buck.txt
+++ b/Documentation/dev-buck.txt
@@ -5,7 +5,7 @@
There is currently no binary distribution of Buck, so it has to be manually
built and installed. Apache Ant is required. Currently only Linux and Mac
-OS are supported. Gerrit's buck wrappers require Python version 2.6 or higher.
+OS are supported. Buck requires Python version 2.7 to be installed.
Clone the git and build it:
diff --git a/Documentation/dev-contributing.txt b/Documentation/dev-contributing.txt
index 5e307f9..95a5554 100644
--- a/Documentation/dev-contributing.txt
+++ b/Documentation/dev-contributing.txt
@@ -171,11 +171,13 @@
* Define any static interfaces next in your class.
* Define non static interfaces after static interfaces in your
class.
- * Next you should define static types and members.
+ * Next you should define static types, members, and methods, in
+ decreasing order of visibility (public to private).
* Finally instance members, then constructors, and then instance
methods.
- * Some common exceptions are private helper static methods which
- might appear near the instance methods which they help.
+ * Some common exceptions are private helper static methods, which
+ might appear near the instance methods which they help (but may
+ also appear at the top).
* Getters and setters for the same instance field should usually
be near each other baring a good reason not to.
* If you are using assisted injection, the factory for your class
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-rename.png b/Documentation/images/user-review-ui-side-by-side-diff-screen-rename.png
new file mode 100644
index 0000000..b4d83ba
--- /dev/null
+++ b/Documentation/images/user-review-ui-side-by-side-diff-screen-rename.png
Binary files differ
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 5d7fe95..49ecc44 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -490,7 +490,10 @@
----
As response a link:#change-info[ChangeInfo] entity is returned that
-describes the change.
+describes the change. This response will contain all votes for each
+label and include one combined vote. The combined label vote is
+calculated in the following order (from highest to lowest):
+REJECTED > APPROVED > DISLIKED > RECOMMENDED.
.Response
----
@@ -544,12 +547,6 @@
}
},
"Code-Review": {
- "recommended": {
- "_account_id": 1000097,
- "name": "Jane Roe",
- "email": "jane.roe@example.com",
- "username": "jroe"
- },
"disliked": {
"_account_id": 1000096,
"name": "John Doe",
@@ -2923,7 +2920,7 @@
[[comment-input]]
=== CommentInput
-The `CommitInput` entity contains information for creating an inline
+The `CommentInput` entity contains information for creating an inline
comment.
[options="header",width="50%",cols="1,^1,5"]
diff --git a/Documentation/user-review-ui.txt b/Documentation/user-review-ui.txt
index 00b957e..81ceb06 100644
--- a/Documentation/user-review-ui.txt
+++ b/Documentation/user-review-ui.txt
@@ -756,14 +756,20 @@
image::images/user-review-ui-side-by-side-diff-screen-patch-sets.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-patch-sets.png"]
+The download icon next to the patch set list allows to download the
+patch. Unless the mime type of the file is configured as safe, the
+download file is a zip archive that contains the patch file.
+
If the compared patches are identical, this is highlighted by a red
`No Differences` label in the screen header.
image::images/user-review-ui-side-by-side-diff-screen-no-differences.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-no-differences.png"]
-The download icon next to the patch set list allows to download the
-patch. Unless the mime type of the file is configured as safe, the
-download file is a zip archive that contains the patch file.
+If a file was renamed, the old and new file paths are shown in the
+header together with a similarity index that shows how much of the file
+content is unmodified.
+
+image::images/user-review-ui-side-by-side-diff-screen-rename.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-rename.png"]
For navigating between the patches in a patch set there are navigation
buttons on the right side of the screen header. The left arrow button
diff --git a/bucklets/gerrit_plugin.bucklet b/bucklets/gerrit_plugin.bucklet
new file mode 100644
index 0000000..eb10456
--- /dev/null
+++ b/bucklets/gerrit_plugin.bucklet
@@ -0,0 +1,15 @@
+#
+# Dummy to make the co-existence of core and standalone plugins possible.
+# Intentionaly left empty as this doesn't suppose to have any side effects
+# in tree build, i. e.:
+#
+# cookbook-plugin/BUCK include this line:
+# include_defs('//bucklets/gerrit_plugin.bucklet')
+#
+# When executing from the Gerrit tree:
+# buck build plugins/cookbook-plugin
+#
+# this line has no effect.
+#
+# When compiling from standalone cookbook-plugin, bucklets directory points
+# to cloned bucklets library that includes real gerrit_plugin.bucklet code.
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 5c5746b..fc55411 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -21,6 +21,7 @@
import com.google.common.base.Joiner;
import com.google.common.primitives.Chars;
+import com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -128,9 +129,9 @@
userSession = new RestSession(server, user);
initSsh(admin);
db = reviewDbProvider.open();
- atrScope.set(atrScope.newContext(reviewDbProvider, sshSession,
- identifiedUserFactory.create(Providers.of(db), admin.getId())));
- sshSession = new SshSession(server, admin);
+ Context ctx = newRequestContext(admin);
+ atrScope.set(ctx);
+ sshSession = ctx.getSession();
project = new Project.NameKey("p");
createProject(sshSession, project.get());
git = cloneProject(sshSession.getUrl() + "/" + project.get());
@@ -194,6 +195,19 @@
return gApi.changes().id(id).get(s);
}
+ protected List<ChangeInfo> query(String q) throws RestApiException {
+ return gApi.changes().query(q).get();
+ }
+
+ private Context newRequestContext(TestAccount account) {
+ return atrScope.newContext(reviewDbProvider, new SshSession(server, admin),
+ identifiedUserFactory.create(Providers.of(db), account.getId()));
+ }
+
+ protected Context setApiUser(TestAccount account) {
+ return atrScope.set(newRequestContext(account));
+ }
+
protected static Gson newGson() {
return OutputFormat.JSON_COMPACT.newGson();
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index a01525a..c79b198 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -186,9 +186,9 @@
@Test
public void queryChangesNoResults() throws Exception {
createChange();
- List<ChangeInfo> results = gApi.changes().query("status:open").get();
+ List<ChangeInfo> results = query("status:open");
assertEquals(1, results.size());
- results = gApi.changes().query("status:closed").get();
+ results = query("status:closed");
assertTrue(results.isEmpty());
}
@@ -196,7 +196,7 @@
public void queryChangesOneTerm() throws Exception {
PushOneCommit.Result r1 = createChange();
PushOneCommit.Result r2 = createChange();
- List<ChangeInfo> results = gApi.changes().query("status:open").get();
+ List<ChangeInfo> results = query("status:open");
assertEquals(2, results.size());
assertEquals(r2.getChangeId(), results.get(0).changeId);
assertEquals(r1.getChangeId(), results.get(1).changeId);
@@ -206,9 +206,7 @@
public void queryChangesMultipleTerms() throws Exception {
PushOneCommit.Result r1 = createChange();
createChange();
- List<ChangeInfo> results = gApi.changes()
- .query("status:open " + r1.getChangeId())
- .get();
+ List<ChangeInfo> results = query("status:open " + r1.getChangeId());
assertEquals(r1.getChangeId(), Iterables.getOnlyElement(results).changeId);
}
@@ -232,8 +230,7 @@
@Test
public void queryChangesNoOptions() throws Exception {
PushOneCommit.Result r = createChange();
- ChangeInfo result = Iterables.getOnlyElement(
- gApi.changes().query(r.getChangeId()).get());
+ ChangeInfo result = Iterables.getOnlyElement(query(r.getChangeId()));
assertNull(result.labels);
assertNull(result.messages);
assertNull(result.revisions);
@@ -256,4 +253,29 @@
assertEquals(r.getPatchSetId().get(), rev._number);
assertFalse(rev.actions.isEmpty());
}
+
+ @Test
+ public void queryChangesOwnerWithDifferentUsers() throws Exception {
+ PushOneCommit.Result r = createChange();
+ assertEquals(r.getChangeId(),
+ Iterables.getOnlyElement(query("owner:self")).changeId);
+ setApiUser(user);
+ assertTrue(query("owner:self").isEmpty());
+ }
+
+ @Test
+ public void checkReviewedFlagBeforeAndAfterReview() throws Exception {
+ PushOneCommit.Result r = createChange();
+ AddReviewerInput in = new AddReviewerInput();
+ in.reviewer = user.email;
+ gApi.changes()
+ .id(r.getChangeId())
+ .addReviewer(in);
+
+ setApiUser(user);
+ assertNull(get(r.getChangeId()).reviewed);
+
+ revision(r).review(ReviewInput.recommend());
+ assertTrue(get(r.getChangeId()).reviewed);
+ }
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/BUCK b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/BUCK
index d63d195..f42a134 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/BUCK
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/BUCK
@@ -10,6 +10,10 @@
acceptance_tests(
srcs = OTHER_TESTS,
+ deps = [
+ ':submit_util',
+ '//lib/joda:joda-time',
+ ],
labels = ['rest'],
)
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java
index 6352b50..acf50db 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java
@@ -14,6 +14,8 @@
package com.google.gerrit.acceptance.rest.change;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.SECONDS;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
@@ -23,14 +25,55 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.testutil.ConfigSuite;
import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.lib.Config;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeUtils;
+import org.joda.time.DateTimeUtils.MillisProvider;
+import org.junit.After;
+import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
import java.io.IOException;
import java.util.Iterator;
+import java.util.concurrent.atomic.AtomicLong;
+@RunWith(ConfigSuite.class)
public class ChangeMessagesIT extends AbstractDaemonTest {
+ private String systemTimeZone;
+ private volatile long clockStepMs;
+
+ @ConfigSuite.Config
+ public static Config noteDbEnabled() {
+ Config cfg = new Config();
+ cfg.setBoolean("notedb", null, "write", true);
+ cfg.setBoolean("notedb", "changeMessages", "read", true);
+ return cfg;
+ }
+
+ @Before
+ public void setTimeForTesting() {
+ systemTimeZone = System.setProperty("user.timezone", "US/Eastern");
+ clockStepMs = MILLISECONDS.convert(1, SECONDS);
+ final AtomicLong clockMs = new AtomicLong(
+ new DateTime(2009, 9, 30, 17, 0, 0).getMillis());
+
+ DateTimeUtils.setCurrentMillisProvider(new MillisProvider() {
+ @Override
+ public long getMillis() {
+ return clockMs.getAndAdd(clockStepMs);
+ }
+ });
+ }
+
+ @After
+ public void resetTime() {
+ DateTimeUtils.setCurrentMillisSystem();
+ System.setProperty("user.timezone", systemTimeZone);
+ }
@Test
public void messagesNotReturnedByDefault() throws Exception {
diff --git a/gerrit-antlr/BUCK b/gerrit-antlr/BUCK
index 57e7e1d..03c3c1e 100644
--- a/gerrit-antlr/BUCK
+++ b/gerrit-antlr/BUCK
@@ -17,8 +17,8 @@
java_library(
name = 'lib',
- srcs = [genfile('query_antlr.src.zip')],
- deps = PARSER_DEPS + [':query_antlr'],
+ srcs = [':query_antlr'],
+ deps = PARSER_DEPS,
)
# Hack necessary to expose ANTLR generated code as JAR to Eclipse.
@@ -28,9 +28,10 @@
deps = [':lib'],
out = 'query_parser.jar',
)
+
prebuilt_jar(
name = 'query_parser',
- binary_jar = genfile('query_parser.jar'),
- deps = PARSER_DEPS + [':query_link'],
+ binary_jar = ':query_link',
+ deps = PARSER_DEPS,
visibility = ['PUBLIC'],
)
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java
index 28629b9..337aefd 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java
@@ -60,7 +60,7 @@
return name;
}
- public static String defaultAbbreviation(String name) {
+ private static String getAbbreviation(String name) {
StringBuilder abbr = new StringBuilder();
for (int i = 0; i < name.length(); i++) {
char c = name.charAt(i);
@@ -128,7 +128,7 @@
values = sortValues(valueList);
defaultValue = 0;
- abbreviation = defaultAbbreviation(name);
+ abbreviation = getAbbreviation(name);
functionName = "MaxWithBlock";
maxNegative = Short.MIN_VALUE;
@@ -151,14 +151,6 @@
return psa.getLabelId().get().equalsIgnoreCase(name);
}
- public String getAbbreviation() {
- return abbreviation;
- }
-
- public void setAbbreviation(String abbreviation) {
- this.abbreviation = abbreviation;
- }
-
public String getFunctionName() {
return functionName;
}
diff --git a/gerrit-gwtui/BUCK b/gerrit-gwtui/BUCK
index 8dc277a..f1c9e6b 100644
--- a/gerrit-gwtui/BUCK
+++ b/gerrit-gwtui/BUCK
@@ -95,11 +95,10 @@
prebuilt_jar(
name = 'diffy_logo',
- binary_jar = genfile('diffy_images.jar'),
+ binary_jar = ':diffy_image_files_ln',
deps = [
'//lib:LICENSE-diffy',
'//lib:LICENSE-CC-BY3.0',
- ':diffy_image_files_ln',
],
)
diff --git a/gerrit-gwtui/gwt.defs b/gerrit-gwtui/gwt.defs
index b903977..0e10116 100644
--- a/gerrit-gwtui/gwt.defs
+++ b/gerrit-gwtui/gwt.defs
@@ -63,9 +63,8 @@
)
prebuilt_jar(
name = '%s_gwtxml_lib' % gwt_name,
- binary_jar = genfile(jar),
- gwt_jar = genfile(jar),
- deps = [':%s_gwtxml_gen' % gwt_name],
+ binary_jar = ':%s_gwtxml_gen' % gwt_name,
+ gwt_jar = ':%s_gwtxml_gen' % gwt_name,
)
gwt_binary(
name = gwt_name,
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java
index b472bbd..d315fa3 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java
@@ -68,7 +68,8 @@
this.anonymousProvider = anonymousProvider;
this.identified = identified;
- if (!GitSmartHttpTools.isGitClient(request)) {
+ if (request.getRequestURI() == null
+ || !GitSmartHttpTools.isGitClient(request)) {
String cookie = readCookie();
if (cookie != null) {
key = new Key(cookie);
@@ -184,6 +185,10 @@
}
private void saveCookie() {
+ if (response == null) {
+ return;
+ }
+
final String token;
final int ageSeconds;
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/H2CacheBasedWebSession.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/H2CacheBasedWebSession.java
index 3dfb9fb..4b68f02 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/H2CacheBasedWebSession.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/H2CacheBasedWebSession.java
@@ -17,6 +17,7 @@
import static java.util.concurrent.TimeUnit.MINUTES;
import com.google.common.cache.Cache;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.httpd.WebSessionManager.Val;
import com.google.gerrit.server.AnonymousUser;
@@ -56,7 +57,7 @@
@Inject
H2CacheBasedWebSession(
HttpServletRequest request,
- HttpServletResponse response,
+ @Nullable HttpServletResponse response,
WebSessionManagerFactory managerFactory,
@Named(WebSessionManager.CACHE_NAME) Cache<String, Val> cache,
AuthConfig authConfig,
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/gitweb/GitWebServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/gitweb/GitWebServlet.java
index f831ab5..2902335 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/gitweb/GitWebServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/gitweb/GitWebServlet.java
@@ -34,6 +34,7 @@
import com.google.gerrit.httpd.GitWebConfig;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.LocalDiskRepositoryManager;
@@ -85,18 +86,21 @@
private final LocalDiskRepositoryManager repoManager;
private final ProjectControl.Factory projectControl;
private final Provider<AnonymousUser> anonymousUserProvider;
+ private final Provider<CurrentUser> userProvider;
private final EnvList _env;
@Inject
GitWebServlet(final LocalDiskRepositoryManager repoManager,
final ProjectControl.Factory projectControl,
final Provider<AnonymousUser> anonymousUserProvider,
+ final Provider<CurrentUser> userProvider,
final SitePaths site,
final GerritConfig gerritConfig, final GitWebConfig gitWebConfig)
throws IOException {
this.repoManager = repoManager;
this.projectControl = projectControl;
this.anonymousUserProvider = anonymousUserProvider;
+ this.userProvider = userProvider;
this.gitwebCgi = gitWebConfig.getGitwebCGI();
this.deniedActions = new HashSet<>();
@@ -377,7 +381,14 @@
throw new NoSuchProjectException(nameKey);
}
} catch (NoSuchProjectException e) {
- rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
+ if (userProvider.get().isIdentifiedUser()) {
+ rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
+ } else {
+ // Allow anonymous users a chance to login.
+ // Avoid leaking information by not distinguishing between
+ // project not existing and no access rights.
+ rsp.sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ }
return;
}
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 45b55c1..907760b 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
@@ -29,6 +29,7 @@
import com.google.gerrit.server.plugins.Plugin;
import com.google.gerrit.server.plugins.PluginsCollection;
import com.google.gerrit.server.plugins.ReloadPluginListener;
+import com.google.gerrit.server.plugins.ServerPlugin;
import com.google.gerrit.server.plugins.StartPluginListener;
import com.google.gerrit.server.ssh.SshInfo;
import com.google.gwtexpui.server.CacheHeaders;
@@ -82,6 +83,13 @@
private static final long serialVersionUID = 1L;
private static final Logger log
= LoggerFactory.getLogger(HttpPluginServlet.class);
+ private static final Comparator<JarEntry> JAR_ENTRY_COMPARATOR_BY_NAME =
+ new Comparator<JarEntry>() {
+ @Override
+ public int compare(JarEntry a, JarEntry b) {
+ return a.getName().compareTo(b.getName());
+ }
+ };
private final MimeUtilFileTypeRegistry mimeUtil;
private final Provider<String> webUrl;
@@ -268,7 +276,7 @@
}
if (file.startsWith(holder.staticPrefix)) {
- JarFile jar = holder.plugin.getJarFile();
+ JarFile jar = jarFileOf(holder.plugin);
if (jar != null) {
JarEntry entry = jar.getJarEntry(file);
if (exists(entry)) {
@@ -286,7 +294,7 @@
} else if (file.startsWith(holder.docPrefix) && file.endsWith("/")) {
res.sendRedirect(uri + "index.html");
} else if (file.startsWith(holder.docPrefix)) {
- JarFile jar = holder.plugin.getJarFile();
+ JarFile jar = jarFileOf(holder.plugin);
JarEntry entry = jar.getJarEntry(file);
if (!exists(entry)) {
entry = findSource(jar, file);
@@ -368,18 +376,9 @@
}
}
}
- Collections.sort(cmds, new Comparator<JarEntry>() {
- @Override
- public int compare(JarEntry a, JarEntry b) {
- return a.getName().compareTo(b.getName());
- }
- });
- Collections.sort(docs, new Comparator<JarEntry>() {
- @Override
- public int compare(JarEntry a, JarEntry b) {
- return a.getName().compareTo(b.getName());
- }
- });
+
+ Collections.sort(cmds, JAR_ENTRY_COMPARATOR_BY_NAME);
+ Collections.sort(docs, JAR_ENTRY_COMPARATOR_BY_NAME);
StringBuilder md = new StringBuilder();
md.append(String.format("# Plugin %s #\n", pluginName));
@@ -632,6 +631,14 @@
return data;
}
+ private static JarFile jarFileOf(Plugin plugin) {
+ if(plugin instanceof ServerPlugin) {
+ return ((ServerPlugin) plugin).getJarFile();
+ } else {
+ return null;
+ }
+ }
+
private static class PluginHolder {
final Plugin plugin;
final GuiceFilter filter;
@@ -648,7 +655,7 @@
}
private static String getPrefix(Plugin plugin, String attr, String def) {
- JarFile jarFile = plugin.getJarFile();
+ JarFile jarFile = jarFileOf(plugin);
if (jarFile == null) {
return def;
}
diff --git a/gerrit-plugin-gwtui/BUCK b/gerrit-plugin-gwtui/BUCK
index 911016c..419176d 100644
--- a/gerrit-plugin-gwtui/BUCK
+++ b/gerrit-plugin-gwtui/BUCK
@@ -26,7 +26,7 @@
name = 'gwtui-api-lib2',
srcs = SRCS,
resources = glob(['src/main/**/*']),
- deps = ['//gerrit-gwtui-common:client-lib2'],
+ exported_deps = ['//gerrit-gwtui-common:client-lib2'],
provided_deps = DEPS,
visibility = ['PUBLIC'],
)
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
index 86f77db..dbce048 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
@@ -415,6 +415,7 @@
subject = other.subject;
topic = other.topic;
mergeable = other.mergeable;
+ lastSha1MergeTested = other.lastSha1MergeTested;
}
/** Legacy 32 bit integer identity for a change. */
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java
new file mode 100644
index 0000000..6ef04fb
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java
@@ -0,0 +1,76 @@
+// Copyright (C) 2014 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;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.gerrit.reviewdb.client.ChangeMessage;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Utility functions to manipulate ChangeMessages.
+ * <p>
+ * These methods either query for and update ChangeMessages in the NoteDb or
+ * ReviewDb, depending on the state of the NotesMigration.
+ */
+@Singleton
+public class ChangeMessagesUtil {
+ private static List<ChangeMessage> sortChangeMessages(
+ Iterable<ChangeMessage> changeMessage) {
+ return ChangeNotes.MESSAGE_BY_TIME.sortedCopy(changeMessage);
+ }
+
+ private final NotesMigration migration;
+
+ @VisibleForTesting
+ @Inject
+ public ChangeMessagesUtil(NotesMigration migration) {
+ this.migration = migration;
+ }
+
+ public List<ChangeMessage> byChange(ReviewDb db, ChangeNotes notes) throws OrmException {
+ if (!migration.readChangeMessages()) {
+ return
+ sortChangeMessages(db.changeMessages().byChange(notes.getChangeId()));
+ } else {
+ return sortChangeMessages(notes.load().getChangeMessages().values());
+ }
+ }
+
+ public List<ChangeMessage> byPatchSet(ReviewDb db, ChangeNotes notes,
+ PatchSet.Id psId) throws OrmException {
+ if (!migration.readChangeMessages()) {
+ return sortChangeMessages(db.changeMessages().byPatchSet(psId));
+ }
+ return notes.load().getChangeMessages().get(psId);
+ }
+
+ public void addChangeMessage(ReviewDb db, ChangeUpdate update,
+ ChangeMessage changeMessage) throws OrmException {
+ if (changeMessage != null) {
+ update.setChangeMessage(changeMessage.getMessage());
+ db.changeMessages().insert(Collections.singleton(changeMessage));
+ }
+ }
+}
\ No newline at end of file
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
index b083850..388e1cf 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
@@ -25,12 +25,14 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.mail.AbandonedSender;
import com.google.gerrit.server.mail.ReplyToChangeSender;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
@@ -42,7 +44,6 @@
import org.slf4j.LoggerFactory;
import java.io.IOException;
-import java.util.Collections;
@Singleton
public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
@@ -54,18 +55,24 @@
private final Provider<ReviewDb> dbProvider;
private final ChangeJson json;
private final ChangeIndexer indexer;
+ private final ChangeUpdate.Factory updateFactory;
+ private final ChangeMessagesUtil cmUtil;
@Inject
Abandon(ChangeHooks hooks,
AbandonedSender.Factory abandonedSenderFactory,
Provider<ReviewDb> dbProvider,
ChangeJson json,
- ChangeIndexer indexer) {
+ ChangeIndexer indexer,
+ ChangeUpdate.Factory updateFactory,
+ ChangeMessagesUtil cmUtil) {
this.hooks = hooks;
this.abandonedSenderFactory = abandonedSenderFactory;
this.dbProvider = dbProvider;
this.json = json;
this.indexer = indexer;
+ this.updateFactory = updateFactory;
+ this.cmUtil = cmUtil;
}
@Override
@@ -84,6 +91,7 @@
}
ChangeMessage message;
+ ChangeUpdate update;
ReviewDb db = dbProvider.get();
db.changes().beginTransaction(change.getId());
try {
@@ -104,12 +112,16 @@
throw new ResourceConflictException("change is "
+ status(db.changes().get(req.getChange().getId())));
}
+
+ //TODO(yyonas): atomic update was not propagated
+ update = updateFactory.create(control, change.getLastUpdatedOn());
message = newMessage(input, caller, change);
- db.changeMessages().insert(Collections.singleton(message));
+ cmUtil.addChangeMessage(db, update, message);
db.commit();
} finally {
db.rollback();
}
+ update.commit();
CheckedFuture<?, IOException> indexFuture =
indexer.indexAsync(change.getId());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
index d8344f8..3c210d4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -27,6 +27,7 @@
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.mail.CreateChangeSender;
@@ -62,6 +63,7 @@
private final GitReferenceUpdated gitRefUpdated;
private final ChangeHooks hooks;
private final ApprovalsUtil approvalsUtil;
+ private final ChangeMessagesUtil cmUtil;
private final MergeabilityChecker mergeabilityChecker;
private final CreateChangeSender.Factory createChangeSenderFactory;
@@ -85,6 +87,7 @@
GitReferenceUpdated gitRefUpdated,
ChangeHooks hooks,
ApprovalsUtil approvalsUtil,
+ ChangeMessagesUtil cmUtil,
MergeabilityChecker mergeabilityChecker,
CreateChangeSender.Factory createChangeSenderFactory,
@Assisted RefControl refControl,
@@ -95,6 +98,7 @@
this.gitRefUpdated = gitRefUpdated;
this.hooks = hooks;
this.approvalsUtil = approvalsUtil;
+ this.cmUtil = cmUtil;
this.mergeabilityChecker = mergeabilityChecker;
this.createChangeSenderFactory = createChangeSenderFactory;
this.refControl = refControl;
@@ -181,20 +185,22 @@
approvalsUtil.addApprovals(db, update, labelTypes, patchSet, patchSetInfo,
change, ctl, approvals);
if (messageIsForChange()) {
- insertMessage(db);
+ cmUtil.addChangeMessage(db, update, changeMessage);
}
db.commit();
} finally {
db.rollback();
}
-
- update.commit();
+ if (messageIsForChange()) {
+ update.commit();
+ }
CheckedFuture<?, IOException> f = mergeabilityChecker.newCheck()
.addChange(change)
.reindex()
.runAsync();
- if (!messageIsForChange()) {
- insertMessage(db);
+
+ if(!messageIsForChange()) {
+ commitMessageNotForChange();
}
gitRefUpdated.fire(change.getProject(), patchSet.getRefName(),
ObjectId.zeroId(), commit);
@@ -220,14 +226,23 @@
return change;
}
+ private void commitMessageNotForChange() throws OrmException,
+ IOException {
+ ReviewDb db = dbProvider.get();
+ if (changeMessage != null) {
+ Change otherChange =
+ db.changes().get(changeMessage.getPatchSetId().getParentKey());
+ ChangeControl otherControl =
+ refControl.getProjectControl().controlFor(otherChange);
+ ChangeUpdate updateForOtherChange =
+ updateFactory.create(otherControl, change.getLastUpdatedOn());
+ cmUtil.addChangeMessage(db, updateForOtherChange, changeMessage);
+ updateForOtherChange.commit();
+ }
+ }
+
private boolean messageIsForChange() {
return changeMessage != null
&& changeMessage.getKey().getParentKey().equals(change.getKey());
}
-
- private void insertMessage(ReviewDb db) throws OrmException {
- if (changeMessage != null) {
- db.changeMessages().insert(Collections.singleton(changeMessage));
- }
- }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index 067affa..653310d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -74,12 +74,14 @@
import com.google.gerrit.reviewdb.client.UserIdentity;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.WebLinks;
import com.google.gerrit.server.account.AccountInfo;
import com.google.gerrit.server.extensions.webui.UiActions;
import com.google.gerrit.server.git.LabelNormalizer;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
@@ -88,7 +90,6 @@
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeData.ChangedLines;
import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -100,7 +101,6 @@
import java.util.Collections;
import java.util.Comparator;
import java.util.EnumSet;
-import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -108,22 +108,8 @@
public class ChangeJson {
private static final Logger log = LoggerFactory.getLogger(ChangeJson.class);
- private static final ResultSet<ChangeMessage> NO_MESSAGES =
- new ResultSet<ChangeMessage>() {
- @Override
- public Iterator<ChangeMessage> iterator() {
- return toList().iterator();
- }
-
- @Override
- public List<ChangeMessage> toList() {
- return Collections.emptyList();
- }
-
- @Override
- public void close() {
- }
- };
+ private static final List<ChangeMessage> NO_MESSAGES =
+ ImmutableList.of();
private final Provider<ReviewDb> db;
private final LabelNormalizer labelNormalizer;
@@ -140,6 +126,7 @@
private final Revisions revisions;
private final Provider<WebLinks> webLinks;
private final EnumSet<ListChangesOption> options;
+ private final ChangeMessagesUtil cmUtil;
private AccountInfo.Loader accountLoader;
@@ -159,7 +146,8 @@
DynamicMap<DownloadCommand> downloadCommands,
DynamicMap<RestView<ChangeResource>> changeViews,
Revisions revisions,
- Provider<WebLinks> webLinks) {
+ Provider<WebLinks> webLinks,
+ ChangeMessagesUtil cmUtil) {
this.db = db;
this.labelNormalizer = ln;
this.userProvider = user;
@@ -174,6 +162,7 @@
this.changeViews = changeViews;
this.revisions = revisions;
this.webLinks = webLinks;
+ this.cmUtil = cmUtil;
options = EnumSet.noneOf(ListChangesOption.class);
}
@@ -642,8 +631,7 @@
private Collection<ChangeMessageInfo> messages(ChangeControl ctl, ChangeData cd,
Map<PatchSet.Id, PatchSet> map)
throws OrmException {
- List<ChangeMessage> messages =
- db.get().changeMessages().byChange(cd.getId()).toList();
+ List<ChangeMessage> messages = cmUtil.byChange(db.get(), cd.notes());
if (messages.isEmpty()) {
return Collections.emptyList();
}
@@ -705,18 +693,18 @@
if (userProvider.get().isIdentifiedUser()) {
Account.Id self = ((IdentifiedUser) userProvider.get()).getAccountId();
for (List<ChangeData> batch : Iterables.partition(all, 50)) {
- List<ResultSet<ChangeMessage>> m =
+ List<List<ChangeMessage>> m =
Lists.newArrayListWithCapacity(batch.size());
for (ChangeData cd : batch) {
PatchSet.Id ps = cd.change().currentPatchSetId();
if (ps != null && cd.change().getStatus().isOpen()) {
- m.add(db.get().changeMessages().byPatchSet(ps));
+ m.add(cmUtil.byPatchSet(db.get(), cd.notes(), ps));
} else {
m.add(NO_MESSAGES);
}
}
for (int i = 0; i < m.size(); i++) {
- if (isChangeReviewed(self, batch.get(i), m.get(i).toList())) {
+ if (isChangeReviewed(self, batch.get(i), m.get(i))) {
reviewed.add(batch.get(i).getId());
}
}
@@ -728,12 +716,8 @@
private boolean isChangeReviewed(Account.Id self, ChangeData cd,
List<ChangeMessage> msgs) throws OrmException {
// Sort messages to keep the most recent ones at the beginning.
- Collections.sort(msgs, new Comparator<ChangeMessage>() {
- @Override
- public int compare(ChangeMessage a, ChangeMessage b) {
- return b.getWrittenOn().compareTo(a.getWrittenOn());
- }
- });
+ msgs = ChangeNotes.MESSAGE_BY_TIME.sortedCopy(msgs);
+ Collections.reverse(msgs);
Account.Id changeOwnerId = cd.change().getOwner();
for (ChangeMessage cm : msgs) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
index a1245fb..3c53f7b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
@@ -27,6 +27,7 @@
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.DeleteReviewer.Input;
@@ -40,7 +41,6 @@
import com.google.inject.Singleton;
import java.io.IOException;
-import java.util.Collections;
import java.util.List;
@Singleton
@@ -51,6 +51,7 @@
private final Provider<ReviewDb> dbProvider;
private final ChangeUpdate.Factory updateFactory;
private final ApprovalsUtil approvalsUtil;
+ private final ChangeMessagesUtil cmUtil;
private final ChangeIndexer indexer;
private final IdentifiedUser.GenericFactory userFactory;
@@ -58,11 +59,13 @@
DeleteReviewer(Provider<ReviewDb> dbProvider,
ChangeUpdate.Factory updateFactory,
ApprovalsUtil approvalsUtil,
+ ChangeMessagesUtil cmUtil,
ChangeIndexer indexer,
IdentifiedUser.GenericFactory userFactory) {
this.dbProvider = dbProvider;
this.updateFactory = updateFactory;
this.approvalsUtil = approvalsUtil;
+ this.cmUtil = cmUtil;
this.indexer = indexer;
this.userFactory = userFactory;
}
@@ -111,7 +114,7 @@
((IdentifiedUser) control.getCurrentUser()).getAccountId(),
TimeUtil.nowTs(), rsrc.getChange().currentPatchSetId());
changeMessage.setMessage(msg.toString());
- db.changeMessages().insert(Collections.singleton(changeMessage));
+ cmUtil.addChangeMessage(db, update, changeMessage);
}
db.commit();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
index c059bf4..044e899 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -29,6 +29,7 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalCopier;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.events.CommitReceivedEvent;
@@ -41,6 +42,7 @@
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
+import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.ssh.NoSshInfo;
import com.google.gerrit.server.ssh.SshInfo;
import com.google.gerrit.server.util.TimeUtil;
@@ -83,12 +85,14 @@
private final PatchSetInfoFactory patchSetInfoFactory;
private final ReviewDb db;
private final ChangeUpdate.Factory updateFactory;
+ private final ChangeControl.GenericFactory ctlFactory;
private final GitReferenceUpdated gitRefUpdated;
private final CommitValidators.Factory commitValidatorsFactory;
private final MergeabilityChecker mergeabilityChecker;
private final ReplacePatchSetSender.Factory replacePatchSetFactory;
private final ApprovalsUtil approvalsUtil;
private final ApprovalCopier approvalCopier;
+ private final ChangeMessagesUtil cmUtil;
private final Repository git;
private final RevWalk revWalk;
@@ -110,8 +114,10 @@
public PatchSetInserter(ChangeHooks hooks,
ReviewDb db,
ChangeUpdate.Factory updateFactory,
+ ChangeControl.GenericFactory ctlFactory,
ApprovalsUtil approvalsUtil,
ApprovalCopier approvalCopier,
+ ChangeMessagesUtil cmUtil,
PatchSetInfoFactory patchSetInfoFactory,
GitReferenceUpdated gitRefUpdated,
CommitValidators.Factory commitValidatorsFactory,
@@ -127,8 +133,10 @@
this.hooks = hooks;
this.db = db;
this.updateFactory = updateFactory;
+ this.ctlFactory = ctlFactory;
this.approvalsUtil = approvalsUtil;
this.approvalCopier = approvalCopier;
+ this.cmUtil = cmUtil;
this.patchSetInfoFactory = patchSetInfoFactory;
this.gitRefUpdated = gitRefUpdated;
this.commitValidatorsFactory = commitValidatorsFactory;
@@ -212,7 +220,7 @@
}
public Change insert() throws InvalidChangeOperationException, OrmException,
- IOException {
+ IOException, NoSuchChangeException {
init();
validate();
@@ -273,17 +281,19 @@
}
if (messageIsForChange()) {
- insertMessage(db);
+ cmUtil.addChangeMessage(db, update, changeMessage);
}
if (copyLabels) {
approvalCopier.copy(db, ctl, patchSet);
}
db.commit();
- update.commit();
+ if (messageIsForChange()) {
+ update.commit();
+ }
if (!messageIsForChange()) {
- insertMessage(db);
+ commitMessageNotForChange(updatedChange);
}
if (sendMail) {
@@ -317,6 +327,20 @@
return updatedChange;
}
+ private void commitMessageNotForChange(Change updatedChange)
+ throws OrmException, NoSuchChangeException, IOException {
+ if (changeMessage != null) {
+ Change otherChange =
+ db.changes().get(changeMessage.getPatchSetId().getParentKey());
+ ChangeControl otherControl =
+ ctlFactory.controlFor(otherChange, user);
+ ChangeUpdate updateForOtherChange =
+ updateFactory.create(otherControl, updatedChange.getLastUpdatedOn());
+ cmUtil.addChangeMessage(db, updateForOtherChange, changeMessage);
+ updateForOtherChange.commit();
+ }
+ }
+
private void init() throws IOException {
if (sshInfo == null) {
sshInfo = new NoSshInfo();
@@ -368,12 +392,6 @@
.equals(patchSet.getId().getParentKey());
}
- private void insertMessage(ReviewDb db) throws OrmException {
- if (changeMessage != null) {
- db.changeMessages().insert(Collections.singleton(changeMessage));
- }
- }
-
public class ChangeModifiedException extends InvalidChangeOperationException {
private static final long serialVersionUID = 1L;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index d504743..288172e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -46,6 +46,7 @@
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountsCollection;
@@ -82,6 +83,7 @@
private final ChangeData.Factory changeDataFactory;
private final ChangeUpdate.Factory updateFactory;
private final ApprovalsUtil approvalsUtil;
+ private final ChangeMessagesUtil cmUtil;
private final ChangeIndexer indexer;
private final AccountsCollection accounts;
private final EmailReviewComments.Factory email;
@@ -100,6 +102,7 @@
ChangeData.Factory changeDataFactory,
ChangeUpdate.Factory updateFactory,
ApprovalsUtil approvalsUtil,
+ ChangeMessagesUtil cmUtil,
ChangeIndexer indexer,
AccountsCollection accounts,
EmailReviewComments.Factory email,
@@ -109,6 +112,7 @@
this.changeDataFactory = changeDataFactory;
this.updateFactory = updateFactory;
this.approvalsUtil = approvalsUtil;
+ this.cmUtil = cmUtil;
this.indexer = indexer;
this.accounts = accounts;
this.email = email;
@@ -144,7 +148,7 @@
update = updateFactory.create(revision.getControl(), timestamp);
dirty |= insertComments(revision, input.comments, input.drafts);
dirty |= updateLabels(revision, update, input.labels);
- dirty |= insertMessage(revision, input.message);
+ dirty |= insertMessage(revision, input.message, update);
if (dirty) {
db.get().changes().update(Collections.singleton(change));
db.get().commit();
@@ -505,8 +509,8 @@
labelDelta.add(new LabelVote(name, value).format());
}
- private boolean insertMessage(RevisionResource rsrc, String msg)
- throws OrmException {
+ private boolean insertMessage(RevisionResource rsrc, String msg,
+ ChangeUpdate update) throws OrmException {
msg = Strings.nullToEmpty(msg).trim();
StringBuilder buf = new StringBuilder();
@@ -534,7 +538,7 @@
"Patch Set %d:%s",
rsrc.getPatchSet().getPatchSetId(),
buf.toString()));
- db.get().changeMessages().insert(Collections.singleton(message));
+ cmUtil.addChangeMessage(db.get(), update, message);
return true;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java
index 8d96f2d..9676762 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java
@@ -25,10 +25,12 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.PutTopic.Input;
import com.google.gerrit.server.index.ChangeIndexer;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.server.AtomicUpdate;
@@ -38,7 +40,6 @@
import com.google.inject.Singleton;
import java.io.IOException;
-import java.util.Collections;
@Singleton
class PutTopic implements RestModifyView<ChangeResource, Input>,
@@ -46,6 +47,8 @@
private final Provider<ReviewDb> dbProvider;
private final ChangeIndexer indexer;
private final ChangeHooks hooks;
+ private final ChangeUpdate.Factory updateFactory;
+ private final ChangeMessagesUtil cmUtil;
static class Input {
@DefaultInput
@@ -54,10 +57,13 @@
@Inject
PutTopic(Provider<ReviewDb> dbProvider, ChangeIndexer indexer,
- ChangeHooks hooks) {
+ ChangeHooks hooks, ChangeUpdate.Factory updateFactory,
+ ChangeMessagesUtil cmUtil) {
this.dbProvider = dbProvider;
this.indexer = indexer;
this.hooks = hooks;
+ this.updateFactory = updateFactory;
+ this.cmUtil = cmUtil;
}
@Override
@@ -94,6 +100,7 @@
currentUser.getAccountId(), TimeUtil.nowTs(),
change.currentPatchSetId());
cmsg.setMessage(summary);
+ ChangeUpdate update;
db.changes().beginTransaction(change.getId());
try {
@@ -106,11 +113,16 @@
return change;
}
});
- db.changeMessages().insert(Collections.singleton(cmsg));
+
+ //TODO(yyonas): atomic update was not propagated
+ update = updateFactory.create(control);
+ cmUtil.addChangeMessage(db, update, cmsg);
+
db.commit();
} finally {
db.rollback();
}
+ update.commit();
CheckedFuture<?, IOException> indexFuture =
indexer.indexAsync(change.getId());
hooks.doTopicChangedHook(change, currentUser.getAccount(),
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
index 083de08..7a31ebe 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
@@ -26,11 +26,13 @@
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gerrit.server.mail.ReplyToChangeSender;
import com.google.gerrit.server.mail.RestoredSender;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
@@ -42,7 +44,6 @@
import org.slf4j.LoggerFactory;
import java.io.IOException;
-import java.util.Collections;
@Singleton
public class Restore implements RestModifyView<ChangeResource, RestoreInput>,
@@ -54,18 +55,24 @@
private final Provider<ReviewDb> dbProvider;
private final ChangeJson json;
private final MergeabilityChecker mergeabilityChecker;
+ private final ChangeMessagesUtil cmUtil;
+ private final ChangeUpdate.Factory updateFactory;
@Inject
Restore(ChangeHooks hooks,
RestoredSender.Factory restoredSenderFactory,
Provider<ReviewDb> dbProvider,
ChangeJson json,
- MergeabilityChecker mergeabilityChecker) {
+ MergeabilityChecker mergeabilityChecker,
+ ChangeMessagesUtil cmUtil,
+ ChangeUpdate.Factory updateFactory) {
this.hooks = hooks;
this.restoredSenderFactory = restoredSenderFactory;
this.dbProvider = dbProvider;
this.json = json;
this.mergeabilityChecker = mergeabilityChecker;
+ this.cmUtil = cmUtil;
+ this.updateFactory = updateFactory;
}
@Override
@@ -82,6 +89,7 @@
}
ChangeMessage message;
+ ChangeUpdate update;
ReviewDb db = dbProvider.get();
db.changes().beginTransaction(change.getId());
try {
@@ -102,12 +110,16 @@
throw new ResourceConflictException("change is "
+ status(db.changes().get(req.getChange().getId())));
}
+
+ //TODO(yyonas): atomic update was not propagated
+ update = updateFactory.create(control);
message = newMessage(input, caller, change);
- db.changeMessages().insert(Collections.singleton(message));
+ cmUtil.addChangeMessage(db, update, message);
db.commit();
} finally {
db.rollback();
}
+ update.commit();
CheckedFuture<?, IOException> f = mergeabilityChecker.newCheck()
.addChange(change)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
index be2788a..3719028 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
@@ -20,11 +20,11 @@
import com.google.common.base.Optional;
import com.google.common.base.Predicate;
import com.google.common.base.Strings;
+import com.google.common.collect.FluentIterable;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Table;
import com.google.gerrit.common.data.ParameterizedString;
@@ -44,6 +44,7 @@
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
@@ -103,6 +104,7 @@
private final IdentifiedUser.GenericFactory userFactory;
private final ChangeUpdate.Factory updateFactory;
private final ApprovalsUtil approvalsUtil;
+ private final ChangeMessagesUtil cmUtil;
private final MergeQueue mergeQueue;
private final ChangeIndexer indexer;
private final LabelNormalizer labelNormalizer;
@@ -118,6 +120,7 @@
IdentifiedUser.GenericFactory userFactory,
ChangeUpdate.Factory updateFactory,
ApprovalsUtil approvalsUtil,
+ ChangeMessagesUtil cmUtil,
MergeQueue mergeQueue,
AccountsCollection accounts,
ChangesCollection changes,
@@ -130,6 +133,7 @@
this.userFactory = userFactory;
this.updateFactory = updateFactory;
this.approvalsUtil = approvalsUtil;
+ this.cmUtil = cmUtil;
this.mergeQueue = mergeQueue;
this.accounts = accounts;
this.changes = changes;
@@ -226,16 +230,16 @@
*/
public ChangeMessage getConflictMessage(RevisionResource rsrc)
throws OrmException {
- return Iterables.getFirst(Iterables.filter(
- Lists.reverse(dbProvider.get().changeMessages()
- .byChange(rsrc.getChange().getId())
- .toList()),
- new Predicate<ChangeMessage>() {
- @Override
- public boolean apply(ChangeMessage input) {
- return input.getAuthor() == null;
- }
- }), null);
+ return FluentIterable.from(cmUtil.byPatchSet(dbProvider.get(), rsrc.getNotes(),
+ rsrc.getPatchSet().getId()))
+ .filter(new Predicate<ChangeMessage>() {
+ @Override
+ public boolean apply(ChangeMessage input) {
+ return input.getAuthor() == null;
+ }
+ })
+ .last()
+ .orNull();
}
public Change submit(RevisionResource rsrc, IdentifiedUser caller,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/AsyncReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/AsyncReceiveCommits.java
index aaf0273..85eba42 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/AsyncReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/AsyncReceiveCommits.java
@@ -23,7 +23,6 @@
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.FactoryModuleBuilder;
import com.google.inject.Inject;
-
import com.google.inject.name.Named;
import com.google.inject.PrivateModule;
import com.google.inject.Provides;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
index 74056ab..06e17ce 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
@@ -42,6 +42,7 @@
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
@@ -54,6 +55,7 @@
import com.google.gerrit.server.mail.MergeFailSender;
import com.google.gerrit.server.mail.MergedSender;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl;
@@ -88,7 +90,6 @@
import java.io.IOException;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
@@ -137,9 +138,11 @@
private final PatchSetInfoFactory patchSetInfoFactory;
private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final ChangeControl.GenericFactory changeControlFactory;
+ private final ChangeUpdate.Factory updateFactory;
private final MergeQueue mergeQueue;
private final MergeValidators.Factory mergeValidatorsFactory;
private final ApprovalsUtil approvalsUtil;
+ private final ChangeMessagesUtil cmUtil;
private final Branch.NameKey destBranch;
private ProjectState destProject;
@@ -173,6 +176,7 @@
final MergeFailSender.Factory mfsf,
final PatchSetInfoFactory psif, final IdentifiedUser.GenericFactory iuf,
final ChangeControl.GenericFactory changeControlFactory,
+ final ChangeUpdate.Factory updateFactory,
final MergeQueue mergeQueue, @Assisted final Branch.NameKey branch,
final ChangeHooks hooks, final AccountCache accountCache,
final TagCache tagCache,
@@ -182,7 +186,8 @@
final RequestScopePropagator requestScopePropagator,
final ChangeIndexer indexer,
final MergeValidators.Factory mergeValidatorsFactory,
- final ApprovalsUtil approvalsUtil) {
+ final ApprovalsUtil approvalsUtil,
+ final ChangeMessagesUtil cmUtil) {
repoManager = grm;
schemaFactory = sf;
notesFactory = nf;
@@ -193,6 +198,7 @@
patchSetInfoFactory = psif;
identifiedUserFactory = iuf;
this.changeControlFactory = changeControlFactory;
+ this.updateFactory = updateFactory;
this.mergeQueue = mergeQueue;
this.hooks = hooks;
this.accountCache = accountCache;
@@ -204,6 +210,7 @@
this.indexer = indexer;
this.mergeValidatorsFactory = mergeValidatorsFactory;
this.approvalsUtil = approvalsUtil;
+ this.cmUtil = cmUtil;
destBranch = branch;
toMerge = ArrayListMultimap.create();
potentiallyStillSubmittable = new ArrayList<>();
@@ -224,7 +231,7 @@
}
}
- public void merge() throws MergeException {
+ public void merge() throws MergeException, NoSuchChangeException, IOException {
setDestProject();
try {
openSchema();
@@ -395,7 +402,7 @@
inserter = repo.newObjectInserter();
}
- private RefUpdate openBranch() throws MergeException, OrmException {
+ private RefUpdate openBranch() throws MergeException, OrmException, NoSuchChangeException {
try {
final RefUpdate branchUpdate = repo.updateRef(destBranch.get());
if (branchUpdate.getOldObjectId() != null) {
@@ -442,7 +449,7 @@
}
private ListMultimap<SubmitType, Change> validateChangeList(
- final List<Change> submitted) throws MergeException {
+ final List<Change> submitted) throws MergeException, NoSuchChangeException {
final ListMultimap<SubmitType, Change> toSubmit =
ArrayListMultimap.create();
@@ -658,7 +665,7 @@
hooks.doRefUpdatedHook(destBranch, branchUpdate, account);
}
- private void updateChangeStatus(final List<Change> submitted) {
+ private void updateChangeStatus(final List<Change> submitted) throws NoSuchChangeException {
for (final Change c : submitted) {
final CodeReviewCommit commit = commits.get(c.getId());
final CommitMergeStatus s = commit != null ? commit.getStatusCode() : null;
@@ -824,7 +831,8 @@
}
private void setMerged(Change c, ChangeMessage msg)
- throws OrmException, IOException {
+ throws OrmException, IOException, NoSuchChangeException {
+ ChangeUpdate update = null;
try {
db.changes().beginTransaction(c.getId());
@@ -835,8 +843,13 @@
c = setMergedPatchSet(c.getId(), merged);
PatchSetApproval submitter =
approvalsUtil.getSubmitter(db, commit.notes(), merged);
- addMergedMessage(submitter, msg);
+ ChangeControl control = commit.getControl();
+ update = updateFactory.create(control, c.getLastUpdatedOn());
+ // TODO(yyonas): we need to be able to change the author of the message
+ // is not the person for whom the change was made. addMergedMessage
+ // did this in the past.
+ cmUtil.addChangeMessage(db, update, msg);
db.commit();
sendMergedEmail(c, submitter);
@@ -853,6 +866,7 @@
db.rollback();
}
indexer.index(db, c);
+ update.commit();
}
private Change setMergedPatchSet(Change.Id changeId, final PatchSet.Id merged)
@@ -881,16 +895,6 @@
});
}
- private void addMergedMessage(PatchSetApproval submitter, ChangeMessage msg)
- throws OrmException {
- if (msg != null) {
- if (submitter != null && msg.getAuthor() == null) {
- msg.setAuthor(submitter.getAccountId());
- }
- db.changeMessages().insert(Collections.singleton(msg));
- }
- }
-
private void sendMergedEmail(final Change c, final PatchSetApproval from) {
workQueue.getDefaultQueue()
.submit(requestScopePropagator.wrap(new Runnable() {
@@ -933,11 +937,11 @@
c, identifiedUserFactory.create(c.getOwner()));
}
- private void setNew(CodeReviewCommit c, ChangeMessage msg) {
+ private void setNew(CodeReviewCommit c, ChangeMessage msg) throws NoSuchChangeException, IOException {
sendMergeFail(c.notes(), msg, true);
}
- private void setNew(Change c, ChangeMessage msg) throws OrmException {
+ private void setNew(Change c, ChangeMessage msg) throws OrmException, NoSuchChangeException, IOException {
sendMergeFail(notesFactory.create(c), msg, true);
}
@@ -947,7 +951,8 @@
private RetryStatus getRetryStatus(
@Nullable PatchSetApproval submitter,
- ChangeMessage msg) {
+ ChangeMessage msg,
+ ChangeNotes notes) {
if (submitter != null
&& TimeUtil.nowMs() - submitter.getGranted().getTime()
> MAX_SUBMIT_WINDOW) {
@@ -955,8 +960,7 @@
}
try {
- ChangeMessage last = Iterables.getLast(db.changeMessages().byChange(
- msg.getPatchSetId().getParentKey()), null);
+ ChangeMessage last = Iterables.getLast(cmUtil.byChange(db, notes));
if (last != null) {
if (Objects.equal(last.getAuthor(), msg.getAuthor())
&& Objects.equal(last.getMessage(), msg.getMessage())) {
@@ -975,7 +979,7 @@
}
private void sendMergeFail(ChangeNotes notes, final ChangeMessage msg,
- boolean makeNew) {
+ boolean makeNew) throws NoSuchChangeException, IOException {
PatchSetApproval submitter = null;
try {
submitter = approvalsUtil.getSubmitter(
@@ -985,7 +989,7 @@
}
if (!makeNew) {
- RetryStatus retryStatus = getRetryStatus(submitter, msg);
+ RetryStatus retryStatus = getRetryStatus(submitter, msg, notes);
if (retryStatus == RetryStatus.RETRY_NO_MESSAGE) {
return;
} else if (retryStatus == RetryStatus.UNSUBMIT) {
@@ -996,6 +1000,7 @@
final boolean setStatusNew = makeNew;
final Change c = notes.getChange();
Change change = null;
+ ChangeUpdate update = null;
try {
db.changes().beginTransaction(c.getId());
try {
@@ -1013,7 +1018,11 @@
return c;
}
});
- db.changeMessages().insert(Collections.singleton(msg));
+ ChangeControl control = changeControl(change);
+
+ //TODO(yyonas): atomic change is not propagated.
+ update = updateFactory.create(control, c.getLastUpdatedOn());
+ cmUtil.addChangeMessage(db, update, msg);
db.commit();
} finally {
db.rollback();
@@ -1021,6 +1030,9 @@
} catch (OrmException err) {
log.warn("Cannot record merge failure message", err);
}
+ if (update != null) {
+ update.commit();
+ }
CheckedFuture<?, IOException> indexFuture;
if (change != null) {
@@ -1083,7 +1095,7 @@
}
}
- private void abandonAllOpenChanges() {
+ private void abandonAllOpenChanges() throws NoSuchChangeException {
Exception err = null;
try {
openSchema();
@@ -1105,8 +1117,13 @@
}
private void abandonOneChange(Change change) throws OrmException,
- IOException {
+ NoSuchChangeException, IOException {
db.changes().beginTransaction(change.getId());
+
+ //TODO(dborowitz): support InternalUser in ChangeUpdate
+ ChangeControl control = changeControlFactory.controlFor(change,
+ identifiedUserFactory.create(change.getOwner()));
+ ChangeUpdate update = updateFactory.create(control);
try {
change = db.changes().atomicUpdate(
change.getId(),
@@ -1120,6 +1137,7 @@
return null;
}
});
+
if (change != null) {
ChangeMessage msg = new ChangeMessage(
new ChangeMessage.Key(
@@ -1129,12 +1147,15 @@
change.getLastUpdatedOn(),
change.currentPatchSetId());
msg.setMessage("Project was deleted.");
- db.changeMessages().insert(Collections.singleton(msg));
+
+ //TODO(yyonas): atomic change is not propagated.
+ cmUtil.addChangeMessage(db, update, msg);
db.commit();
indexer.index(db, change);
}
} finally {
db.rollback();
}
+ update.commit();
}
-}
+}
\ No newline at end of file
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
index b9b5621..8902af9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
@@ -38,6 +38,7 @@
import com.google.inject.assistedinject.AssistedInject;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
+import org.eclipse.jgit.errors.LargeObjectException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.errors.NoMergeBaseException;
import org.eclipse.jgit.errors.NoMergeBaseException.MergeBaseFailureReason;
@@ -395,6 +396,9 @@
final ThreeWayMerger m = newThreeWayMerger(repo, createDryRunInserter());
try {
return m.merge(new AnyObjectId[] {mergeTip, toMerge});
+ } catch (LargeObjectException e) {
+ log.warn("Cannot merge due to LargeObjectException: " + toMerge.name());
+ return false;
} catch (NoMergeBaseException e) {
return false;
} catch (IOException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
index c4c1b7f..a905385 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
@@ -130,7 +130,6 @@
private static final String KEY_LOCAL_DEFAULT = "local-default";
private static final String LABEL = "label";
- private static final String KEY_ABBREVIATION = "abbreviation";
private static final String KEY_FUNCTION = "function";
private static final String KEY_DEFAULT_VALUE = "defaultValue";
private static final String KEY_COPY_MIN_SCORE = "copyMinScore";
@@ -673,10 +672,6 @@
"Invalid label \"%s\"", name)));
continue;
}
- String abbr = rc.getString(LABEL, name, KEY_ABBREVIATION);
- if (abbr != null) {
- label.setAbbreviation(abbr);
- }
String functionName = Objects.firstNonNull(
rc.getString(LABEL, name, KEY_FUNCTION), "MaxWithBlock");
@@ -1050,15 +1045,6 @@
LabelType label = e.getValue();
toUnset.remove(name);
rc.setString(LABEL, name, KEY_FUNCTION, label.getFunctionName());
-
- if (!LabelType.defaultAbbreviation(name)
- .equals(label.getAbbreviation())) {
- rc.setString(
- LABEL, name, KEY_ABBREVIATION, label.getAbbreviation());
- } else {
- rc.unset(LABEL, name, KEY_ABBREVIATION);
- }
-
rc.setInt(LABEL, name, KEY_DEFAULT_VALUE, label.getDefaultValue());
if (label.isCopyMinScore()) {
rc.setBoolean(LABEL, name, KEY_COPY_MIN_SCORE, true);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index ac0a95e..3267e5f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -69,6 +69,7 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalCopier;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
@@ -97,6 +98,7 @@
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.ProjectState;
@@ -279,6 +281,7 @@
private final ChangeHooks hooks;
private final ApprovalsUtil approvalsUtil;
private final ApprovalCopier approvalCopier;
+ private final ChangeMessagesUtil cmUtil;
private final GitRepositoryManager repoManager;
private final ProjectCache projectCache;
private final String canonicalWebUrl;
@@ -344,6 +347,7 @@
final ChangeHooks hooks,
final ApprovalsUtil approvalsUtil,
final ApprovalCopier approvalCopier,
+ final ChangeMessagesUtil cmUtil,
final ProjectCache projectCache,
final GitRepositoryManager repoManager,
final TagCache tagCache,
@@ -383,6 +387,7 @@
this.hooks = hooks;
this.approvalsUtil = approvalsUtil;
this.approvalCopier = approvalCopier;
+ this.cmUtil = cmUtil;
this.projectCache = projectCache;
this.repoManager = repoManager;
this.canonicalWebUrl = canonicalWebUrl;
@@ -576,54 +581,59 @@
}
for (final ReceiveCommand c : commands) {
- if (c.getResult() == OK) {
- switch (c.getType()) {
- case CREATE:
- if (isHead(c) || isConfig(c)) {
- autoCloseChanges(c);
+ if (c.getResult() == OK) {
+ try {
+ switch (c.getType()) {
+ case CREATE:
+ if (isHead(c) || isConfig(c)) {
+ autoCloseChanges(c);
+ }
+ break;
+
+ case UPDATE: // otherwise known as a fast-forward
+ tagCache.updateFastForward(project.getNameKey(),
+ c.getRefName(),
+ c.getOldId(),
+ c.getNewId());
+ if (isHead(c) || isConfig(c)) {
+ autoCloseChanges(c);
+ }
+ break;
+
+ case UPDATE_NONFASTFORWARD:
+ if (isHead(c) || isConfig(c)) {
+ autoCloseChanges(c);
+ }
+ break;
+
+ case DELETE:
+ break;
}
- break;
- case UPDATE: // otherwise known as a fast-forward
- tagCache.updateFastForward(project.getNameKey(),
- c.getRefName(),
- c.getOldId(),
- c.getNewId());
- if (isHead(c) || isConfig(c)) {
- autoCloseChanges(c);
+ if (isConfig(c)) {
+ projectCache.evict(project);
+ ProjectState ps = projectCache.get(project.getNameKey());
+ repoManager.setProjectDescription(project.getNameKey(), //
+ ps.getProject().getDescription());
}
- break;
- case UPDATE_NONFASTFORWARD:
- if (isHead(c) || isConfig(c)) {
- autoCloseChanges(c);
+ if (!MagicBranch.isMagicBranch(c.getRefName())) {
+ // We only fire gitRefUpdated for direct refs updates.
+ // Events for change refs are fired when they are created.
+ //
+ gitRefUpdated.fire(project.getNameKey(), c.getRefName(),
+ c.getOldId(), c.getNewId());
+ hooks.doRefUpdatedHook(
+ new Branch.NameKey(project.getNameKey(), c.getRefName()),
+ c.getOldId(),
+ c.getNewId(),
+ currentUser.getAccount());
}
- break;
-
- case DELETE:
- break;
+ } catch (NoSuchChangeException e) {
+ c.setResult(REJECTED_OTHER_REASON,
+ "No such change: " + e.getMessage());
+ }
}
-
- if (isConfig(c)) {
- projectCache.evict(project);
- ProjectState ps = projectCache.get(project.getNameKey());
- repoManager.setProjectDescription(project.getNameKey(), //
- ps.getProject().getDescription());
- }
-
- if (!MagicBranch.isMagicBranch(c.getRefName())) {
- // We only fire gitRefUpdated for direct refs updates.
- // Events for change refs are fired when they are created.
- //
- gitRefUpdated.fire(project.getNameKey(), c.getRefName(),
- c.getOldId(), c.getNewId());
- hooks.doRefUpdatedHook(
- new Branch.NameKey(project.getNameKey(), c.getRefName()),
- c.getOldId(),
- c.getNewId(),
- currentUser.getAccount());
- }
- }
}
closeProgress.end();
commandProgress.end();
@@ -1928,7 +1938,7 @@
ListenableFuture<PatchSet.Id> future = changeUpdateExector.submit(
requestScopePropagator.wrap(new Callable<PatchSet.Id>() {
@Override
- public PatchSet.Id call() throws OrmException, IOException {
+ public PatchSet.Id call() throws OrmException, IOException, NoSuchChangeException {
try {
if (caller == Thread.currentThread()) {
return insertPatchSet(db);
@@ -1993,7 +2003,7 @@
new ChangeMessage(new ChangeMessage.Key(change.getId(), ChangeUtil
.messageUUID(db)), me, newPatchSet.getCreatedOn(), newPatchSet.getId());
msg.setMessage("Uploaded patch set " + newPatchSet.getPatchSetId() + ".");
- db.changeMessages().insert(Collections.singleton(msg));
+ cmUtil.addChangeMessage(db, update, msg);
if (mergedIntoRef == null) {
// Change should be new, so it can go through review again.
@@ -2049,7 +2059,7 @@
if (mergedIntoRef != null) {
// Change was already submitted to a branch, close it.
//
- markChangeMergedByPush(db, this);
+ markChangeMergedByPush(db, this, changeCtl);
}
if (cmd.getResult() == NOT_ATTEMPTED) {
@@ -2256,7 +2266,7 @@
return true;
}
- private void autoCloseChanges(final ReceiveCommand cmd) {
+ private void autoCloseChanges(final ReceiveCommand cmd) throws NoSuchChangeException {
final RevWalk rw = rp.getRevWalk();
try {
rw.reset();
@@ -2355,7 +2365,7 @@
result.newPatchSet = ps;
result.info = patchSetInfoFactory.get(commit, psi);
result.mergedIntoRef = refName;
- markChangeMergedByPush(db, result);
+ markChangeMergedByPush(db, result, result.changeCtl);
hooks.doChangeMergedHook(
change, currentUser.getAccount(), result.newPatchSet, db);
sendMergedEmail(result);
@@ -2383,11 +2393,13 @@
return r;
}
- private void markChangeMergedByPush(ReviewDb db, final ReplaceRequest result)
- throws OrmException, IOException {
+ private void markChangeMergedByPush(ReviewDb db, final ReplaceRequest result,
+ ChangeControl control) throws OrmException, IOException {
Change.Id id = result.change.getId();
db.changes().beginTransaction(id);
Change change;
+
+ ChangeUpdate update;
try {
change = db.changes().atomicUpdate(id, new AtomicUpdate<Change>() {
@Override
@@ -2420,12 +2432,15 @@
result.info.getKey());
msg.setMessage(msgBuf.toString());
- db.changeMessages().insert(Collections.singleton(msg));
+ update = updateFactory.create(control, change.getLastUpdatedOn());
+
+ cmUtil.addChangeMessage(db, update, msg);
db.commit();
} finally {
db.rollback();
}
indexer.index(db, change);
+ update.commit();
}
private void sendMergedEmail(final ReplaceRequest result) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 37be365..f8bfd94 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -29,6 +29,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableSetMultimap;
+import com.google.common.collect.LinkedListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
@@ -39,6 +40,7 @@
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
@@ -60,6 +62,7 @@
import org.eclipse.jgit.util.RawParseUtils;
import java.io.IOException;
+import java.nio.charset.Charset;
import java.sql.Timestamp;
import java.util.Collection;
import java.util.Collections;
@@ -78,6 +81,15 @@
}
});
+ public static final Ordering<ChangeMessage> MESSAGE_BY_TIME =
+ Ordering.natural().onResultOf(
+ new Function<ChangeMessage, Timestamp>() {
+ @Override
+ public Timestamp apply(ChangeMessage input) {
+ return input.getWrittenOn();
+ }
+ });
+
@Singleton
public static class Factory {
private final GitRepositoryManager repoManager;
@@ -101,6 +113,7 @@
Table<Account.Id, String, Optional<PatchSetApproval>>> approvals;
private final Map<Account.Id, ReviewerState> reviewers;
private final List<SubmitRecord> submitRecords;
+ private final Multimap<PatchSet.Id, ChangeMessage> changeMessages;
private Change.Status status;
private Parser(Change.Id changeId, ObjectId tip, RevWalk walk) {
@@ -110,6 +123,7 @@
approvals = Maps.newHashMap();
reviewers = Maps.newLinkedHashMap();
submitRecords = Lists.newArrayListWithExpectedSize(1);
+ changeMessages = LinkedListMultimap.create();
}
private void parseAll() throws ConfigInvalidException, IOException {
@@ -136,12 +150,21 @@
return ImmutableListMultimap.copyOf(result);
}
+ private ImmutableListMultimap<PatchSet.Id, ChangeMessage> buildMessages() {
+ for (Collection<ChangeMessage> v : changeMessages.asMap().values()) {
+ Collections.sort((List<ChangeMessage>) v, MESSAGE_BY_TIME);
+ }
+ return ImmutableListMultimap.copyOf(changeMessages);
+ }
+
private void parse(RevCommit commit) throws ConfigInvalidException {
if (status == null) {
status = parseStatus(commit);
}
PatchSet.Id psId = parsePatchSetId(commit);
Account.Id accountId = parseIdent(commit);
+ parseChangeMessage(psId, accountId, commit);
+
if (submitRecords.isEmpty()) {
// Only parse the most recent set of submit records; any older ones are
@@ -189,6 +212,63 @@
return new PatchSet.Id(changeId, psId);
}
+ private void parseChangeMessage(PatchSet.Id psId, Account.Id accountId,
+ RevCommit commit) {
+ byte[] raw = commit.getRawBuffer();
+ int size = raw.length;
+ Charset enc = RawParseUtils.parseEncoding(raw);
+
+ int subjectStart = RawParseUtils.commitMessage(raw, 0);
+ if (subjectStart < 0 || subjectStart >= size) {
+ return;
+ }
+
+ int subjectEnd = RawParseUtils.endOfParagraph(raw, subjectStart);
+ if (subjectEnd == size) {
+ return;
+ }
+
+ int changeMessageStart;
+
+ if (raw[subjectEnd] == '\n') {
+ changeMessageStart = subjectEnd + 2; //\n\n ends paragraph
+ } else if (raw[subjectEnd] == '\r') {
+ changeMessageStart = subjectEnd + 4; //\r\n\r\n ends paragraph
+ } else {
+ return;
+ }
+
+ int ptr = size - 1;
+ int changeMessageEnd = -1;
+ while(ptr > changeMessageStart) {
+ ptr = RawParseUtils.prevLF(raw, ptr, '\r');
+ if (ptr == -1) {
+ break;
+ }
+ if (raw[ptr] == '\n') {
+ changeMessageEnd = ptr - 1;
+ break;
+ } else if (raw[ptr] == '\r') {
+ changeMessageEnd = ptr - 3;
+ break;
+ }
+ }
+
+ if (ptr <= changeMessageStart) {
+ return;
+ }
+
+ String changeMsgString = RawParseUtils.decode(enc, raw,
+ changeMessageStart, changeMessageEnd + 1);
+ ChangeMessage changeMessage = new ChangeMessage(
+ new ChangeMessage.Key(psId.getParentKey(), commit.name()),
+ accountId,
+ new Timestamp(commit.getCommitterIdent().getWhen().getTime()),
+ psId);
+ changeMessage.setMessage(changeMsgString);
+ changeMessages.put(psId, changeMessage);
+ }
+
private void parseApproval(PatchSet.Id psId, Account.Id accountId,
RevCommit commit, String line) throws ConfigInvalidException {
Table<Account.Id, String, Optional<PatchSetApproval>> curr =
@@ -353,6 +433,7 @@
private ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals;
private ImmutableSetMultimap<ReviewerState, Account.Id> reviewers;
private ImmutableList<SubmitRecord> submitRecords;
+ private ImmutableListMultimap<PatchSet.Id, ChangeMessage> changeMessages;
@VisibleForTesting
ChangeNotes(GitRepositoryManager repoManager, Change change) {
@@ -405,6 +486,11 @@
return submitRecords;
}
+ /** @return change messages by patch set, in chronological order. */
+ public ImmutableListMultimap<PatchSet.Id, ChangeMessage> getChangeMessages() {
+ return changeMessages;
+ }
+
@Override
protected String getRefName() {
return ChangeNoteUtil.changeRefName(change.getId());
@@ -426,6 +512,7 @@
change.setStatus(parser.status);
}
approvals = parser.buildApprovals();
+ changeMessages = parser.buildMessages();
ImmutableSetMultimap.Builder<ReviewerState, Account.Id> reviewers =
ImmutableSetMultimap.builder();
@@ -434,6 +521,7 @@
reviewers.put(e.getValue(), e.getKey());
}
this.reviewers = reviewers.build();
+
submitRecords = ImmutableList.copyOf(parser.submitRecords);
} finally {
walk.release();
@@ -444,6 +532,7 @@
approvals = ImmutableListMultimap.of();
reviewers = ImmutableSetMultimap.of();
submitRecords = ImmutableList.of();
+ changeMessages = ImmutableListMultimap.of();
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 74ca198..12b276f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -88,6 +88,7 @@
private String subject;
private PatchSet.Id psId;
private List<SubmitRecord> submitRecords;
+ private String changeMessage;
@AssistedInject
private ChangeUpdate(
@@ -186,6 +187,10 @@
this.psId = psId;
}
+ public void setChangeMessage(String changeMessage) {
+ this.changeMessage = changeMessage;
+ }
+
public void putReviewer(Account.Id reviewer, ReviewerState type) {
checkArgument(type != ReviewerState.REMOVED, "invalid ReviewerType");
reviewers.put(reviewer, type);
@@ -297,6 +302,13 @@
msg.append("Update patch set ").append(ps);
}
msg.append("\n\n");
+
+ if (changeMessage != null) {
+ msg.append(changeMessage);
+ msg.append("\n\n");
+ }
+
+
addFooter(msg, FOOTER_PATCH_SET, ps);
if (status != null) {
addFooter(msg, FOOTER_STATUS, status.name().toLowerCase());
@@ -352,7 +364,8 @@
return approvals.isEmpty()
&& reviewers.isEmpty()
&& status == null
- && submitRecords == null;
+ && submitRecords == null
+ && changeMessage == null;
}
private static StringBuilder addFooter(StringBuilder sb, FooterKey footer) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java
index 5321046..db72def 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java
@@ -35,17 +35,21 @@
Config cfg = new Config();
cfg.setBoolean("notedb", null, "write", true);
cfg.setBoolean("notedb", "patchSetApprovals", "read", true);
+ cfg.setBoolean("notedb", "changeMessages", "read", true);
return new NotesMigration(cfg);
}
private final boolean write;
private final boolean readPatchSetApprovals;
+ private final boolean readChangeMessages;
@Inject
NotesMigration(@GerritServerConfig Config cfg) {
write = cfg.getBoolean("notedb", null, "write", false);
readPatchSetApprovals =
cfg.getBoolean("notedb", "patchSetApprovals", "read", false);
+ readChangeMessages =
+ cfg.getBoolean("notedb", "changeMessages", "read", false);
}
public boolean write() {
@@ -55,4 +59,8 @@
public boolean readPatchSetApprovals() {
return readPatchSetApprovals;
}
+
+ public boolean readChangeMessages() {
+ return readChangeMessages;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/AutoRegisterModules.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/AutoRegisterModules.java
index bd8a524..6f1204b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/AutoRegisterModules.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/AutoRegisterModules.java
@@ -23,7 +23,7 @@
import com.google.gerrit.extensions.annotations.Export;
import com.google.gerrit.extensions.annotations.ExtensionPoint;
import com.google.gerrit.extensions.annotations.Listen;
-import com.google.gerrit.server.plugins.JarScanner.ExtensionMetaData;
+import com.google.gerrit.server.plugins.PluginContentScanner.ExtensionMetaData;
import com.google.inject.AbstractModule;
import com.google.inject.Module;
import com.google.inject.Scopes;
@@ -34,12 +34,11 @@
import java.util.Arrays;
import java.util.Map;
import java.util.Set;
-import java.util.jar.JarFile;
class AutoRegisterModules {
private final String pluginName;
private final PluginGuiceEnvironment env;
- private final JarFile jarFile;
+ private final PluginContentScanner scanner;
private final ClassLoader classLoader;
private final ModuleGenerator sshGen;
private final ModuleGenerator httpGen;
@@ -53,11 +52,11 @@
AutoRegisterModules(String pluginName,
PluginGuiceEnvironment env,
- JarFile jarFile,
+ PluginContentScanner scanner,
ClassLoader classLoader) {
this.pluginName = pluginName;
this.env = env;
- this.jarFile = jarFile;
+ this.scanner = scanner;
this.classLoader = classLoader;
this.sshGen = env.hasSshModule() ? env.newSshModuleGenerator() : null;
this.httpGen = env.hasHttpModule() ? env.newHttpModuleGenerator() : null;
@@ -111,7 +110,7 @@
private void scan() throws InvalidPluginException {
Map<Class<? extends Annotation>, Iterable<ExtensionMetaData>> extensions =
- JarScanner.scan(jarFile, pluginName, Arrays.asList(Export.class, Listen.class));
+ scanner.scan(pluginName, Arrays.asList(Export.class, Listen.class));
for (ExtensionMetaData export : extensions.get(Export.class)) {
export(export);
}
@@ -123,18 +122,18 @@
private void export(ExtensionMetaData def) throws InvalidPluginException {
Class<?> clazz;
try {
- clazz = Class.forName(def.getClassName(), false, classLoader);
+ clazz = Class.forName(def.className, false, classLoader);
} catch (ClassNotFoundException err) {
throw new InvalidPluginException(String.format(
"Cannot load %s with @Export(\"%s\")",
- def.getClassName(), def.getAnnotationValue()), err);
+ def.className, def.annotationValue), err);
}
Export export = clazz.getAnnotation(Export.class);
if (export == null) {
PluginLoader.log.warn(String.format(
"In plugin %s asm incorrectly parsed %s with @Export(\"%s\")",
- pluginName, clazz.getName(), def.getAnnotationValue()));
+ pluginName, clazz.getName(), def.annotationValue));
return;
}
@@ -162,11 +161,11 @@
private void listen(ExtensionMetaData def) throws InvalidPluginException {
Class<?> clazz;
try {
- clazz = Class.forName(def.getClassName(), false, classLoader);
+ clazz = Class.forName(def.className, false, classLoader);
} catch (ClassNotFoundException err) {
throw new InvalidPluginException(String.format(
"Cannot load %s with @Listen",
- def.getClassName()), err);
+ def.className), err);
}
Listen listen = clazz.getAnnotation(Listen.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java
index c4fe1ec..6ce3464 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java
@@ -24,6 +24,7 @@
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.common.collect.Sets;
@@ -38,6 +39,7 @@
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.Type;
+import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.lang.annotation.Annotation;
@@ -46,10 +48,12 @@
import java.util.Enumeration;
import java.util.Map;
import java.util.Set;
+import java.util.jar.Attributes;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
+import java.util.jar.Manifest;
-public class JarScanner {
+public class JarScanner implements PluginContentScanner {
private static final int SKIP_ALL = ClassReader.SKIP_CODE
| ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES;
private static final Function<ClassData, ExtensionMetaData> CLASS_DATA_TO_EXTENSION_META_DATA =
@@ -61,27 +65,19 @@
}
};
- public static class ExtensionMetaData {
- private final String className;
- private final String annotationValue;
+ private final JarFile jarFile;
- private ExtensionMetaData(String className, String annotationValue) {
- this.className = className;
- this.annotationValue = annotationValue;
- }
-
- public String getAnnotationValue() {
- return annotationValue;
- }
-
- public String getClassName() {
- return className;
+ public JarScanner(File srcFile) throws InvalidPluginException {
+ try {
+ this.jarFile = new JarFile(srcFile);
+ } catch (IOException e) {
+ throw new InvalidPluginException("Cannot scan plugin file " + srcFile, e);
}
}
- public static Map<Class<? extends Annotation>, Iterable<ExtensionMetaData>> scan(
- JarFile jarFile, String pluginName,
- Iterable<Class<? extends Annotation>> annotations)
+ @Override
+ public Map<Class<? extends Annotation>, Iterable<ExtensionMetaData>> scan(
+ String pluginName, Iterable<Class<? extends Annotation>> annotations)
throws InvalidPluginException {
Set<String> descriptors = Sets.newHashSet();
Multimap<String, JarScanner.ClassData> rawMap = ArrayListMultimap.create();
@@ -262,4 +258,62 @@
public void visitEnd() {
}
}
+
+ @Override
+ public Optional<PluginEntry> getEntry(String resourcePath) throws IOException {
+ JarEntry jarEntry = jarFile.getJarEntry(resourcePath);
+ if (jarEntry == null || jarEntry.getSize() == 0) {
+ return Optional.absent();
+ }
+
+ return Optional.of(resourceOf(jarEntry));
+ }
+
+ @Override
+ public Enumeration<PluginEntry> entries() {
+ return Collections.enumeration(Lists.transform(
+ Collections.list(jarFile.entries()),
+ new Function<JarEntry, PluginEntry>() {
+ public PluginEntry apply(JarEntry jarEntry) {
+ try {
+ return resourceOf(jarEntry);
+ } catch (IOException e) {
+ throw new IllegalArgumentException("Cannot convert jar entry "
+ + jarEntry + " to a resource", e);
+ }
+ }
+ }));
+ }
+
+ @Override
+ public InputStream getInputStream(PluginEntry entry)
+ throws IOException {
+ return jarFile.getInputStream(jarFile
+ .getEntry(entry.getName()));
+ }
+
+ @Override
+ public Manifest getManifest() throws IOException {
+ return jarFile.getManifest();
+ }
+
+ private PluginEntry resourceOf(JarEntry jarEntry) throws IOException {
+ return new PluginEntry(jarEntry.getName(), jarEntry.getTime(),
+ jarEntry.getSize(), attributesOf(jarEntry));
+ }
+
+ private Map<Object, String> attributesOf(JarEntry jarEntry)
+ throws IOException {
+ Attributes attributes = jarEntry.getAttributes();
+ if (attributes == null) {
+ return Collections.emptyMap();
+ }
+ return Maps.transformEntries(attributes,
+ new Maps.EntryTransformer<Object, Object, String>() {
+ @Override
+ public String transformEntry(Object key, Object value) {
+ return (String) value;
+ }
+ });
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JsPlugin.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JsPlugin.java
index ff72576..63f69b5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JsPlugin.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JsPlugin.java
@@ -28,7 +28,6 @@
import org.eclipse.jgit.internal.storage.file.FileSnapshot;
import java.io.File;
-import java.util.jar.JarFile;
class JsPlugin extends Plugin {
private Injector httpInjector;
@@ -67,11 +66,6 @@
}
@Override
- public JarFile getJarFile() {
- return null;
- }
-
- @Override
public Injector getSysInjector() {
return null;
}
@@ -109,4 +103,9 @@
new JavaScriptPlugin(fileName));
}
}
+
+ @Override
+ public PluginContentScanner getContentScanner() {
+ return PluginContentScanner.EMPTY;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ListPlugins.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ListPlugins.java
index 577ee0c..c5611dd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ListPlugins.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ListPlugins.java
@@ -137,7 +137,7 @@
version = p.getVersion();
disabled = p.isDisabled() ? true : null;
- if (p.getJarFile() != null) {
+ if (p.getSrcFile() != null) {
indexUrl = String.format("plugins/%s/", p.getName());
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java
index 988669a..ff6f0ae 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java
@@ -29,7 +29,6 @@
import java.util.Collections;
import java.util.List;
import java.util.jar.Attributes;
-import java.util.jar.JarFile;
import java.util.jar.Manifest;
public abstract class Plugin {
@@ -124,7 +123,7 @@
abstract void stop(PluginGuiceEnvironment env);
- public abstract JarFile getJarFile();
+ public abstract PluginContentScanner getContentScanner();
public abstract Injector getSysInjector();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginContentScanner.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginContentScanner.java
new file mode 100644
index 0000000..0228509
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginContentScanner.java
@@ -0,0 +1,130 @@
+// Copyright (C) 2014 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.plugins;
+
+import com.google.common.base.Optional;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.annotation.Annotation;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.Map;
+import java.util.jar.Manifest;
+
+/**
+ * Scans the plugin returning classes and resources.
+ *
+ * Gerrit uses the scanner to automatically discover the classes
+ * and resources exported by the plugin for auto discovery
+ * of exported SSH commands, Servlets and listeners.
+ */
+public interface PluginContentScanner {
+
+ /**
+ * Scanner without resources.
+ */
+ PluginContentScanner EMPTY = new PluginContentScanner() {
+ @Override
+ public Manifest getManifest() throws IOException {
+ return new Manifest();
+ }
+
+ @Override
+ public Map<Class<? extends Annotation>, Iterable<ExtensionMetaData>> scan(
+ String pluginName, Iterable<Class<? extends Annotation>> annotations)
+ throws InvalidPluginException {
+ return Collections.emptyMap();
+ }
+
+ @Override
+ public Optional<PluginEntry> getEntry(String resourcePath)
+ throws IOException {
+ return Optional.absent();
+ }
+
+ @Override
+ public InputStream getInputStream(PluginEntry entry) throws IOException {
+ throw new FileNotFoundException("Empty plugin");
+ }
+
+ @Override
+ public Enumeration<PluginEntry> entries() {
+ return Collections.emptyEnumeration();
+ }
+ };
+
+ /**
+ * Plugin class extension meta-data
+ *
+ * Class name and annotation value of the class
+ * provided by a plugin to extend an existing
+ * extension point in Gerrit.
+ */
+ public static class ExtensionMetaData {
+ public final String className;
+ public final String annotationValue;
+
+ public ExtensionMetaData(String className, String annotationValue) {
+ this.className = className;
+ this.annotationValue = annotationValue;
+ }
+ }
+
+ /**
+ * Return the plugin meta-data manifest
+ *
+ * @return Manifest of the plugin or null if plugin has no meta-data
+ * @throws IOException if an I/O problem occurred whilst accessing the Manifest
+ */
+ Manifest getManifest() throws IOException;
+
+ /**
+ * Scans the plugin for declared public annotated classes
+ *
+ * @param pluginName the plugin name
+ * @param annotations annotations declared by the plugin classes
+ * @return map of annotations and associated plugin classes found
+ * @throws InvalidPluginException if the plugin is not valid or corrupted
+ */
+ Map<Class<? extends Annotation>, Iterable<ExtensionMetaData>> scan(
+ String pluginName, Iterable<Class<? extends Annotation>> annotations)
+ throws InvalidPluginException;
+
+ /**
+ * Return the plugin resource associated to a path
+ *
+ * @param resourcePath full path of the resource inside the plugin package
+ * @return the resource object or Optional.absent() if the resource was not found
+ * @throws IOException if there was a problem retrieving the resource
+ */
+ Optional<PluginEntry> getEntry(String resourcePath) throws IOException;
+
+ /**
+ * Return the InputStream of the resource entry
+ *
+ * @param entry resource entry inside the plugin package
+ * @return the resource input stream
+ * @throws IOException if there was an I/O problem accessing the resource
+ */
+ InputStream getInputStream(PluginEntry entry) throws IOException;
+
+ /**
+ * Return all the resources inside a plugin
+ *
+ * @return the enumeration of all resources found
+ */
+ Enumeration<PluginEntry> entries();
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginEntry.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginEntry.java
new file mode 100644
index 0000000..6022f02
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginEntry.java
@@ -0,0 +1,64 @@
+// Copyright (C) 2014 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.plugins;
+
+import java.util.Collections;
+import java.util.Map;
+
+/**
+ * Plugin static resource entry
+ *
+ * Bean representing a static resource inside a plugin.
+ * All static resources are available at <plugin web url>/static
+ * and served by the HttpPluginServlet.
+ */
+public class PluginEntry {
+ public static final String ATTR_CHARACTER_ENCODING = "Character-Encoding";
+ public static final String ATTR_CONTENT_TYPE = "Content-Type";
+
+ private static final Map<Object,String> EMPTY_ATTRS = Collections.emptyMap();
+
+ private final String name;
+ private final long time;
+ private final long size;
+ private final Map<Object, String> attrs;
+
+ public PluginEntry(String name, long time, long size,
+ Map<Object, String> attrs) {
+ this.name = name;
+ this.time = time;
+ this.size = size;
+ this.attrs = attrs;
+ }
+
+ public PluginEntry(String name, long time, long size) {
+ this(name, time, size, EMPTY_ATTRS);
+ }
+
+ public String getName() {
+ return name;
+ }
+
+ public long getTime() {
+ return time;
+ }
+
+ public long getSize() {
+ return size;
+ }
+
+ public Map<Object, String> getAttrs() {
+ return attrs;
+ }
+}
\ No newline at end of file
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java
index c2c8d03..b56a10c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java
@@ -589,8 +589,8 @@
Plugin plugin = new ServerPlugin(name, url,
pluginUserFactory.create(name),
- srcJar, snapshot,
- jarFile, manifest,
+ srcJar, snapshot, new JarFile(srcJar),
+ new JarScanner(srcJar),
new File(dataDir, name), type, pluginLoader,
sysModule, sshModule, httpModule);
cleanupHandles.put(plugin, new CleanupHandle(tmp, jarFile));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java
index 8dae5ae..052cdc7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java
@@ -35,12 +35,13 @@
import org.eclipse.jgit.internal.storage.file.FileSnapshot;
import java.io.File;
+import java.io.IOException;
import java.util.List;
import java.util.jar.Attributes;
import java.util.jar.JarFile;
import java.util.jar.Manifest;
-class ServerPlugin extends Plugin {
+public class ServerPlugin extends Plugin {
/** Unique key that changes whenever a plugin reloads. */
public static final class CacheKey {
@@ -59,6 +60,7 @@
private final JarFile jarFile;
private final Manifest manifest;
+ private final PluginContentScanner scanner;
private final File dataDir;
private final String pluginCanonicalWebUrl;
private final ClassLoader classLoader;
@@ -78,28 +80,39 @@
File srcJar,
FileSnapshot snapshot,
JarFile jarFile,
- Manifest manifest,
+ PluginContentScanner scanner,
File dataDir,
ApiType apiType,
ClassLoader classLoader,
@Nullable Class<? extends Module> sysModule,
@Nullable Class<? extends Module> sshModule,
- @Nullable Class<? extends Module> httpModule) {
+ @Nullable Class<? extends Module> httpModule)
+ throws InvalidPluginException {
super(name, srcJar, pluginUser, snapshot, apiType);
this.pluginCanonicalWebUrl = pluginCanonicalWebUrl;
this.jarFile = jarFile;
- this.manifest = manifest;
+ this.scanner = scanner;
this.dataDir = dataDir;
this.classLoader = classLoader;
this.sysModule = sysModule;
this.sshModule = sshModule;
this.httpModule = httpModule;
+ this.manifest = getPluginManifest(scanner);
}
File getSrcJar() {
return getSrcFile();
}
+ private Manifest getPluginManifest(PluginContentScanner scanner)
+ throws InvalidPluginException {
+ try {
+ return scanner.getManifest();
+ } catch (IOException e) {
+ throw new InvalidPluginException("Cannot get plugin manifest", e);
+ }
+ }
+
@Nullable
public String getVersion() {
Attributes main = manifest.getMainAttributes();
@@ -136,7 +149,7 @@
AutoRegisterModules auto = null;
if (sysModule == null && sshModule == null && httpModule == null) {
- auto = new AutoRegisterModules(getName(), env, jarFile, classLoader);
+ auto = new AutoRegisterModules(getName(), env, scanner, classLoader);
auto.discover();
}
@@ -270,4 +283,9 @@
manager.add(handle);
}
}
+
+ @Override
+ public PluginContentScanner getContentScanner() {
+ return scanner;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index da19d19..ed64015 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -32,6 +32,7 @@
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -152,7 +153,8 @@
* @return instance for testing.
*/
static ChangeData createForTest(Change.Id id, int currentPatchSetId) {
- ChangeData cd = new ChangeData(null, null, null, null, null, null, null, null, id);
+ ChangeData cd = new ChangeData(null, null, null, null, null,
+ null, null, null, null, id);
cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId));
return cd;
}
@@ -163,6 +165,7 @@
private final IdentifiedUser.GenericFactory userFactory;
private final ChangeNotes.Factory notesFactory;
private final ApprovalsUtil approvalsUtil;
+ private final ChangeMessagesUtil cmUtil;
private final PatchListCache patchListCache;
private final NotesMigration notesMigration;
private final Change.Id legacyId;
@@ -190,6 +193,7 @@
IdentifiedUser.GenericFactory userFactory,
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
+ ChangeMessagesUtil cmUtil,
PatchListCache patchListCache,
NotesMigration notesMigration,
@Assisted ReviewDb db,
@@ -200,6 +204,7 @@
this.userFactory = userFactory;
this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil;
+ this.cmUtil = cmUtil;
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
legacyId = id;
@@ -212,6 +217,7 @@
IdentifiedUser.GenericFactory userFactory,
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
+ ChangeMessagesUtil cmUtil,
PatchListCache patchListCache,
NotesMigration notesMigration,
@Assisted ReviewDb db,
@@ -222,6 +228,7 @@
this.userFactory = userFactory;
this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil;
+ this.cmUtil = cmUtil;
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
legacyId = c.getId();
@@ -235,6 +242,7 @@
IdentifiedUser.GenericFactory userFactory,
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
+ ChangeMessagesUtil cmUtil,
PatchListCache patchListCache,
NotesMigration notesMigration,
@Assisted ReviewDb db,
@@ -245,6 +253,7 @@
this.userFactory = userFactory;
this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil;
+ this.cmUtil = cmUtil;
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
legacyId = c.getChange().getId();
@@ -521,7 +530,7 @@
public List<ChangeMessage> messages()
throws OrmException {
if (messages == null) {
- messages = db.changeMessages().byChange(legacyId).toList();
+ messages = cmUtil.byChange(db, notes);
}
return messages;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
index db9b29d..173060b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
@@ -103,12 +103,6 @@
return lt;
}
}
-
- for (LabelType lt : types.getLabelTypes()) {
- if (toFind.equalsIgnoreCase(lt.getAbbreviation())) {
- return lt;
- }
- }
return null;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/AllProjectsCreator.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/AllProjectsCreator.java
index 7712650..1d8c126 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/AllProjectsCreator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/AllProjectsCreator.java
@@ -215,7 +215,6 @@
new LabelValue((short) 0, "No score"),
new LabelValue((short) -1, "I would prefer this is not merged as is"),
new LabelValue((short) -2, "This shall not be merged")));
- type.setAbbreviation("CR");
type.setCopyMinScore(true);
c.getLabelSections().put(type.getName(), type);
return type;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_77.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_77.java
index facfb75..a5166b6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_77.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_77.java
@@ -203,8 +203,7 @@
Statement catStmt = ((JdbcSchema) db).getConnection().createStatement();
try {
ResultSet catRs = catStmt.executeQuery(
- "SELECT category_id, name, abbreviated_name, function_name, "
- + " copy_min_score"
+ "SELECT category_id, name, function_name, copy_min_score"
+ " FROM approval_categories"
+ " ORDER BY position, name");
PreparedStatement valStmt = ((JdbcSchema) db).getConnection().prepareStatement(
@@ -224,7 +223,6 @@
LegacyLabelType type =
new LegacyLabelType(getLabelName(catRs.getString("name")), values);
type.setId(id);
- type.setAbbreviation(catRs.getString("abbreviated_name"));
type.setFunctionName(catRs.getString("function_name"));
type.setCopyMinScore("Y".equals(catRs.getString("copy_min_score")));
types.add(type);
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
index f2ad057..f912099 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -32,6 +32,7 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
@@ -213,6 +214,31 @@
}
@Test
+ public void changeMessageCommitFormatSimple() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.setChangeMessage("Just a little code change.\n"
+ + "How about a new line");
+ update.commit();
+ assertEquals("refs/changes/01/1/meta", update.getRefName());
+
+ RevWalk walk = new RevWalk(repo);
+ try {
+ RevCommit commit = walk.parseCommit(update.getRevision());
+ walk.parseBody(commit);
+ assertEquals("Update patch set 1\n"
+ + "\n"
+ + "Just a little code change.\n"
+ + "How about a new line\n"
+ + "\n"
+ + "Patch-set: 1\n",
+ commit.getFullMessage());
+ } finally {
+ walk.release();
+ }
+ }
+
+ @Test
public void approvalTombstoneCommitFormat() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
@@ -608,6 +634,204 @@
assertEquals((short) 2, psas.get(1).getValue());
}
+ @Test
+ public void changeMessageOnePatchSet() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.putReviewer(changeOwner.getAccount().getId(), REVIEWER);
+ update.setChangeMessage("Just a little code change.\n");
+ update.commit();
+ PatchSet.Id ps1 = c.currentPatchSetId();
+
+ ChangeNotes notes = newNotes(c);
+ ListMultimap<PatchSet.Id, ChangeMessage> changeMessages =
+ notes.getChangeMessages();
+ assertEquals(1, changeMessages.keySet().size());
+
+ ChangeMessage cm = Iterables.getOnlyElement(changeMessages.get(ps1));
+ assertEquals("Just a little code change.\n",
+ cm.getMessage());
+ assertEquals(changeOwner.getAccount().getId(),
+ cm.getAuthor());
+ assertEquals(ps1, cm.getPatchSetId());
+ }
+
+ @Test
+ public void changeMessagesMultiplePatchSets() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.putReviewer(changeOwner.getAccount().getId(), REVIEWER);
+ update.setChangeMessage("This is the change message for the first PS.");
+ update.commit();
+ PatchSet.Id ps1 = c.currentPatchSetId();
+
+ incrementPatchSet(c);
+ update = newUpdate(c, changeOwner);
+
+ update.setChangeMessage("This is the change message for the second PS.");
+ update.commit();
+ PatchSet.Id ps2 = c.currentPatchSetId();
+
+ ChangeNotes notes = newNotes(c);
+ ListMultimap<PatchSet.Id, ChangeMessage> changeMessages =
+ notes.getChangeMessages();
+ assertEquals(2, changeMessages.keySet().size());
+
+ ChangeMessage cm1 = Iterables.getOnlyElement(changeMessages.get(ps1));
+ assertEquals("This is the change message for the first PS.",
+ cm1.getMessage());
+ assertEquals(changeOwner.getAccount().getId(),
+ cm1.getAuthor());
+
+ ChangeMessage cm2 = Iterables.getOnlyElement(changeMessages.get(ps2));
+ assertEquals(ps1, cm1.getPatchSetId());
+ assertEquals("This is the change message for the second PS.",
+ cm2.getMessage());
+ assertEquals(changeOwner.getAccount().getId(), cm2.getAuthor());
+ assertEquals(ps2, cm2.getPatchSetId());
+ }
+
+ @Test
+ public void noChangeMessage() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.putReviewer(changeOwner.getAccount().getId(), REVIEWER);
+ update.commit();
+
+ RevWalk walk = new RevWalk(repo);
+ try {
+ RevCommit commit = walk.parseCommit(update.getRevision());
+ walk.parseBody(commit);
+ assertEquals("Update patch set 1\n"
+ + "\n"
+ + "Patch-set: 1\n"
+ + "Reviewer: Change Owner <1@gerrit>\n",
+ commit.getFullMessage());
+ } finally {
+ walk.release();
+ }
+
+ ChangeNotes notes = newNotes(c);
+ ListMultimap<PatchSet.Id, ChangeMessage> changeMessages =
+ notes.getChangeMessages();
+ assertEquals(0, changeMessages.keySet().size());
+ }
+
+ @Test
+ public void changeMessageWithTrailingDoubleNewline() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.setChangeMessage("Testing trailing double newline\n"
+ + "\n");
+ update.commit();
+ PatchSet.Id ps1 = c.currentPatchSetId();
+
+ RevWalk walk = new RevWalk(repo);
+ try {
+ RevCommit commit = walk.parseCommit(update.getRevision());
+ walk.parseBody(commit);
+ assertEquals("Update patch set 1\n"
+ + "\n"
+ + "Testing trailing double newline\n"
+ + "\n"
+ + "\n"
+ + "\n"
+ + "Patch-set: 1\n",
+ commit.getFullMessage());
+ } finally {
+ walk.release();
+ }
+
+ ChangeNotes notes = newNotes(c);
+ ListMultimap<PatchSet.Id, ChangeMessage> changeMessages =
+ notes.getChangeMessages();
+ assertEquals(1, changeMessages.keySet().size());
+
+ ChangeMessage cm1 = Iterables.getOnlyElement(changeMessages.get(ps1));
+ assertEquals("Testing trailing double newline\n" + "\n", cm1.getMessage());
+ assertEquals(changeOwner.getAccount().getId(), cm1.getAuthor());
+
+ }
+
+ @Test
+ public void changeMessageWithMultipleParagraphs() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.setChangeMessage("Testing paragraph 1\n"
+ + "\n"
+ + "Testing paragraph 2\n"
+ + "\n"
+ + "Testing paragraph 3");
+ update.commit();
+ PatchSet.Id ps1 = c.currentPatchSetId();
+
+ RevWalk walk = new RevWalk(repo);
+ try {
+ RevCommit commit = walk.parseCommit(update.getRevision());
+ walk.parseBody(commit);
+ assertEquals("Update patch set 1\n"
+ + "\n"
+ + "Testing paragraph 1\n"
+ + "\n"
+ + "Testing paragraph 2\n"
+ + "\n"
+ + "Testing paragraph 3\n"
+ + "\n"
+ + "Patch-set: 1\n",
+ commit.getFullMessage());
+ } finally {
+ walk.release();
+ }
+
+ ChangeNotes notes = newNotes(c);
+ ListMultimap<PatchSet.Id, ChangeMessage> changeMessages =
+ notes.getChangeMessages();
+ assertEquals(1, changeMessages.keySet().size());
+
+ ChangeMessage cm1 = Iterables.getOnlyElement(changeMessages.get(ps1));
+ assertEquals("Testing paragraph 1\n"
+ + "\n"
+ + "Testing paragraph 2\n"
+ + "\n"
+ + "Testing paragraph 3", cm1.getMessage());
+ assertEquals(changeOwner.getAccount().getId(), cm1.getAuthor());
+ }
+
+ @Test
+ public void changeMessageMultipleInOnePatchSet() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.putReviewer(changeOwner.getAccount().getId(), REVIEWER);
+ update.setChangeMessage("First change message.\n");
+ update.commit();
+
+ PatchSet.Id ps1 = c.currentPatchSetId();
+
+ update = newUpdate(c, changeOwner);
+ update.putReviewer(changeOwner.getAccount().getId(), REVIEWER);
+ update.setChangeMessage("Second change message.\n");
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+ ListMultimap<PatchSet.Id, ChangeMessage> changeMessages =
+ notes.getChangeMessages();
+ assertEquals(1, changeMessages.keySet().size());
+
+ List<ChangeMessage> cm = changeMessages.get(ps1);
+ assertEquals(2, cm.size());
+ assertEquals("First change message.\n",
+ cm.get(0).getMessage());
+ assertEquals(changeOwner.getAccount().getId(),
+ cm.get(0).getAuthor());
+ assertEquals(ps1, cm.get(0).getPatchSetId());
+ assertEquals("Second change message.\n",
+ cm.get(1).getMessage());
+ assertEquals(changeOwner.getAccount().getId(),
+ cm.get(1).getAuthor());
+ assertEquals(ps1, cm.get(1).getPatchSetId());
+ }
+
+
private Change newChange() {
Change.Id changeId = new Change.Id(1);
Change c = new Change(
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java
index 15a66ab..996aafa 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java
@@ -128,7 +128,6 @@
LabelType codeReview = getLabelTypes().byLabel("Code-Review");
assertNotNull(codeReview);
assertEquals("Code-Review", codeReview.getName());
- assertEquals("CR", codeReview.getAbbreviation());
assertEquals(0, codeReview.getDefaultValue());
assertEquals("MaxWithBlock", codeReview.getFunctionName());
assertTrue(codeReview.isCopyMinScore());
diff --git a/gerrit-war/BUCK b/gerrit-war/BUCK
index 7e11aea..fc73973 100644
--- a/gerrit-war/BUCK
+++ b/gerrit-war/BUCK
@@ -49,8 +49,7 @@
prebuilt_jar(
name = 'log4j-config',
- binary_jar = genfile('log4j-config.jar'),
- deps = [':log4j-config__jar'],
+ binary_jar = ':log4j-config__jar',
visibility = [
'//:',
'//tools/eclipse:classpath',
@@ -59,8 +58,7 @@
prebuilt_jar(
name = 'version',
- binary_jar = genfile('version.jar'),
- deps = [':gen_version'],
+ binary_jar = ':gen_version',
visibility = ['//:'],
)
diff --git a/lib/codemirror/BUCK b/lib/codemirror/BUCK
index ea6e2a1..e8539c6 100644
--- a/lib/codemirror/BUCK
+++ b/lib/codemirror/BUCK
@@ -1,5 +1,6 @@
include_defs('//lib/maven.defs')
include_defs('//lib/codemirror/cm3.defs')
+include_defs('//lib/codemirror/closure.defs')
VERSION = '28a638a984'
SHA1 = '68f8f136092a5965778186fb401a33be34cf73ed'
@@ -8,46 +9,52 @@
ZIP = 'codemirror-%s.zip' % VERSION
TOP = 'codemirror-%s' % VERSION
+CLOSURE_COMPILER_ARGS = [
+ '--compilation_level SIMPLE_OPTIMIZATIONS',
+ '--warning_level QUIET'
+]
+
genrule(
name = 'css',
cmd = ';'.join([
':>$OUT',
"echo '/** @license' >>$OUT",
- 'unzip -p %s %s/LICENSE >>$OUT' % (ZIP, TOP),
+ 'unzip -p $(location :zip) %s/LICENSE >>$OUT' % TOP,
"echo '*/' >>$OUT",
] +
- ['unzip -p %s %s/%s >>$OUT' % (ZIP, TOP, n)
+ ['unzip -p $(location :zip) %s/%s >>$OUT' % (TOP, n)
for n in CM3_CSS + CM3_THEMES]
),
- srcs = [genfile(ZIP)],
- deps = [':download'],
+ deps = [':zip'],
out = 'cm3.css',
)
-# TODO(sop) Minify with Closure JavaScript compiler.
genrule(
- name = 'js',
+ name = 'cm3-verbose',
cmd = ';'.join([
':>$OUT',
"echo '/** @license' >>$OUT",
- 'unzip -p %s %s/LICENSE >>$OUT' % (ZIP, TOP),
+ 'unzip -p $(location :zip) %s/LICENSE >>$OUT' % TOP,
"echo '*/' >>$OUT",
] +
- ['unzip -p %s %s/%s >>$OUT' % (ZIP, TOP, n)
+ ['unzip -p $(location :zip) %s/%s >>$OUT' % (TOP, n)
for n in CM3_JS]
),
- srcs = [genfile(ZIP)],
- deps = [':download'],
- out = 'cm3.js',
+ deps = [':zip'],
+ out = 'cm3-verbose.js',
+)
+
+js_minify(
+ name = 'js',
+ generated = [':cm3-verbose'],
+ compiler_args = CLOSURE_COMPILER_ARGS,
+ out = 'cm3.js'
)
prebuilt_jar(
name = 'codemirror',
- binary_jar = genfile('codemirror.jar'),
- deps = [
- ':jar',
- '//lib:LICENSE-codemirror',
- ],
+ binary_jar = ':jar',
+ deps = ['//lib:LICENSE-codemirror'],
visibility = ['PUBLIC'],
)
@@ -55,35 +62,28 @@
name = 'jar',
cmd = ';'.join([
'cd $TMP',
- 'unzip -q $SRCDIR/%s %s' % (
- ZIP,
- ' '.join(['%s/mode/%s' % (TOP, n) for n in CM3_MODES])),
+ 'unzip -q $(location :zip) %s' %
+ ' '.join(['%s/mode/%s' % (TOP, n) for n in CM3_MODES]),
+ ';'.join(['$(exe :js_minifier) ' +
+ ' '.join(CLOSURE_COMPILER_ARGS) +
+ ' --js_output_file %s/mode/%s.min --js %s/mode/%s'
+ % (TOP, n, TOP, n) for n in CM3_MODES]),
+ ';'.join(['mv %s/mode/%s.min %s/mode/%s' % (TOP, n, TOP, n) for n in CM3_MODES]),
'mkdir net',
'mv %s net/codemirror' % TOP,
'mkdir net/codemirror/lib',
- 'mv $SRCDIR/cm3.css net/codemirror/lib',
- 'mv $SRCDIR/cm3.js net/codemirror/lib',
+ 'cp $(location :css) net/codemirror/lib',
+ 'cp $(location :js) net/codemirror/lib',
'zip -qr $OUT *'
]),
- srcs = [
- genfile(ZIP),
- genfile('cm3.css'),
- genfile('cm3.js'),
- ],
- deps = [
- ':download',
- ':css',
- ':js',
- ],
out = 'codemirror.jar',
)
genrule(
- name = 'download',
+ name = 'zip',
cmd = '$(exe //tools:download_file)' +
' -o $OUT' +
' -u ' + URL +
' -v ' + SHA1,
- deps = ['//tools:download_file'],
- out = 'codemirror-' + VERSION + '.zip',
+ out = ZIP,
)
diff --git a/lib/codemirror/closure.defs b/lib/codemirror/closure.defs
new file mode 100644
index 0000000..e602b9f
--- /dev/null
+++ b/lib/codemirror/closure.defs
@@ -0,0 +1,50 @@
+# https://code.google.com/p/closure-compiler/wiki/BinaryDownloads?tm=2
+CLOSURE_VERSION = '20140407'
+CLOSURE_COMPILER_URL = 'http://dl.google.com/closure-compiler/compiler-%s.zip' % CLOSURE_VERSION
+COMPILER = 'compiler.jar'
+CLOSURE_COMPILER_SHA1 = 'eeb02bfd45eb4a080b66dd423eaee4bdd1d674e9'
+
+def js_minify(
+ name,
+ out,
+ compiler_args = [],
+ srcs = [],
+ generated = []):
+ cmd = ['$(exe :js_minifier) --js_output_file $OUT'] + compiler_args
+ if srcs:
+ cmd.append('$SRCS')
+ if generated:
+ cmd.extend(['$(location %s)' % n for n in generated])
+
+ genrule(
+ name = name,
+ cmd = ' '.join(cmd),
+ srcs = srcs,
+ out = out,
+)
+
+java_binary(
+ name = 'js_minifier',
+ main_class = 'com.google.javascript.jscomp.CommandLineRunner',
+ deps = [':compiler-jar']
+)
+
+prebuilt_jar(
+ name = 'compiler-jar',
+ binary_jar = ':compiler',
+)
+
+genrule(
+ name = 'compiler',
+ cmd = 'unzip -p $(location :closure-compiler-zip) %s >$OUT' % COMPILER,
+ out = COMPILER,
+)
+
+genrule(
+ name = 'closure-compiler-zip',
+ cmd = '$(exe //tools:download_file)' +
+ ' -o $OUT' +
+ ' -u ' + CLOSURE_COMPILER_URL +
+ ' -v ' + CLOSURE_COMPILER_SHA1,
+ out = 'closure-compiler.zip',
+)
diff --git a/lib/guice/BUCK b/lib/guice/BUCK
index 946bf9d..162ad07 100644
--- a/lib/guice/BUCK
+++ b/lib/guice/BUCK
@@ -1,6 +1,7 @@
include_defs('//lib/maven.defs')
VERSION = '4.0-beta'
+COOKIE_PATCH = '4.0-beta-98-g8d88344'
EXCLUDE = [
'META-INF/DEPENDENCIES',
'META-INF/LICENSE',
@@ -41,8 +42,9 @@
maven_jar(
name = 'guice-servlet',
- id = 'com.google.inject.extensions:guice-servlet:' + VERSION,
- sha1 = '46b44984f254c0bf92d0c972fad1a70292ada28e',
+ id = 'com.google.inject.extensions:guice-servlet:' + COOKIE_PATCH,
+ repository = GERRIT,
+ sha1 = 'fa17d57a083fe9fc86b93f2dc37069573a2e65cd',
license = 'Apache2.0',
deps = [':guice'],
exclude = EXCLUDE,
diff --git a/lib/jgit/BUCK b/lib/jgit/BUCK
index 07671a5..65d6c62 100644
--- a/lib/jgit/BUCK
+++ b/lib/jgit/BUCK
@@ -69,17 +69,15 @@
prebuilt_jar(
name = 'Edit',
- binary_jar = genfile('edit-src.jar'),
- deps = [':jgit_edit_src'],
+ binary_jar = ':jgit_edit_src',
visibility = ['PUBLIC'],
)
genrule(
name = 'jgit_edit_src',
- cmd = 'unzip -qd $TMP $SRCS org/eclipse/jgit/diff/Edit.java;' +
+ cmd = 'unzip -qd $TMP $(location :jgit_src) ' +
+ 'org/eclipse/jgit/diff/Edit.java;' +
'cd $TMP;' +
'zip -Dq $OUT org/eclipse/jgit/diff/Edit.java',
- srcs = [genfile('jgit/org.eclipse.jgit-%s-src.jar' % VERS)],
out = 'edit-src.jar',
- deps = [':jgit_src']
)
diff --git a/lib/maven.defs b/lib/maven.defs
index 0ac5005..67dad4e 100644
--- a/lib/maven.defs
+++ b/lib/maven.defs
@@ -79,9 +79,8 @@
cmd.append('--unsign')
genrule(
- name = name + '__download_bin',
+ name = '%s__download_bin' % name,
cmd = ' '.join(cmd),
- deps = ['//tools:download_file'],
out = binjar,
)
license = ['//lib:LICENSE-' + license]
@@ -91,32 +90,30 @@
if src_sha1:
cmd.extend(['-v', src_sha1])
genrule(
- name = name + '__download_src',
+ name = '%s__download_src' % name,
cmd = ' '.join(cmd),
- deps = ['//tools:download_file'],
out = srcjar,
)
prebuilt_jar(
- name = name + '_src',
- binary_jar = genfile(srcjar),
- deps = license + [':' + name + '__download_src'],
+ name = '%s_src' % name,
+ binary_jar = ':%s__download_src' % name,
+ deps = license,
visibility = visibility,
)
else:
srcjar = None
genrule(
- name = name + '__download_src',
+ name = '%s__download_src' % name,
cmd = ':>$OUT',
- out = '__' + name + '__no_src',
+ out = '__%s__no_src' % name,
)
- srcdep = [':' + name + '__download_src'] if srcjar else []
if exported_deps:
prebuilt_jar(
- name = name + '__jar',
- deps = deps + srcdep + license + [':' + name + '__download_bin'],
- binary_jar = genfile(binjar),
- source_jar = genfile(srcjar) if srcjar else None,
+ name = '%s__jar' % name,
+ deps = deps,
+ binary_jar = ':%s__download_bin' % name,
+ source_jar = ':%s__download_src' % name if srcjar else None,
)
java_library(
name = name,
@@ -126,9 +123,9 @@
else:
prebuilt_jar(
name = name,
- deps = deps + srcdep + license + [':' + name + '__download_bin'],
- binary_jar = genfile(binjar),
- source_jar = genfile(srcjar) if srcjar else None,
+ deps = deps,
+ binary_jar = ':%s__download_bin' % name,
+ source_jar = ':%s__download_src' % name if srcjar else None,
visibility = visibility,
)
@@ -141,18 +138,17 @@
binjar = name + '.jar'
srcjar = name + '-src.jar'
genrule(
- name = name + '__local_bin',
+ name = '%s__local_bin' % name,
cmd = 'ln -s %s $OUT' % jar,
out = binjar)
if src:
genrule(
- name = name + '__local_src',
+ name = '%s__local_src' % name,
cmd = 'ln -s %s $OUT' % src,
out = srcjar)
prebuilt_jar(
- name = name + '_src',
- deps = [':' + name + '__local_src'],
- binary_jar = genfile(srcjar),
+ name = '%s_src' % name,
+ binary_jar = ':%s__local_src' % name,
visibility = visibility,
)
else:
@@ -160,8 +156,8 @@
prebuilt_jar(
name = name,
- deps = deps + [':' + name + '__local_bin'],
- binary_jar = genfile(binjar),
- source_jar = genfile(srcjar) if srcjar else None,
+ deps = deps,
+ binary_jar = ':%s__local_bin' % name,
+ source_jar = ':%s__local_src' % name if srcjar else None,
visibility = visibility,
)
diff --git a/lib/prolog/prolog.defs b/lib/prolog/prolog.defs
index 62e4c6d..d0636f5 100644
--- a/lib/prolog/prolog.defs
+++ b/lib/prolog/prolog.defs
@@ -23,16 +23,12 @@
' $TMP $OUT ' +
' '.join(srcs),
srcs = srcs,
- deps = ['//lib/prolog:compiler'],
out = name + '.src.zip',
)
java_library(
name = name + '__lib',
- srcs = [genfile(name + '.src.zip')],
- deps = [
- ':' + name + '__pl2j',
- '//lib/prolog:prolog-cafe',
- ] + deps,
+ srcs = [':' + name + '__pl2j'],
+ deps = ['//lib/prolog:prolog-cafe'] + deps,
)
genrule(
name = name + '__ln',
@@ -42,7 +38,6 @@
)
prebuilt_jar(
name = name,
- binary_jar = genfile(name + '.jar'),
- deps = [':%s__ln' % name],
+ binary_jar = ':%s__ln' % name,
visibility = visibility,
)
diff --git a/plugins/BUCK b/plugins/BUCK
index 480cd4c..134f275 100644
--- a/plugins/BUCK
+++ b/plugins/BUCK
@@ -28,11 +28,11 @@
';'.join(['echo >&2 plugins/'+n+' is required.' for n in NEED]) +
(';echo >&2;exit 1;' if NEED else '') +
'mkdir -p $TMP/WEB-INF/plugins;' +
- 'for s in $SRCS;do ln -s $s $TMP/WEB-INF/plugins;done;' +
+ 'for s in ' +
+ ' '.join(['$(location //%s/%s:%s)' % (BASE, n, n) for n in HAVE]) +
+ ';do ln -s $s $TMP/WEB-INF/plugins;done;' +
'cd $TMP;' +
'zip -qr $OUT .',
- srcs = [genfile('%s/%s.jar' % (n, n)) for n in HAVE],
- deps = ['//%s/%s:%s' % (BASE, n, n) for n in HAVE],
out = 'core.zip',
visibility = ['//:release'],
)
diff --git a/plugins/replication b/plugins/replication
index d92b6f2..3f67e32 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit d92b6f2f92b06f0f189439029a2abb7107d23838
+Subproject commit 3f67e32dc01955cdec9bac64b19fc7cfcb20b816
diff --git a/tools/build.defs b/tools/build.defs
index 9ce28f2..8b858cd 100644
--- a/tools/build.defs
+++ b/tools/build.defs
@@ -14,12 +14,8 @@
# These definitions support building a runnable version of Gerrit.
-DOCS_SRC = genfile('Documentation/html.zip')
+DOCS_HTML = '//Documentation:html'
DOCS_LIB = '//Documentation:index_lib'
-DOCS_DEP = [
- '//Documentation:html',
- '//Documentation:index_lib',
-]
LIBS = [
'//gerrit-war:log4j-config',
'//gerrit-war:init',
@@ -50,24 +46,19 @@
for l in pgmlibs:
cmd.extend(['--pgmlib', l])
- src = []
dep = []
if docs:
- src.append(DOCS_SRC)
- dep.extend(DOCS_DEP)
+ cmd.append('$(location %s)' % DOCS_HTML)
+ dep.append(DOCS_LIB)
cmd.extend(['--lib', DOCS_LIB])
if context:
for t in context:
- dep.append(t)
cmd.append('$(location %s)' % t)
- if src:
- cmd.append('$SRCS')
genrule(
name = name,
cmd = ' '.join(cmd),
- srcs = src,
- deps = libs + pgmlibs + dep + ['//tools:pack_war'],
+ deps = libs + pgmlibs + dep,
out = name + '.war',
visibility = visibility,
)
diff --git a/tools/default.defs b/tools/default.defs
index 4c6e898..16f5d10 100644
--- a/tools/default.defs
+++ b/tools/default.defs
@@ -28,7 +28,6 @@
cmd = '$(exe //lib/antlr:antlr-tool) -o $TMP $SRCS;' +
'cd $TMP;' +
'zip -qr $OUT .',
- deps = ['//lib/antlr:antlr-tool'],
out = out,
)
@@ -110,8 +109,7 @@
if gwt_module:
prebuilt_jar(
name = '%s-static-jar' % name,
- binary_jar = genfile('%s-static.zip' % name),
- deps = [':%s-static' % name],
+ binary_jar = ':%s-static' % name,
)
genrule(
name = '%s-static' % name,
@@ -121,7 +119,6 @@
';cd $TMP' +
';zip -qr $OUT .',
out = '%s-static.zip' % name,
- deps = [':%s__gwt_application' % name]
)
gwt_binary(
name = name + '__gwt_application',
@@ -136,10 +133,10 @@
java_binary(
name = name,
- manifest_file = genfile('MANIFEST.MF'),
+ manifest_file = ':%s__manifest' % name,
+ merge_manifests = False,
deps = [
':%s__plugin' % name,
- ':%s__manifest' % name,
] + static_jars,
visibility = visibility,
)
@@ -189,7 +186,6 @@
'-d $TMP',
]) + ';jar cf $OUT -C $TMP .',
srcs = srcs,
- deps = deps,
out = name + '.jar',
visibility = visibility,
)
diff --git a/tools/download_file.py b/tools/download_file.py
index d4ddf1c..d5d8b66 100755
--- a/tools/download_file.py
+++ b/tools/download_file.py
@@ -28,6 +28,7 @@
CACHE_DIR = path.join(GERRIT_HOME, 'buck-cache')
LOCAL_PROPERTIES = 'local.properties'
+
def hashfile(p):
d = sha1()
with open(p, 'rb') as f:
@@ -38,6 +39,7 @@
d.update(b)
return d.hexdigest()
+
def safe_mkdirs(d):
if path.isdir(d):
return
@@ -47,6 +49,7 @@
if not path.isdir(d):
raise err
+
def download_properties(root_dir):
""" Get the download properties.
@@ -73,6 +76,7 @@
pass
return p
+
def cache_entry(args):
if args.v:
h = args.v
@@ -81,6 +85,7 @@
name = '%s-%s' % (path.basename(args.o), h)
return path.join(CACHE_DIR, name)
+
opts = OptionParser()
opts.add_option('-o', help='local output file')
opts.add_option('-u', help='URL to download')
@@ -137,30 +142,24 @@
exclude += args.x
if args.exclude_java_sources:
try:
- zf = ZipFile(cache_ent, 'r')
- try:
+ with ZipFile(cache_ent, 'r') as zf:
for n in zf.namelist():
if n.endswith('.java'):
exclude.append(n)
- finally:
- zf.close()
except (BadZipfile, LargeZipFile) as err:
- print('error opening %s: %s' % (cache_ent, err), file=stderr)
+ print('error opening %s: %s' % (cache_ent, err), file=stderr)
exit(1)
if args.unsign:
try:
- zf = ZipFile(cache_ent, 'r')
- try:
+ with ZipFile(cache_ent, 'r') as zf:
for n in zf.namelist():
if (n.endswith('.RSA')
or n.endswith('.SF')
or n.endswith('.LIST')):
exclude.append(n)
- finally:
- zf.close()
except (BadZipfile, LargeZipFile) as err:
- print('error opening %s: %s' % (cache_ent, err), file=stderr)
+ print('error opening %s: %s' % (cache_ent, err), file=stderr)
exit(1)
safe_mkdirs(path.dirname(args.o))
diff --git a/tools/eclipse/project.py b/tools/eclipse/project.py
index 50644eb..2008316 100755
--- a/tools/eclipse/project.py
+++ b/tools/eclipse/project.py
@@ -32,7 +32,7 @@
])
ROOT = path.abspath(__file__)
-for _ in range(0, 3):
+while not path.exists(path.join(ROOT, '.buckconfig')):
ROOT = path.dirname(ROOT)
opts = OptionParser()
diff --git a/tools/gwt-constants.defs b/tools/gwt-constants.defs
index 44d2a62..cc09d3e 100644
--- a/tools/gwt-constants.defs
+++ b/tools/gwt-constants.defs
@@ -6,7 +6,6 @@
]
GWT_PLUGIN_DEPS = [
- '//gerrit-gwtui-common:client',
'//gerrit-plugin-gwtui:gwtui-api-lib',
'//lib/gwt:user',
]
diff --git a/tools/maven/mvn.py b/tools/maven/mvn.py
index 9e36b48..cc10816 100644
--- a/tools/maven/mvn.py
+++ b/tools/maven/mvn.py
@@ -16,9 +16,8 @@
from __future__ import print_function
from optparse import OptionParser
from os import path, environ
-
+from subprocess import check_output
from sys import stderr
-from tools.util import check_output
opts = OptionParser()
opts.add_option('--repository', help='maven repository id')
@@ -34,7 +33,7 @@
exit(1)
root = path.abspath(__file__)
-for _ in range(0, 3):
+while not path.exists(path.join(root, '.buckconfig')):
root = path.dirname(root)
if 'install' == args.a:
diff --git a/tools/pack_war.py b/tools/pack_war.py
index 09ff054..ee87b51 100755
--- a/tools/pack_war.py
+++ b/tools/pack_war.py
@@ -16,9 +16,8 @@
from __future__ import print_function
from optparse import OptionParser
from os import getcwd, chdir, makedirs, path, symlink
-from subprocess import check_call
+from subprocess import check_call, check_output
import sys
-from util import check_output
opts = OptionParser()
opts.add_option('-o', help='path to write WAR to')
@@ -31,6 +30,7 @@
root = war[:war.index('buck-out')]
jars = set()
+
def link_jars(libs, directory):
makedirs(directory)
while not path.isfile('.buckconfig'):
@@ -51,7 +51,7 @@
try:
for s in ctx:
check_call(['unzip', '-q', '-d', war, s])
- check_call(['zip', '-9qr', args.o, '.'], cwd = war)
+ check_call(['zip', '-9qr', args.o, '.'], cwd=war)
except KeyboardInterrupt:
print('Interrupted by user', file=sys.stderr)
exit(1)
diff --git a/tools/util.py b/tools/util.py
index 9115ac7..6dfe719 100644
--- a/tools/util.py
+++ b/tools/util.py
@@ -14,13 +14,6 @@
from os import path
-try:
- from subprocess import check_output
-except ImportError:
- from subprocess import Popen, PIPE
- def check_output(*cmd):
- return Popen(*cmd, stdout=PIPE).communicate()[0]
-
REPO_ROOTS = {
'GERRIT': 'http://gerrit-maven.storage.googleapis.com',
'GERRIT_API': 'https://gerrit-api.commondatastorage.googleapis.com/release',
@@ -29,6 +22,7 @@
'MAVEN_LOCAL': 'file://' + path.expanduser('~/.m2/repository'),
}
+
def resolve_url(url, redirects):
""" Resolve URL of a Maven artifact.
