Issue 1101868 in chromium: Java checkdeps is skipped in CI

[wychen via monorail]

Status: Untriaged
Owner: ----
CC: ja...@chromium.org, dav...@chromium.org
Components: Test>Android
OS: Android
Pri: 1
Type: Bug

New issue 1101868 by wyc...@chromium.org: Java checkdeps is skipped in CI
https://bugs.chromium.org/p/chromium/issues/detail?id=1101868

Java checkdeps is only working in CQ, when Java files are touched, but not in CI. Existing DEPS violations won't be reported by CI. If a new violation somehow slip through CQ, it won't be caught later in CI.

This is noticed when doing drive-by code review (https://chromium-review.googlesource.com/c/chromium/src/+/2278178/2/chrome/android/features/tab_ui/java/DEPS#5).

This is a regression introduced in https://chromium.googlesource.com/chromium/buildtools/+/b0f94d04e26f1cdaf4d2357e0f9eeaffa894b872. That CL skips Java checkdeps when there's no Java changes, in order to make PRESUBMIT faster, and to reduce warning message spamming. However, it also accidentally skips Java checkdeps in CI because there's no diff in CI. See more context in issue 778442.

Given how expensive the prescan step is, I think the desired behavior should be:
1) For CLs with Java changes, PRESUBMIT should fail on DEPS violations introduced in the CL, and preferably ignore other DEPS violations
2) For CLs without Java changes, PRESUBMIT should skip Java prescan in order to make it fast
3) In CI, checkdeps should report all existing violations.

For 1) and 2), it is already working as intended. For 3), before we clean up existing offenders, we cannot re-enable the checking in CI. Otherwise the waterfall would be consistently red.

I hacked a locally fixed checkdeps and run a scan, and there are 133 files containing 308 illegal imports. The offenders are reasonably clustered, so the fixing work load should not be as scary as it looks.

--
You received this message because:
1. You were specifically CC'd on the issue

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

Reply to this email to add a comment or make updates.

[sheriffbot via monorail]

Comment #1 on issue 1101868 by sheriffbot: Java checkdeps is skipped in CI
https://bugs.chromium.org/p/chromium/issues/detail?id=1101868#c1

This issue hasn't been updated in the last 30 days - please update it or consider lowering its priority.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

[wnwen via monorail]

Updates:
Cc: mhe...@chromium.org wn...@chromium.org
Components: Build
Labels: BuildHealth
Status: Available

Comment #2 on issue 1101868 by wn...@chromium.org: Java checkdeps is skipped in CI
https://bugs.chromium.org/p/chromium/issues/detail?id=1101868#c2

Adding BuildHealth label and cc'ing mheikal@ as Mohamed just fixed a pydeps issue.