Updated cq_client

R=akuegel@chromium.org
BUG=562427

Review URL: https://codereview.chromium.org/1475033006

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@297726 0039d316-1c4b-4281-b951-d872f2087c98

third_party/cq_client/README.md

@@ -7,7 +7,18 @@ which should only be updated as a whole using Glyco (when available, see
 The canonical version is located at `https://chrome-internal.googlesource.com/
 infra/infra_internal/+/master/commit_queue/cq_client`.
 
-To generate `cq_pb2.py` and `cq.pb.go`, please use protoc version 2.6.1:
+You'll need to use protoc version 2.6.1 and
+recent golang/protobuf package. Sadly, the latter has no tags no versions.
+
+You can get protobuf by downloading archive from https://github.com/google/protobuf/tree/v2.6.1,
+and manually building it. As for golang compiler, if you have go configured,
+just
+
+    go get -u github.com/golang/protobuf/{proto,protoc-gen-go}
+
+TODO(tandrii,sergiyb): decide how to pin the go protobuf generator.
+
+To generate `cq_pb2.py` and `cq.pb.go`:
 
     cd commit_queue/cq_client
     protoc cq.proto --python_out $(pwd) --go_out $(pwd)
@@ -15,3 +26,4 @@ To generate `cq_pb2.py` and `cq.pb.go`, please use protoc version 2.6.1:
 Additionally, please make sure to use proto3-compatible syntax, e.g. no default
 values, no required fields. Ideally, we should use proto3 generator already,
 however alpha version thereof is still unstable.
+

third_party/cq_client/cq.pb.go

@@ -11,6 +11,7 @@ It is generated from these files:
 It has these top-level messages:
 	Config
 	Rietveld
+	Gerrit
 	Verifiers
 */
 package cq
@@ -54,6 +55,9 @@ type Config struct {
 	InProduction *bool `protobuf:"varint,8,opt,name=in_production" json:"in_production,omitempty"`
 	// Configuration options for Rietveld code review.
 	Rietveld *Rietveld `protobuf:"bytes,9,opt,name=rietveld" json:"rietveld,omitempty"`
+	// EXPERIMENTAL! Configuration options for Gerrit code review.
+	// TODO(tandrii): update this doc (GERRIT).
+	Gerrit *Gerrit `protobuf:"bytes,15,opt,name=gerrit" json:"gerrit,omitempty"`
 	// This can be used to override the Git repository URL used to checkout and
 	// commit changes on CQ host. This should only be used in case, when the
 	// source repository is not supported by luci-config (e.g. GitHub).
@@ -63,15 +67,8 @@ type Config struct {
 	// that use gnumbd where CQ should commit into a pending ref.
 	TargetRef *string `protobuf:"bytes,11,opt,name=target_ref" json:"target_ref,omitempty"`
 	// Deprecated. URL of the SVN repository. We are deprecating SVN support.
-	SvnRepoUrl *string `protobuf:"bytes,12,opt,name=svn_repo_url" json:"svn_repo_url,omitempty"`
-	// Deprecated. Should be set to true, when the project's SVN repository does
-	// not have server-side hooks configured.
-	ServerHooksMissing *bool `protobuf:"varint,13,opt,name=server_hooks_missing" json:"server_hooks_missing,omitempty"`
-	// Deprecated. Specifies a list of verifiers that are run on a local checkout
-	// with patch applied. The only remaining use case for this is PRESUBMIT_CHECK
-	// verifier, which we are deprecating as well.
-	VerifiersWithPatch *Verifiers `protobuf:"bytes,14,opt,name=verifiers_with_patch" json:"verifiers_with_patch,omitempty"`
-	XXX_unrecognized   []byte     `json:"-"`
+	SvnRepoUrl       *string `protobuf:"bytes,12,opt,name=svn_repo_url" json:"svn_repo_url,omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
 }
 
 func (m *Config) Reset()         { *m = Config{} }
@@ -141,6 +138,13 @@ func (m *Config) GetRietveld() *Rietveld {
 	return nil
 }
 
+func (m *Config) GetGerrit() *Gerrit {
+	if m != nil {
+		return m.Gerrit
+	}
+	return nil
+}
+
 func (m *Config) GetGitRepoUrl() string {
 	if m != nil && m.GitRepoUrl != nil {
 		return *m.GitRepoUrl
@@ -162,20 +166,6 @@ func (m *Config) GetSvnRepoUrl() string {
 	return ""
 }
 
-func (m *Config) GetServerHooksMissing() bool {
-	if m != nil && m.ServerHooksMissing != nil {
-		return *m.ServerHooksMissing
-	}
-	return false
-}
-
-func (m *Config) GetVerifiersWithPatch() *Verifiers {
-	if m != nil {
-		return m.VerifiersWithPatch
-	}
-	return nil
-}
-
 type Rietveld struct {
 	// Required. URL of the codereview site.
 	Url *string `protobuf:"bytes,1,opt,name=url" json:"url,omitempty"`
@@ -206,6 +196,44 @@ func (m *Rietveld) GetProjectBases() []string {
 	return nil
 }
 
+// Gerrit CQ is EXPERIMENTAL! See http://crbug.com/493899 for more info.
+//
+// Unlike Rietveld, Gerrit doesn't need a separate url.
+// Instead, the git_repo_url must be specified on the Gerrit instance,
+// and CQ will deduce Gerrit url from it.
+//
+// TODO(tandrii): support Rietveld and Gerrit at the same time.
+// This basically requires to start two CQ instances, instead of one.
+//
+// For example, if https://chromium.googlesource.com/infra/infra.git is your
+// repo url provided in `git_repo_url` above, then
+// https://chromium-review.googlesource.com/#/admin/projects/infra/infra should
+// show general properties of your project.
+//
+// Also,
+// https://chromium-review.googlesource.com/#/admin/projects/infra/infra,access
+// should show ACLs for refs in your project, but you may need to be admin to
+// see it. This will come handy to enable and customize the CQ-related workflows
+// for your project.
+type Gerrit struct {
+	// If set, tells CQ to set score on a given label to mark result of CQ run.
+	// Typically, this is Commit-Queue-Verified label.
+	// If not set, CQ will just try to hit submit button.
+	CqVerifiedLabel  *string `protobuf:"bytes,1,opt,name=cq_verified_label" json:"cq_verified_label,omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
+}
+
+func (m *Gerrit) Reset()         { *m = Gerrit{} }
+func (m *Gerrit) String() string { return proto.CompactTextString(m) }
+func (*Gerrit) ProtoMessage()    {}
+
+func (m *Gerrit) GetCqVerifiedLabel() string {
+	if m != nil && m.CqVerifiedLabel != nil {
+		return *m.CqVerifiedLabel
+	}
+	return ""
+}
+
 // Verifiers are various types of checks that a Commit Queue performs on a CL.
 // All verifiers must pass in order for a CL to be landed. Configuration file
 // describes types of verifiers that should be applied to each CL and their
@@ -213,6 +241,7 @@ func (m *Rietveld) GetProjectBases() []string {
 type Verifiers struct {
 	// This verifier is used to ensure that an LGTM was posted to the code review
 	// site from a valid project committer.
+	// This verifier is not supported with Gerrit.
 	ReviewerLgtm *Verifiers_ReviewerLgtmVerifier `protobuf:"bytes,1,opt,name=reviewer_lgtm" json:"reviewer_lgtm,omitempty"`
 	// This verifier is used to check tree status before committing a CL. If the
 	// tree is closed, then the verifier will wait until it is reopened.
@@ -354,7 +383,7 @@ type Verifiers_TryJobVerifier_Builder struct {
 	Name *string `protobuf:"bytes,1,opt,name=name" json:"name,omitempty"`
 	// When true, the builder is triggered by CQ. Otherwise, it is expected to
 	// be triggered from another tryjob. Default value is true.
-	Triggered *bool `protobuf:"varint,2,opt,name=triggered" json:"triggered,omitempty"`
+	TriggeredByCq *bool `protobuf:"varint,2,opt,name=triggered_by_cq" json:"triggered_by_cq,omitempty"`
 	// When this field is present, it marks given builder as experimental. It
 	// is only executed on a given percentage of the CLs and the outcome does
 	// not affect the decicion whether a CL can land or not. This is typically
@@ -374,9 +403,9 @@ func (m *Verifiers_TryJobVerifier_Builder) GetName() string {
 	return ""
 }
 
-func (m *Verifiers_TryJobVerifier_Builder) GetTriggered() bool {
-	if m != nil && m.Triggered != nil {
-		return *m.Triggered
+func (m *Verifiers_TryJobVerifier_Builder) GetTriggeredByCq() bool {
+	if m != nil && m.TriggeredByCq != nil {
+		return *m.TriggeredByCq
 	}
 	return false
 }
@@ -480,3 +509,17 @@ type Verifiers_SignCLAVerifier struct {
 func (m *Verifiers_SignCLAVerifier) Reset()         { *m = Verifiers_SignCLAVerifier{} }
 func (m *Verifiers_SignCLAVerifier) String() string { return proto.CompactTextString(m) }
 func (*Verifiers_SignCLAVerifier) ProtoMessage()    {}
+
+func init() {
+	proto.RegisterType((*Config)(nil), "Config")
+	proto.RegisterType((*Rietveld)(nil), "Rietveld")
+	proto.RegisterType((*Gerrit)(nil), "Gerrit")
+	proto.RegisterType((*Verifiers)(nil), "Verifiers")
+	proto.RegisterType((*Verifiers_ReviewerLgtmVerifier)(nil), "Verifiers.ReviewerLgtmVerifier")
+	proto.RegisterType((*Verifiers_TreeStatusLgtmVerifier)(nil), "Verifiers.TreeStatusLgtmVerifier")
+	proto.RegisterType((*Verifiers_TryJobVerifier)(nil), "Verifiers.TryJobVerifier")
+	proto.RegisterType((*Verifiers_TryJobVerifier_Builder)(nil), "Verifiers.TryJobVerifier.Builder")
+	proto.RegisterType((*Verifiers_TryJobVerifier_Bucket)(nil), "Verifiers.TryJobVerifier.Bucket")
+	proto.RegisterType((*Verifiers_TryJobVerifier_TryJobRetryConfig)(nil), "Verifiers.TryJobVerifier.TryJobRetryConfig")
+	proto.RegisterType((*Verifiers_SignCLAVerifier)(nil), "Verifiers.SignCLAVerifier")
+}

third_party/cq_client/cq.proto

@@ -39,6 +39,10 @@ message Config {
   // Configuration options for Rietveld code review.
   optional Rietveld rietveld = 9;
 
+  // EXPERIMENTAL! Configuration options for Gerrit code review.
+  // TODO(tandrii): update this doc (GERRIT).
+  optional Gerrit gerrit = 15;
+
   // This can be used to override the Git repository URL used to checkout and
   // commit changes on CQ host. This should only be used in case, when the
   // source repository is not supported by luci-config (e.g. GitHub).
@@ -51,15 +55,6 @@ message Config {
 
   // Deprecated. URL of the SVN repository. We are deprecating SVN support.
   optional string svn_repo_url = 12;
-
-  // Deprecated. Should be set to true, when the project's SVN repository does
-  // not have server-side hooks configured.
-  optional bool server_hooks_missing = 13;
-
-  // Deprecated. Specifies a list of verifiers that are run on a local checkout
-  // with patch applied. The only remaining use case for this is PRESUBMIT_CHECK
-  // verifier, which we are deprecating as well.
-  optional Verifiers verifiers_with_patch = 14;
 }
 
 message Rietveld {
@@ -74,6 +69,32 @@ message Rietveld {
   repeated string project_bases = 2;
 }
 
+// Gerrit CQ is EXPERIMENTAL! See http://crbug.com/493899 for more info.
+//
+// Unlike Rietveld, Gerrit doesn't need a separate url.
+// Instead, the git_repo_url must be specified on the Gerrit instance,
+// and CQ will deduce Gerrit url from it.
+//
+// TODO(tandrii): support Rietveld and Gerrit at the same time.
+// This basically requires to start two CQ instances, instead of one.
+//
+// For example, if https://chromium.googlesource.com/infra/infra.git is your
+// repo url provided in `git_repo_url` above, then
+// https://chromium-review.googlesource.com/#/admin/projects/infra/infra should
+// show general properties of your project.
+//
+// Also,
+// https://chromium-review.googlesource.com/#/admin/projects/infra/infra,access
+// should show ACLs for refs in your project, but you may need to be admin to
+// see it. This will come handy to enable and customize the CQ-related workflows
+// for your project.
+message Gerrit {
+  // If set, tells CQ to set score on a given label to mark result of CQ run.
+  // Typically, this is Commit-Queue-Verified label.
+  // If not set, CQ will just try to hit submit button.
+  optional string cq_verified_label = 1;
+}
+
 // Verifiers are various types of checks that a Commit Queue performs on a CL.
 // All verifiers must pass in order for a CL to be landed. Configuration file
 // describes types of verifiers that should be applied to each CL and their
@@ -81,6 +102,7 @@ message Rietveld {
 message Verifiers {
   // This verifier is used to ensure that an LGTM was posted to the code review
   // site from a valid project committer.
+  // This verifier is not supported with Gerrit.
   optional ReviewerLgtmVerifier reviewer_lgtm = 1;
 
   // This verifier is used to check tree status before committing a CL. If the
@@ -130,7 +152,7 @@ message Verifiers {
 
       // When true, the builder is triggered by CQ. Otherwise, it is expected to
       // be triggered from another tryjob. Default value is true.
-      optional bool triggered = 2;
+      optional bool triggered_by_cq = 2;
 
       // When this field is present, it marks given builder as experimental. It
       // is only executed on a given percentage of the CLs and the outcome does

third_party/cq_client/test/__init__.py

+
+

third_party/cq_client/test/cq_gerrit.cfg

+version: 1
+cq_name: "infra"
+cq_status_url: "https://chromium-cq-status.appspot.com"
+hide_ref_in_committed_msg: true
+commit_burst_delay: 600
+max_commit_burst: 10
+in_production: false
+git_repo_url: "https://chromium.googlesource.com/infra/infra.git"
+target_ref: "refs/pending/heads/master"
+
+gerrit {
+  cq_verified_label: "Commit-Queue-Verified"
+}
+
+verifiers {
+  tree_status: {
+     tree_status_url: "https://infra-status.appspot.com"
+  }
+
+  try_job {
+    buckets {
+      name: "tryserver.blink"
+      builders { name: "android_blink_compile_dbg" }
+      builders { name: "android_blink_compile_rel" }
+      builders {
+        name: "win_blink_rel"
+        triggered_by_cq: false
+      }
+    }
+    buckets {
+      name: "tryserver.chromium.linux"
+      builders {
+        name: "android_arm64_dbg_recipe"
+      }
+      builders {
+        name: "linux_chromium_rel_ng"
+        experiment_percentage: 10
+      }
+    }
+    buckets {
+      name: "tryserver.chromium.mac"
+      builders {
+        name: "ios_dbg_simulator_ninja"
+        experiment_percentage: 100
+      }
+    }
+    try_job_retry_config {
+      try_job_retry_quota: 10
+      global_retry_quota: 11
+      failure_retry_weight: 12
+      transient_failure_retry_weight: 13
+    }
+  }
+}

third_party/cq_client/test/cq_example.cfg → third_party/cq_client/test/cq_rietveld.cfg

@@ -31,7 +31,7 @@ verifiers {
       builders { name: "android_blink_compile_rel" }
       builders {
         name: "win_blink_rel"
-        triggered: true
+        triggered_by_cq: false
       }
     }
     buckets {

third_party/cq_client/test/validate_config_test.py

@@ -16,10 +16,24 @@ TEST_DIR = os.path.dirname(os.path.abspath(__file__))
 
 
 class TestValidateConfig(unittest.TestCase):
-  def test_is_valid(self):
-    with open(os.path.join(TEST_DIR, 'cq_example.cfg'), 'r') as test_config:
+  def test_is_valid_rietveld(self):
+    with open(os.path.join(TEST_DIR, 'cq_rietveld.cfg'), 'r') as test_config:
       self.assertTrue(validate_config.IsValid(test_config.read()))
 
+  def test_is_valid_gerrit(self):
+    with open(os.path.join(TEST_DIR, 'cq_gerrit.cfg'), 'r') as test_config:
+      self.assertTrue(validate_config.IsValid(test_config.read()))
+
+  def test_one_codereview(self):
+    with open(os.path.join(TEST_DIR, 'cq_gerrit.cfg'), 'r') as gerrit_config:
+      data = gerrit_config.read()
+    data += '\n'.join([
+        'rietveld{',
+        'url: "https://blabla.com"',
+        '}'
+    ])
+    self.assertFalse(validate_config.IsValid(data))
+
   def test_has_field(self):
     config = cq_pb2.Config()
 

third_party/cq_client/validate_config.py

@@ -23,16 +23,12 @@ from cq_client import cq_pb2
 
 REQUIRED_FIELDS = [
   'version',
-  'rietveld',
-  'rietveld.url',
   'verifiers',
   'cq_name',
 ]
 
 LEGACY_FIELDS = [
   'svn_repo_url',
-  'server_hooks_missing',
-  'verifiers_with_patch',
 ]
 
 EMAIL_REGEXP = '^[^@]+@[^@]+\.[^@]+$'
@@ -92,7 +88,22 @@ def IsValid(cq_config):
     logging.error('Failed to parse config as protobuf:\n%s', e)
     return False
 
-  for fname in REQUIRED_FIELDS:
+  if _HasField(config, 'gerrit'):
+    if _HasField(config, 'rietveld'):
+      logging.error('gerrit and rietveld are not supported at the same time.')
+      return False
+    # TODO(tandrii): validate gerrit.
+    required_fields = REQUIRED_FIELDS + ['gerrit.cq_verified_label']
+    if _HasField(config, 'verifiers.reviewer_lgtm'):
+      logging.error('reviewer_lgtm verifier is not supported with Gerrit.')
+      return False
+  elif _HasField(config, 'rietveld'):
+    required_fields = REQUIRED_FIELDS + ['rietveld.url']
+  else:
+    logging.error('either rietveld gerrit are required fields.')
+    return False
+
+  for fname in required_fields:
     if not _HasField(config, fname):
       logging.error('%s is a required field', fname)
       return False
@@ -111,5 +122,4 @@ def IsValid(cq_config):
 
   # TODO(sergiyb): For each field, check valid values depending on its
   # semantics, e.g. email addresses, regular expressions etc.
-
   return True