Open Bug 1466381 Opened 3 years ago Updated 1 year ago

Some pushlog ranges don't work

Categories

(Developer Services :: Mercurial: Pushlog, defect, P5)

defect

Tracking

(Not tracked)

People

(Reporter: jorgk-bmo, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: in-triage)

Attachments

(1 obsolete file)

Summary: Some pushlog ranged don't work → Some pushlog ranges don't work
Component: Mercurial: hg.mozilla.org → Mercurial: Pushlog
Keywords: in-triage
Assignee: nobody → sheehan
From our team meeting, this is likely an issue with either the bound conditions on the revision range, or related to the revisions coming form the same push. I'll try and find the edge case shortly.
Status: NEW → ASSIGNED
When a pushlog revision-to-revision query includes a set of changes from the same push, the query returns an empty result. This is because the "where" clause on the underlying sqlite query is inclusive on on end and exclusive on the other (ie [a, b) in the corresponding mathematical notation). This is frustrating for developers who are trying to look for regressions between two commits that occurred on the same push.

This commit changes the endpoints of changeset queries to be inclusive on both sides (ie [a, b]).
The problem here is that due to the way we land changes on different branches it's very common for changes that landed separately on an integration branch (inbound or autoland) to be pushed together to mozilla-central (by a sheriff doing a merge).

More concretely: clicking the first two links in comment 0 will show that they both have "push id	34083", so they were pushed together to mozilla-central. Editing the URLs to point to inbound instead gives:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b9d55ec3055
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f62ecdf59b6

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3b9d55ec3055&tochange=1f62ecdf59b6

...which shows what looks like a plausible range of changesets.

This is a lousy user experience, I agree. With the benefit of hindsight I think the pushlog ought to be aware of all repositories that changesets get pushed to, so that for any given changeset you could see when it was pushed to any other repo, like "Pushed to mozilla-inbound on date X in push 123. Pushed to mozilla-central on date Y in push 456."

I'm not 100% sure how you'd manage queries against the pushlog in this scenario, but it would at least be feasible.
Attachment #8985412 - Attachment is obsolete: true
Flags: needinfo?(sheehan)
Flags: needinfo?(lars)

When I took a stab at fixing this originally, I tried changing the SQL query to be inclusive on both the start and end push in a range. It was pointed out to me in review that this would change the results of over a decade of pushlog query links in bug comments, mailing list posts, etc. So making the change in such a way that it changes the pushlog query is out of the question.

The alternative proposed by Ted is to extend pushlog so that it is aware of all repos a changeset can be pushed to, essentially performing the "swap mozilla-central for integration/autoland" trick automatically. This is an interesting idea, but it isn't something I have the time to implement right now, and probably won't in the near future.

Jorg, if you have any ideas on how we can fix this bug without breaking existing links and without having to redesign the UX, I'd be happy to hear them.

Assignee: sheehan → nobody
Blocks: pushlogux
Severity: critical → normal
Status: ASSIGNED → NEW
Flags: needinfo?(sheehan)
Flags: needinfo?(lars)
Priority: -- → P5

I really have no clue here. I also don't understand the problem. To me, we're querying into a continuous list of changesets, which are of course grouped into "push groups" onto mozilla-central. Why does it matter where changesets were coming from, autoland or inbound, when we're querying M-C? I don't understand that if I have a list from 1 to 200, why I can get a sublist of 50 to 150, but not 50 to 60, even if items were added to the list in chunks of 1-100 and 101-200. But obviously I don't understand how the pushlog is organised. Sorry about the ignorant reply.

You need to log in before you can comment on or make changes to this bug.