New issue
Advanced search Search tips

Issue 856884 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 3
Type: Bug

Blocking:
issue 855791



Sign in to add a comment

hardcoded buildtools gn location in traffic_annotation

Project Member Reported by scottmg@chromium.org, Jun 27 2018

Issue description

Cc: msramek@chromium.org privard@chromium.org georgesak@chromium.org rhalavati@chromium.org battre@chromium.org
Status: Started (was: Assigned)
Hi traffic_annotation OWNERS,

I'm in the process of moving GN out of buildtools in https://chromium-review.googlesource.com/c/chromium/src/+/1112840 and I noticed https://cs.chromium.org/chromium/src/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc?type=cs&q=buildtools.*gn&sq=package:chromium&g=0&l=622 might break as a result.

Is there a tryjob that I can run, or some manual test I should run for this code?

Thanks
Hi scottmg@,

The tests use the rolled binary of traffic_annotator_auditor, hence testing it using trybots requires some extra steps. I recommend building the binary, copying it to tools/traffic_annotation/bin/[platform], and running it there locally.

P.S., gn is here used to check if a file is used in building chrome::chrome target or not. If there is another way to test that, it can also be used instead.

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 27 2018

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

commit 0895c7977a5c312aca132b23a3a9d46f78f77ee4
Author: Scott Graham <scottmg@chromium.org>
Date: Wed Jun 27 16:10:36 2018

Pull GN via CIPD package

The gn binary will be downloaded into third_party/gn. This CL rolls
buildtools forward to a revision where gn is no longer in buildtools.

Because buildtools has no way to cleanup after itself, add
build/util/gn_cleanup.py to delete the old binaries so that we don't
accidentally depend on them.

Update mb.py to use the new location.

Update explicit location in ios/web_view/BUILD.gn for the new location.

Update explicit location in
tools/traffic_annotation/auditor/traffic_annotation_auditor.cc for the new
location.

Update explicit location in tools/licenses.py with for the new location.

Bug: 855791
Bug:  794764 
Bug: 856883
Bug:  856884 
Bug: 856878
Bug:  856899 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I92c908faf4f868850eafa1b4adf6e7c33c365116
Reviewed-on: https://chromium-review.googlesource.com/1112840
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570792}
[modify] https://crrev.com/0895c7977a5c312aca132b23a3a9d46f78f77ee4/DEPS
[add] https://crrev.com/0895c7977a5c312aca132b23a3a9d46f78f77ee4/build/util/gn_cleanup.py
[modify] https://crrev.com/0895c7977a5c312aca132b23a3a9d46f78f77ee4/ios/web_view/BUILD.gn
[modify] https://crrev.com/0895c7977a5c312aca132b23a3a9d46f78f77ee4/tools/licenses.py
[modify] https://crrev.com/0895c7977a5c312aca132b23a3a9d46f78f77ee4/tools/mb/mb.py
[modify] https://crrev.com/0895c7977a5c312aca132b23a3a9d46f78f77ee4/tools/mb/mb_unittest.py
[modify] https://crrev.com/0895c7977a5c312aca132b23a3a9d46f78f77ee4/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc

Blocking: 855791
Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 27 2018

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

commit 35383539aa83f27185a661215773ece946535676
Author: Scott Graham <scottmg@chromium.org>
Date: Wed Jun 27 16:27:35 2018

Revert "Pull GN via CIPD package"

This reverts commit 0895c7977a5c312aca132b23a3a9d46f78f77ee4.

Reason for revert: Missed a hardcoded gn location: https://crbug.com/857107.

Original change's description:
> Pull GN via CIPD package
> 
> The gn binary will be downloaded into third_party/gn. This CL rolls
> buildtools forward to a revision where gn is no longer in buildtools.
> 
> Because buildtools has no way to cleanup after itself, add
> build/util/gn_cleanup.py to delete the old binaries so that we don't
> accidentally depend on them.
> 
> Update mb.py to use the new location.
> 
> Update explicit location in ios/web_view/BUILD.gn for the new location.
> 
> Update explicit location in
> tools/traffic_annotation/auditor/traffic_annotation_auditor.cc for the new
> location.
> 
> Update explicit location in tools/licenses.py with for the new location.
> 
> Bug: 855791
> Bug:  794764 
> Bug: 856883
> Bug:  856884 
> Bug: 856878
> Bug:  856899 
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> Change-Id: I92c908faf4f868850eafa1b4adf6e7c33c365116
> Reviewed-on: https://chromium-review.googlesource.com/1112840
> Commit-Queue: Scott Graham <scottmg@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570792}

TBR=dpranke@chromium.org,scottmg@chromium.org

Change-Id: I89e180710b5ce483106fa3b4ff6e8dec06780c07
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 855791,  794764 , 856883,  856884 , 856878,  856899 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/1117219
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570799}
[modify] https://crrev.com/35383539aa83f27185a661215773ece946535676/DEPS
[delete] https://crrev.com/d0ed5774569fc56f4a0c74affafda8a7ad2a02aa/build/util/gn_cleanup.py
[modify] https://crrev.com/35383539aa83f27185a661215773ece946535676/ios/web_view/BUILD.gn
[modify] https://crrev.com/35383539aa83f27185a661215773ece946535676/tools/licenses.py
[modify] https://crrev.com/35383539aa83f27185a661215773ece946535676/tools/mb/mb.py
[modify] https://crrev.com/35383539aa83f27185a661215773ece946535676/tools/mb/mb_unittest.py
[modify] https://crrev.com/35383539aa83f27185a661215773ece946535676/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 28 2018

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

commit fed0e8ec393c317037a824b63b3e9690b8b0a4fc
Author: Scott Graham <scottmg@chromium.org>
Date: Thu Jun 28 05:02:14 2018

Pull GN via CIPD package

This is a reland of
https://chromium-review.googlesource.com/c/chromium/src/+/1112840.


The gn binary will be downloaded into third_party/gn. This CL rolls
buildtools forward to a revision where the gn binary is a stub that
prints an error message and returns 1. This is to avoid depending
on the old location, and helping users relying on gn auto regen.

Update mb.py to use the new location.

Update explicit location in ios/web_view/BUILD.gn for the new location.

Update explicit location in
tools/traffic_annotation/auditor/traffic_annotation_auditor.cc for the new
location.

Update explicit location in tools/licenses.py with for the new location.

Update explicit location in testing/libfuzzer/gen_fuzzer_owners.py.

Bug: 855791
Bug: 856883
Bug:  856884 
Bug: 856878
Bug:  856899 
Bug: 857107
Bug:  857110 
Change-Id: Iedaa29161e56cc90beafd802efca3a6c12859c58
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/1117264
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571031}
[modify] https://crrev.com/fed0e8ec393c317037a824b63b3e9690b8b0a4fc/DEPS
[modify] https://crrev.com/fed0e8ec393c317037a824b63b3e9690b8b0a4fc/ios/web_view/BUILD.gn
[modify] https://crrev.com/fed0e8ec393c317037a824b63b3e9690b8b0a4fc/testing/libfuzzer/gen_fuzzer_owners.py
[modify] https://crrev.com/fed0e8ec393c317037a824b63b3e9690b8b0a4fc/tools/licenses.py
[modify] https://crrev.com/fed0e8ec393c317037a824b63b3e9690b8b0a4fc/tools/mb/mb.py
[modify] https://crrev.com/fed0e8ec393c317037a824b63b3e9690b8b0a4fc/tools/mb/mb_unittest.py
[modify] https://crrev.com/fed0e8ec393c317037a824b63b3e9690b8b0a4fc/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 28 2018

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

commit fb40db1204f14ad0e420652787c4a70ce374c4a4
Author: Ramin Halavati <rhalavati@chromium.org>
Date: Thu Jun 28 07:04:27 2018

Roll traffic_annotation_auditor for Linux

The new binary for traffic_annotation_auditor after the change in
crrev.com/c/1117264 is rolled.

Bug:  856884 
Change-Id: Ief41cd698f8190b3642582a2ddabf15724f91a2f
TBR: georgesak@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1117682
Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571041}
[modify] https://crrev.com/fb40db1204f14ad0e420652787c4a70ce374c4a4/tools/traffic_annotation/auditor/README.md
[modify] https://crrev.com/fb40db1204f14ad0e420652787c4a70ce374c4a4/tools/traffic_annotation/bin/README.md
[modify] https://crrev.com/fb40db1204f14ad0e420652787c4a70ce374c4a4/tools/traffic_annotation/bin/linux64/traffic_annotation_auditor.sha1

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 28 2018

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

commit 081bb19ac0c6fe255ffc8ea85a4277dbc06efa85
Author: Ramin Halavati <rhalavati@chromium.org>
Date: Thu Jun 28 08:43:00 2018

Roll traffic_annotation_auditor binary for Windows.

The new binary for traffic_annotation_auditor after the change in
crrev.com/c/1117264 is rolled.

Bug:  856884 
Change-Id: I76523df5e58ace767ac19c619c9bb32da5a25674
TBR: georgesak@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1117686
Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571048}
[modify] https://crrev.com/081bb19ac0c6fe255ffc8ea85a4277dbc06efa85/tools/traffic_annotation/bin/README.md
[modify] https://crrev.com/081bb19ac0c6fe255ffc8ea85a4277dbc06efa85/tools/traffic_annotation/bin/win32/traffic_annotation_auditor.exe.sha1

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 28 2018

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

commit 8962649114490c145c5cba76fe290752014d6432
Author: Xiaoqian Dai <xdai@chromium.org>
Date: Thu Jun 28 17:07:46 2018

Revert "Pull GN via CIPD package"

This reverts commit fed0e8ec393c317037a824b63b3e9690b8b0a4fc.

Reason for revert: It broke most of the chrome_informational builders at BuildPackages stage. See  crbug.com/857133  for details. 

