From 6ca0726997d2b0ac6c13bb6a28515947ef158f20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Raymond?= Date: Sun, 22 Oct 2017 19:18:07 +0200 Subject: [PATCH 1/9] Prolog cookbook: fix several examples. * The Example 13: 1+1=2 Code-Review was broken, the fix was missing in I131e2008ba previous fix. Fix the following error: test output invalid result: submit(label(Code-Review,ok(_535EC2E6))). Reason: A label with the status Code-Review: OK must contain a user. * Example 6: Make change submittable if commit author is "John Doe" was probably missing the user(ID) Change-Id: I4126f01d979b4511faa83d4f779f83599c51c29a --- Documentation/prolog-cookbook.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Documentation/prolog-cookbook.txt b/Documentation/prolog-cookbook.txt index 1c114e2478..78497ebcda 100644 --- a/Documentation/prolog-cookbook.txt +++ b/Documentation/prolog-cookbook.txt @@ -544,6 +544,7 @@ submit_rule(submit(Author)) :- Author = label('Author-is-John-Doe', need(_)). submit_rule(submit(Author)) :- + A = user(1000000), gerrit:commit_author(A, 'John Doe', 'john.doe@example.com'), Author = label('Author-is-John-Doe', ok(A)). ---- @@ -899,7 +900,10 @@ add_category_min_score(In, Category, Min, P) :- findall(X, gerrit:commit_label(label(Category,X),R),Z), sum_list(Z, Sum), Sum >= Min, !, - P = [label(Category,ok(R)) | In]. + gerrit:commit_label(label(Category, V), U), + V >= 1, + !, + P = [label(Category,ok(U)) | In]. add_category_min_score(In, Category,Min,P) :- P = [label(Category,need(Min)) | In]. From c6ca3dd56e4a3f699ac74645e167f75c7823f251 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Mon, 23 Apr 2018 16:38:44 +0200 Subject: [PATCH 2/9] Documentation: clarify what happens if you override a label in a child. Change-Id: I15faf68b4b10bd39e5ec1f807ef9ec55e3e5af43 (cherry picked from commit f6b0bb8c83e6233fd8657b2cb6729a51649096e4) --- Documentation/config-labels.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt index c53d1fdbdb..d24f56b865 100644 --- a/Documentation/config-labels.txt +++ b/Documentation/config-labels.txt @@ -161,7 +161,9 @@ properties in the child project's configuration; all properties from the parent definition must be redefined in the child. To remove a label in a child project, add an empty label with the same -name as in the parent. +name as in the parent. This will override the parent label with +a label containing the defaults (`function = MaxWithBlock`, +`defaultValue = 0` and no further allowed values) [[label_layout]] === Layout From 5e35a134c9f3e4db9b3aaa2cd3780f082e60503a Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 24 Apr 2018 11:41:50 +0900 Subject: [PATCH 3/9] AccountManager: Use Logger's built-in string formatting Change-Id: Ib533311ee31a5ca1cd25fee1c8d481ec420dfece --- .../java/com/google/gerrit/server/account/AccountManager.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java index 92f0b1a473..0ad1a147fe 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java @@ -181,9 +181,7 @@ public class AccountManager { if (!realm.allowsEdit(AccountFieldName.USER_NAME) && who.getUserName() != null && !eq(user.getUserName(), who.getUserName())) { - log.warn( - String.format( - "Not changing already set username %s to %s", user.getUserName(), who.getUserName())); + log.warn("Not changing already set username {} to {}", user.getUserName(), who.getUserName()); } if (toUpdate != null) { From 3ecb519e431c127799a6f274575724b46b0348b3 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 24 Apr 2018 11:43:14 +0900 Subject: [PATCH 4/9] AccountManager: Reduce logs to debug level Change-Id: I4128976ae4cdcc6a4b28cc3b79f4d594f5ab4ac9 --- .../com/google/gerrit/server/account/AccountManager.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java index 0ad1a147fe..4c62287f60 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java @@ -124,7 +124,7 @@ public class AccountManager { } // New account, automatically create and return. // - log.info("External ID not found. Attempting to create new account."); + log.debug("External ID not found. Attempting to create new account."); return create(db, who); } @@ -362,16 +362,16 @@ public class AccountManager { public AuthResult link(Account.Id to, AuthRequest who) throws AccountException, OrmException, IOException { try (ReviewDb db = schema.open()) { - log.info("Link another authentication identity to an existing account"); + log.debug("Link another authentication identity to an existing account"); ExternalId extId = findExternalId(db, who.getExternalIdKey()); if (extId != null) { if (!extId.accountId().equals(to)) { throw new AccountException("Identity in use by another account"); } - log.info("Updating existing external ID data"); + log.debug("Updating existing external ID data"); update(db, who, extId); } else { - log.info("Linking new external ID to the existing account"); + log.debug("Linking new external ID to the existing account"); externalIdsUpdateFactory .create() .insert( From 6dcaddf465517e4a694557784f342a915384c389 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Sat, 21 Apr 2018 18:30:21 +0200 Subject: [PATCH 5/9] Delete index after each test in elasticsearch tests Since change I6539845e0 the index is not deleted after each test method because of a problem where the index was not deleted quickly enough and the index for the next test could not be created due to the previous one still existing. The indices are all kept and then only deleted at the end of the run. Leaving all the indices results in running out of file descriptors which results in tests failing when many tests are run in sequence. Since we now give each test index a unique name, we no longer suffer from the original problem described above. We can safely attempt to delete the index and it doesn't matter if it does not get deleted quickly enough and still exists when the next test is started. The tests for accounts, groups and projects are not currently affected by this problem because they have far fewer test methods than the tests for changes. The same fix is done for those tests anyway to make sure we do not run into the same problem in future. Bug: Issue 8816 Change-Id: I1646f5b802dd2d15b974d4c4d75ab337c4c6f462 --- .../testing/ElasticTestUtils.java | 32 +++++++++++++++++-- .../ElasticQueryAccountsTest.java | 14 +++++++- .../ElasticQueryChangesTest.java | 14 +++++++- .../elasticsearch/ElasticQueryGroupsTest.java | 14 +++++++- 4 files changed, 69 insertions(+), 5 deletions(-) diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/testing/ElasticTestUtils.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/testing/ElasticTestUtils.java index ca9673e20a..9ac582a46a 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/testing/ElasticTestUtils.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/testing/ElasticTestUtils.java @@ -109,8 +109,36 @@ public final class ElasticTestUtils { return new ElasticNodeInfo(node, elasticDir, getHttpPort(node)); } - public static void deleteAllIndexes(ElasticNodeInfo nodeInfo) { - nodeInfo.node.client().admin().indices().prepareDelete("_all").execute(); + public static void deleteAllIndexes(ElasticNodeInfo nodeInfo, String prefix) { + Schema changeSchema = ChangeSchemaDefinitions.INSTANCE.getLatest(); + nodeInfo + .node + .client() + .admin() + .indices() + .prepareDelete(String.format("%s%s_%04d", prefix, CHANGES, changeSchema.getVersion())) + .execute() + .actionGet(); + + Schema accountSchema = AccountSchemaDefinitions.INSTANCE.getLatest(); + nodeInfo + .node + .client() + .admin() + .indices() + .prepareDelete(String.format("%s%s_%04d", prefix, ACCOUNTS, accountSchema.getVersion())) + .execute() + .actionGet(); + + Schema groupSchema = GroupSchemaDefinitions.INSTANCE.getLatest(); + nodeInfo + .node + .client() + .admin() + .indices() + .prepareDelete(String.format("%s%s_%04d", prefix, GROUPS, groupSchema.getVersion())) + .execute() + .actionGet(); } public static class NodeInfo { diff --git a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryAccountsTest.java b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryAccountsTest.java index 6b3ebc5b64..57654360c9 100644 --- a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryAccountsTest.java +++ b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryAccountsTest.java @@ -22,6 +22,7 @@ import com.google.inject.Guice; import com.google.inject.Injector; import java.util.concurrent.ExecutionException; import org.eclipse.jgit.lib.Config; +import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -46,11 +47,22 @@ public class ElasticQueryAccountsTest extends AbstractQueryAccountsTest { } } + private String testName() { + return testName.getMethodName().toLowerCase() + "_"; + } + + @After + public void cleanupIndex() { + if (nodeInfo != null) { + ElasticTestUtils.deleteAllIndexes(nodeInfo, testName()); + } + } + @Override protected Injector createInjector() { Config elasticsearchConfig = new Config(config); InMemoryModule.setDefaults(elasticsearchConfig); - String indicesPrefix = testName.getMethodName().toLowerCase() + "_"; + String indicesPrefix = testName(); ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix); ElasticTestUtils.createAllIndexes(nodeInfo, indicesPrefix); return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration)); diff --git a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java index 9ae667cb79..ee90449f10 100644 --- a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java +++ b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java @@ -24,6 +24,7 @@ import com.google.inject.Injector; import java.util.concurrent.ExecutionException; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.Config; +import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Rule; @@ -53,11 +54,22 @@ public class ElasticQueryChangesTest extends AbstractQueryChangesTest { } } + private String testName() { + return testName.getMethodName().toLowerCase() + "_"; + } + + @After + public void cleanupIndex() { + if (nodeInfo != null) { + ElasticTestUtils.deleteAllIndexes(nodeInfo, testName()); + } + } + @Override protected Injector createInjector() { Config elasticsearchConfig = new Config(config); InMemoryModule.setDefaults(elasticsearchConfig); - String indicesPrefix = testName.getMethodName().toLowerCase() + "_"; + String indicesPrefix = testName(); ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix); ElasticTestUtils.createAllIndexes(nodeInfo, indicesPrefix); return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration)); diff --git a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryGroupsTest.java b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryGroupsTest.java index c7f7475bbf..b9912d3a9a 100644 --- a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryGroupsTest.java +++ b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryGroupsTest.java @@ -22,6 +22,7 @@ import com.google.inject.Guice; import com.google.inject.Injector; import java.util.concurrent.ExecutionException; import org.eclipse.jgit.lib.Config; +import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -46,11 +47,22 @@ public class ElasticQueryGroupsTest extends AbstractQueryGroupsTest { } } + private String testName() { + return testName.getMethodName().toLowerCase() + "_"; + } + + @After + public void cleanupIndex() { + if (nodeInfo != null) { + ElasticTestUtils.deleteAllIndexes(nodeInfo, testName()); + } + } + @Override protected Injector createInjector() { Config elasticsearchConfig = new Config(config); InMemoryModule.setDefaults(elasticsearchConfig); - String indicesPrefix = testName.getMethodName().toLowerCase() + "_"; + String indicesPrefix = testName(); ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix); ElasticTestUtils.createAllIndexes(nodeInfo, indicesPrefix); return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration)); From 679c04efe0acae5026c00dfd259808d104026c73 Mon Sep 17 00:00:00 2001 From: Inderjot Kaur Ratol Date: Mon, 23 Apr 2018 13:54:54 -0400 Subject: [PATCH 6/9] Add more debug logging to account creation As explained in issue 8803 new LDAP users are still facing issues due to missing DB entries. These extra log statements can help to get a more focused idea of the problem in the future. Bug: Issue 8803 Change-Id: I03df282d0611259d196160082cd787ed8232bb79 --- .../google/gerrit/server/account/AccountManager.java | 5 +++++ .../google/gerrit/server/account/ChangeUserName.java | 11 +++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java index 4c62287f60..a4c39e07aa 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java @@ -214,10 +214,12 @@ public class AccountManager { private AuthResult create(ReviewDb db, AuthRequest who) throws OrmException, AccountException, IOException, ConfigInvalidException { Account.Id newId = new Account.Id(db.nextAccountId()); + log.debug("Assigning new Id {} to account", newId); Account account = new Account(newId, TimeUtil.nowTs()); ExternalId extId = ExternalId.createWithEmail(who.getExternalIdKey(), newId, who.getEmailAddress()); + log.debug("Created external Id: {}", extId); account.setFullName(who.getDisplayName()); account.setPreferredEmail(extId.email()); @@ -272,10 +274,13 @@ public class AccountManager { db.accountGroupMembers().insert(Collections.singleton(m)); } + log.debug("Username from AuthRequest: {}", who.getUserName()); if (who.getUserName() != null) { + log.debug("Setting username for: {}", who.getUserName()); // Only set if the name hasn't been used yet, but was given to us. // IdentifiedUser user = userFactory.create(newId); + log.debug("Identified user {} was created from {}", user, who.getUserName()); try { changeUserNameFactory.create(db, user, who.getUserName()).call(); } catch (NameAlreadyUsedException e) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java index 978331eca8..c9a7aabf21 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java @@ -33,13 +33,16 @@ import java.util.Collection; import java.util.concurrent.Callable; import java.util.regex.Pattern; import org.eclipse.jgit.errors.ConfigInvalidException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** Operation to change the username of an account. */ public class ChangeUserName implements Callable { - public static final String USERNAME_CANNOT_BE_CHANGED = "Username cannot be changed."; - + private static final Logger log = LoggerFactory.getLogger(ChangeUserName.class); private static final Pattern USER_NAME_PATTERN = Pattern.compile(Account.USER_NAME_PATTERN); + public static final String USERNAME_CANNOT_BE_CHANGED = "Username cannot be changed."; + /** Generic factory to change any user's username. */ public interface Factory { ChangeUserName create(ReviewDb db, IdentifiedUser user, String newUsername); @@ -79,6 +82,9 @@ public class ChangeUserName implements Callable { .filter(e -> e.isScheme(SCHEME_USERNAME)) .collect(toSet()); if (!old.isEmpty()) { + log.error( + "External id with scheme \"username:\" already exists for the user {}", + user.getAccountId()); throw new IllegalStateException(USERNAME_CANNOT_BE_CHANGED); } @@ -97,6 +103,7 @@ public class ChangeUserName implements Callable { } } externalIdsUpdate.insert(db, ExternalId.create(key, user.getAccountId(), null, password)); + log.info("Created the new external Id with key: {}", key); } catch (OrmDuplicateKeyException dupeErr) { // If we are using this identity, don't report the exception. // From 6ca35faf9d392a994661e06fe65d3c32ec023496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Ar=C3=A8s?= Date: Wed, 11 Apr 2018 09:12:37 -0400 Subject: [PATCH 7/9] Remove duplication between both index modules Change-Id: I16d866bb1b4c5d7887a29827cbd71b7911d141e1 --- .../elasticsearch/ElasticIndexModule.java | 60 ++++--------- .../gerrit/lucene/LuceneIndexModule.java | 67 +++++--------- .../server/index/AbstractIndexModule.java | 88 +++++++++++++++++++ 3 files changed, 127 insertions(+), 88 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/index/AbstractIndexModule.java diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticIndexModule.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticIndexModule.java index 9573fd5689..b98a4145b0 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticIndexModule.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticIndexModule.java @@ -14,24 +14,14 @@ package com.google.gerrit.elasticsearch; -import com.google.gerrit.lifecycle.LifecycleModule; -import com.google.gerrit.server.config.GerritServerConfig; +import com.google.gerrit.server.index.AbstractIndexModule; import com.google.gerrit.server.index.AbstractVersionManager; -import com.google.gerrit.server.index.IndexConfig; -import com.google.gerrit.server.index.IndexModule; -import com.google.gerrit.server.index.SingleVersionModule; import com.google.gerrit.server.index.account.AccountIndex; import com.google.gerrit.server.index.change.ChangeIndex; import com.google.gerrit.server.index.group.GroupIndex; -import com.google.inject.Provides; -import com.google.inject.Singleton; -import com.google.inject.assistedinject.FactoryModuleBuilder; import java.util.Map; -import org.eclipse.jgit.lib.Config; -public class ElasticIndexModule extends LifecycleModule { - private final int threads; - private final Map singleVersions; +public class ElasticIndexModule extends AbstractIndexModule { public static ElasticIndexModule singleVersionWithExplicitVersions( Map versions, int threads) { @@ -43,44 +33,26 @@ public class ElasticIndexModule extends LifecycleModule { } private ElasticIndexModule(Map singleVersions, int threads) { - this.singleVersions = singleVersions; - this.threads = threads; + super(singleVersions, threads); } @Override - protected void configure() { - install( - new FactoryModuleBuilder() - .implement(AccountIndex.class, ElasticAccountIndex.class) - .build(AccountIndex.Factory.class)); - install( - new FactoryModuleBuilder() - .implement(ChangeIndex.class, ElasticChangeIndex.class) - .build(ChangeIndex.Factory.class)); - install( - new FactoryModuleBuilder() - .implement(GroupIndex.class, ElasticGroupIndex.class) - .build(GroupIndex.Factory.class)); - - install(new IndexModule(threads)); - if (singleVersions == null) { - install(new MultiVersionModule()); - } else { - install(new SingleVersionModule(singleVersions)); - } + protected Class getAccountIndex() { + return ElasticAccountIndex.class; } - @Provides - @Singleton - IndexConfig getIndexConfig(@GerritServerConfig Config cfg) { - return IndexConfig.fromConfig(cfg); + @Override + protected Class getChangeIndex() { + return ElasticChangeIndex.class; } - private static class MultiVersionModule extends LifecycleModule { - @Override - public void configure() { - bind(AbstractVersionManager.class).to(ElasticVersionManager.class); - listener().to(ElasticVersionManager.class); - } + @Override + protected Class getGroupIndex() { + return ElasticGroupIndex.class; + } + + @Override + protected Class getVersionManager() { + return ElasticVersionManager.class; } } diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java index 5e857908cd..a78504f59b 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java @@ -15,23 +15,18 @@ package com.google.gerrit.lucene; import com.google.common.collect.ImmutableMap; -import com.google.gerrit.lifecycle.LifecycleModule; import com.google.gerrit.server.config.GerritServerConfig; +import com.google.gerrit.server.index.AbstractIndexModule; import com.google.gerrit.server.index.AbstractVersionManager; import com.google.gerrit.server.index.IndexConfig; -import com.google.gerrit.server.index.IndexModule; -import com.google.gerrit.server.index.SingleVersionModule; import com.google.gerrit.server.index.account.AccountIndex; import com.google.gerrit.server.index.change.ChangeIndex; import com.google.gerrit.server.index.group.GroupIndex; -import com.google.inject.Provides; -import com.google.inject.Singleton; -import com.google.inject.assistedinject.FactoryModuleBuilder; import java.util.Map; import org.apache.lucene.search.BooleanQuery; import org.eclipse.jgit.lib.Config; -public class LuceneIndexModule extends LifecycleModule { +public class LuceneIndexModule extends AbstractIndexModule { public static LuceneIndexModule singleVersionAllLatest(int threads) { return new LuceneIndexModule(ImmutableMap.of(), threads); } @@ -49,50 +44,34 @@ public class LuceneIndexModule extends LifecycleModule { return cfg.getBoolean("index", "lucene", "testInmemory", false); } - private final int threads; - private final Map singleVersions; - private LuceneIndexModule(Map singleVersions, int threads) { - this.singleVersions = singleVersions; - this.threads = threads; + super(singleVersions, threads); } @Override - protected void configure() { - install( - new FactoryModuleBuilder() - .implement(AccountIndex.class, LuceneAccountIndex.class) - .build(AccountIndex.Factory.class)); - install( - new FactoryModuleBuilder() - .implement(ChangeIndex.class, LuceneChangeIndex.class) - .build(ChangeIndex.Factory.class)); - install( - new FactoryModuleBuilder() - .implement(GroupIndex.class, LuceneGroupIndex.class) - .build(GroupIndex.Factory.class)); - - install(new IndexModule(threads)); - if (singleVersions == null) { - install(new MultiVersionModule()); - } else { - install(new SingleVersionModule(singleVersions)); - } + protected Class getAccountIndex() { + return LuceneAccountIndex.class; } - @Provides - @Singleton - IndexConfig getIndexConfig(@GerritServerConfig Config cfg) { + @Override + protected Class getChangeIndex() { + return LuceneChangeIndex.class; + } + + @Override + protected Class getGroupIndex() { + return LuceneGroupIndex.class; + } + + @Override + protected Class getVersionManager() { + return LuceneVersionManager.class; + } + + @Override + protected IndexConfig getIndexConfig(@GerritServerConfig Config cfg) { BooleanQuery.setMaxClauseCount( cfg.getInt("index", "maxTerms", BooleanQuery.getMaxClauseCount())); - return IndexConfig.fromConfig(cfg); - } - - private static class MultiVersionModule extends LifecycleModule { - @Override - public void configure() { - bind(AbstractVersionManager.class).to(LuceneVersionManager.class); - listener().to(LuceneVersionManager.class); - } + return super.getIndexConfig(cfg); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/AbstractIndexModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/AbstractIndexModule.java new file mode 100644 index 0000000000..ec7804369c --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/AbstractIndexModule.java @@ -0,0 +1,88 @@ +// Copyright (C) 2018 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.index; + +import com.google.gerrit.lifecycle.LifecycleModule; +import com.google.gerrit.server.config.GerritServerConfig; +import com.google.gerrit.server.index.account.AccountIndex; +import com.google.gerrit.server.index.change.ChangeIndex; +import com.google.gerrit.server.index.group.GroupIndex; +import com.google.inject.AbstractModule; +import com.google.inject.Provides; +import com.google.inject.Singleton; +import com.google.inject.assistedinject.FactoryModuleBuilder; +import java.util.Map; +import org.eclipse.jgit.lib.Config; + +public abstract class AbstractIndexModule extends AbstractModule { + + private final int threads; + private final Map singleVersions; + + protected AbstractIndexModule(Map singleVersions, int threads) { + this.singleVersions = singleVersions; + this.threads = threads; + } + + @Override + protected void configure() { + install( + new FactoryModuleBuilder() + .implement(AccountIndex.class, getAccountIndex()) + .build(AccountIndex.Factory.class)); + install( + new FactoryModuleBuilder() + .implement(ChangeIndex.class, getChangeIndex()) + .build(ChangeIndex.Factory.class)); + install( + new FactoryModuleBuilder() + .implement(GroupIndex.class, getGroupIndex()) + .build(GroupIndex.Factory.class)); + + install(new IndexModule(threads)); + if (singleVersions == null) { + install(new MultiVersionModule()); + } else { + install(new SingleVersionModule(singleVersions)); + } + } + + protected abstract Class getAccountIndex(); + + protected abstract Class getChangeIndex(); + + protected abstract Class getGroupIndex(); + + protected abstract Class getVersionManager(); + + @Provides + @Singleton + IndexConfig provideIndexConfig(@GerritServerConfig Config cfg) { + return getIndexConfig(cfg); + } + + protected IndexConfig getIndexConfig(@GerritServerConfig Config cfg) { + return IndexConfig.fromConfig(cfg); + } + + private class MultiVersionModule extends LifecycleModule { + @Override + public void configure() { + Class versionManagerClass = getVersionManager(); + bind(AbstractVersionManager.class).to(versionManagerClass); + listener().to(versionManagerClass); + } + } +} From 3aa1562c1ea3c91335bd0d323ded97ef977fa8e7 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Wed, 6 Dec 2017 14:33:02 +0100 Subject: [PATCH 8/9] Make _bower_archive rule deterministic This avoids it being rerun every time bazel starts up. Change-Id: I2679c85cbd9deb2597ffd83f7184d5d9b34d07b6 --- tools/bzl/js.bzl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/bzl/js.bzl b/tools/bzl/js.bzl index 783c79c0c7..fba8c16cad 100644 --- a/tools/bzl/js.bzl +++ b/tools/bzl/js.bzl @@ -91,7 +91,11 @@ def _bower_archive(ctx): "cd bower_components", "unzip %s" % ctx.path(download_name), "cd ..", - "zip -r %s bower_components" % renamed_name,])) + "find . -exec touch -t 198001010000 '{}' ';'", + "zip -r %s bower_components" % renamed_name, + "cd ..", + "rm -rf ${TMP}", + ])) dep_version = ctx.attr.semver if ctx.attr.semver else ctx.attr.version ctx.file(version_name, From ec122943013e194e2510413bf31e25707d85199a Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Tue, 24 Apr 2018 18:26:56 +0200 Subject: [PATCH 9/9] Make zip generation (more) reproducible The 'touch' command uses the system default timezone if none is supplied. This makes zip files dependent on the timezone of the generating machine. Zip by default stores extended attributes (eg. ctime, atime) on Unix systems. These can be further sources of non-determinism, so switch it off with --no-extra Change-Id: I7e102be7368ef339cbaf47524b90688c32a09238 --- polygerrit-ui/app/BUILD | 2 ++ tools/bzl/javadoc.bzl | 4 +++- tools/bzl/js.bzl | 12 +++++++++--- tools/bzl/pkg_war.bzl | 4 +++- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/polygerrit-ui/app/BUILD b/polygerrit-ui/app/BUILD index f422cdbd20..0c4238ad23 100644 --- a/polygerrit-ui/app/BUILD +++ b/polygerrit-ui/app/BUILD @@ -141,6 +141,8 @@ genrule2( ("tar -hcf- $(locations :pg_code) |" + " tar --strip-components=2 -C $$TMP/ -xf-"), "cd $$TMP", + "TZ=UTC", + "export TZ", "find . -exec touch -t 198001010000 '{}' ';'", "zip -rq $$ROOT/$@ *", ]), diff --git a/tools/bzl/javadoc.bzl b/tools/bzl/javadoc.bzl index 18ca129648..f550526b90 100644 --- a/tools/bzl/javadoc.bzl +++ b/tools/bzl/javadoc.bzl @@ -28,6 +28,8 @@ def _impl(ctx): source = ctx.outputs.zip.path + ".source" external_docs = ["http://docs.oracle.com/javase/8/docs/api"] + ctx.attr.external_docs cmd = [ + "TZ=UTC", + "export TZ", "rm -rf %s" % source, "mkdir %s" % source, " && ".join(["unzip -qud %s %s" % (source, j.path) for j in source_jars]), @@ -50,7 +52,7 @@ def _impl(ctx): ":".join(transitive_jar_paths), "-d %s" % dir]), "find %s -exec touch -t 198001010000 '{}' ';'" % dir, - "(cd %s && zip -qr ../%s *)" % (dir, ctx.outputs.zip.basename), + "(cd %s && zip -Xqr ../%s *)" % (dir, ctx.outputs.zip.basename), ] ctx.action( inputs = list(transitive_jar_set) + list(source_jars) + ctx.files._jdk, diff --git a/tools/bzl/js.bzl b/tools/bzl/js.bzl index fba8c16cad..9f2a3bf094 100644 --- a/tools/bzl/js.bzl +++ b/tools/bzl/js.bzl @@ -86,13 +86,15 @@ def _bower_archive(ctx): _bash(ctx, " && " .join([ "TMP=$(mktemp -d || mktemp -d -t bazel-tmp)", + "TZ=UTC", + "export UTC", "cd $TMP", "mkdir bower_components", "cd bower_components", "unzip %s" % ctx.path(download_name), "cd ..", "find . -exec touch -t 198001010000 '{}' ';'", - "zip -r %s bower_components" % renamed_name, + "zip -Xr %s bower_components" % renamed_name, "cd ..", "rm -rf ${TMP}", ])) @@ -164,11 +166,13 @@ def _js_component(ctx): name = name[:-4] dest = "%s/%s" % (dir, name) cmd = " && ".join([ + "TZ=UTC", + "export TZ", "mkdir -p %s" % dest, "cp %s %s/" % (' '.join([s.path for s in ctx.files.srcs]), dest), "cd %s" % dir, "find . -exec touch -t 198001010000 '{}' ';'", - "zip -qr ../%s *" % ctx.outputs.zip.basename + "zip -Xqr ../%s *" % ctx.outputs.zip.basename ]) ctx.action( @@ -243,13 +247,15 @@ def _bower_component_bundle_impl(ctx): outputs=[out_zip], command=" && ".join([ "p=$PWD", + "TZ=UTC", + "export TZ", "rm -rf %s.dir" % out_zip.path, "mkdir -p %s.dir/bower_components" % out_zip.path, "cd %s.dir/bower_components" % out_zip.path, "for z in %s; do unzip -q $p/$z ; done" % " ".join(sorted([z.path for z in zips])), "cd ..", "find . -exec touch -t 198001010000 '{}' ';'", - "zip -qr $p/%s bower_components/*" % out_zip.path, + "zip -Xqr $p/%s bower_components/*" % out_zip.path, ]), mnemonic="BowerCombine") diff --git a/tools/bzl/pkg_war.bzl b/tools/bzl/pkg_war.bzl index ebb632fd06..59ede4cb40 100644 --- a/tools/bzl/pkg_war.bzl +++ b/tools/bzl/pkg_war.bzl @@ -54,9 +54,11 @@ def _add_file(in_file, output): def _make_war(input_dir, output): return '(%s)' % ' && '.join([ 'root=$(pwd)', + 'TZ=UTC', + 'export TZ', 'cd %s' % input_dir, "find . -exec touch -t 198001010000 '{}' ';' 2> /dev/null", - 'zip -9qr ${root}/%s .' % (output.path), + 'zip -X -9qr ${root}/%s .' % (output.path), ]) def _war_impl(ctx):