henry pushed to branch base-browser-146.0a1-16.0-2 at The Tor Project / Applications / Tor Browser

Commits:

1 changed file:

Changes:

  • tools/base_browser/tb-dev
    ... ... @@ -6,6 +6,7 @@ Useful tools for working on tor-browser repository.
    6 6
    7 7 
     import argparse
    8 8 
     import atexit
    9 
    +import functools
    9 10 
     import json
    10 11 
     import os
    11 12 
     import re
    ... ... @@ -14,8 +15,15 @@ import sys
    14 15 
     import tempfile
    15 16 
     import termios
    16 17 
     import urllib.request
    18 
    +from collections.abc import Callable, Iterable, Iterator
    19 
    +from types import ModuleType
    20 
    +from typing import Any, NotRequired, TypedDict, TypeVar
    17 21
    18 
    -import argcomplete
    22 
    +argcomplete: None | ModuleType = None
    23 
    +try:
    24 
    +    import argcomplete
    25 
    +except ImportError:
    26 
    +    pass
    19 27
    20 28 
     GIT_PATH = "/usr/bin/git"
    21 29 
     UPSTREAM_URLS = {
    ... ... @@ -36,9 +44,14 @@ class TbDevException(Exception):
    36 44 
         pass
    37 45
    38 46
    39 
    -def git_run(args, check=True, env=None):
    47 
    +def git_run(
    48 
    +    args: list[str], check: bool = True, env: None | dict[str, str] = None
    49 
    +) -> None:
    40 50 
         """
    41 51 
         Run a git command with output sent to stdout.
    52 
    +    :param args: The arguments to pass to git.
    53 
    +    :param check: Whether to check for success.
    54 
    +    :param env: Optional environment to set.
    42 55 
         """
    43 56 
         if env is not None:
    44 57 
             tmp_env = dict(os.environ)
    ... ... @@ -51,46 +64,122 @@ def git_run(args, check=True, env=None):
    51 64 
             raise TbDevException(str(err)) from err
    52 65
    53 66
    54 
    -def git_get(args):
    67 
    +def git_run_pager(
    68 
    +    args: list[str] | None = None,
    69 
    +    arg_sequence: Iterable[list[str]] | None = None,
    70 
    +    pager_prefix: None | str = None,
    71 
    +) -> None:
    55 72 
         """
    56 
    -    Run a git command with each non-empty line returned in a list.
    73 
    +    Run a sequence of git commands with the output concatenated and sent to the
    74 
    +    git pager.
    75 
    +    :param args: The arguments to pass to git, or `None` if a sequence is desired.
    76 
    +    :param arg_sequence: A sequence representing several git commands.
    77 
    +    :param pager_prefix: An optional text to send to the pager first.
    78 
    +    """
    79 
    +    if arg_sequence is None:
    80 
    +        if args is not None:
    81 
    +            arg_sequence = (args,)
    82 
    +        else:
    83 
    +            raise ValueError("Missing `arg_sequence` or `args`")
    84 
    +    elif args is not None:
    85 
    +        raise ValueError("Unexpected both args and arg_sequence")
    86 
    +
    87 
    +    pager = git_get(["var", "GIT_PAGER"])
    88 
    +    if not pager:
    89 
    +        raise TbDevException("Missing a GIT_PAGER")
    90 
    +    command = [pager]
    91 
    +    if os.path.basename(pager) == "less":
    92 
    +        # Show colours.
    93 
    +        command.append("-R")
    94 
    +
    95 
    +    pager_process = subprocess.Popen(command, stdin=subprocess.PIPE, text=True)
    96 
    +    assert pager_process.stdin is not None
    97 
    +
    98 
    +    if pager_prefix is not None:
    99 
    +        pager_process.stdin.write(pager_prefix)
    100 
    +        pager_process.stdin.flush()
    101 
    +
    102 
    +    for git_args in arg_sequence:
    103 
    +        subprocess.run(
    104 
    +            [GIT_PATH, "--no-pager", *git_args], check=False, stdout=pager_process.stdin
    105 
    +        )
    106 
    +
    107 
    +    pager_process.stdin.close()
    108 
    +
    109 
    +    status = pager_process.wait()
    110 
    +    if status != 0:
    111 
    +        raise TbDevException(f"git pager {pager} exited with status {status}")
    112 
    +
    113 
    +
    114 
    +def git_get(args: list[str], strip: bool = True, check: bool = True) -> str:
    115 
    +    """
    116 
    +    Return the output from a git command.
    117 
    +    :param args: The arguments to send to git.
    118 
    +    :param strip: Whether to strip the whitespace from the output.
    119 
    +    :param check: Whether to check for success.
    120 
    +    :returns: The stdout.
    57 121 
         """
    58 122 
         try:
    59 123 
             git_process = subprocess.run(
    60 
    -            [GIT_PATH, *args], text=True, stdout=subprocess.PIPE, check=True
    124 
    +            [GIT_PATH, *args], text=True, stdout=subprocess.PIPE, check=check
    61 125 
             )
    62 126 
         except subprocess.CalledProcessError as err:
    63 127 
             raise TbDevException(str(err)) from err
    64 
    -    return [line for line in git_process.stdout.split("\n") if line]
    128 
    +    ret = git_process.stdout
    129 
    +    if strip:
    130 
    +        ret = ret.strip()
    131 
    +    return ret
    65 132
    66 133
    67 
    -local_root = None
    134 
    +def git_lines(args: list[str]) -> Iterator[str]:
    135 
    +    """
    136 
    +    Yields the non-empty lines returned by the git command.
    137 
    +    :param args: The arguments to send to git.
    138 
    +    :yield: The lines.
    139 
    +    """
    140 
    +    for line in git_get(args, strip=False).split("\n"):
    141 
    +        if not line:
    142 
    +            continue
    143 
    +        yield line
    144 
    +
    145 
    +
    146 
    +def git_path_args(path_iter: Iterable[str]) -> Iterator[str]:
    147 
    +    """
    148 
    +    Generate the trailing arguments to specify paths in git commands, includes
    149 
    +    the "--" separator just before the paths.
    150 
    +    :param path_iter: The paths that should be passed in.
    151 
    +    :yields: The git arguments.
    152 
    +    """
    153 
    +    yield "--"
    154 
    +    for path in path_iter:
    155 
    +        yield f":(literal){path}"
    68 156
    69 157
    70 
    -def get_local_root():
    158 
    +@functools.cache
    159 
    +def get_local_root() -> str:
    71 160 
         """
    72 161 
         Get the path for the tor-browser root directory.
    162 
    +    :returns: The local root.
    73 163 
         """
    74 
    -    global local_root
    75 
    -    if local_root is None:
    76 
    -        try:
    77 
    -            # Make sure we have a matching remote in this git repository.
    78 
    -            if get_upstream_details()["is-browser-repo"]:
    79 
    -                local_root = git_get(["rev-parse", "--show-toplevel"])[0]
    80 
    -            else:
    81 
    -                local_root = ""
    82 
    -        except TbDevException:
    83 
    -            local_root = ""
    84 
    -    return local_root
    164 
    +    try:
    165 
    +        # Make sure we have a matching remote in this git repository.
    166 
    +        if get_upstream_details()["is-browser-repo"] == "True":
    167 
    +            return git_get(["rev-parse", "--show-toplevel"])
    168 
    +        else:
    169 
    +            return ""
    170 
    +    except TbDevException:
    171 
    +        return ""
    85 172
    86 173
    87 
    -def determine_upstream_details():
    174 
    +@functools.cache
    175 
    +def get_upstream_details() -> dict[str, str]:
    88 176 
         """
    89 
    -    Determine details about the upstream.
    177 
    +    Get details about the upstream repository.
    178 
    +    :returns: The details.
    90 179 
         """
    91 180 
         remote_urls = {
    92 
    -        remote: git_get(["remote", "get-url", remote])[0]
    93 
    -        for remote in git_get(["remote"])
    181 
    +        remote: git_get(["remote", "get-url", remote])
    182 
    +        for remote in git_lines(["remote"])
    94 183 
         }
    95 184
    96 185 
         matches = {
    ... ... @@ -102,7 +191,7 @@ def determine_upstream_details():
    102 191 
         }
    103 192
    104 193 
         is_browser_repo = len(matches) > 0
    105 
    -    details = {"is-browser-repo": is_browser_repo}
    194 
    +    details = {"is-browser-repo": str(is_browser_repo)}
    106 195
    107 196 
         origin_remote_repo = matches.get("origin", None)
    108 197 
         upstream_remote_repo = matches.get("upstream", None)
    ... ... @@ -125,31 +214,30 @@ def determine_upstream_details():
    125 214 
         return details
    126 215
    127 216
    128 
    -cached_upstream_details = None
    129 
    -
    130 
    -
    131 
    -def get_upstream_details():
    132 
    -    """
    133 
    -    Get details about the upstream repository.
    134 
    -    """
    135 
    -    global cached_upstream_details
    136 
    -    if cached_upstream_details is None:
    137 
    -        cached_upstream_details = determine_upstream_details()
    138 
    -    return cached_upstream_details
    139 
    -
    140 
    -
    141 217 
     class Reference:
    142 218 
         """Represents a git reference to a commit."""
    143 219
    144 
    -    def __init__(self, name, commit):
    145 
    -        self.name = name
    220 
    +    _REFS_REGEX = re.compile(r"refs/[a-z]+/")
    221 
    +
    222 
    +    def __init__(self, full_name: str, commit: str) -> None:
    223 
    +        """
    224 
    +        :param full_name: The full reference name. E.g. "refs/tags/MyTag".
    225 
    +        :param commit: The commit hash for the commit this reference points to.
    226 
    +        """
    227 
    +        match = self.__class__._REFS_REGEX.match(full_name)
    228 
    +        if not match:
    229 
    +            raise ValueError(f"Invalid reference name {full_name}")
    230 
    +        self.full_name = full_name
    231 
    +        self.name = full_name[match.end() :]
    146 232 
             self.commit = commit
    147 233
    148 234
    149 
    -def get_refs(ref_type, name_start):
    235 
    +def get_refs(ref_type: str, name_start: str) -> Iterator[Reference]:
    150 236 
         """
    151 
    -    Get a list of references that match the given 'ref_type' ("tag" or "remote"
    152 
    -    or "head") that starts with the given 'name_start'.
    237 
    +    Get a list of references that match the given conditions.
    238 
    +    :param ref_type: The ref type to search for ("tag" or "remote" or "head").
    239 
    +    :param name_start: The ref name start to match against.
    240 
    +    :yield: The matching references.
    153 241 
         """
    154 242 
         if ref_type == "tag":
    155 243 
             ref_start = "refs/tags/"
    ... ... @@ -163,56 +251,83 @@ def get_refs(ref_type, name_start):
    163 251 
         fstring = "%(*objectname),%(objectname),%(refname)"
    164 252 
         pattern = f"{ref_start}{name_start}**"
    165 253
    166 
    -    def line_to_ref(line):
    254 
    +    def line_to_ref(line: str) -> Reference:
    167 255 
             [objectname_reference, objectname, ref_name] = line.split(",", 2)
    168 256 
             # For annotated tags, the objectname_reference is non-empty and points
    169 257 
             # to an actual commit.
    170 258 
             # For remotes, heads and lightweight tags, the objectname_reference will
    171 259 
             # be empty and objectname will point directly to the commit.
    172 
    -        return Reference(
    173 
    -            ref_name.replace(ref_start, "", 1), objectname_reference or objectname
    174 
    -        )
    260 
    +        return Reference(ref_name, objectname_reference or objectname)
    175 261
    176 
    -    return [
    262 
    +    return (
    177 263 
             line_to_ref(line)
    178 
    -        for line in git_get(["for-each-ref", f"--format={fstring}", pattern])
    179 
    -    ]
    264 
    +        for line in git_lines(["for-each-ref", f"--format={fstring}", pattern])
    265 
    +    )
    180 266
    181 267
    182 
    -def get_nearest_ref(ref_type, name_start, search_from):
    268 
    +def get_firefox_ref(search_from: str) -> Reference:
    183 269 
         """
    184 
    -    Search backwards from the 'search_from' commit to find the first commit
    185 
    -    that matches the given 'ref_type' that starts with the given 'name_start'.
    270 
    +    Search for the commit that comes from firefox.
    271 
    +    :param search_from: The commit to search backwards from.
    272 
    +    :returns: The firefox reference.
    186 273 
         """
    187 
    -    ref_list = get_refs(ref_type, name_start)
    274 
    +    # Only search a limited history that should include the FIREFOX_ tag.
    275 
    +    search_commits = [c for c in git_lines(["rev-list", "-1000", search_from])]
    276 
    +
    277 
    +    firefox_tag_prefix = "FIREFOX_"
    188 278
    189 
    -    for commit in git_get(["rev-list", "-1000", search_from]):
    190 
    -        for ref in ref_list:
    279 
    +    existing_tags = list(get_refs("tag", firefox_tag_prefix))
    280 
    +    for commit in search_commits:
    281 
    +        for ref in existing_tags:
    191 282 
                 if commit == ref.commit:
    192 283 
                     return ref
    193 284
    194 
    -    raise TbDevException(f"No {name_start} commit found in the last 1000 commits")
    195 
    -
    196 
    -
    197 
    -def get_firefox_ref(search_from):
    285 
    +    # Might just need to fetch tags from the remote.
    286 
    +    upstream = get_upstream_details().get("remote", None)
    287 
    +    if upstream:
    288 
    +        remote_ref: None | Reference = None
    289 
    +        search_index = len(search_commits)
    290 
    +        # Search the remote for a tag that is in our history.
    291 
    +        # We want to avoid triggering a long fetch, so we just want to grab the
    292 
    +        # tag that already points to a commit in our history.
    293 
    +        for line in git_lines(
    294 
    +            ["ls-remote", upstream, f"refs/tags/{firefox_tag_prefix}*"]
    295 
    +        ):
    296 
    +            objectname, name = line.split("\t", 1)
    297 
    +            for index in range(search_index):
    298 
    +                if search_commits[index] == objectname:
    299 
    +                    # Remove trailing "^{}" for commits pointed to by
    300 
    +                    # annotated tags.
    301 
    +                    remote_ref = Reference(re.sub(r"\^\{\}$", "", name), objectname)
    302 
    +                    # Only continue to search for references that are even
    303 
    +                    # closer to `search_from`.
    304 
    +                    search_index = index
    305 
    +                    break
    306 
    +        if remote_ref is not None:
    307 
    +            # Get a local copy of just this tag.
    308 
    +            git_run(["fetch", "--no-tags", upstream, "tag", remote_ref.name])
    309 
    +            return ref
    310 
    +
    311 
    +    raise TbDevException("Unable to find FIREFOX_ tag")
    312 
    +
    313 
    +
    314 
    +def get_upstream_tracking_branch(search_from: str) -> str:
    198 315 
         """
    199 
    -    Search backwards from the 'search_from' commit to find the commit that comes
    200 
    -    from firefox.
    316 
    +    :param search_from: The commit reference.
    317 
    +    :returns: The upstream branch reference name.
    201 318 
         """
    202 
    -    return get_nearest_ref("tag", "FIREFOX_", search_from)
    203 
    -
    204 
    -
    205 
    -def get_upstream_tracking_branch(search_from):
    206 
    -    return git_get(["rev-parse", "--abbrev-ref", f"{search_from}@{{upstream}}"])[0]
    319 
    +    return git_get(["rev-parse", "--abbrev-ref", f"{search_from}@{{upstream}}"])
    207 320
    208 321
    209 
    -def get_upstream_basis_commit(search_from):
    322 
    +def get_upstream_basis_commit(search_from: str) -> str:
    210 323 
         """
    211 324 
         Get the first common ancestor of search_from that is also in its upstream
    212 325 
         branch.
    326 
    +    :param search_from: The commit reference.
    327 
    +    :returns: The upstream commit hash.
    213 328 
         """
    214 329 
         upstream_branch = get_upstream_tracking_branch(search_from)
    215 
    -    commit = git_get(["merge-base", search_from, upstream_branch])[0]
    330 
    +    commit = git_get(["merge-base", search_from, upstream_branch])
    216 331 
         # Verify that the upstream commit shares the same firefox basis. Otherwise,
    217 332 
         # this would indicate that the upstream is on an early or later FIREFOX
    218 333 
         # base.
    ... ... @@ -226,26 +341,82 @@ def get_upstream_basis_commit(search_from):
    226 341 
         return commit
    227 342
    228 343
    229 
    -def get_changed_files(from_commit, staged=False):
    344 
    +class FileChange:
    345 
    +    """Represents a git change to a commit."""
    346 
    +
    347 
    +    def __init__(self, status: str, path: str, new_path: str) -> None:
    348 
    +        """
    349 
    +        :param status: The file change status used within git diff. E.g. "M" for
    350 
    +          modified, or "D" for deleted.
    351 
    +        :param path: The source file path.
    352 
    +        :param new_path: The file path after the change.
    353 
    +        """
    354 
    +        self.status = status
    355 
    +        self.path = path
    356 
    +        self.new_path = new_path
    357 
    +
    358 
    +
    359 
    +RAW_DIFF_PATH_PATTERN = r"(?P<path>[^\0]*)\0"
    360 
    +RAW_DIFF_LINE_REGEX = re.compile(
    361 
    +    r":[0-7]+ [0-7]+ [0-9a-f]+ [0-9a-f]+ (?P<status>[ADMTUXRC])[0-9]*\0"
    362 
    +    + RAW_DIFF_PATH_PATTERN
    363 
    +)
    364 
    +RAW_DIFF_PATH_REGEX = re.compile(RAW_DIFF_PATH_PATTERN)
    365 
    +
    366 
    +
    367 
    +def parse_raw_diff_line(raw_output: str) -> tuple[FileChange, int]:
    230 368 
         """
    231 
    -    Get a list of filenames relative to the current working directory that have
    369 
    +    Parse the --raw diff output from git.
    370 
    +    :param raw_output: The raw output.
    371 
    +    :returns: The change for this line, and the offset for the end of the raw
    372 
    +      diff line.
    373 
    +    """
    374 
    +    match = RAW_DIFF_LINE_REGEX.match(raw_output)
    375 
    +    if not match:
    376 
    +        raise ValueError(f"Invalid raw output: {raw_output[:50]}...")
    377 
    +    path = os.path.relpath(os.path.join(get_local_root(), match.group("path")))
    378 
    +    status = match.group("status")
    379 
    +    if status in ("R", "C"):
    380 
    +        match = RAW_DIFF_PATH_REGEX.match(raw_output, pos=match.end())
    381 
    +        if not match:
    382 
    +            raise ValueError(f"Invalid raw output for rename: {raw_output[:50]}...")
    383 
    +        new_path = os.path.relpath(os.path.join(get_local_root(), match.group("path")))
    384 
    +    else:
    385 
    +        new_path = path
    386 
    +
    387 
    +    return FileChange(status, path, new_path), match.end()
    388 
    +
    389 
    +
    390 
    +def get_changed_files(
    391 
    +    from_commit: None | str = None, staged: bool = False
    392 
    +) -> Iterator[FileChange]:
    393 
    +    """
    394 
    +    Get a list of file changes relative to the current working directory that have
    232 395 
         been changed since 'from_commit' (non-inclusive).
    396 
    +    :param from_commit: The commit to compare against, otherwise use the git
    397 
    +      diff default.
    398 
    +    :param staged: Whether to limit the diff to staged changes.
    399 
    +    :yield: The file changes.
    233 400 
         """
    234 
    -    args = ["diff"]
    401 
    +    args = ["diff", "-z", "--raw"]
    235 402 
         if staged:
    236 403 
             args.append("--staged")
    237 
    -    args.append("--name-only")
    238 
    -    args.append(from_commit)
    239 
    -    return [
    240 
    -        os.path.relpath(os.path.join(get_local_root(), filename))
    241 
    -        for filename in git_get(args)
    242 
    -    ]
    404 
    +    if from_commit:
    405 
    +        args.append(from_commit)
    406 
    +    raw_output = git_get(args, strip=False)
    407 
    +    while raw_output:
    408 
    +        file_change, end = parse_raw_diff_line(raw_output)
    409 
    +        yield file_change
    410 
    +        raw_output = raw_output[end:]
    243 411
    244 412
    245 
    -def file_contains(filename, regex):
    413 
    +def file_contains(filename: str, regex: re.Pattern[str]) -> bool:
    246 414 
         """
    247 415 
         Return whether the file is a utf-8 text file containing the regular
    248 416 
         expression given by 'regex'.
    417 
    +    :param filename: The file path.
    418 
    +    :param regex: The pattern to search for.
    419 
    +    :returns: Whether the pattern was matched.
    249 420 
         """
    250 421 
         with open(filename, encoding="utf-8") as file:
    251 422 
             try:
    ... ... @@ -258,9 +429,10 @@ def file_contains(filename, regex):
    258 429 
         return False
    259 430
    260 431
    261 
    -def get_gitlab_default():
    432 
    +def get_gitlab_default() -> str:
    262 433 
         """
    263 434 
         Get the name of the default branch on gitlab.
    435 
    +    :returns: The branch name.
    264 436 
         """
    265 437 
         repo_name = get_upstream_details().get("repo-name", None)
    266 438 
         if repo_name is None:
    ... ... @@ -283,12 +455,14 @@ def get_gitlab_default():
    283 455 
         )
    284 456
    285 457 
         with urllib.request.urlopen(gitlab_request, timeout=20) as response:
    286 
    -        return json.load(response)["data"]["project"]["repository"]["rootRef"]
    458 
    +        default = json.load(response)["data"]["project"]["repository"]["rootRef"]
    459 
    +        assert isinstance(default, str)
    460 
    +        return default
    287 461
    288 462
    289 
    -def within_browser_root():
    463 
    +def within_browser_root() -> bool:
    290 464 
         """
    291 
    -    Whether we are with the tor browser root.
    465 
    +    :returns: Whether we are with the tor browser root.
    292 466 
         """
    293 467 
         root = get_local_root()
    294 468 
         if not root:
    ... ... @@ -301,24 +475,24 @@ def within_browser_root():
    301 475 
     # * -------------------- *
    302 476
    303 477
    304 
    -def show_firefox_commit(_args):
    478 
    +def show_firefox_commit(_args: argparse.Namespace) -> None:
    305 479 
         """
    306 480 
         Print the tag name and commit for the last firefox commit below the current
    307 481 
         HEAD.
    308 482 
         """
    309 483 
         ref = get_firefox_ref("HEAD")
    310 
    -    print(ref.name)
    484 
    +    print(ref.full_name)
    311 485 
         print(ref.commit)
    312 486
    313 487
    314 
    -def show_upstream_basis_commit(_args):
    488 
    +def show_upstream_basis_commit(_args: argparse.Namespace) -> None:
    315 489 
         """
    316 490 
         Print the last upstream commit for the current HEAD.
    317 491 
         """
    318 492 
         print(get_upstream_basis_commit("HEAD"))
    319 493
    320 494
    321 
    -def show_log(args):
    495 
    +def show_log(args: argparse.Namespace) -> None:
    322 496 
         """
    323 497 
         Show the git log between the current HEAD and the last firefox commit.
    324 498 
         """
    ... ... @@ -326,7 +500,7 @@ def show_log(args):
    326 500 
         git_run(["log", f"{commit}..HEAD", *args.gitargs], check=False)
    327 501
    328 502
    329 
    -def show_files_containing(args):
    503 
    +def show_files_containing(args: argparse.Namespace) -> None:
    330 504 
         """
    331 505 
         List all the files that that have been modified for tor browser, that also
    332 506 
         contain a regular expression.
    ... ... @@ -336,33 +510,32 @@ def show_files_containing(args):
    336 510 
         except re.error as err:
    337 511 
             raise TbDevException(f"{args.regex} is not a valid python regex") from err
    338 512
    339 
    -    file_list = get_changed_files(get_firefox_ref("HEAD").commit)
    340 
    -
    341 
    -    for filename in file_list:
    342 
    -        if not os.path.isfile(filename):
    513 
    +    for file_change in get_changed_files(get_firefox_ref("HEAD").commit):
    514 
    +        path = file_change.new_path
    515 
    +        if not os.path.isfile(path):
    343 516 
                 # deleted ofile
    344 517 
                 continue
    345 
    -        if file_contains(filename, regex):
    346 
    -            print(filename)
    518 
    +        if file_contains(path, regex):
    519 
    +            print(path)
    347 520
    348 521
    349 
    -def show_changed_files(_args):
    522 
    +def show_changed_files(_args: argparse.Namespace) -> None:
    350 523 
         """
    351 524 
         List all the files that have been modified relative to upstream.
    352 525 
         """
    353 
    -    for filename in get_changed_files(get_upstream_basis_commit("HEAD")):
    354 
    -        print(filename)
    526 
    +    for file_change in get_changed_files(get_upstream_basis_commit("HEAD")):
    527 
    +        print(file_change.new_path)
    355 528
    356 529
    357 
    -def lint_changed_files(args):
    530 
    +def lint_changed_files(args: argparse.Namespace) -> None:
    358 531 
         """
    359 532 
         Lint all the files that have been modified relative to upstream.
    360 533 
         """
    361 534 
         os.chdir(get_local_root())
    362 535 
         file_list = [
    363 
    -        f
    536 
    +        f.new_path
    364 537 
             for f in get_changed_files(get_upstream_basis_commit("HEAD"))
    365 
    -        if os.path.isfile(f)  # Not deleted
    538 
    +        if os.path.isfile(f.new_path)  # Not deleted
    366 539 
         ]
    367 540 
         # We add --warnings since clang only reports whitespace issues as warnings.
    368 541 
         subprocess.run(
    ... ... @@ -371,10 +544,18 @@ def lint_changed_files(args):
    371 544 
         )
    372 545
    373 546
    374 
    -def prompt_user(prompt, convert):
    547 
    +# TODO: replace with "prompt_user[T](..., T]) -> T" after python 3.12 is the
    548 
    +# minimum mach version.
    549 
    +T = TypeVar("T")
    550 
    +
    551 
    +
    552 
    +def prompt_user(prompt: str, convert: Callable[[str], T]) -> T:
    375 553 
         """
    376 
    -    Ask the user for some input until the given converter returns without
    377 
    -    throwing a ValueError.
    554 
    +    Ask the user for some input.
    555 
    +    :param prompt: The prompt to show the user.
    556 
    +    :param convert: A method to convert the response into a type. Should
    557 
    +      throw `ValueError` if the user should be re-prompted for a valid input.
    558 
    +    :returns: The first valid user response.
    378 559 
         """
    379 560 
         while True:
    380 561 
             # Flush out stdin.
    ... ... @@ -388,8 +569,12 @@ def prompt_user(prompt, convert):
    388 569 
                 pass
    389 570
    390 571
    391 
    -def binary_reply_default_no(value):
    392 
    -    """Process a 'y' or 'n' reply, defaulting to 'n' if empty."""
    572 
    +def binary_reply_default_no(value: str) -> bool:
    573 
    +    """
    574 
    +    Process a 'y' or 'n' reply, defaulting to 'n' if empty.
    575 
    +    :param value: The user input.
    576 
    +    :returns: Whether the answer is yes.
    577 
    +    """
    393 578 
         if value == "":
    394 579 
             return False
    395 580 
         if value.lower() == "y":
    ... ... @@ -399,121 +584,737 @@ def binary_reply_default_no(value):
    399 584 
         raise ValueError()
    400 585
    401 586
    402 
    -def get_fixup_for_file(filename, firefox_commit):
    403 
    -    """Find the commit the given file should fix up."""
    587 
    +class FixupTarget:
    588 
    +    """Represents a commit that can be targeted by a fixup."""
    589 
    +
    590 
    +    def __init__(self, commit: str, short_ref: str, title: str) -> None:
    591 
    +        """
    592 
    +        :param commit: The commit hash for the commit.
    593 
    +        :param short_ref: The shortened commit hash for display.
    594 
    +        :param title: The first line of the commit message.
    595 
    +        """
    596 
    +        self.commit = commit
    597 
    +        self.short_ref = short_ref
    598 
    +        self.title = title
    599 
    +        self.changes: list[FileChange] = []
    600 
    +        self.fixups: list[FixupTarget] = []
    601 
    +        self.target: None | FixupTarget = None
    602 
    +
    603 
    +    _FIXUP_REGEX = re.compile(r"^fixup! +")
    604 
    +
    605 
    +    def trim_fixup(self) -> tuple[str, int]:
    606 
    +        """
    607 
    +        Trim the "fixup!" prefixes.
    608 
    +        :returns: The stripped commit title and the fixup depth (how many fixups
    609 
    +          prefixes there were).
    610 
    +        """
    611 
    +        title = self.title
    612 
    +        depth = 0
    613 
    +        while True:
    614 
    +            match = self.__class__._FIXUP_REGEX.match(title)
    615 
    +            if not match:
    616 
    +                return title, depth
    617 
    +            title = title[match.end() :]
    618 
    +            depth += 1
    619 
    +
    620 
    +    def touches_path(
    621 
    +        self, path: str, filter_status: None | str = None, check_dir: bool = False
    622 
    +    ) -> bool:
    623 
    +        """
    624 
    +        Whether this target, or one of its fixups or target, touches the given
    625 
    +        path.
    626 
    +        :param path: The path to check.
    627 
    +        :param filter_status: Limit the detected changes to the given status(es).
    628 
    +        :param check_dir: Whether we should treat `path` as a directory and check for
    629 
    +          files within it.
    630 
    +        :returns: Whether this target matches.
    631 
    +        """
    632 
    +        # NOTE: In the case of renames, we generally assume that renames occur
    633 
    +        # in the fixup targets. E.g. "Commit 1" creates the file "file.txt", and
    634 
    +        # "fixup! Commit 1" renames it to "new.txt". In this case, if the
    635 
    +        # FixupTarget for "Commit 1" is passed in "file.txt" it will match. And
    636 
    +        # if it is passed in "new.txt" it will also match via the self.fixups
    637 
    +        # field, which will include the "fixup! Commit 1" rename.
    638 
    +        # But the "fixup ! Commit 1" FixupTargets will only match with
    639 
    +        # "file.txt" if they occurred before the rename fixup, and will only
    640 
    +        # match with "new.txt" if they occur after the rename fixup. With the
    641 
    +        # exception of the rename fixup itself, which will match both.
    642 
    +        #
    643 
    +        # In principle, we could identify a file across renames (have a mapping
    644 
    +        # from each commit to what the file is called at that stage) and match
    645 
    +        # using this file identifier. Similar to the "--follow" git diff
    646 
    +        # argument. This would then cover cases where a rename occurs between
    647 
    +        # the commit and its fixups, and allow fixups before the rename to also
    648 
    +        # match. However, the former case is unexpected and the latter case
    649 
    +        # would not be that useful.
    650 
    +        if self._touches_path_basis(path, filter_status, check_dir):
    651 
    +            return True
    652 
    +        # Mark this as a valid target for the path if one of our fixups changes
    653 
    +        # this path.
    654 
    +        # NOTE: We use _touch_path_basis to prevent recursion. This means we
    655 
    +        # will only check one layer up or down, but we only expect fixups of
    656 
    +        # up to depth 1.
    657 
    +        for fixup_target in self.fixups:
    658 
    +            if fixup_target._touches_path_basis(path, filter_status, check_dir):
    659 
    +                return True
    660 
    +        # Mark this as a valid target if our target changes this path.
    661 
    +        if self.target is not None and self.target._touches_path_basis(
    662 
    +            path, filter_status, check_dir
    663 
    +        ):
    664 
    +            return True
    665 
    +        return False
    666 
    +
    667 
    +    def _touches_path_basis(
    668 
    +        self, path: str, filter_status: None | str, check_dir: bool
    669 
    +    ) -> bool:
    670 
    +        """
    671 
    +        Whether this target touches the given path.
    672 
    +        :param path: The path to check.
    673 
    +        :param filter_status: Limit the detected changes to the given status.
    674 
    +        :param check_dir: Whether we should treat `path` as a directory and check for
    675 
    +          files within it.
    676 
    +        :returns: Whether this target matches.
    677 
    +        """
    678 
    +        for file_change in self.changes:
    679 
    +            if filter_status is not None and file_change.status not in filter_status:
    680 
    +                continue
    681 
    +            for test_path in (file_change.path, file_change.new_path):
    682 
    +                if check_dir:
    683 
    +                    if os.path.commonpath((os.path.dirname(test_path), path)) == path:
    684 
    +                        # test_path's directory matches the path or is within it.
    685 
    +                        return True
    686 
    +                elif test_path == path:
    687 
    +                    return True
    688 
    +        return False
    689 
    +
    690 
    +
    691 
    +def get_fixup_targets(
    692 
    +    target_list: list[FixupTarget],
    693 
    +    from_commit: str,
    694 
    +    to_commit: str,
    695 
    +    fixup_depth: int = 0,
    696 
    +) -> None:
    697 
    +    """
    698 
    +    Find all the commits that can be targeted by a fixup between the given
    699 
    +    commits.
    700 
    +    :param target_list: The list to fill with targets. Appended in the order of
    701 
    +      `from_commit` to `to_commit`.
    702 
    +    :param from_commit: The commit to start from (non-inclusive).
    703 
    +    :param to_commit: The commit to end on (inclusive).
    704 
    +    :param fixup_depth: The maximum "depth" of fixups. I.e. how many "fixup!"
    705 
    +      prefixes to allow.
    706 
    +    """
    707 
    +    raw_output = git_get(
    708 
    +        [
    709 
    +            "log",
    710 
    +            "--pretty=format:%H,%h,%s",
    711 
    +            "--reverse",
    712 
    +            "--raw",
    713 
    +            "-z",
    714 
    +            f"{from_commit}..{to_commit}",
    715 
    +        ],
    716 
    +        strip=False,
    717 
    +    )
    718 
    +    pretty_regex = re.compile(
    719 
    +        r"(?P<commit>[0-9a-f]+),(?P<short_ref>[0-9a-f]+),(?P<title>[^\n\0]*)\n"
    720 
    +    )
    721 
    +    excluded_regex_list = [
    722 
    +        re.compile(r"^Bug [0-9]+.*r="),  # Backported Mozilla bug.
    723 
    +        re.compile(r"^dropme! "),
    724 
    +    ]
    725 
    +
    726 
    +    while raw_output:
    727 
    +        match = pretty_regex.match(raw_output)
    728 
    +        if not match:
    729 
    +            raise ValueError(f"Invalid pretty format: {raw_output[:100]}...")
    730 
    +        fixup_target = FixupTarget(
    731 
    +            match.group("commit"), match.group("short_ref"), match.group("title")
    732 
    +        )
    733 
    +        raw_output = raw_output[match.end() :]
    734 
    +        while raw_output and raw_output[0] != "\0":
    735 
    +            file_change, end = parse_raw_diff_line(raw_output)
    736 
    +            fixup_target.changes.append(file_change)
    737 
    +            raw_output = raw_output[end:]
    738 
    +        if raw_output:
    739 
    +            # Skip over the "\0".
    740 
    +            raw_output = raw_output[1:]
    741 
    +
    742 
    +        for regex in excluded_regex_list:
    743 
    +            if regex.match(fixup_target.title):
    744 
    +                # Exclude from the list.
    745 
    +                continue
    746 
    +
    747 
    +        trimmed_title, depth = fixup_target.trim_fixup()
    748 
    +        if depth:
    749 
    +            original_target = None
    750 
    +            for target in target_list:
    751 
    +                if target.title == trimmed_title:
    752 
    +                    original_target = target
    753 
    +                    break
    754 
    +
    755 
    +            if original_target:
    756 
    +                original_target.fixups.append(fixup_target)
    757 
    +                fixup_target.target = original_target
    758 
    +                if depth > fixup_depth:
    759 
    +                    # Exclude from the list.
    760 
    +                    continue
    761 
    +
    762 
    +        target_list.append(fixup_target)
    763 
    +
    764 
    +
    765 
    +class NewCommitBasis:
    766 
    +    def __init__(self) -> None:
    767 
    +        self.staged_paths: set[str] = set()
    768 
    +        self.adding_paths: set[str] = set()
    769 
    +
    770 
    +    def add(self, paths: Iterable[str], staged: bool) -> None:
    771 
    +        """
    772 
    +        Add a path to include in this commit.
    773 
    +        :param paths: The paths to add.
    774 
    +        :param staged: Whether we are adding already staged changes.
    775 
    +        """
    776 
    +        if staged:
    777 
    +            self.staged_paths.update(paths)
    778 
    +            return
    779 
    +
    780 
    +        self.adding_paths.update(paths)
    781 
    +
    782 
    +
    783 
    +class NewCommit(NewCommitBasis):
    784 
    +    """Represents a new commit that we want to create."""
    404 785
    405 
    -    def parse_log_line(line):
    406 
    -        [commit, short_ref, title] = line.split(",", 2)
    407 
    -        return {"commit": commit, "short-ref": short_ref, "title": title}
    786 
    +    def __init__(self, alias: str) -> None:
    787 
    +        """
    788 
    +        :param alias: The alias name for the commit.
    789 
    +        """
    790 
    +        super().__init__()
    791 
    +        self.alias = alias
    408 792
    409 
    -    options = [
    410 
    -        parse_log_line(line)
    411 
    -        for line in git_get(
    412 
    -            [
    413 
    -                "log",
    414 
    -                "--pretty=format:%H,%h,%s",
    415 
    -                f"{firefox_commit}..HEAD",
    416 
    -                "--",
    417 
    -                filename,
    418 
    -            ]
    793 
    +
    794 
    +class NewFixup(NewCommitBasis):
    795 
    +    """Represents a new fixup commit that we want to create."""
    796 
    +
    797 
    +    def __init__(self, target: FixupTarget) -> None:
    798 
    +        """
    799 
    +        :param target: The commit to target with the fixup.
    800 
    +        """
    801 
    +        super().__init__()
    802 
    +        self.target = target
    803 
    +
    804 
    +
    805 
    +def get_suggested_fixup_targets_for_change(
    806 
    +    file_change: FileChange,
    807 
    +    fixup_target_list: list[FixupTarget],
    808 
    +    firefox_directories_lazy: Callable[[], set[str]],
    809 
    +) -> Iterator[FixupTarget]:
    810 
    +    """
    811 
    +    Find the suggested fixup targets for the given file change.
    812 
    +    :param file_change: The file change to get a suggestion for.
    813 
    +    :param fixup_target_list: The list to choose from.
    814 
    +    :param firefox_directories_lazy: Lazy method to return the firefox
    815 
    +      directories.
    816 
    +    :yield: The suggested fixup targets.
    817 
    +    """
    818 
    +
    819 
    +    def filter_list(
    820 
    +        path: str, filter_status: None | str = None, check_dir: bool = False
    821 
    +    ) -> Iterator[FixupTarget]:
    822 
    +        return (
    823 
    +            t
    824 
    +            for t in fixup_target_list
    825 
    +            if t.touches_path(path, filter_status=filter_status, check_dir=check_dir)
    419 826 
             )
    827 
    +
    828 
    +    if file_change.status == "D":
    829 
    +        # Deleted.
    830 
    +        # Find the commit that introduced this file or previously deleted it.
    831 
    +        # I.e. added the file ("A"), renamed it ("R"), or deleted it ("D").
    832 
    +        yield from filter_list(file_change.path, filter_status="ARD")
    833 
    +        return
    834 
    +
    835 
    +    if file_change.status == "A":
    836 
    +        # First check to see if this file name was actually touched before.
    837 
    +        yielded_target = False
    838 
    +        for target in filter_list(file_change.path):
    839 
    +            yielded_target = True
    840 
    +            yield target
    841 
    +        if yielded_target:
    842 
    +            return
    843 
    +        # Else, find commits that introduced files in the same directory, or
    844 
    +        # deleted in them, if they are not firefox directories.
    845 
    +        dir_path = file_change.path
    846 
    +        while True:
    847 
    +            dir_path = os.path.dirname(dir_path)
    848 
    +            if not dir_path or dir_path in firefox_directories_lazy():
    849 
    +                return
    850 
    +
    851 
    +            yielded_target = False
    852 
    +            for target in filter_list(dir_path, filter_status="ARD", check_dir=True):
    853 
    +                yielded_target = True
    854 
    +                yield target
    855 
    +
    856 
    +            if yielded_target:
    857 
    +                return
    858 
    +            # Else, search one directory higher.
    859 
    +
    860 
    +    if file_change.status == "R":
    861 
    +        # Renamed.
    862 
    +        # Find the commit that introduced the original name for this file.
    863 
    +        yield from filter_list(file_change.path, filter_status="AR")
    864 
    +        return
    865 
    +
    866 
    +    # Modified.
    867 
    +    yield from filter_list(file_change.path)
    868 
    +
    869 
    +
    870 
    +def ask_for_target(
    871 
    +    file_change_list: list[FileChange],
    872 
    +    new_commits_list: list[NewCommit | NewFixup],
    873 
    +    suggested_fixup_target_list: list[FixupTarget],
    874 
    +    full_fixup_target_list: list[FixupTarget],
    875 
    +    staged: bool = False,
    876 
    +) -> bool:
    877 
    +    """
    878 
    +    Ask the user to choose a target.
    879 
    +    :param file_change_list: The file changes to ask for.
    880 
    +    :param new_commits_list: The list of pending new commits, may be added to.
    881 
    +    :param suggested_fixup_target_list: The list of suggested target fixups
    882 
    +      to choose from.
    883 
    +    :param staged: Whether this is for staged changes.
    884 
    +    :returns: `True` if the operation should be aborted.
    885 
    +    """
    886 
    +
    887 
    +    new_paths = [c.new_path for c in file_change_list]
    888 
    +    all_paths = set(new_paths).union(c.path for c in file_change_list)
    889 
    +    non_fixup_commits: list[NewCommit] = [
    890 
    +        n for n in new_commits_list if isinstance(n, NewCommit)
    420 891 
         ]
    421 
    -    if not options:
    422 
    -        print(f"No commit found for {filename}")
    423 
    -        return None
    424 892
    425 
    -    def valid_index(val):
    893 
    +    shown_list: list[NewCommit | FixupTarget] = (
    894 
    +        non_fixup_commits + suggested_fixup_target_list
    895 
    +    )
    896 
    +
    897 
    +    can_skip = not staged
    898 
    +    shown_full = False
    899 
    +
    900 
    +    index_offset = 2
    901 
    +
    902 
    +    def valid_response(val: str) -> tuple[str, None | NewCommit | FixupTarget]:
    903 
    +        val = val.strip()
    904 
    +
    905 
    +        if val == "h":
    906 
    +            return "help", None
    907 
    +
    908 
    +        if val == "a":
    909 
    +            return "abort", None
    910 
    +
    426 911 
             if val == "d":
    427 
    -            return val
    912 
    +            return "diff", None
    913 
    +
    914 
    +        if val == "f":
    915 
    +            if shown_full:
    916 
    +                # Already done once.
    917 
    +                raise ValueError()
    918 
    +            return "full-list", None
    428 919
    920 
    +        is_patch_full = val.startswith("P")
    429 921 
             is_patch = val.startswith("p")
    430 
    -        if is_patch:
    431 
    -            val = val[1:]
    922 
    +        if is_patch or is_patch_full:
    923 
    +            index = int(val[1:], base=10)  # Raises ValueError if not integer.
    924 
    +        else:
    925 
    +            index = int(val, base=10)  # Raises ValueError if not integer.
    926 
    +            if index == 0:
    927 
    +                if not can_skip:
    928 
    +                    raise ValueError()
    929 
    +                return "skip", None
    930 
    +
    931 
    +            if index == 1:
    932 
    +                return "new", None
    432 933
    433 
    -        # May raise a ValueError.
    434 
    -        as_index = int(val)
    435 
    -        if as_index < 0 or as_index > len(options):
    934 
    +        index -= index_offset
    935 
    +
    936 
    +        if index < 0 or index >= len(shown_list):
    436 937 
                 raise ValueError()
    437 938
    438 
    -        if as_index == 0:
    439 
    -            if is_patch:
    939 
    +        selected = shown_list[index]
    940 
    +
    941 
    +        if is_patch_full:
    942 
    +            return "patch-full", selected
    943 
    +        if is_patch:
    944 
    +            return "patch", selected
    945 
    +        return "target", selected
    946 
    +
    947 
    +    def alias_response(val: str) -> str:
    948 
    +        # Choose a default alias name if none is given.
    949 
    +        val = val.strip() or f"New commit {len(non_fixup_commits)}"
    950 
    +        for new_commit in non_fixup_commits:
    951 
    +            if new_commit.alias == val:
    952 
    +                # Already in use.
    440 953 
                     raise ValueError()
    441 
    -            return None
    954 
    +        return val
    955 
    +
    956 
    +    def print_index_option(index: int, description: str) -> None:
    957 
    +        print(f"  \x1b[1m{index}\x1b[0m: {description}")
    442 958
    443 
    -        return (is_patch, options[as_index - 1]["commit"])
    959 
    +    def in_pink(text: str) -> str:
    960 
    +        return f"\x1b[1;38;5;212m{text}\x1b[0m"
    444 961
    962 
    +    prefix_str = "For " + (in_pink("staged") if staged else "unstaged") + " changes to"
    963 
    +    if len(new_paths) == 1:
    964 
    +        print(f"{prefix_str} {in_pink(new_paths[0])}:")
    965 
    +    else:
    966 
    +        print(f"{prefix_str}:")
    967 
    +        for path in new_paths:
    968 
    +            print(f"  {in_pink(path)}")
    969 
    +    print("")
    970 
    +
    971 
    +    show_help = True
    972 
    +    reshow_list = True
    445 973 
         while True:
    446 
    -        print(f"For {filename}:\n")
    447 
    -        print("  \x1b[1m0\x1b[0m: None")
    448 
    -        for index, opt in enumerate(options):
    449 
    -            print(
    450 
    -                f"  \x1b[1m{index + 1}\x1b[0m: "
    451 
    -                + f"\x1b[1;38;5;212m{opt['short-ref']}\x1b[0m "
    452 
    -                + opt["title"]
    453 
    -            )
    974 
    +        if reshow_list:
    975 
    +            if can_skip:
    976 
    +                print_index_option(0, "Skip")
    977 
    +            print_index_option(1, "New commit")
    978 
    +            for index, target in enumerate(shown_list, start=index_offset):
    979 
    +                if isinstance(target, NewCommit):
    980 
    +                    print_index_option(index, f"Add to new commit: {target.alias}")
    981 
    +                else:
    982 
    +                    print_index_option(
    983 
    +                        index, f"Fixup: {in_pink(target.short_ref)} {target.title}"
    984 
    +                    )
    985 
    +            reshow_list = False
    454 986 
             print("")
    455 
    -        response = prompt_user(
    456 
    -            "Choose an <index> to fixup, or '0' to skip this file, "
    457 
    -            "or 'd' to view the pending diff, "
    458 
    -            "or 'p<index>' to view the patch for the index: ",
    459 
    -            valid_index,
    987 
    +
    988 
    +        response, selected = prompt_user(
    989 
    +            (
    990 
    +                "Choose an <index> to target. Type 'h' for additional options: "
    991 
    +                if show_help
    992 
    +                else "Choose an <index> to target or an option: "
    993 
    +            ),
    994 
    +            valid_response,
    460 995 
             )
    461 
    -        if response is None:
    462 
    -            # Skip this file.
    463 
    -            return None
    464 996
    465 
    -        if response == "d":
    466 
    -            git_run(["diff", "--", filename])
    997 
    +        if response == "help":
    998 
    +            print("Options:")
    999 
    +            for option, desc in (
    1000 
    +                ("h", "show the available options."),
    1001 
    +                ("a", "abort this commit operation and all pending commits."),
    1002 
    +                (
    1003 
    +                    ("", "")
    1004 
    +                    if shown_full
    1005 
    +                    else (
    1006 
    +                        "f",
    1007 
    +                        "show the full list of fixup targets, rather than just the suggested ones.",
    1008 
    +                    )
    1009 
    +                ),
    1010 
    +                ("d", "view the diff for the pending file changes."),
    1011 
    +                (
    1012 
    +                    "P<index>",
    1013 
    +                    "view the patch for the index (including its relevant fixups).",
    1014 
    +                ),
    1015 
    +                (
    1016 
    +                    "p<index>",
    1017 
    +                    "view the patch for the index (including its relevant fixups), "
    1018 
    +                    "limited to the current files.",
    1019 
    +                ),
    1020 
    +            ):
    1021 
    +                if not option:
    1022 
    +                    # Skip this option.
    1023 
    +                    continue
    1024 
    +                print(f"  \x1b[1m{option[0]}\x1b[0m{option[1:].ljust(7)}: {desc}")
    1025 
    +            # Do not show the help option again.
    1026 
    +            show_help = False
    1027 
    +            continue
    1028 
    +
    1029 
    +        if response == "abort":
    1030 
    +            return True
    1031 
    +
    1032 
    +        if response == "skip":
    1033 
    +            return False
    1034 
    +
    1035 
    +        if response == "new":
    1036 
    +            new_alias = prompt_user(
    1037 
    +                "Enter an optional temporary alias for this new commit: ",
    1038 
    +                alias_response,
    1039 
    +            )
    1040 
    +            new_commit = NewCommit(new_alias)
    1041 
    +            new_commit.add(all_paths, staged)
    1042 
    +            new_commits_list.append(new_commit)
    1043 
    +            return False
    1044 
    +
    1045 
    +        if response == "target":
    1046 
    +            assert selected is not None
    1047 
    +
    1048 
    +            if isinstance(selected, NewCommit):
    1049 
    +                # Adding to a new commit.
    1050 
    +                selected.add(all_paths, staged)
    1051 
    +                return False
    1052 
    +
    1053 
    +            for new_fixup in new_commits_list:
    1054 
    +                if not isinstance(new_fixup, NewFixup):
    1055 
    +                    continue
    1056 
    +                if new_fixup.target == selected:
    1057 
    +                    # We already have a pending fixup commit that targets this
    1058 
    +                    # selected target. Add this path to the same commit.
    1059 
    +                    new_fixup.add(all_paths, staged)
    1060 
    +                    return False
    1061 
    +
    1062 
    +            new_fixup = NewFixup(selected)
    1063 
    +            new_fixup.add(all_paths, staged)
    1064 
    +            new_commits_list.append(new_fixup)
    1065 
    +            return False
    1066 
    +
    1067 
    +        if response == "full-list":
    1068 
    +            shown_list = non_fixup_commits + full_fixup_target_list
    1069 
    +            shown_full = True
    1070 
    +            reshow_list = True
    467 1071 
                 continue
    468 1072
    469 
    -        view_patch, commit = response
    470 
    -        if view_patch:
    471 
    -            git_run(["log", "-p", "-1", commit, "--", filename])
    1073 
    +        if response == "diff":
    1074 
    +            git_args = ["diff", "--color"]
    1075 
    +            if staged:
    1076 
    +                git_args.append("--staged")
    1077 
    +            git_args.extend(git_path_args(all_paths))
    1078 
    +            git_run_pager(git_args)
    472 1079 
                 continue
    473 1080
    474 
    -        return commit
    1081 
    +        if response in ("patch", "patch-full"):
    1082 
    +            assert selected is not None
    1083 
    +
    1084 
    +            filter_paths = response == "patch"
    1085 
    +
    1086 
    +            if isinstance(selected, NewCommit):
    1087 
    +                git_sequence = [
    1088 
    +                    ["diff", "--color", "--staged", *git_path_args((path,))]
    1089 
    +                    for path in selected.staged_paths
    1090 
    +                    if not filter_paths or path in all_paths
    1091 
    +                ]
    1092 
    +                git_sequence.extend(
    1093 
    +                    ["diff", "--color", *git_path_args((path,))]
    1094 
    +                    for path in selected.adding_paths
    1095 
    +                    if not filter_paths or path in all_paths
    1096 
    +                )
    1097 
    +
    1098 
    +                # Show what the expected patch will be for the new commit.
    1099 
    +                git_run_pager(
    1100 
    +                    arg_sequence=git_sequence, pager_prefix=f"{selected.alias}\n\n"
    1101 
    +                )
    1102 
    +            else:
    1103 
    +                # Show the log entry for the FixupTarget and each of its fixups.
    1104 
    +                # Order with the commmit closest to HEAD first. We expect
    1105 
    +                # selected.fixups to match this order.
    1106 
    +                git_sequence = []
    1107 
    +                # If `filter_paths` is set, we want to limit the log to the
    1108 
    +                # paths, and try to track any renames in the commit history.
    1109 
    +                prev_log_paths: None | set[str] = None
    1110 
    +                # For the first commit in the sequence, we use the old path
    1111 
    +                # names (rather than `c.new_path`) since we expect the commit
    1112 
    +                # which is closest to us to use the older names.
    1113 
    +                log_paths: None | set[str] = (
    1114 
    +                    {c.path for c in file_change_list} if filter_paths else None
    1115 
    +                )
    1116 
    +                for target in (*selected.fixups, selected):
    1117 
    +                    git_args = [
    1118 
    +                        "log",
    1119 
    +                        "--color",
    1120 
    +                        "-p",
    1121 
    +                        f"{target.commit}~1..{target.commit}",
    1122 
    +                    ]
    1123 
    +                    if filter_paths:
    1124 
    +                        assert log_paths is not None
    1125 
    +                        # Track the renamed paths.
    1126 
    +                        prev_log_paths = log_paths.copy()
    1127 
    +                        for file_change in target.changes:
    1128 
    +                            if (
    1129 
    +                                file_change.status == "R"
    1130 
    +                                and file_change.new_path in log_paths
    1131 
    +                            ):
    1132 
    +                                # file was renamed in this change.
    1133 
    +                                # Update log_paths to the new name.
    1134 
    +                                # NOTE: This should have a similar effect to the
    1135 
    +                                # --follow option for git log for a single file
    1136 
    +                                # NOTE: File renames will not be properly
    1137 
    +                                # tracked if a rename occurs outside of
    1138 
    +                                # `selected.changes` or
    1139 
    +                                # `selected.fixups[].changes`, but this is
    1140 
    +                                # unexpected.
    1141 
    +                                log_paths.remove(file_change.new_path)
    1142 
    +                                log_paths.add(file_change.path)
    1143 
    +
    1144 
    +                        # NOTE: This log entry may be empty if none of the paths
    1145 
    +                        # match.
    1146 
    +                        # NOTE: We include both log_paths and prev_log_paths to
    1147 
    +                        # show renames in the diff output.
    1148 
    +                        git_args.extend(git_path_args(log_paths | prev_log_paths))
    1149 
    +                    git_sequence.append(git_args)
    1150 
    +                # Combine all the logs into one.
    1151 
    +                git_run_pager(arg_sequence=git_sequence)
    1152 
    +            continue
    1153 
    +
    1154 
    +        raise ValueError(f"Unexpected response: {response}")
    475 1155
    476 1156
    477 
    -def auto_fixup(_args):
    1157 
    +def auto_commit(_args: argparse.Namespace) -> None:
    478 1158 
         """
    479 
    -    Automatically find and fix up commits using the current unstaged changes.
    1159 
    +    Automatically find and fix up commits for any pending changes.
    480 1160 
         """
    1161 
    +    # Want git log and add to be run from the root.
    1162 
    +    os.chdir(get_local_root())
    481 1163 
         # Only want to search as far back as the firefox commit.
    482 1164 
         firefox_commit = get_firefox_ref("HEAD").commit
    483 1165
    484 
    -    staged_files = get_changed_files("HEAD", staged=True)
    485 
    -    if staged_files:
    486 
    -        raise TbDevException(f"Have already staged files: {staged_files}")
    1166 
    +    staged_changes = [f for f in get_changed_files(staged=True)]
    1167 
    +    if staged_changes:
    1168 
    +        print("Existing staged changes for:")
    1169 
    +        for file_change in staged_changes:
    1170 
    +            print(f"  {file_change.new_path}")
    1171 
    +        if not prompt_user(
    1172 
    +            "Include staged changes? (y/\x1b[4mn\x1b[0m)", binary_reply_default_no
    1173 
    +        ):
    1174 
    +            raise TbDevException("Cannot continue with pending staged changes")
    1175 
    +        print("")
    487 1176
    488 
    -    fixups = {}
    489 
    -    for filename in get_changed_files("HEAD"):
    490 
    -        commit = get_fixup_for_file(filename, firefox_commit)
    491 
    -        if commit is None:
    1177 
    +    full_target_list: list[FixupTarget] = []
    1178 
    +    # Determine if HEAD points to a branch or not and has an upstream commit.
    1179 
    +    # We choose check=False since the exit status is non-zero when we are in a
    1180 
    +    # detached state.
    1181 
    +    head_symbolic_ref = git_get(["symbolic-ref", "-q", "HEAD"], check=False)
    1182 
    +    if not head_symbolic_ref or not bool(
    1183 
    +        git_get(["for-each-ref", "--format=%(upstream)", head_symbolic_ref])
    1184 
    +    ):
    1185 
    +        # Unexpected, but not fatal.
    1186 
    +        print("HEAD has no upstream tracking!")
    1187 
    +        # Just include all commits since firefox_commit with no fixup depth
    1188 
    +        get_fixup_targets(full_target_list, firefox_commit, "HEAD", fixup_depth=0)
    1189 
    +    else:
    1190 
    +        upstream_commit = get_upstream_basis_commit("HEAD")
    1191 
    +        # Only include "fixup!" commits that are between here and the upstream
    1192 
    +        # tracking commit.
    1193 
    +        get_fixup_targets(
    1194 
    +            full_target_list, firefox_commit, upstream_commit, fixup_depth=0
    1195 
    +        )
    1196 
    +        get_fixup_targets(full_target_list, upstream_commit, "HEAD", fixup_depth=1)
    1197 
    +
    1198 
    +    # full_target_list is ordered with the earlier commits first. Reverse this.
    1199 
    +    full_target_list.reverse()
    1200 
    +    # Also reverse the fixups order to follow the same order.
    1201 
    +    for target in full_target_list:
    1202 
    +        target.fixups.reverse()
    1203 
    +
    1204 
    +    # Lazy load the list of firefox directories since they are unlikely to be
    1205 
    +    # needed.
    1206 
    +    @functools.cache
    1207 
    +    def firefox_directories_lazy() -> set[str]:
    1208 
    +        return {
    1209 
    +            dir_name
    1210 
    +            for dir_name in git_get(
    1211 
    +                [
    1212 
    +                    "ls-tree",
    1213 
    +                    "-r",
    1214 
    +                    "-d",
    1215 
    +                    "--name-only",
    1216 
    +                    "--full-tree",
    1217 
    +                    "-z",
    1218 
    +                    firefox_commit,
    1219 
    +                ],
    1220 
    +                strip=False,
    1221 
    +            ).split("\0")
    1222 
    +            if dir_name
    1223 
    +        }
    1224 
    +
    1225 
    +    # Check untracked files to be added.
    1226 
    +    for path in git_get(
    1227 
    +        ["ls-files", "--other", "--exclude-standard", "-z"], strip=False
    1228 
    +    ).split("\0"):
    1229 
    +        if not path:
    492 1230 
                 continue
    493 
    -        if commit not in fixups:
    494 
    -            fixups[commit] = [filename]
    495 
    -        else:
    496 
    -            fixups[commit].append(filename)
    1231 
    +        if prompt_user(
    1232 
    +            f"Start tracking file `{path}`? (y/\x1b[4mn\x1b[0m)",
    1233 
    +            binary_reply_default_no,
    1234 
    +        ):
    1235 
    +            # Include in the git diff output, but do not stage.
    1236 
    +            git_run(["add", "--intent-to-add", path])
    497 1237 
             print("")
    498 1238
    499 
    -    for commit, files in fixups.items():
    500 
    -        print("")
    501 
    -        git_run(["add", *files])
    502 
    -        git_run(["commit", f"--fixup={commit}"])
    1239 
    +    aborted = False
    1240 
    +    new_commits_list: list[NewCommit | NewFixup] = []
    1241 
    +    # First go through staged changes.
    1242 
    +    if staged_changes:
    1243 
    +        common_fixup_targets = None
    1244 
    +        for change in staged_changes:
    1245 
    +            target_iter = get_suggested_fixup_targets_for_change(
    1246 
    +                change, full_target_list, firefox_directories_lazy
    1247 
    +            )
    1248 
    +            if common_fixup_targets is None:
    1249 
    +                common_fixup_targets = set(target_iter)
    1250 
    +            else:
    1251 
    +                common_fixup_targets.intersection_update(target_iter)
    1252 
    +
    1253 
    +        assert common_fixup_targets is not None
    1254 
    +
    1255 
    +        aborted = ask_for_target(
    1256 
    +            staged_changes,
    1257 
    +            new_commits_list,
    1258 
    +            # Sort in the same order as full_target_list.
    1259 
    +            [target for target in full_target_list if target in common_fixup_targets],
    1260 
    +            full_target_list,
    1261 
    +            staged=True,
    1262 
    +        )
    503 1263 
             print("")
    504 1264
    505 
    -        if prompt_user(
    506 
    -            "Edit fixup commit message? (y/\x1b[4mn\x1b[0m)", binary_reply_default_no
    507 
    -        ):
    1265 
    +    if not aborted:
    1266 
    +        for file_change in get_changed_files():
    1267 
    +            target_list = list(
    1268 
    +                get_suggested_fixup_targets_for_change(
    1269 
    +                    file_change, full_target_list, firefox_directories_lazy
    1270 
    +                )
    1271 
    +            )
    1272 
    +            aborted = ask_for_target(
    1273 
    +                [file_change],
    1274 
    +                new_commits_list,
    1275 
    +                target_list,
    1276 
    +                full_target_list,
    1277 
    +                staged=False,
    1278 
    +            )
    1279 
    +            print("")
    1280 
    +            if aborted:
    1281 
    +                break
    1282 
    +
    1283 
    +    if aborted:
    1284 
    +        return
    1285 
    +
    1286 
    +    # NOTE: Only the first commit can include staged changes.
    1287 
    +    # This should already be the case, but we want to double check.
    1288 
    +    for commit_index in range(1, len(new_commits_list)):
    1289 
    +        if new_commits_list[commit_index].staged_paths:
    1290 
    +            raise ValueError(f"Staged changes for commit {commit_index}")
    1291 
    +
    1292 
    +    for new_commit in new_commits_list:
    1293 
    +        print("")
    1294 
    +        if new_commit.adding_paths:
    1295 
    +            git_run(["add", *git_path_args(new_commit.adding_paths)])
    1296 
    +        if isinstance(new_commit, NewFixup):
    1297 
    +            git_run(["commit", f"--fixup={new_commit.target.commit}"])
    1298 
    +            print("")
    1299 
    +            is_double_fixup = bool(new_commit.target.target)
    1300 
    +            if not is_double_fixup and prompt_user(
    1301 
    +                "Edit fixup commit message? (y/\x1b[4mn\x1b[0m)",
    1302 
    +                binary_reply_default_no,
    1303 
    +            ):
    1304 
    +                git_run(["commit", "--amend"])
    1305 
    +                print("")
    1306 
    +        else:
    1307 
    +            git_run(["commit", "-m", new_commit.alias])
    508 1308 
                 git_run(["commit", "--amend"])
    1309 
    +            print("")
    509 1310
    510 1311
    511 
    -def clean_fixups(_args):
    1312 
    +def clean_fixups(_args: argparse.Namespace) -> None:
    512 1313 
         """
    513 1314 
         Perform an interactive rebase that automatically applies fixups, similar to
    514 1315 
         --autosquash but also works on fixups of fixups.
    515 1316 
         """
    516 
    -    user_editor = git_get(["var", "GIT_SEQUENCE_EDITOR"])[0]
    1317 
    +    user_editor = git_get(["var", "GIT_SEQUENCE_EDITOR"])
    517 1318 
         sub_editor = os.path.join(
    518 1319 
             os.path.dirname(os.path.realpath(__file__)), FIXUP_PREPROCESSOR_EDITOR
    519 1320 
         )
    ... ... @@ -525,7 +1326,7 @@ def clean_fixups(_args):
    525 1326 
         )
    526 1327
    527 1328
    528 
    -def show_default(_args):
    1329 
    +def show_default(_args: argparse.Namespace) -> None:
    529 1330 
         """
    530 1331 
         Print the default branch name from gitlab.
    531 1332 
         """
    ... ... @@ -536,7 +1337,7 @@ def show_default(_args):
    536 1337 
         print(f"{upstream}/{default_branch}")
    537 1338
    538 1339
    539 
    -def branch_from_default(args):
    1340 
    +def branch_from_default(args: argparse.Namespace) -> None:
    540 1341 
         """
    541 1342 
         Fetch the default gitlab branch from upstream and create a new local branch.
    542 1343 
         """
    ... ... @@ -557,7 +1358,7 @@ def branch_from_default(args):
    557 1358 
         )
    558 1359
    559 1360
    560 
    -def move_to_default(args):
    1361 
    +def move_to_default(args: argparse.Namespace) -> None:
    561 1362 
         """
    562 1363 
         Fetch the default gitlab branch from upstream and move the specified
    563 1364 
         branch's commits on top. A new branch will be created tracking the default
    ... ... @@ -569,7 +1370,7 @@ def move_to_default(args):
    569 1370 
         if branch_name is None:
    570 1371 
             # Use current branch as default.
    571 1372 
             try:
    572 
    -            branch_name = git_get(["branch", "--show-current"])[0]
    1373 
    +            branch_name = git_get(["branch", "--show-current"])
    573 1374 
             except IndexError:
    574 1375 
                 raise TbDevException("No current branch")
    575 1376
    ... ... @@ -608,7 +1409,7 @@ def move_to_default(args):
    608 1409 
         git_run(["cherry-pick", f"{current_basis}..{old_branch_name}"], check=False)
    609 1410
    610 1411
    611 
    -def show_range_diff(args):
    1412 
    +def show_range_diff(args: argparse.Namespace) -> None:
    612 1413 
         """
    613 1414 
         Show the range diff between two branches, from their firefox bases.
    614 1415 
         """
    ... ... @@ -624,21 +1425,21 @@ def show_range_diff(args):
    624 1425 
         )
    625 1426
    626 1427
    627 
    -def show_diff_diff(args):
    1428 
    +def show_diff_diff(args: argparse.Namespace) -> None:
    628 1429 
         """
    629 1430 
         Show the diff between the diffs of two branches, relative to their firefox
    630 1431 
         bases.
    631 1432 
         """
    632 
    -    config_res = git_get(["config", "--get", "diff.tool"])
    633 
    -    if not config_res:
    1433 
    +    try:
    1434 
    +        diff_tool = next(git_lines(["config", "--get", "diff.tool"]))
    1435 
    +    except StopIteration:
    634 1436 
             raise TbDevException("No diff.tool configured for git")
    635 
    -    diff_tool = config_res[0]
    636 1437
    637 1438 
         # Filter out parts of the diff we expect to be different.
    638 1439 
         index_regex = re.compile(r"index [0-9a-f]{12}\.\.[0-9a-f]{12}")
    639 1440 
         lines_regex = re.compile(r"@@ -[0-9]+,[0-9]+ \+[0-9]+,[0-9]+ @@(?P<rest>.*)")
    640 1441
    641 
    -    def save_diff(branch):
    1442 
    +    def save_diff(branch: str) -> str:
    642 1443 
             firefox_commit = get_firefox_ref(branch).commit
    643 1444 
             file_desc, file_name = tempfile.mkstemp(
    644 1445 
                 text=True, prefix=f'{branch.split("/")[-1]}-'
    ... ... @@ -653,6 +1454,7 @@ def show_diff_diff(args):
    653 1454 
             )
    654 1455
    655 1456 
             with os.fdopen(file_desc, "w") as file:
    1457 
    +            assert diff_process.stdout is not None
    656 1458 
                 for line in diff_process.stdout:
    657 1459 
                     if index_regex.match(line):
    658 1460 
                         # Fake data that will match.
    ... ... @@ -665,7 +1467,7 @@ def show_diff_diff(args):
    665 1467 
                         continue
    666 1468 
                     file.write(line)
    667 1469
    668 
    -        status = diff_process.poll()
    1470 
    +        status = diff_process.wait()
    669 1471 
             if status != 0:
    670 1472 
                 raise TbDevException(f"git diff exited with status {status}")
    671 1473
    ... ... @@ -681,7 +1483,7 @@ def show_diff_diff(args):
    681 1483 
     # * -------------------- *
    682 1484
    683 1485
    684 
    -def branch_complete(prefix, parsed_args, **kwargs):
    1486 
    +def branch_complete(prefix: str, **_kwargs: Any) -> list[str]:
    685 1487 
         """
    686 1488 
         Complete the argument with a branch name.
    687 1489 
         """
    ... ... @@ -689,7 +1491,7 @@ def branch_complete(prefix, parsed_args, **kwargs):
    689 1491 
             return []
    690 1492 
         try:
    691 1493 
             branches = [ref.name for ref in get_refs("head", "")]
    692 
    -        branches.extend([ref.name for ref in get_refs("remote", "")])
    1494 
    +        branches.extend(ref.name for ref in get_refs("remote", ""))
    693 1495 
             branches.append("HEAD")
    694 1496 
         except Exception:
    695 1497 
             return []
    ... ... @@ -699,7 +1501,20 @@ def branch_complete(prefix, parsed_args, **kwargs):
    699 1501 
     parser = argparse.ArgumentParser()
    700 1502 
     subparsers = parser.add_subparsers(required=True)
    701 1503
    702 
    -for name, details in {
    1504 
    +
    1505 
    +class ArgConfig(TypedDict):
    1506 
    +    help: str
    1507 
    +    metavar: NotRequired[str]
    1508 
    +    nargs: NotRequired[str]
    1509 
    +    completer: NotRequired[Callable[[str], list[str]]]
    1510 
    +
    1511 
    +
    1512 
    +class CommandConfig(TypedDict):
    1513 
    +    func: Callable[[argparse.Namespace], None]
    1514 
    +    args: NotRequired[dict[str, ArgConfig]]
    1515 
    +
    1516 
    +
    1517 
    +all_commands: dict[str, CommandConfig] = {
    703 1518 
         "show-upstream-basis-commit": {
    704 1519 
             "func": show_upstream_basis_commit,
    705 1520 
         },
    ... ... @@ -716,8 +1531,8 @@ for name, details in {
    716 1531 
                 },
    717 1532 
             },
    718 1533 
         },
    719 
    -    "auto-fixup": {
    720 
    -        "func": auto_fixup,
    1534 
    +    "auto-commit": {
    1535 
    +        "func": auto_commit,
    721 1536 
         },
    722 1537 
         "clean-fixups": {
    723 1538 
             "func": clean_fixups,
    ... ... @@ -794,20 +1609,25 @@ for name, details in {
    794 1609 
                 "regex": {"help": "the regex that the files must contain"},
    795 1610 
             },
    796 1611 
         },
    797 
    -}.items():
    798 
    -    help_message = re.sub(r"\s+", " ", details["func"].__doc__).strip()
    1612 
    +}
    1613 
    +
    1614 
    +for name, command_config in all_commands.items():
    1615 
    +    help_message = command_config["func"].__doc__
    1616 
    +    assert isinstance(help_message, str)
    1617 
    +    help_message = re.sub(r"\s+", " ", help_message).strip()
    799 1618 
         sub = subparsers.add_parser(name, help=help_message)
    800 
    -    sub.set_defaults(func=details["func"])
    801 
    -    for arg, keywords in details.get("args", {}).items():
    1619 
    +    sub.set_defaults(func=command_config["func"])
    1620 
    +    for arg, keywords in command_config.get("args", {}).items():
    802 1621 
             completer = None
    803 1622 
             if "completer" in keywords:
    804 1623 
                 completer = keywords["completer"]
    805 1624 
                 del keywords["completer"]
    806 1625 
             sub_arg = sub.add_argument(arg, **keywords)
    807 
    -        if completer is not None:
    808 
    -            sub_arg.completer = completer
    1626 
    +        if completer is not None and argcomplete is not None:
    1627 
    +            sub_arg.completer = completer  # type: ignore
    809 1628
    810 
    -argcomplete.autocomplete(parser)
    1629 
    +if argcomplete is not None:
    1630 
    +    argcomplete.autocomplete(parser)
    811 1631
    812 1632 
     try:
    813 1633 
         if not within_browser_root():

    • —
    View it on GitLab.
    You're receiving this email because of your account on gitlab.torproject.org. Manage all notifications · Help Notification message regarding https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/f12904c221c038ad500ef2cfce771280c8e26b41...18b3272d08f419edcbdef65988c592a74625b7a7 at 1765464256