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

Broken handling of backouts affecting multiple bugs #1904

Open
jgraham opened this issue Jul 6, 2023 · 2 comments · May be fixed by #1933
Open

Broken handling of backouts affecting multiple bugs #1904

jgraham opened this issue Jul 6, 2023 · 2 comments · May be fixed by #1933
Assignees

Comments

@jgraham
Copy link
Member

jgraham commented Jul 6, 2023

When handling backouts we try to extract a list of all the bugs/commits that they touch, lookup the sync for each commit, and apply the backout to that commit. It seems that once we've applied the backout to a sync we won't apply it to further syncs.

This is rather broken if a backout touches multiple wpt bugs. Instead of directly applying the actual backout, if we have a set of commits that were backed out, potentially belonging to multiple bugs/syncs, for handling the backout we should construct a corresponding set of web-platform-tests commits as reverts for the original commits, rather than just from applying the backout commit itself.

@jgraham
Copy link
Member Author

jgraham commented Jul 6, 2023

I think this is what caused the confusion in https://bugzilla.mozilla.org/show_bug.cgi?id=1824242

@jgraham
Copy link
Member Author

jgraham commented Jul 6, 2023

@lutien This might be one you want to look at when you have a chance. I think my description is probably not very clear, so let me know if you have questions.

To expand it a little, let's imagine we have bugs 1 and 2 with some wpt-touching commits like so:

Commit C1 - Bug 1
Commit C2 - Bug 1
Commit C3 - Bug 2

That creates two upstream syncs; S1 for Bug 1 with commits C1 and C2, and S2 for Bug 2 with commit C3.

Then we get a backout commit for all of them

Commit B1 - reverts C1 C2 C3, Bug 1 Bug 2

In the sync we're currently applying all the parts of B1 that touch web-platform-tests to (I think one or the other of) S1/S2. But what we should instead do when we see B1 is apply a commit to S1 that specifically reverts the changes in C1 and C2, and one to S2 that specifically reverts the changes in C3. That then just relies on the fact that things in backouts really are just reverts and don't introduce additional changes (one might worry about merge conflict resolution, but I think in practice that isn't a problem in m-c).

Off the top of my head, I'm not totally sure if one gecko commit applying to multiple upstream PRs will cause data model problems. I don't think so, but I may have forgotten something.

@lutien lutien self-assigned this Jul 6, 2023
@lutien lutien linked a pull request Jul 26, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants