New issue
Advanced search Search tips

Issue 878303 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Regression

Blocking:
issue v8:8107



Sign in to add a comment

V8: Implement --no-autocc flag in git-cl-upload to to disable auto-CCing v8-reviews@ on automatic CLs

Project Member Reported by machenb...@chromium.org, Aug 28

Issue description

We'd like to not CC v8-reviews@ on CLs from auto-rollers, but keep adding it on all other CLs.

When switching to gerrit we solved this by uploading automatic CLs as private. This is not supported any longer after https://crbug.com/877964 so we need a new solution.
 
I've sent a heads-up to v8-reviews@ which might get spammed again now.
Blocking: v8:8107
Components: Infra>SDK
Labels: -Pri-2 Pri-1
Auto-CC emails on CLs are added by git-cl tool [1] based on the the CC_LIST value in the codereview.settings file [2]. As linked code suggests, git-cl does not add auto-CC for private CLs and that's what's we've been using so far. How about adding another explicit git-cl flag to prevent auto-CCing?

  git cl upload --no-autocc ...

[1]: https://cs.chromium.org/chromium/tools/depot_tools/git_cl.py?l=3153&rcl=25c380cf9d2df4bb72fc63f7435346bc49f7720c
[2]: https://cs.chromium.org/chromium/src/v8/codereview.settings?l=4&rcl=abc91d4174d4019369c60b31b45a440e8ece6682
Cc: ehmaldonado@chromium.org
Re #4. That sounds good to me. How would that flag interact with the --cc flag?

I'll try to get to this tomorrow or Friday. Happy to do code reviews as well...
It won't interact with --cc flag, because --cc flag is about explicit CCs, whereas --no-autocc is about CC's being added implicitly. In other words, any people added with -cc flag will still be added to CL even if --no-autocc is specified.
Owner: serg...@chromium.org
Status: Assigned (was: Untriaged)
I'll work on this tomorrow.
Summary: V8: Implement --no-autocc flag in git-cl-upload to to disable auto-CCing v8-reviews@ on automatic CLs (was: V8: Figure out how to not auto-CC v8-reviews@ on automatic CLs)
CL: https://crrev.com/c/1230074
Status: Started (was: Assigned)
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/1aa405fd859a3bd625b0d61184d6e4a3cf95c0b4

commit 1aa405fd859a3bd625b0d61184d6e4a3cf95c0b4
Author: Sergiy Byelozyorov <sergiyb@chromium.org>
Date: Tue Sep 18 17:38:43 2018

Add an option to disable adding CC emails automatically

R=ehmaldonado@chromium.org, machenbach@chromium.org

Bug:  878303 
Change-Id: Ia90001025babe692e481eb74fdbb4a66ce9a22f8
Reviewed-on: https://chromium-review.googlesource.com/1230074
Commit-Queue: Sergiy Byelozyorov <sergiyb@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>

[modify] https://crrev.com/1aa405fd859a3bd625b0d61184d6e4a3cf95c0b4/tests/git_cl_test.py
[modify] https://crrev.com/1aa405fd859a3bd625b0d61184d6e4a3cf95c0b4/git_cl.py

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/37812790f2c5b3025e3cffc92c880ff3bb9a7f66

commit 37812790f2c5b3025e3cffc92c880ff3bb9a7f66
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Tue Sep 18 19:32:01 2018

Roll src/third_party/depot_tools 804797362e1d..1aa405fd859a (5 commits)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/804797362e1d..1aa405fd859a


git log 804797362e1d..1aa405fd859a --date=short --no-merges --format='%ad %ae %s'
2018-09-18 sergiyb@chromium.org Add an option to disable adding CC emails automatically
2018-09-18 nodir@google.com [gclient] Fix typo in get_gerrit_patch_root
2018-09-18 nodir@google.com [bot_update] Add gclient.get_gerrit_patch_root
2018-09-18 vadimsh@chromium.org [cipd] Pin hashes of CIPD packages.
2018-09-18 borenet@chromium.org Revert "gerrit_util: Support OAuth2 bearer tokens in CookieAuthenticator"


Created with:
  gclient setdep -r src/third_party/depot_tools@1aa405fd859a

The AutoRoll server is located here: https://autoroll.skia.org/r/depot-tools-chromium-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.



BUG= chromium:878303 , chromium:694348 , chromium:694348 , chromium:870166 
TBR=agable@chromium.org

Change-Id: I439b74e4283ec9962446dd2b14a101175f7fbb4d
Reviewed-on: https://chromium-review.googlesource.com/1231496
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#592145}
[modify] https://crrev.com/37812790f2c5b3025e3cffc92c880ff3bb9a7f66/DEPS

