New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 879554 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

generate_telemetry_build.py suggests to add irrelevant changes to my commit

Project Member Reported by pasko@chromium.org, Aug 31

Issue description

I am in catapult checkout as part of my chromium checkout, with master being at:
cd6bcbe53ba75242740c2244d1d3b4c556e03039

but this has not rolled to Chromium yet, being at d2f6e76cbc6b7d6b29023587e61751ac08a71bcf (maybe that's important, maybe not, I don't know)

A fresh new branch created with base on this master would fail on 'git cl upload' with:
Running presubmit upload checks ...
ERROR:root:Diff found. Please rerun generate_telemetry_build.py.

Running the script produces this diff, which I think I should *not* include into my commit, contrary to the recommendation:
shell> python generate_telemetry_build.py
shell> git diff
diff --git a/BUILD.gn b/BUILD.gn
index d087a5439..662f631ec 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -23,7 +23,9 @@ group("telemetry_chrome_test_support") {
     "codereview.settings",
     "navbar.md",
     "pylintrc",
+    "tags",
     "bin/",
+    "catapult_base/",
     "catapult_build/",
     "dashboard/",
     "dependency_manager/",
@@ -39,6 +41,7 @@ group("telemetry_chrome_test_support") {
 
   data += [
     "common/OWNERS",
+    "common/battor/",
     "common/bin/",
     "common/eslint/",
     "common/lab/",


 
Status: WontFix (was: Assigned)
you need to clean your local checkout of any folders/files that have been removed from repo already:

git clean -f

That should resolve this issue.

I don't want to include this clean step in the generator because if people have local files that they are working on, I don't want to accidentally remove it.
Status: Available (was: WontFix)
I think we can still improve this by having the script running "git status -u" and warn users that it gonna affect fidelity of the script.

Even better, maybe script should ignore folders that are not tracked by git
Status: Assigned (was: Available)
Ah, "git status -u" seems a good improvement.

I'll look into that.
Cc: -nednguyen@chromium.org zmo@chromium.org kbr@chromium.org
Owner: nednguyen@chromium.org
zmo@ is out, and this has been causing many catapult blockage ( issue 881543 ). I am taking this bug.
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 7

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/febcf8764dc1ab9a2b3cd87c3911c4924db45e30

commit febcf8764dc1ab9a2b3cd87c3911c4924db45e30
Author: nednguyen <nednguyen@google.com>
Date: Fri Sep 07 18:44:05 2018

Ignore files/directory that are not tracked by git when generating BUILD.gn for Telemetry

Previously, if developers working on catapult/ patches have files or directories in the
top level catapult directory that are untracked by git, generate_telemetry_build.py script
will include those files in BUILD.gn. This CL makes sure that the script
ignores those files instead.

Test: patch this CL, add random local files into catapult/,
run ./generate_telemetry_build.py --> expect that BUILD.gn doesn't change

Bug:  chromium:879554 
Change-Id: I2c05bacd09360685002273be6e8a8be5ae79c577
Reviewed-on: https://chromium-review.googlesource.com/1213349
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Kenneth Russell <kbr@chromium.org>

[modify] https://crrev.com/febcf8764dc1ab9a2b3cd87c3911c4924db45e30/generate_telemetry_build.py

Status: Fixed (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 7

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

commit dd19a2123e6a476fb81010166b9661bfb8c74bbc
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Sep 07 21:07:52 2018

Roll src/third_party/catapult 32654a45d3da..febcf8764dc1 (6 commits)

https://chromium.googlesource.com/catapult.git/+log/32654a45d3da..febcf8764dc1


git log 32654a45d3da..febcf8764dc1 --date=short --no-merges --format='%ad %ae %s'
2018-09-07 nednguyen@google.com Ignore files/directory that are not tracked by git when generating BUILD.gn for Telemetry
2018-09-07 jbudorick@chromium.org devil: move gn files over from chromium.
2018-09-07 nastasoiuf@google.com Additional features for stacked plot
2018-09-07 simonhatch@chromium.org Dashboard - Remove TestMetadata._pre_put_hook
2018-09-07 simonhatch@chromium.org Dashboard - Remove stray print
2018-09-07 simonhatch@chromium.org Dashboard - Refactor find_anomalies to support stats later


Created with:
  gclient setdep -r src/third_party/catapult@febcf8764dc1

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-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.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:879554 , chromium:881910 ,chromium:866423,chromium:881523
TBR=sullivan@chromium.org

Change-Id: I1f41015a567033be0e588947b705ae02435521be
Reviewed-on: https://chromium-review.googlesource.com/1213776
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#589641}
[modify] https://crrev.com/dd19a2123e6a476fb81010166b9661bfb8c74bbc/DEPS

Sign in to add a comment