Original change's description:
> Pull GN via CIPD package
> 
> This is a reland of
> https://chromium-review.googlesource.com/c/chromium/src/+/1112840.
> 
> 
> The gn binary will be downloaded into third_party/gn. This CL rolls
> buildtools forward to a revision where the gn binary is a stub that
> prints an error message and returns 1. This is to avoid depending
> on the old location, and helping users relying on gn auto regen.
> 
> Update mb.py to use the new location.
> 
> Update explicit location in ios/web_view/BUILD.gn for the new location.
> 
> Update explicit location in
> tools/traffic_annotation/auditor/traffic_annotation_auditor.cc for the new
> location.
> 
> Update explicit location in tools/licenses.py with for the new location.
> 
> Update explicit location in testing/libfuzzer/gen_fuzzer_owners.py.
> 
> Bug: 855791
> Bug: 856883
> Bug:  856884 
> Bug: 856878
> Bug:  856899 
> Bug: 857107
> Bug:  857110 
> Change-Id: Iedaa29161e56cc90beafd802efca3a6c12859c58
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> Reviewed-on: https://chromium-review.googlesource.com/1117264
> Commit-Queue: Scott Graham <scottmg@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#571031}

TBR=dpranke@chromium.org,scottmg@chromium.org

Change-Id: I28962c879b7bc24e809b29427122bb425e9450a5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 855791, 856883,  856884 , 856878,  856899 , 857107,  857110 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/1118825
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571170}
[modify] https://crrev.com/8962649114490c145c5cba76fe290752014d6432/DEPS
[modify] https://crrev.com/8962649114490c145c5cba76fe290752014d6432/ios/web_view/BUILD.gn
[modify] https://crrev.com/8962649114490c145c5cba76fe290752014d6432/testing/libfuzzer/gen_fuzzer_owners.py
[modify] https://crrev.com/8962649114490c145c5cba76fe290752014d6432/tools/licenses.py
[modify] https://crrev.com/8962649114490c145c5cba76fe290752014d6432/tools/mb/mb.py
[modify] https://crrev.com/8962649114490c145c5cba76fe290752014d6432/tools/mb/mb_unittest.py
[modify] https://crrev.com/8962649114490c145c5cba76fe290752014d6432/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 29 2018

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

commit 19c20b3f57a846fe1150ad7e09d1afc2c355d291
Author: Ramin Halavati <rhalavati@chromium.org>
Date: Fri Jun 29 06:46:12 2018

Revert "Roll traffic_annotation_auditor binary for Windows."

This reverts commit 081bb19ac0c6fe255ffc8ea85a4277dbc06efa85.

Reason for revert: crrev.com/c/1117264 got reverted, and this CL is based on that.

Original change's description:
> Roll traffic_annotation_auditor binary for Windows.
> 
> The new binary for traffic_annotation_auditor after the change in
> crrev.com/c/1117264 is rolled.
> 
> Bug:  856884 
> Change-Id: I76523df5e58ace767ac19c619c9bb32da5a25674
> TBR: georgesak@chromium.org
> Reviewed-on: https://chromium-review.googlesource.com/1117686
> Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
> Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#571048}

TBR=georgesak@chromium.org,rhalavati@chromium.org

Change-Id: I2706aa14d74d1c858d6e53c2703baf55c041f340
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  856884 
Reviewed-on: https://chromium-review.googlesource.com/1119805
Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571412}
[modify] https://crrev.com/19c20b3f57a846fe1150ad7e09d1afc2c355d291/tools/traffic_annotation/bin/README.md
[modify] https://crrev.com/19c20b3f57a846fe1150ad7e09d1afc2c355d291/tools/traffic_annotation/bin/win32/traffic_annotation_auditor.exe.sha1

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 29 2018

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

commit aba525ad92c9cc783570611ddb7b2cc06d217e55
Author: Ramin Halavati <rhalavati@chromium.org>
Date: Fri Jun 29 06:47:11 2018

Revert "Roll traffic_annotation_auditor for Linux"

This reverts commit fb40db1204f14ad0e420652787c4a70ce374c4a4.

Reason for revert: crrev.com/c/1117264 got reverted, and this CL is based on that.

Original change's description:
> Roll traffic_annotation_auditor for Linux
> 
> The new binary for traffic_annotation_auditor after the change in
> crrev.com/c/1117264 is rolled.
> 
> Bug:  856884 
> Change-Id: Ief41cd698f8190b3642582a2ddabf15724f91a2f
> TBR: georgesak@chromium.org
> Reviewed-on: https://chromium-review.googlesource.com/1117682
> Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
> Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#571041}

TBR=georgesak@chromium.org,rhalavati@chromium.org

Change-Id: Idac821023b403db1d9182b54616bcc50904a8401
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  856884 
Reviewed-on: https://chromium-review.googlesource.com/1119785
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571413}
[modify] https://crrev.com/aba525ad92c9cc783570611ddb7b2cc06d217e55/tools/traffic_annotation/auditor/README.md
[modify] https://crrev.com/aba525ad92c9cc783570611ddb7b2cc06d217e55/tools/traffic_annotation/bin/README.md
[modify] https://crrev.com/aba525ad92c9cc783570611ddb7b2cc06d217e55/tools/traffic_annotation/bin/linux64/traffic_annotation_auditor.sha1

Sign in to add a comment