New issue
Advanced search Search tips

Issue 827197 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Apr 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

"gn analyze" do not take into account deps from depfiles

Project Member Reported by agrieve@chromium.org, Mar 29 2018

Issue description

Noticed this when updating proguard flags file and linux_android_rel_ng did not think there was anything that changed:

https://chromium-review.googlesource.com/c/chromium/src/+/986532
https://ci.chromium.org/buildbot/tryserver.chromium.android/linux_android_rel_ng/517390

Proguard flags can be defined on libraries and then not used until file proguard step, several targets away. Thus, they are written to .build_config files and added to the final proguard step's depfile.

Having depfiles considered by analyze is a bit tricky since they are always one build out-of-date (not written until after a build). If a bot is doing a clean build, it would have no depfiles, and no way to know that the proguard flags affect anything.

Perhaps maybe the fix here is to just mark proguard flags as inputs to java libraries even though they don't actually use them?

dpranke - does this sound like the right approach?
 
I didn't follow the "one build out-of-date" part, and I'm not really familiar with proguard at all, but, apart from that, I think, yes, it's probably correct to mark them as inputs to the relevant java libraries, if you want the library to be rebuilt (or if the .build_config file needs to be updated).
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 3 2018

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

commit b9aea9a73fd3a5e5a25fef3144f08547c7bb73df
Author: Andrew Grieve <agrieve@chromium.org>
Date: Tue Apr 03 20:02:36 2018

Android: Make proguard_configs explicit inputs for "gn analyze"

gn analyze does not use depfile information, which causes it to not know
that builds need to happen when proguard flags change.

To work around this, this change marks the proguard_configs as inputs to
(somewhat arbitrary) java actions.

Bug:  827197 
Change-Id: I1522daa0e3782ac1989cf4e234d8409663bf1357
Reviewed-on: https://chromium-review.googlesource.com/985900
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547799}
[modify] https://crrev.com/b9aea9a73fd3a5e5a25fef3144f08547c7bb73df/build/config/android/internal_rules.gni
[modify] https://crrev.com/b9aea9a73fd3a5e5a25fef3144f08547c7bb73df/build/config/android/rules.gni

Status: WontFix (was: Available)
"one build out-of-date": .d files are deps from the most recent build. The next build could have changes to .gn files, thus the .ninja files will be regenerated, but the .d files will still be from the previous build.


The immediate problem of proguard change not triggering tests is fixed with #2. I think this is just a fundamental gotcha with using depfiles for dependencies that we'll need to be mindful of.

Sign in to add a comment