opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.10'

Browse Source 
Conflicts:
	gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java

Change-Id: I64ca4005da7d3c676954b4da397e5c9566c934a1
This commit is contained in:
Shawn Pearce
2014-08-27 17:18:48 -07:00
parent bce80ffca4 9954c4e249
commit 137c303fdd
13 changed files with 55 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
Documentation/cmd-review.txt
View File

@@ -82,7 +82,8 @@ branch.
--abandon::
Abandon the specified change(s).
(option is mutually exclusive with --submit and --restore)
(option is mutually exclusive with --submit, --restore, --publish and
--delete)
--restore::
Restore the specified abandoned change(s).
@@ -91,7 +92,7 @@ branch.
--submit::
-s::
Submit the specified patch set(s) for merging.
(option is mutually exclusive with --abandon)
(option is mutually exclusive with --abandon, --publish and --delete)
--publish::
Publish the specified draft patch set(s).

2
Documentation/cmd-set-account.txt
View File

@@ -41,7 +41,7 @@ This command is intended to be used in scripts.
Required; Full name, email-address, SSH username or account id.
--full-name::
Display name of the user account.
Set the display name for the user account.
+
Names containing spaces should be quoted in single quotes (').
This most likely requires double quoting the value, for example

4
Documentation/rest-api-changes.txt
View File

@@ -3236,7 +3236,7 @@ The number of the line for which the comment was done. +
If range is set, this equals the end line of the range. +
If neither line nor range is set, it's a file comment.
|`range` |optional|
The range of the comment as a link:rest-api.html#comment-range[CommentRange]
The range of the comment as a link:#comment-range[CommentRange]
entity.
|`in_reply_to` |optional|
The URL encoded UUID of the comment to which this comment is a reply.
@@ -3275,7 +3275,7 @@ The number of the line for which the comment should be added. +
If neither line nor range is set, a file comment is added. +
If range is set, this should equal the end line of the range.
|`range` |optional|
The range of the comment as a link:rest-api.html#comment-range[CommentRange]
The range of the comment as a link:#comment-range[CommentRange]
entity.
|`in_reply_to` |optional|
The URL encoded UUID of the comment to which this comment is a reply.

13
gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecker.java
View File

@@ -333,6 +333,11 @@ public class MergeabilityChecker implements GitReferenceUpdatedListener {
ReviewDb db = context.getReviewDbProvider().get();
try {
PatchSet ps = db.patchSets().get(change.currentPatchSetId());
if (ps == null) {
// Cannot compute mergeability if current patch set is missing.
return false;
}
Mergeable m = mergeable.get();
m.setForce(force);
@@ -345,10 +350,10 @@ public class MergeabilityChecker implements GitReferenceUpdatedListener {
// change is closed
return false;
} catch (Exception e) {
String msg = "Failed to update mergeability flags for project "
+ change.getDest().getParentKey() + " on update of "
+ change.getDest().get();
log.error(msg, e);
log.error(String.format(
"cannot update mergeability flag of change %d in project %s after update of %s",
change.getId().get(),
change.getDest().getParentKey(), change.getDest().get()), e);
throw e;
} finally {
tl.setContext(old);

10
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
View File

@@ -655,14 +655,17 @@ public class MergeOp {
private void fireRefUpdated(RefUpdate branchUpdate) {
gitRefUpdated.fire(destBranch.getParentKey(), branchUpdate);
hooks.doRefUpdatedHook(destBranch, branchUpdate, getAccount(mergeTip));
}
private Account getAccount(CodeReviewCommit codeReviewCommit) {
Account account = null;
PatchSetApproval submitter = approvalsUtil.getSubmitter(
db, mergeTip.notes(), mergeTip.getPatchsetId());
db, codeReviewCommit.notes(), codeReviewCommit.getPatchsetId());
if (submitter != null) {
account = accountCache.get(submitter.getAccountId()).getAccount();
}
hooks.doRefUpdatedHook(destBranch, branchUpdate, account);
return account;
}
private void updateChangeStatus(final List<Change> submitted) throws NoSuchChangeException {
@@ -726,7 +729,8 @@ public class MergeOp {
if (mergeTip != null && (branchTip == null || branchTip != mergeTip)) {
SubmoduleOp subOp =
subOpFactory.create(destBranch, mergeTip, rw, repo,
destProject.getProject(), submitted, commits);
destProject.getProject(), submitted, commits,
getAccount(mergeTip));
try {
subOp.update();
} catch (SubmoduleException e) {

3
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@@ -2353,7 +2353,8 @@ public class ReceiveCommits {
subOpFactory.create(
new Branch.NameKey(project.getNameKey(), cmd.getRefName()),
codeReviewCommit, rw, repo, project, new ArrayList<Change>(),
new HashMap<Change.Id, CodeReviewCommit>());
new HashMap<Change.Id, CodeReviewCommit>(),
currentUser.getAccount());
subOp.update();
} catch (InsertException e) {
log.error("Can't insert patchset", e);

12
gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
View File

@@ -15,7 +15,9 @@
package com.google.gerrit.server.git;
import com.google.common.base.Strings;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.Nullable;
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.Project;
@@ -68,7 +70,7 @@ public class SubmoduleOp {
public interface Factory {
SubmoduleOp create(Branch.NameKey destBranch, RevCommit mergeTip,
RevWalk rw, Repository db, Project destProject, List<Change> submitted,
Map<Change.Id, CodeReviewCommit> commits);
Map<Change.Id, CodeReviewCommit> commits, Account account);
}
private static final Logger log = LoggerFactory.getLogger(SubmoduleOp.class);
@@ -88,6 +90,8 @@ public class SubmoduleOp {
private final GitReferenceUpdated gitRefUpdated;
private final SchemaFactory<ReviewDb> schemaFactory;
private final Set<Branch.NameKey> updatedSubscribers;
private final Account account;
private final ChangeHooks changeHooks;
@Inject
public SubmoduleOp(@Assisted final Branch.NameKey destBranch,
@@ -97,7 +101,8 @@ public class SubmoduleOp {
@Assisted Project destProject, @Assisted List<Change> submitted,
@Assisted final Map<Change.Id, CodeReviewCommit> commits,
@GerritPersonIdent final PersonIdent myIdent,
GitRepositoryManager repoManager, GitReferenceUpdated gitRefUpdated) {
GitRepositoryManager repoManager, GitReferenceUpdated gitRefUpdated,
@Assisted Account account, ChangeHooks changeHooks) {
this.destBranch = destBranch;
this.mergeTip = mergeTip;
this.rw = rw;
@@ -110,6 +115,8 @@ public class SubmoduleOp {
this.myIdent = myIdent;
this.repoManager = repoManager;
this.gitRefUpdated = gitRefUpdated;
this.account = account;
this.changeHooks = changeHooks;
updatedSubscribers = new HashSet<>();
}
@@ -344,6 +351,7 @@ public class SubmoduleOp {
case NEW:
case FAST_FORWARD:
gitRefUpdated.fire(subscriber.getParentKey(), rfu);
changeHooks.doRefUpdatedHook(subscriber, rfu, account);
// TODO since this is performed "in the background" no mail will be
// sent to inform users about the updated branch
break;

2
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
View File

@@ -164,7 +164,7 @@ public class CherryPick extends SubmitStrategy {
ChangeUtil.nextPatchSetId(args.repo, n.change().currentPatchSetId());
final PatchSet ps = new PatchSet(id);
ps.setCreatedOn(TimeUtil.nowTs());
ps.setUploader(submitAudit.getAccountId());
ps.setUploader(cherryPickUser.getAccountId());
ps.setRevision(new RevId(newCommit.getId().getName()));
final RefUpdate ru;

5
gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java
View File

@@ -164,7 +164,10 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
case LDAP:
if (accountResolver.find(nameOrEmail) == null) {
// account does not exist, try to create it
return createAccountByLdap(nameOrEmail);
Account a = createAccountByLdap(nameOrEmail);
if (a != null) {
return a;
}
}
break;
default:

2
gerrit-server/src/main/java/com/google/gerrit/server/util/SystemLog.java
View File

@@ -25,7 +25,6 @@ import org.apache.log4j.Appender;
import org.apache.log4j.AsyncAppender;
import org.apache.log4j.DailyRollingFileAppender;
import org.apache.log4j.Layout;
import org.apache.log4j.Level;
import org.apache.log4j.LogManager;
import org.apache.log4j.Logger;
import org.apache.log4j.helpers.OnlyOnceErrorHandler;
@@ -64,7 +63,6 @@ public class SystemLog {
dst.setFile(new File(resolve(logdir), name).getPath());
dst.setImmediateFlush(true);
dst.setAppend(true);
dst.setThreshold(Level.INFO);
dst.setErrorHandler(new DieErrorHandler());
dst.activateOptions();
dst.setErrorHandler(new OnlyOnceErrorHandler());

13
gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.git;
import static org.easymock.EasyMock.capture;
import static org.easymock.EasyMock.createNiceMock;
import static org.easymock.EasyMock.createStrictMock;
import static org.easymock.EasyMock.eq;
import static org.easymock.EasyMock.expect;
@@ -22,6 +23,7 @@ import static org.easymock.EasyMock.replay;
import static org.easymock.EasyMock.verify;
import static org.junit.Assert.assertEquals;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
@@ -79,6 +81,7 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
private Provider<String> urlProvider;
private GitRepositoryManager repoManager;
private GitReferenceUpdated gitRefUpdated;
private ChangeHooks changeHooks;
@SuppressWarnings("unchecked")
@Override
@@ -92,6 +95,7 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
urlProvider = createStrictMock(Provider.class);
repoManager = createStrictMock(GitRepositoryManager.class);
gitRefUpdated = createStrictMock(GitReferenceUpdated.class);
changeHooks = createNiceMock(ChangeHooks.class);
}
private void doReplay() {
@@ -138,7 +142,7 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
final SubmoduleOp submoduleOp =
new SubmoduleOp(branchNameKey, mergeTip, new RevWalk(realDb), urlProvider,
schemaFactory, realDb, null, new ArrayList<Change>(), null, null,
null, null);
null, null, null, null);
submoduleOp.update();
@@ -654,7 +658,8 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
new SubmoduleOp(sourceBranchNameKey, sourceMergeTip, new RevWalk(
sourceRepository), urlProvider, schemaFactory, sourceRepository,
new Project(sourceBranchNameKey.getParentKey()), submitted,
mergedCommits, myIdent, repoManager, gitRefUpdated);
mergedCommits, myIdent, repoManager, gitRefUpdated, null,
changeHooks);
submoduleOp.update();
@@ -758,7 +763,7 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
new SubmoduleOp(sourceBranchNameKey, sourceMergeTip, new RevWalk(
sourceRepository), urlProvider, schemaFactory, sourceRepository,
new Project(sourceBranchNameKey.getParentKey()), submitted,
mergedCommits, myIdent, repoManager, gitRefUpdated);
mergedCommits, myIdent, repoManager, gitRefUpdated, null, changeHooks);
submoduleOp.update();
@@ -912,7 +917,7 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
new SubmoduleOp(mergedBranch, mergeTip, new RevWalk(realDb),
urlProvider, schemaFactory, realDb, new Project(mergedBranch
.getParentKey()), new ArrayList<Change>(), null, null,
repoManager, null);
repoManager, null, null, null);
submoduleOp.update();
}

6
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
View File

@@ -186,6 +186,9 @@ public class ReviewCommand extends SshCommand {
throw error("json and message are mutually exclusive");
}
}
if (deleteDraftPatchSet && submitChange) {
throw error("delete and submit actions are mutually exclusive");
}
boolean ok = true;
ReviewInput input = null;
@@ -242,6 +245,9 @@ public class ReviewCommand extends SshCommand {
if (changeComment == null) {
changeComment = "";
}
if (notify == null) {
notify = NotifyHandling.ALL;
}
ReviewInput review = new ReviewInput();
review.message = Strings.emptyToNull(changeComment);

8
gerrit-war/src/main/resources/log4j.properties
View File

@@ -12,14 +12,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
log4j.rootCategory=DEBUG, stderr
log4j.rootCategory=INFO, stderr
log4j.appender.stderr=org.apache.log4j.ConsoleAppender
log4j.appender.stderr.target=System.err
log4j.appender.stderr.layout=org.apache.log4j.PatternLayout
log4j.appender.stderr.layout.ConversionPattern=[%d] %-5p %c %x: %m%n
log4j.logger.com.google.gerrit=INFO
# Silence non-critical messages from MINA SSHD.
#
log4j.logger.org.apache.mina=WARN
@@ -28,10 +26,6 @@ log4j.logger.org.apache.sshd.server=WARN
log4j.logger.org.apache.sshd.common.keyprovider.FileKeyPairProvider=INFO
log4j.logger.com.google.gerrit.sshd.GerritServerSession=WARN
# Silence non-critical messages from Jetty.
#
log4j.logger.org.eclipse.jetty=INFO
# Silence non-critical messages from mime-util.
#
log4j.logger.eu.medsea.mimeutil=WARN