[llvm-dev] Phabricator sending spurious "This revision was not accepted when it landed" emails

[Alex Brachet-Mialot via llvm-dev]

Those emails say
"This revision was not accepted when it landed; it landed in state
"Needs Review".
This revision was landed with ongoing or failed builds."
I think the problem here is the part about failed builds. Harbormaster
seemingly is always complaining about failed builds. Might this be the
cause of these emails?

Alex

On Tue, Jul 21, 2020 at 6:41 PM Mehdi AMINI via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
>
>
> On Tue, Jul 21, 2020 at 11:42 AM David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>> On Tue, Jul 21, 2020 at 11:29 AM Mehdi AMINI <joker.eph at gmail.com> wrote:
>>>
>>>
>>>
>>> On Tue, Jul 21, 2020 at 11:07 AM David Blaikie <dblaikie at gmail.com> wrote:
>>>>
>>>> +Mehdi AMINI who's taking some (shared?) ownership of Phabricator these days.
>>>>
>>>> Mehdi - was Phab updated recently (such that we might've picked up new semantics)?
>>>
>>>
>>> No: I upgraded the hardware and the OS, but not Phab itself yet.
>>>
>>> I have a test instance running with an upgraded Phab though, it may have been sending duplicate emails in the last day or two when I didn't notice I had the email daemon running.
>>>
>>>>
>>>>
>>>> On Tue, Jul 21, 2020 at 4:25 AM Jay Foad via llvm-dev <llvm-dev at lists.llvm.org> wrote:
>>>>>
>>>>> Has anyone else noticed Phabricator sending emails saying:
>>>>>   This revision was not accepted when it landed; it landed in state
>>>>> "Needs Review".
>>>>> when the review clearly has been accepted by someone?
>>>>>
>>>>> Some recent examples:
>>>>> https://reviews.llvm.org/D83952
>>>
>>>
>>> Seems like this one closed as expected without the message? http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200720/808734.html
>>
>>
>> In my inbox I have two emails for that review.
>> (though, also, on the commits list, I do see this: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200720/808735.html - though my mail client (gmail)  is rendering this one as in the same thread/doesn't seem to show the "[Differential]" prefix in the subject - not sure what's going on there, usually gmail is /too/ ready to group mails by actual subject text, rather than by thread ids in the email hedaers... )
>
>
> Thanks! I dug a bit and found out that these came from my test instance, I ran an upgraded Phabricator instance and didn't disable polling the repository: so it closed the revision without having any approval.
>
> --
> Mehdi
>
>
>>
>>
>>>>>
>>>>> https://reviews.llvm.org/D80116
>>>
>>>
>>> Same here: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200720/808778.html
>>>
>>> Can you forward me the email you received for these revisions?
>>
>>
>> Similarly, the separate "[Differential]" email seems to be unthreaded and contains the problematic "not accepted when it landed" text: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200720/808779.html
>>
>>>
>>>
>>>
>>>>
>>>>
>>>> Hard for me to tell what happened here. I wonder if it's related to making changes after review/before committing. While that's common in LLVM, I could imagine a review tool (especially if we picked up a newer version - as I don't think it's always had this behavior) might get fussy about that - perhaps it'd be configurable, so it'd say "this was committed with extra changes" but not "This was committed without review".
>>>>
>>>> Do you have any examples that didn't have post-approval-pre-commit changes that still got this annotation about being committed without review?
>>>>
>>>>> https://reviews.llvm.org/D81267
>>>>
>>>>
>>>> Last one seems more clear - one of the reviewers (rupprecht) still had the review marked "requires changes", so it was committed without closure on that
>>>
>>>
>>> Indeed this one shows the message: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200713/807554.html
>>>
>>> --
>>> Mehdi
>>>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev