Fix ordering of commits in the network graph.

What does this MR do?

• We upgraded rugged to 0.25.1.1 in !10286 (merged) for %9.1

• Prior to this upgrade, the default sort order for commits returned by Gitlab::Git::Repository#find_commits was Rugged::SORT_DATE, which the graph relied on.

• While upgrading rugged, the MR also changed this default to Rugged::SORT_NONE, which broke commit ordering in the graph.

• This MR adds an option to Gitlab::Git::Repository#find_commits to sort by date, and changes the graph builder Network::Graph so it explictly requests the :date sort order

What are the relevant issue numbers?

Closes #30973 (closed)

Tasks

• Investigation
• Implementation
• Tests
  • Added
    • Add more tests for Network::Graph
    • Compare results of this fix v/s 9.0 and confirm that there are no changes
  • Passing
• Meta
  • CHANGELOG entry created
  • API support added
  • Branch has no merge conflicts with master
  • Squashed related commits together
  • Added screenshots
  • Check for clean merge with EE
  • Documentation added/updated
• Review
  • Reviewer
  • Maintainer
• Wait for merge

marked the checklist item Implementation as completed

mentioned in commit 33b2e089

added 2 commits

• 91dfc1bb - Fix ordering of commits in the network graph.
• 33b2e089 - Add CHANGELOG entry for !10936 (merged)

Compare with previous version

The relevant commit from !10286 (merged) is https://gitlab.com/gitlab-org/gitlab-ce/commit/91e150266adeeaa60c8626a3dd1ddc467347744f. I explicitly changed it, because in a new version of rugged (and precisely libgit2) the implementation of Rugged::SORT_DATE changed and our tests were failing. I described it here.

I hope that for the network graph Rugged::SORT_DATE will produce the same results as in the previous version of rugged.

And thanks for taking this bug @timothyandrew

assigned to @adamniedzielski

The specs for the Network graph are pretty sparse. I think we need to add a spec to make sure that it is generating the right JSON data.

@adamniedzielski: The libgit2 change you mention in 91e15026 only concerns SORT_NONE and SORT_TOPO, and doesn't seem to touch SORT_DATE. I'm sure I'm missing something here, though.

I hope that for the network graph Rugged::SORT_DATE will produce the same results as in the previous version of rugged.

I changed the strategy for the network graph to SORT_DATE, and eyeballed it against a network graph from 9.0, and it all looked the same. I can run a more extensive comparison, just to be sure. (Edit: I compared the JSON outputs for the network graphs on 9.0.0 and this branch, and I can confirm that the diff is empty between them)

The specs for the Network graph are pretty sparse. I think we need to add a spec to make sure that it is generating the right JSON data.

@stanhu: Agreed!

marked the checklist item Added as incomplete

mentioned in commit 6fdf4ba9

added 2 commits

• df3ded83 - Fix ordering of commits in the network graph.
• 6fdf4ba9 - Add CHANGELOG entry for !10936 (merged)

Compare with previous version

marked the checklist item Added as completed

marked the checklist item Add more tests for Network::Graph as completed

marked the checklist item Check for clean merge with EE as completed

marked the checklist item Meta as completed

Great, thanks for jumping on this quickly, @timothyandrew!

@jameslopez: Could you take the initial review for this MR? Please feel free to reassign!

assigned to @jameslopez

@timothyandrew looking I think we don't need Next Patch Release here since it's used mainly for issues.

removed Next Patch Release label

@timothyandrew LGTM! left a couple of minor comments. Thanks!

assigned to @timothyandrew

resolved all discussions

mentioned in commit 3f1c7d85

Thanks for the review, @jameslopez!

@DouweM: Could you take care of the final review for this MR?

assigned to @DouweM

added 2 commits

• a7e67604 - Fix ordering of commits in the network graph.
• 3f1c7d85 - Add CHANGELOG entry for !10936 (merged)

Compare with previous version

approved this merge request

enabled an automatic merge when the pipeline for 3f1c7d85 succeeds

merged

mentioned in commit 4d4aefc5

Picked into 9-1-stable, will go into 9.1.2

mentioned in commit c6b13536

mentioned in issue #30973 (closed)

mentioned in merge request !11057 (merged)

mentioned in issue #31939 (moved)

mentioned in issue gitlab#9696