git cl: add reviewers and ccs to git push command if possible.

Also fix a typo in ValidateEmail function which didn't support
email addresses with '-' in them, e.g., infra-dev@chromium.org.

R=ehmaldonado

Bug: 875089
Change-Id: I2d73c1473527c9bf62e25e9f88250196b783fcb0
Reviewed-on: https://chromium-review.googlesource.com/c/1242849
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>

git_cl.py

@@ -2942,7 +2942,6 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
         self._GetGerritHost(), reviewers + cc)
     logging.debug('accounts %s are valid, %s invalid', sorted(valid_accounts),
                    set(reviewers + cc).difference(set(valid_accounts)))
-    # TODO(tandrii): add valid reviwers and ccs to push option.
 
     # Extra options that can be specified at push time. Doc:
     # https://gerrit-review.googlesource.com/Documentation/user-upload.html
@@ -2969,6 +2968,22 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
     if options.private:
       refspec_opts.append('private')
 
+    for r in sorted(reviewers):
+      if r in valid_accounts:
+        refspec_opts.append('r=%s' % r)
+        reviewers.remove(r)
+      else:
+        # TODO(tandrii): this should probably be a hard failure.
+        print('WARNING: reviewer %s doesn\'t have a Gerrit account, skipping'
+              % r)
+    for c in sorted(cc):
+      # refspec option will be rejected if cc doesn't correspond to an
+      # account, even though REST call to add such arbitrary cc may succeed.
+      if c in valid_accounts:
+        refspec_opts.append('cc=%s' % c)
+        cc.remove(c)
+
+
     if options.topic:
       # Documentation on Gerrit topics is here:
       # https://gerrit-review.googlesource.com/Documentation/user-upload.html#topic
@@ -3069,7 +3084,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
       self.SetIssue(change_numbers[0])
       self._GitSetBranchConfigValue('gerritsquashhash', ref_to_push)
 
-    if self.GetIssue():
+    if self.GetIssue() and (reviewers or cc):
       # GetIssue() is not set in case of non-squash uploads according to tests.
       # TODO(agable): non-squash uploads in git cl should be removed.
       gerrit_util.AddReviewers(

scm.py

@@ -20,8 +20,9 @@ import subprocess2
 
 
 def ValidateEmail(email):
-  return (re.match(r"^[a-zA-Z0-9._%-+]+@[a-zA-Z0-9._%-]+.[a-zA-Z]{2,6}$", email)
-          is not None)
+  return (
+      re.match(r"^[a-zA-Z0-9._%\-+]+@[a-zA-Z0-9._%-]+.[a-zA-Z]{2,6}$", email)
+      is not None)
 
 
 def GetCasedPath(path):

tests/git_cl_test.py

@@ -1020,9 +1020,18 @@ class TestGitCl(TestCase):
         sorted(reviewers) + ['joe@example.com',
         'chromium-reviews+test-more-cc@chromium.org'] + cc),
        {
-         # TODO(tandrii): add here some valid accounts and make use of them.
+         e: {'email': e}
+         for e in (reviewers + ['joe@example.com'] + cc)
        }),
     ]
+    for r in sorted(reviewers):
+      if r != 'bad-account-or-email':
+        ref_suffix  += ',r=%s' % r
+        reviewers.remove(r)
+    for c in sorted(['joe@example.com'] + cc):
+      ref_suffix += ',cc=%s' % c
+      if c in cc:
+        cc.remove(c)
 
     calls.append((
       (['git', 'push',
@@ -1057,8 +1066,9 @@ class TestGitCl(TestCase):
           (('AddReviewers',
             'chromium-review.googlesource.com', 'my%2Frepo~123456',
             sorted(reviewers),
-            ['joe@example.com', 'chromium-reviews+test-more-cc@chromium.org'] +
-            cc, notify), ''),
+            cc + ['chromium-reviews+test-more-cc@chromium.org'],
+            notify),
+           ''),
       ]
     if tbr:
       calls += [