Page MenuHomePhabricator

Depends-On did not block manual merging of a patch
Closed, DeclinedPublic

Description

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Translate/+/608817 got merged even though it depends on https://gerrit.wikimedia.org/r/c/translatewiki/+/608815 which is not merged at the time of writing.

Note: this is a cross-repository Depends-On, if it matters.

This would have broken translatewiki.net, but we caught this while testing on a canary.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 8 2020, 2:23 PM
Nikerabbit updated the task description. (Show Details)

I don't believe it ever stopped manual merging, just Jenkins automatic merging. The patch was manually merged by @abi_

Nikerabbit renamed this task from Depends-On did not block merging of a patch (broken by the update?) to Depends-On did not block manual merging of a patch.Jul 8 2020, 3:40 PM

I see, I guess this is a feature request then. Is there a task for that already?

hashar closed this task as Declined.Jul 8 2020, 3:55 PM
hashar added a subscriber: hashar.

TLDR: the change has been force merged by a user thus bypassing CI entirely and the cross dependency system

From mediawiki/extensions/Translate.git

git fetch origin refs/changes/17/608817/meta
git log FETCH_HEAD

The patch has been voted Code-Review +2:

commit 36221253e96b3be180109b0010d59a8ea61cb581
Author: Gerrit User 6870 <6870@e9e9afe9-4712-486d-8885-f54b72dd1951>
Date:   Mon Jul 6 11:58:04 2020 +0000

    Update patch set 3
    
    Patch Set 3: Code-Review+2
    
    Patch-set: 3
    Reviewer: Gerrit User 6870 <6870@e9e9afe9-4712-486d-8885-f54b72dd1951>
    Label: Code-Review=+2

And it then has been force submitted by @abi_ :

commit 9a5f509425251c9bacf57406714e1e689296914f
Author: Gerrit User 6870 <6870@e9e9afe9-4712-486d-8885-f54b72dd1951>
Date:   Mon Jul 6 16:39:36 2020 +0000

    Create patch set 4
    
    Change has been successfully rebased and submitted as dee13d0c87a63aa259b4ce985ed1762905bc3055 by Abijeet Patro
    
    Patch-set: 4
    Patch-set-description: Rebase
    Subject: Remove MediaWikiMisc deprecated code
    Status: merged
    Commit: dee13d0c87a63aa259b4ce985ed1762905bc3055
    Tag: autogenerated:gerrit:merged
    Groups: 73212415e35eaf6a58bf671635d6abea1bd7f82a
    Label: Verified=+2 Gerrit User 75 <75@e9e9afe9-4712-486d-8885-f54b72dd1951>
    Label: Verified=+1 Gerrit User 7819 <7819@e9e9afe9-4712-486d-8885-f54b72dd1951>
    Label: Code-Review=+1 Gerrit User 93 <93@e9e9afe9-4712-486d-8885-f54b72dd1951>
    Label: Code-Review=+2
    Label: SUBM=+1
    Submission-id: 608817-validators
    Submitted-with: OK
    Submitted-with: OK: Verified: Gerrit User 75 <75@e9e9afe9-4712-486d-8885-f54b72dd1951>
    Submitted-with: OK: Code-Review: Gerrit User 6870 <6870@e9e9afe9-4712-486d-8885-f54b72dd1951>

Since the change had a Depends-On on a change that was not merged and did not receive a CR+2, the change is known to Zuul but does not enter the pipeline at all. Hence there is no report in Gerrit. I guess that confused the user as to why the change was not processed and it went force merged thus breaking the dependency chain.

hashar added a comment.Jul 8 2020, 3:57 PM

Side note, we have a task more or less to prevent unintentional submit caused by the test pipeline voting Verified+2 and thus enabling a user to submit a change T226123 . Or we could disable submit entirely.