Causes CQ flakiness:
https://ci.chromium.org/buildbot/tryserver.chromium.android/android_n5x_swarming_rel/372297

https://ci.chromium.org/buildbot/tryserver.chromium.win/win7_chromium_rel_ng/114276

On Windows too.

https://ci.chromium.org/buildbot/chromium.win/Win7%20Tests%20%281%29/77710
https://ci.chromium.org/buildbot/chromium.win/Win%207%20Tests%20x64%20%281%29/35318
https://ci.chromium.org/buildbot/chromium.win/Win7%20Tests%20%28dbg%29%281%29/66660

Robbie: is this related to your work with vpython?

I think https://chromium-review.googlesource.com/c/chromium/src/+/944899 is intended to fix it, but the CQ dry run failed with that patch, so the fix is not working.

Yes, that CL is intended to fix it. I'll take a closer look.

I've rolled back the swarming version, which should fix the problem immediately; I'll continue to iterate on that CL in the mean time to see if I can fix it preemptively.

I've corrected https://chromium-review.googlesource.com/c/chromium/src/+/944899 with the help of a manual swarming task https://chromium-swarm.appspot.com/user/task/3bfdcb5ef871df10

Landing the real fix now

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

commit 5a9d75f6abae6a2af0ff4976d00a6a4ae36475d5
Author: Robert Iannucci <iannucci@chromium.org>
Date: Fri Mar 02 05:28:20 2018

Reland "[mb.py] Workaround for mb.py to include "base" software"

This is a reland of 64d2f02cddda0801ee45731afb25abdff3e3440c

It wasn't related to  crbug.com/818054  at all :)

Original change's description:
> [mb.py] Workaround for mb.py to include "base" software
>
> Bug: 812428
> Change-Id: I8561fce9eb8a2da3cf45530ccf1494d6182f48d5
> Reviewed-on: https://chromium-review.googlesource.com/942226
> Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#540310}

Tbr: chengx@chromium.org
Bug: 812428, 818054 
Change-Id: I32685152c837abfb904402a0625a5da1ffbf2a66
Reviewed-on: https://chromium-review.googlesource.com/945369
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540453}
[modify] https://crrev.com/5a9d75f6abae6a2af0ff4976d00a6a4ae36475d5/tools/mb/mb.py

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

commit bbbfd6032b311c9aaf1b0890f5dae34545d5c0ec
Author: Robert Iannucci <iannucci@chromium.org>
Date: Fri Mar 02 06:00:53 2018

[find_dependencies.py] Explicitly exclude all files under python sys.prefix.

When running the dependency finder under a VirtualEnv (such as vpython),
it will accidentally pick up dependencies from the sys.prefix.

Bug: 804174, 818054 
Change-Id: Ic6a677413ffed782ab49a772cc0eb73b7e8a87b2
Reviewed-on: https://chromium-review.googlesource.com/944899
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: David Tu <dtu@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#540459}
[modify] https://crrev.com/bbbfd6032b311c9aaf1b0890f5dae34545d5c0ec/tools/perf/core/find_dependencies.py

Swapping swarming back, should be happy now.

Looks like some of the tests ran on the new swarming before picking up the change. Still watching the tree

Woot, passes: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Tests/67996

find_dependencies is also used by chromeos, FYI. There's a chance this breaks the chromeos build. 

It's possible. Do you have a link to an example build?

AFAIK, find_dependencies isn't actually used by much in chrome... is it possible that chromeos is the only user? The test that broke wasn't actually USING find_dependencies, just a small unittest that checks to see if it sort of vaguely works.

If it turns out that the fix in https://chromium.googlesource.com/chromium/src.git/+/bbbfd6032b311c9aaf1b0890f5dae34545d5c0ec changes the way that find_dependencies works in a CrOS-incompatible way, I would recommend reverting that CL, but also disabling the find_dependencies_unittests (for now). That will restore the find_dependencies script to the pre-change functionality, but won't cause the main waterfall/trybots to go red in chromium.

If we do this we should then adjust the unittests to match the behavior that cros expects (and/or move find_dependencies into cros :))

Ok, I'm going to call this one fixed for now. 

achuith, please reopen for the revert if it causes CrOS issues.

Looks like this did, indeed break chromeos

Issue 818230 has been merged into this issue.

Going to do what I suggested in comment 18

CL: https://chromium-review.googlesource.com/c/chromium/src/+/946747

Actually I think it was just lag on the CI; all those builders are green now...


/me backs away slowly

nvm, https://luci-milo.appspot.com/buildbot/chromiumos.chromium/amd64-generic-telemetry/ is still red

So the breakage I was worried about was here:
https://cs.corp.google.com/chromeos_public/src/third_party/chromiumos-overlay/chromeos-base/chromeos-chrome/chromeos-chrome-9999.ebuild?l=1209

Changing this script can cause telemetry dependencies to not be properly packaged on chromeos, or even break the build. Doesn't look like any of those things happened.

We have a broken unittest, and I don't see why it's only broken on chromeos? It also seems like it ought to be fixable.

Are you aware of go/cros-vm:
https://chromium.googlesource.com/chromiumos/docs/+/master/cros_vm.md

You can run your unittest in a cros VM on your desktop. Should be fairly straightforward:

(shell) .../chrome/src $ cros chrome-sdk --board=amd64-generic \
--download-vm --clear-sdk-cache --log-level info
(sdk) .../chrome/src $ cros_vm --start
(sdk) .../chrome/src $ tools/perf/run_tests \
--browser=cros-chrome --remote=localhost --remote-ssh-port=9222 testFindPythonDependencies

Once the unittest disablement lands, would you mind taking this over achuith? I'm... 3 steps removed from code I know anything about at this point :D.

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

commit 03a17b1a9826e1a8dbe0f23ef37d89acb209497b
Author: Robert Iannucci <iannucci@chromium.org>
Date: Sat Mar 03 02:54:52 2018

[find_dependencies] Disable tests on CrOS.

Bug:  818054 ,818230
Change-Id: Ia3c395bf89845c683bba74b4944e5c673ae3a0b0
Reviewed-on: https://chromium-review.googlesource.com/946747
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Reviewed-by: David Tu <dtu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540729}
[modify] https://crrev.com/03a17b1a9826e1a8dbe0f23ef37d89acb209497b/tools/perf/core/find_dependencies_unittest.py

Ok, https://luci-milo.appspot.com/buildbot/chromiumos.chromium/amd64-generic-telemetry/ is green at least so we don't have a red streak now.

achuith let's chat on monday and see if we can figure out the underlying cause of the test discrepancy?

Sure, we can do that. Please give the VM in #c27 a try - it might be trivially easy for you to figure out why it's broken.

This one should be fixed f'real at this point. Let's continue discussion re: longer term cros fix on issue 818230.