circular dependent cl keep canceling each other out |
||
Issue descriptionhttps://chromium-review.googlesource.com/#/c/344933 and https://chromium-review.googlesource.com/#/c/344809 Those two cls depend on each other but unfortunately keep canceling each other out. Here's a timeline of what happens: 2:53 - both cls are marked CQ+1 2:54 - on 344933: The Pre-Commit Queue failed to submit your change in https://uberchromegw.corp.google.com/i/chromeos/builders/pre-cq-launcher/builds/7020 . CL:344933 depends on CL:344809, which was rejected by the CQ. 3:23 - on 344809: The Pre-Commit Queue failed to apply your change in https://uberchromegw.corp.google.com/i/chromeos/builders/pre-cq-launcher/builds/7020 . CL:344809 depends on CL:344933, which is not marked Commit-Queue>=+1. Seems like it held stale rejection info on 344809?
,
May 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/8c1efbf59d5475399d118a683b6b987fdef6ee07 commit 8c1efbf59d5475399d118a683b6b987fdef6ee07 Author: Aviv Keshet <akeshet@chromium.org> Date: Thu May 26 23:36:01 2016 sync_stages: add some logging to pre-cq launcher BUG= chromium:615244 TEST=None Change-Id: I8a1fe5cfd25b4e2154ed7abdf5cf5599243b398b Reviewed-on: https://chromium-review.googlesource.com/347698 Reviewed-by: Kevin Cheng <kevcheng@chromium.org> Tested-by: Aviv Keshet <akeshet@chromium.org> [modify] https://crrev.com/8c1efbf59d5475399d118a683b6b987fdef6ee07/cbuildbot/stages/sync_stages.py
,
May 27 2016
Note -- https://chromium-review.googlesource.com/#/c/344933 is targeted at a branch, not at master. I wonder if that is the problem or is tickling it (since I'm pretty sure pre-cq works otherwise on closed dependency graphs).
,
May 27 2016
I'm sure that's it. What can we reasonably do to improve our error messages in this case?
,
May 27 2016
Yeah... sorry, my bad. The official master branch for coreboot changed just a few days ago and I hadn't resynced my manifest yet. That was probably it (but it looks like I'll have to do a bunch of rebasing to confirm, so I won't know until tomorrow). Sorry for the noise.
,
May 27 2016
My theory -- the bad logging is coming from validation_pool:471 "raise dep_change.GetMergeException() or PatchRejected(dep_change)" in the case of a patch that returns None from GetMergeException (such as that wrong-branch patch that nevertheless had all the right labels/approvals) we assume it must be some generic "was rejected" reason and raise a failure with the "was rejected by the CQ" message. That seems wrong. We need some check up front to first handle patches for the wrong manifest, or to understand why we even got to this line of validation_pool.
,
May 27 2016
We get to that line because of validation_pool:467 elif limit_to is not None and dep_change not in limit_to: The wrong-branch change is not in limit_to, probably, because of some filtering that takes on wrong branch changes.
,
May 27 2016
The easiest way I can think of to fix this is to just have a better log message emitted by that line (rather than trying to fix it before we even get there). See https://chromium-review.googlesource.com/#/c/347951/
,
May 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/a26b17af27ba6ae82adc03b010b8b227a29bd228 commit a26b17af27ba6ae82adc03b010b8b227a29bd228 Author: Aviv Keshet <akeshet@chromium.org> Date: Fri May 27 17:54:12 2016 validation_pool: clearer logging for ineligible changes This CL address previously unclear logging message "was rejected by the CQ" that was given as the failure reason for patches in a variety of cases that were not really caused by being rejected by the CQ (in particular: when encountering a wrong-manifest change in the dependency graph of a change in the cq or pre-cq). BUG= chromium:615244 TEST=existing unit tests pass Change-Id: I82036266ab98f28d75edab72b428a6639471c1c6 Reviewed-on: https://chromium-review.googlesource.com/347951 Commit-Ready: Aviv Keshet <akeshet@chromium.org> Tested-by: Aviv Keshet <akeshet@chromium.org> Reviewed-by: Don Garrett <dgarrett@chromium.org> Reviewed-by: Kevin Cheng <kevcheng@chromium.org> [modify] https://crrev.com/a26b17af27ba6ae82adc03b010b8b227a29bd228/cbuildbot/validation_pool.py
,
May 31 2016
Looks fixed to me.
,
Nov 14 2016
|
||
►
Sign in to add a comment |
||
Comment 1 by akes...@chromium.org
, May 26 2016Owner: akes...@chromium.org
Status: Started (was: Untriaged)