Preserve the order of uploaded files for diff generation.

Review Request #13999 — Created June 25, 2024 and submitted — Latest diff uploaded

Information

Review Board
release-7.x

Reviewers

When processing uploaded patches, we were sorting them for display
purposes, trying to sort header files before implementation files for
languages like C, C++, and Objective-C.

Pre-sorting these files caused issues with certain types of diffs.
Depending on how a diff was parsed, critical header metadata could end
up being moved lower in a file. This was particularly a problem with
Mercurial, when using header/implementation sorting.

We no longer pre-sort files. Instead, we do this when sorting a list of
filediffs, which happens on demand.

The approach to sorting is different than what we had in the past, to
account for this. Previously, we had a two-stage process: 1) Build a key
for each FileDiff, 2) attempt to sort based on equality of base paths,
base names, and file extensions.

This didn't work particularly well, leading to an inconsistent sorting
when comparing file extensions. Instead, we now leave it to cmp() to
handle the sorting, and put all the smarts into key-building, by
returning a tuple of (base_path, base_name, order_flag, ext). This
helps us leverage the complete smarts of sorting and eases the work on
our end.

These fixes only affect newly-uploaded files. It should have no impact
on any existing diffs, or on comments, at least nothing surface-level.
It will mean that file IDs in diffs won't be in numeric order within a
given displayed diff anymore, but that should be an implementation
detail.

We will want to spend time smoke-testing this, and will schedule it for
7.0.2.

Unit tests pass.

Commits

Files