1193496 - Mozreview: The specified diff file is empty

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect, P1)

Description

[Eric Rescorla (:ekr)]

abort: reviewboard error: "The specified diff file is empty. (HTTP 400, API Error 219)". please try submitting the review again. if that doesn't work, you've likely encountered a bug.

Comment 1

[Gregory Szorc [:gps]]

Per IRC conversations, I think this is because of a try syntax empty commit somehow appearing in the stack.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b6e53369f667 is the rough equivalent of what was attempting to be pushed to mozreview.

https://reviewboard-hg.mozilla.org/gecko/rev/f8b217b03d72 is the tip of the push. https://reviewboard-hg.mozilla.org/gecko/rev/eff1dd5e5062 is likely the commit tripping it up.

I thought we looked for empty commits on the client side. If not, we should definitely do that. Searching for empty ctx.files() is easiest, since diff computation is expensive and empty files should catch most of these failures.

Comment 2

[Gregory Szorc [:gps]]

I found a bug in our empty commit detection that is the root cause of this. Looks like an easy fix.

Comment 3

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: add test demonstrating failure to detect empty commit (bug 1193496); r?smacleod

reviewboard: add test demonstrating failure to detect empty commit (bug 1193496); r?smacleod

The bug will be fixed in the subsequent commit.

Comment 4

[Gregory Szorc [:gps]]

Attachment: MozReview Request: mozreview: screen all reviewed changesets for being empty (bug 1193496); r?smacleod

mozreview: screen all reviewed changesets for being empty (bug 1193496); r?smacleod

The old code wasn't guaranteed to screen all changesets because
pushop.revs may only be populated with heads. The code immediately
following the old code constructed a list of changesets that would be
reviewed. We leverage this list to screen all to-be-reviewed changesets
as empty.

Comment 5

[Steven MacLeod [:smacleod]]

Comment on attachment 8646677 [details]
MozReview Request: reviewboard: add test demonstrating failure to detect empty commit (bug 1193496); r?smacleod

https://reviewboard.mozilla.org/r/15829/#review14659

Ship It!

Comment 6

[Steven MacLeod [:smacleod]]

Comment on attachment 8646678 [details]
MozReview Request: mozreview: screen all reviewed changesets for being empty (bug 1193496); r?smacleod

https://reviewboard.mozilla.org/r/15831/#review14661

Ship It!

Comment 7

[Gregory Szorc [:gps]]

url:        https://hg.mozilla.org/hgcustom/version-control-tools/rev/694fc9bb0ef0861ebd3f96672fad490fef1655ed
changeset:  694fc9bb0ef0861ebd3f96672fad490fef1655ed
user:       Gregory Szorc <[e-mail]>
date:       Tue Aug 18 14:10:41 2015 -0700
description:
reviewboard: add test demonstrating failure to detect empty commit (bug 1193496); r=smacleod

The bug will be fixed in the subsequent commit.

url:        https://hg.mozilla.org/hgcustom/version-control-tools/rev/1de7b45326e24be00566a093ac972ebc324f0b93
changeset:  1de7b45326e24be00566a093ac972ebc324f0b93
user:       Gregory Szorc <[e-mail]>
date:       Tue Aug 18 14:10:49 2015 -0700
description:
mozreview: screen all reviewed changesets for being empty (bug 1193496); r=smacleod

The old code wasn't guaranteed to screen all changesets because
pushop.revs may only be populated with heads. The code immediately
following the old code constructed a list of changesets that would be
reviewed. We leverage this list to screen all to-be-reviewed changesets
as empty.