• 
      

    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