From d50c94e759a7f7b26aa7dfa58b28d78e9b9caadf Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Thu, 15 Jul 2010 12:24:11 -0700 Subject: [PATCH] Support topic branch tags on changes The topic tag can be attached to a change during upload by suffixing the 'refs/for/branch_name' with the topic label, treating the branch as though it were a directory. For example, to tag 'exp/nosql' onto a change headed for the 'sandbox/future-stuff' branch, git push can be used as: git push URL HEAD:refs/for/sandbox/future-stuff/exp-nosql If the topic is supplied, it is displayed in the branch column of a change, in parens. If no topic was set, only the branch is shown. Bug: issue 51 Change-Id: I07d6c137fc9aefa8c1ee1652bf1e7bcde9d33674 Signed-off-by: Shawn O. Pearce --- Documentation/cmd-receive-pack.txt | 5 ++ Documentation/user-upload.txt | 10 +++ .../google/gerrit/common/data/ChangeInfo.java | 6 ++ .../client/changes/ChangeConstants.java | 1 + .../client/changes/ChangeConstants.properties | 1 + .../client/changes/ChangeInfoBlock.java | 16 ++-- .../gerrit/client/changes/ChangeTable.java | 7 +- .../com/google/gerrit/reviewdb/Change.java | 12 +++ .../gerrit/server/git/ReceiveCommits.java | 84 ++++++++++++------- .../gerrit/server/schema/SchemaVersion.java | 2 +- .../gerrit/server/schema/Schema_39.java | 25 ++++++ 11 files changed, 129 insertions(+), 40 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_39.java diff --git a/Documentation/cmd-receive-pack.txt b/Documentation/cmd-receive-pack.txt index 5a5ed6da06..ca965508dd 100644 --- a/Documentation/cmd-receive-pack.txt +++ b/Documentation/cmd-receive-pack.txt @@ -64,6 +64,11 @@ Send a review for a change on the master branch to charlie@example.com: git push --receive-pack='git receive-pack --reviewer charlie@example.com' ssh://review.example.com:29418/project HEAD:refs/for/master ===== +Send reviews, but tagging them with the topic name 'bug42': +===== + git push --receive-pack='git receive-pack --reviewer charlie@example.com' ssh://review.example.com:29418/project HEAD:refs/for/master/bug42 +===== + Also CC two other parties: ===== git push --receive-pack='git receive-pack --reviewer charlie@example.com --cc alice@example.com --cc bob@example.com' ssh://review.example.com:29418/project HEAD:refs/for/master diff --git a/Documentation/user-upload.txt b/Documentation/user-upload.txt index 8034637159..10fddd3c1f 100644 --- a/Documentation/user-upload.txt +++ b/Documentation/user-upload.txt @@ -116,6 +116,16 @@ Other users (e.g. project owners) who have configured Gerrit to notify them of new changes will be automatically sent an email message when the push is completed. +To include a short tag associated with all of the changes in the +same group, such as the local topic branch name, append it after +the destination branch name. In this example the short topic tag +'driver/i42' will be saved on each change this push creates or +updates: + +==== + git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/experimental/driver/i42 +==== + If you are frequently uploading changes to the same Gerrit server, consider adding an SSH host block in `~/.ssh/config` to remember your username, hostname and port number. This permits the use of diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeInfo.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeInfo.java index 3de4ee090f..b5271ec539 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeInfo.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeInfo.java @@ -27,6 +27,7 @@ public class ChangeInfo { protected Change.Status status; protected ProjectInfo project; protected String branch; + protected String topic; protected boolean starred; protected Timestamp lastUpdatedOn; protected String sortKey; @@ -42,6 +43,7 @@ public class ChangeInfo { status = c.getStatus(); project = new ProjectInfo(c.getProject()); branch = c.getDest().getShortName(); + topic = c.getTopic(); lastUpdatedOn = c.getLastUpdatedOn(); sortKey = c.getSortKey(); } @@ -74,6 +76,10 @@ public class ChangeInfo { return branch; } + public String getTopic() { + return topic; + } + public boolean isStarred() { return starred; } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java index 7862bc8e86..9a08932687 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java @@ -80,6 +80,7 @@ public interface ChangeConstants extends Constants { String changeInfoBlockOwner(); String changeInfoBlockProject(); String changeInfoBlockBranch(); + String changeInfoBlockTopic(); String changeInfoBlockUploaded(); String changeInfoBlockUpdated(); String changeInfoBlockStatus(); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties index b1f78b4d79..59d0bd52c1 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties @@ -57,6 +57,7 @@ approvalTableAddReviewer = Add Reviewer changeInfoBlockOwner = Owner changeInfoBlockProject = Project changeInfoBlockBranch = Branch +changeInfoBlockTopic = Topic changeInfoBlockUploaded = Uploaded changeInfoBlockUpdated = Updated changeInfoBlockStatus = Status diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java index c812f3b9b0..a1c1b6bdbf 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java @@ -23,12 +23,9 @@ import com.google.gerrit.client.ui.ProjectLink; import com.google.gerrit.common.data.AccountInfoCache; import com.google.gerrit.reviewdb.Branch; import com.google.gerrit.reviewdb.Change; -import com.google.gwt.user.client.DOM; -import com.google.gwt.user.client.ui.ComplexPanel; import com.google.gwt.user.client.ui.Composite; import com.google.gwt.user.client.ui.FlowPanel; import com.google.gwt.user.client.ui.Grid; -import com.google.gwt.user.client.ui.InlineLabel; import com.google.gwt.user.client.ui.HTMLTable.CellFormatter; import com.google.gwtexpui.clippy.client.CopyableLabel; @@ -37,11 +34,12 @@ public class ChangeInfoBlock extends Composite { private static final int R_OWNER = 1; private static final int R_PROJECT = 2; private static final int R_BRANCH = 3; - private static final int R_UPLOADED = 4; - private static final int R_UPDATED = 5; - private static final int R_STATUS = 6; - private static final int R_PERMALINK = 7; - private static final int R_CNT = 8; + private static final int R_TOPIC = 4; + private static final int R_UPLOADED = 5; + private static final int R_UPDATED = 6; + private static final int R_STATUS = 7; + private static final int R_PERMALINK = 8; + private static final int R_CNT = 9; private final Grid table; @@ -54,6 +52,7 @@ public class ChangeInfoBlock extends Composite { initRow(R_OWNER, Util.C.changeInfoBlockOwner()); initRow(R_PROJECT, Util.C.changeInfoBlockProject()); initRow(R_BRANCH, Util.C.changeInfoBlockBranch()); + initRow(R_TOPIC, Util.C.changeInfoBlockTopic()); initRow(R_UPLOADED, Util.C.changeInfoBlockUploaded()); initRow(R_UPDATED, Util.C.changeInfoBlockUpdated()); initRow(R_STATUS, Util.C.changeInfoBlockStatus()); @@ -85,6 +84,7 @@ public class ChangeInfoBlock extends Composite { table.setWidget(R_OWNER, 1, AccountDashboardLink.link(acc, chg.getOwner())); table.setWidget(R_PROJECT, 1, new ProjectLink(chg.getProject(), chg.getStatus())); table.setText(R_BRANCH, 1, dst.getShortName()); + table.setText(R_TOPIC, 1, chg.getTopic()); table.setText(R_UPLOADED, 1, mediumFormat(chg.getCreatedOn())); table.setText(R_UPDATED, 1, mediumFormat(chg.getLastUpdatedOn())); table.setText(R_STATUS, 1, Util.toLongString(chg.getStatus())); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java index ed45a64e2d..5ede95f9f5 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java @@ -226,7 +226,12 @@ public class ChangeTable extends NavigationTable { table.setWidget(row, C_OWNER, link(c.getOwner())); table.setWidget(row, C_PROJECT, new ProjectLink(c.getProject().getKey(), c .getStatus())); - table.setText(row, C_BRANCH, c.getBranch()); + + String branchText = c.getBranch(); + if (c.getTopic() != null) { + branchText += " (" + c.getTopic() + ")"; + } + table.setText(row, C_BRANCH, branchText); table.setText(row, C_LAST_UPDATE, shortFormat(c.getLastUpdatedOn())); setRowItem(row, c); } diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/Change.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/Change.java index 7fafb13517..e4ae63dc71 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/Change.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/Change.java @@ -351,6 +351,10 @@ public final class Change { @Column(id = 13) protected String subject; + /** Topic name assigned by the user, if any. */ + @Column(id = 14, notNull = false) + protected String topic; + protected Change() { } @@ -456,4 +460,12 @@ public final class Change { open = newStatus.isOpen(); status = newStatus.getCode(); } + + public String getTopic() { + return topic; + } + + public void setTopic(String topic) { + this.topic = topic; + } } 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 63ff58adab..50427666e0 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 @@ -55,7 +55,6 @@ import com.google.gwtorm.client.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; -import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; @@ -159,6 +158,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { private Map refsById; + private String destTopicName; + @Inject ReceiveCommits(final ReviewDb db, final ApprovalTypes approvalTypes, final AccountResolver accountResolver, @@ -579,38 +580,55 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { destBranchName = Constants.R_HEADS + destBranchName; } - if (rp.getAdvertisedRefs().containsKey(destBranchName)) { - // We advertised the branch to the client so we know - // the branch exists. Target this branch for the upload. - // - destBranch = new Branch.NameKey(project.getNameKey(), destBranchName); - - } else { - // We didn't advertise the branch, because it doesn't exist yet. - // Allow it anyway if HEAD is a symbolic reference to the name. - // - final String head; - try { - head = repo.getFullBranch(); - } catch (IOException e) { - log.error("Cannot read HEAD symref", e); - reject(cmd, "internal error"); - return; - } - - if (head.equals(destBranchName)) { - destBranch = new Branch.NameKey(project.getNameKey(), destBranchName); - } - } - - if (destBranch == null) { - String n = destBranchName; - if (n.startsWith(Constants.R_HEADS)) - n = n.substring(Constants.R_HEADS.length()); - reject(cmd, "branch " + n + " not found"); + final String head; + try { + head = repo.getFullBranch(); + } catch (IOException e) { + log.error("Cannot read HEAD symref", e); + reject(cmd, "internal error"); return; } + // Split the destination branch by branch and topic. The topic + // suffix is entirely optional, so it might not even exist. + // + int split = destBranchName.length(); + for (;;) { + String name = destBranchName.substring(0, split); + + if (rp.getAdvertisedRefs().containsKey(name)) { + // We advertised the branch to the client so we know + // the branch exists. Target this branch for the upload. + // + break; + } else if (head.equals(name)) { + // We didn't advertise the branch, because it doesn't exist yet. + // Allow it anyway as HEAD is a symbolic reference to the name. + // + break; + } + + split = name.lastIndexOf('/', split - 1); + if (split <= Constants.R_HEADS.length()) { + String n = destBranchName; + if (n.startsWith(Constants.R_HEADS)) + n = n.substring(Constants.R_HEADS.length()); + reject(cmd, "branch " + n + " not found"); + return; + } + } + + if (split < destBranchName.length()) { + destTopicName = destBranchName.substring(split + 1); + } else { + // We use empty string here to denote the topic wasn't + // supplied, but the caller used the syntax that allows + // for a topic to be given. + // + destTopicName = ""; + } + destBranch = new Branch.NameKey(project.getNameKey(), // + destBranchName.substring(0, split)); destBranchCtl = projectControl.controlForRef(destBranch); if (!destBranchCtl.canUpload()) { reject(cmd); @@ -858,6 +876,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { final Change change = new Change(changeKey, new Change.Id(db.nextChangeId()), me, destBranch); + change.setTopic(destTopicName.isEmpty() ? null : destTopicName); change.nextPatchSetId(); final PatchSet ps = new PatchSet(change.currPatchSetId()); @@ -1158,6 +1177,11 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { @Override public Change update(Change change) { if (change.getStatus().isOpen()) { + if (destTopicName != null) { + change.setTopic(destTopicName.isEmpty() // + ? null // + : destTopicName); + } change.setStatus(Change.Status.NEW); change.setCurrentPatchSet(result.info); ChangeUtil.updated(change); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java index 96c8df0a74..d2638b3ff3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java @@ -32,7 +32,7 @@ import java.util.List; /** A version of the database schema. */ public abstract class SchemaVersion { /** The current schema version. */ - private static final Class C = Schema_38.class; + private static final Class C = Schema_39.class; public static class Module extends AbstractModule { @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_39.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_39.java new file mode 100644 index 0000000000..39ae226c38 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_39.java @@ -0,0 +1,25 @@ +// Copyright (C) 2010 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.schema; + +import com.google.inject.Inject; +import com.google.inject.Provider; + +public class Schema_39 extends SchemaVersion { + @Inject + Schema_39(Provider prior) { + super(prior); + } +}