Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/build/cmd/gerritbot: incorrectly attempts to close a cherry-pick CL #40151

Closed
dmitshur opened this issue Jul 10, 2020 · 2 comments
Closed

x/build/cmd/gerritbot: incorrectly attempts to close a cherry-pick CL #40151

dmitshur opened this issue Jul 10, 2020 · 2 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Contributor

Consider a Gerrit CL that was imported from a GitHub PR, and merged (or closed without merging). For example, see PR #39821.

It's possible that a cherry-pick of that imported Gerrit CL will be made. For example, see CL 240343.

The cherry-pick CL will include the line "GitHub-Pull-Request: golang/go#​39821" quoted from the original CL.

This confuses gerritbot into thinking the backport CL (into another branch) is the same as the original PR. Since the original PR was merged (aka closed), now that #23850 is fixed (/cc @stamblerre), gerritbot attempts to abandon the cherry-pick CL. Fortunately, it fails:

2020/07/10 15:23:59 abandonCL: calling AbandonChange(ctx, "I79f2ad7c836a8a46569f603aca583fdd526d22dc", "GitHub PR golang/go#39821 has been closed.") for CL https://golang.org/cl/240343
2020/07/10 15:23:59 abandonCL(ctx, 240343, "golang/go#39821"): HTTP status 404 Not Found; Multiple changes found for I79f2ad7c836a8a46569f603aca583fdd526d22dc

Things might be different if the cherry-pick was made before the original CL was merged. In either case, gerritbot shouldn't be trying to take this incorrect action. It's not a high priority since the disruption this causes at the moment doesn't seem to be large.

/cc @golang/osp-team

@dmitshur dmitshur added Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 10, 2020
@dmitshur dmitshur added this to the Unreleased milestone Jul 10, 2020
@dmitshur
Copy link
Contributor Author

dmitshur commented Jul 10, 2020

Issue #25020 (/cc @ALTree) might be relevant here. If GerritBot indeed doesn't support branches other than master, then the fix for this issue will be very simple: it should not consider a Gerrit CL targeting a branch other than master as something that should be abandoned.

Edit: Issue #30037 is a better point of reference. It confirms that GerritBot cannot be used to create CLs into branches other than master at this time, so supporting those isn't a high priority.

@gopherbot
Copy link

Change https://golang.org/cl/242157 mentions this issue: cmd/gerritbot: don't abandon CLs for branches other than master

@golang golang locked and limited conversation to collaborators Jul 13, 2021
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

2 participants