Project Member

Comment 14 by bugdroid1@chromium.org, Sep 19

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/8f30ab323cff3f49cd132cf01315a9c984f1432e

commit 8f30ab323cff3f49cd132cf01315a9c984f1432e
Author: Sergiy Byelozyorov <sergiyb@chromium.org>
Date: Wed Sep 19 13:41:36 2018

[tools] Do not auto-CC v8-reviews@ on CLs created by branch creator script

R=machenbach@chromium.org

Bug:  chromium:878303 , chromium:877964
Change-Id: I9f0de35780861f3f121daa9952af70b332c11e98
Reviewed-on: https://chromium-review.googlesource.com/1231176
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Sergiy Byelozyorov <sergiyb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56041}
[modify] https://crrev.com/8f30ab323cff3f49cd132cf01315a9c984f1432e/tools/release/create_release.py
[modify] https://crrev.com/8f30ab323cff3f49cd132cf01315a9c984f1432e/tools/release/git_recipes.py
[modify] https://crrev.com/8f30ab323cff3f49cd132cf01315a9c984f1432e/tools/release/test_scripts.py

Project Member

Comment 16 by bugdroid1@chromium.org, Sep 20

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/a1ea9ea7b9b7553cc9baedb172c830e66d19ec42

commit a1ea9ea7b9b7553cc9baedb172c830e66d19ec42
Author: Sergiy Byelozyorov <sergiyb@chromium.org>
Date: Thu Sep 20 15:01:30 2018

Revert "[V8] Stop auto-CC-ing v8-reviews@ in CLs created by Auto-tag builder"

This reverts commit be49789324d9a64b80d0680f3d5ce83b902e1f6f.

Reason for revert: --no-autocc option has not propagated to branches yet, hence Auto-tag builder that runs on branches fails due to missing option

Original change's description:
> [V8] Stop auto-CC-ing v8-reviews@ in CLs created by Auto-tag builder
> 
> R=​machenbach@chromium.org
> 
> Bug:  878303 , 877964
> Change-Id: I007623fe481bbdba629d1135dd7329d27ce8ff16
> Reviewed-on: https://chromium-review.googlesource.com/1231533
> Commit-Queue: Michael Achenbach <machenbach@chromium.org>
> Auto-Submit: Sergiy Byelozyorov <sergiyb@chromium.org>
> Reviewed-by: Michael Achenbach <machenbach@chromium.org>

TBR=machenbach@chromium.org,sergiyb@chromium.org

Change-Id: Ic10a753d9865b5f4e84fea602aadfb007a28d5df
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  878303 , 877964
Reviewed-on: https://chromium-review.googlesource.com/1236334
Reviewed-by: Sergiy Byelozyorov <sergiyb@chromium.org>
Commit-Queue: Sergiy Byelozyorov <sergiyb@chromium.org>

[modify] https://crrev.com/a1ea9ea7b9b7553cc9baedb172c830e66d19ec42/scripts/slave/README.recipes.md
[modify] https://crrev.com/a1ea9ea7b9b7553cc9baedb172c830e66d19ec42/scripts/slave/recipes/v8/auto_tag.expected/dry_run.json
[modify] https://crrev.com/a1ea9ea7b9b7553cc9baedb172c830e66d19ec42/scripts/slave/recipes/v8/auto_tag.py
[modify] https://crrev.com/a1ea9ea7b9b7553cc9baedb172c830e66d19ec42/scripts/slave/recipes/v8/auto_tag.expected/update.json
[modify] https://crrev.com/a1ea9ea7b9b7553cc9baedb172c830e66d19ec42/scripts/slave/recipes/v8/auto_tag.expected/update_timeout.json

Hmm... so auto-tag also uses the depot_tools pinned in V8? Maybe we can backmerge using the newer version...
Project Member

Comment 18 by bugdroid1@chromium.org, Sep 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/a2c9c08543464a36778cda54dfa58d575bdf0352

commit a2c9c08543464a36778cda54dfa58d575bdf0352
Author: Sergiy Byelozyorov <sergiyb@chromium.org>
Date: Fri Sep 21 09:09:39 2018

Reland "[V8] Stop auto-CC-ing v8-reviews@ in CLs created by Auto-tag builder"

This is a reland of be49789324d9a64b80d0680f3d5ce83b902e1f6f

Original change's description:
> [V8] Stop auto-CC-ing v8-reviews@ in CLs created by Auto-tag builder
>
> R=machenbach@chromium.org
>
> Bug:  878303 , 877964
> Change-Id: I007623fe481bbdba629d1135dd7329d27ce8ff16
> Reviewed-on: https://chromium-review.googlesource.com/1231533
> Commit-Queue: Michael Achenbach <machenbach@chromium.org>
> Auto-Submit: Sergiy Byelozyorov <sergiyb@chromium.org>
> Reviewed-by: Michael Achenbach <machenbach@chromium.org>

TBR=machenbach@chromium.org
Bug:  878303 , 877964
Change-Id: Idcb7ce33381db260f5789e643630b57e6e8773d0
Reviewed-on: https://chromium-review.googlesource.com/1238393
Reviewed-by: Sergiy Byelozyorov <sergiyb@chromium.org>
Commit-Queue: Sergiy Byelozyorov <sergiyb@chromium.org>

[modify] https://crrev.com/a2c9c08543464a36778cda54dfa58d575bdf0352/scripts/slave/README.recipes.md
[modify] https://crrev.com/a2c9c08543464a36778cda54dfa58d575bdf0352/scripts/slave/recipes/v8/auto_tag.expected/dry_run.json
[modify] https://crrev.com/a2c9c08543464a36778cda54dfa58d575bdf0352/scripts/slave/recipes/v8/auto_tag.py
[modify] https://crrev.com/a2c9c08543464a36778cda54dfa58d575bdf0352/scripts/slave/recipes/v8/auto_tag.expected/update.json
[modify] https://crrev.com/a2c9c08543464a36778cda54dfa58d575bdf0352/scripts/slave/recipes/v8/auto_tag.expected/update_timeout.json

It works now, but somehow the created CLs still do auto-CC v8-reviews@. Not sure if this was added by the git-cl tool or by Gerrit itself. Will investigate.
Turns out I've only updated Rietveld's implementation of upload process in git_cl.py and since we only use Gerrit now, nothing's changed with added --no-autocc flag. Adding a fix for Gerrit implementation: https://crrev.com/c/1238571. Tested it locally by creating this CL with --no-autocc flag: https://crrev.com/c/1240138.
Project Member

Comment 21 by bugdroid1@chromium.org, Sep 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/aaf2cc09c6874e394c6c1e4692360cc400d6b388

commit aaf2cc09c6874e394c6c1e4692360cc400d6b388
Author: Sergiy Byelozyorov <sergiyb@chromium.org>
Date: Mon Sep 24 18:02:28 2018

Implement --no-autocc flag for Gerrit

R=ehmaldonado@chromium.org, machenbach@chromium.org

Bug:  878303 
Change-Id: Ic387bbdc9e8ee4b27b17505ca53e8873e20e4cda
Reviewed-on: https://chromium-review.googlesource.com/1238571
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Sergiy Byelozyorov <sergiyb@chromium.org>

[modify] https://crrev.com/aaf2cc09c6874e394c6c1e4692360cc400d6b388/git_cl.py

Project Member

Comment 22 by bugdroid1@chromium.org, Sep 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/429ef673e133c956e1ebcb489f9eae9a7fea492c

commit 429ef673e133c956e1ebcb489f9eae9a7fea492c
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Mon Sep 24 20:24:06 2018

Roll src/third_party/depot_tools 8bdc1b8a0433..aaf2cc09c687 (1 commits)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/8bdc1b8a0433..aaf2cc09c687


git log 8bdc1b8a0433..aaf2cc09c687 --date=short --no-merges --format='%ad %ae %s'
2018-09-24 sergiyb@chromium.org Implement --no-autocc flag for Gerrit


Created with:
  gclient setdep -r src/third_party/depot_tools@aaf2cc09c687

The AutoRoll server is located here: https://autoroll.skia.org/r/depot-tools-chromium-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.



BUG= chromium:878303 
TBR=agable@chromium.org

Change-Id: Iab04c0d298d064f57c569aadd622ea5a338637be
Reviewed-on: https://chromium-review.googlesource.com/1240834
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#593660}
[modify] https://crrev.com/429ef673e133c956e1ebcb489f9eae9a7fea492c/DEPS

Status: Fixed (was: Started)
New CLs created by Auto-tag do not CC v8-reviews anymore, e.g. https://crrev.com/c/1243688 and https://crrev.com/c/1243689.

Interestingly, the bot account is not added as a reviewer despite --tbrs argument on the command line. This is probably due to the fact that the account was never registered on Gerrit. Filed issue 889071 for this.
How about TBR=santa or something? Or is that prohibited soon?

Thanks for fixing this!
Let's see what's the reply on the issue and if they won't be able or would not want to do this for some reason, we can just leave it as is. It's not a major issue was e are able to git-cl-land CLs also without a reviewer assigned and description still says TBR=v8-ci-autoroll-builder@chops-service-accounts.iam.gserviceaccount.com.

Sign in to add a comment