Here's another link to another failing build with the same problem, but on a different builder and from a different CL:

https://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-generic_chromium_compile_only_ng/builds/278845

It doesn't look like this problem is happening on the main waterfalls, so it's unclear to me if there were just a few CLs that underspecified dependencies that we are "correctly" catching, or if this is the result of flakiness already in the committed code (and we know there's some of this) that is just manifesting here.

Ikuta-san, since this is perhaps largely fallout from your changes to the way we do compiles w/ goma, can you work on this?

The relevant code is here, I think: 

https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/chromium/api.py?rcl=cb4c17a892538ffea98b5ad4171f06e310a25df1&l=183

One obvious question: why doesn't raising self.m.step.StepFailure() result in a red step? Related question: is there a way we can actually mark the step that we want to consider as failed (the call on line 183)?

So the way that steps in recipes work is roughly this:

  * run the actual command
  * check it's exit code
    * if the exit code isn't in the `ok_ret` set:
      * change the step's presentation.status to FAILURE
      * raise StepFailure (or InfraFailure if it's an infra_step)

Non-obviously, that logic lives in the engine, right about here: https://github.com/luci/recipes-py/blob/master/recipe_engine/run.py#L403

(we should probably move this logic into the step module itself so that it's closer to user-space and the behavior is more obvious)


However, realizing that this is all that a failing step actually is, the recipe can be modified to give a better representation of what's actually happening by doing something like:

  second_compile = api.step(....)
  if not was_no_op(second_compile):
    second_compile.presentation.status = api.step.FAILURE
    second_compile.presentation.step_text = "This should have been a no-op, but it wasn't!"
    raise api.step.StepFailure("no-op compile wasn't a no-op!")

I think this used to work fine in the compile.py days. I don't understand recipes, so I can't help, sorry.

@thakis - Yes, it did work fine in the compile.py days, and I'm not expecting you to fix this, but mostly just wanted to make you aware of the current issue so you didn't also discover it and re-report it :)

The exit code of `ninja -d explain -n` is 0 even if code tree is dirty. So "compile confirm no-op" looks succeeded.

Which fix is better?
1. check the output of `ninja -d explain -n`
2. `ninja -d explain -n` returns 1 when dirty

We used to do 1; it worked.

note: Unfortunately tikuta is ooo today.
if it's easy and in hurry, I will pay his debt, though...

I don't think this has to be fixed today.

I fix this issue following iannucci's solution.

https://chromium-review.googlesource.com/c/440744/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/0f07e03ad937f9e01a1c9f19300623c470b48ab2

commit 0f07e03ad937f9e01a1c9f19300623c470b48ab2
Author: Takuto Ikuta <tikuta@google.com>
Date: Fri Feb 10 02:39:18 2017

Make sure to fail if build dependency is broken

BUG= 690269 

Change-Id: Id13745b85a981fa54bc042c6eb85a757a84b755a
Reviewed-on: https://chromium-review.googlesource.com/440744
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>

[modify] https://crrev.com/0f07e03ad937f9e01a1c9f19300623c470b48ab2/scripts/slave/recipe_modules/chromium/example.py
[add] https://crrev.com/0f07e03ad937f9e01a1c9f19300623c470b48ab2/scripts/slave/recipe_modules/chromium/example.expected/basic_out_dir_ninja_no_op_failure.json
[modify] https://crrev.com/0f07e03ad937f9e01a1c9f19300623c470b48ab2/scripts/slave/recipe_modules/chromium/api.py

Just saw this failure on a change which just deletes a bunch of files:

https://codereview.chromium.org/2694113002
https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/365646

Is this check flaky?

Not normally, no.

Second run of the bot passed, so something flaky is happening :(

I think it has false-positive.

if build rules miss some dependency, for example
 obj/chrome/browser/test_support_ui/web_ui_test_handler.obj doesn't have explicit dependency on gen/third_party/WebKit/public/web/window_features.mojom.h

there would be race between
 1) compiling web_ui_test_handler.obj
 2) generating window_features.mojom.h

if 1 happens before 2, it will report error like
output obj/chrome/browser/test_support_ui/web_ui_test_handler.obj older than most recent input gen/third_party/WebKit/public/web/window_features.mojom.h (508733603 vs 508733645)

if 2 happens before 1, nothing will be reported.

note: in incremental build, ninja knows such dependency from deps generated by previous build, so such error may not be reported.

That's a true positive, right? It means there's a missing dep, and because of that work might happen in the second build. It's not deterministic as you say, but it's due to a real bug (a missing dep) that needs fixing.

I agree w/ thakis, identifying those cases is exactly the point.

I also agree with thakis to fix the issue.

anyway, could ninja have a tool that reports an error when it detects generated targets are found not in explicit (direct or indirect) deps, but found only in implicit deps?

iannucci's solution looks worked on buildbot.
Thank you for your instruction!

Let me update status to fixed.