henry pushed to branch tor-browser-147.0a1-16.0-1 at The Tor Project / Applications / Tor Browser Commits: 629a5b42 by Beatriz Rizental at 2026-01-12T17:08:09+00:00 fixup! TB 40597: Implement TorSettings module Fix linter issue. - - - - - 55b87f13 by june wilde at 2026-01-12T17:08:11+00:00 fixup! BB 41459: WebRTC fails to build under mingw (Part 2) - - - - - 7b0257e4 by june wilde at 2026-01-12T17:08:12+00:00 fixup! BB 41459: WebRTC fails to build under mingw (Part 1) - - - - - a40941d3 by june wilde at 2026-01-12T17:08:13+00:00 fixup! BB 41459: WebRTC fails to build under mingw (Part 5) - - - - - c1a4a89e by Henry Wilkes at 2026-01-12T17:08:14+00:00 fixup! BB 41803: Add some developer tools for working on tor-browser. TB 44367: Make git_get return the stdout string, rather than a list. Add git_lines to generate lines. - - - - - abf794c5 by Henry Wilkes at 2026-01-12T17:08:15+00:00 fixup! BB 41803: Add some developer tools for working on tor-browser. TB 44367: Use raw diff to get list of file changes. - - - - - 9494d12d by Henry Wilkes at 2026-01-12T17:08:16+00:00 fixup! BB 41803: Add some developer tools for working on tor-browser. TB 44367: Add type annotations and parameter documentation. - - - - - 39511abd by Henry Wilkes at 2026-01-12T17:08:17+00:00 fixup! BB 41803: Add some developer tools for working on tor-browser. TB 44367: Make the argcomplete module optional. - - - - - 7f98a5fd by Henry Wilkes at 2026-01-12T17:08:19+00:00 fixup! BB 41803: Add some developer tools for working on tor-browser. TB 44367: Use function caching instead of global variables. - - - - - b000589a by Henry Wilkes at 2026-01-12T17:08:20+00:00 fixup! BB 41803: Add some developer tools for working on tor-browser. TB 44367: Fetch FIREFOX_ tags from the remote if they are missing. - - - - - da6f0e38 by Henry Wilkes at 2026-01-12T17:08:21+00:00 fixup! BB 41803: Add some developer tools for working on tor-browser. TB 44367: Improve the auto-fixup/auto-commit command. The auto-fixup command was renamed to auto-commit. It now also handles: 1. Already staged changes. 2. Untracked/added files. 3. Removed files. 4. Renamed files. 5. Allowing the user to create a new commit. - - - - - 82513ac3 by Pier Angelo Vendrame at 2026-01-12T17:08:22+00:00 fixup! Tweaks to the build system This reverts commit 4e4d1a17c3fd6148d6ecfdeb3831070e068470af. This reverts commit d0aa909310783cf4bdb219d34f5031d5123f8749. - - - - - 67f0febf by Beatriz Rizental at 2026-01-12T17:08:23+00:00 fixup! TB 43817: Add tests for Tor Browser Bug 43243: Rename marionette.toml to manifest.toml just for consistency sake. All other marionette manifest files are named that. - - - - - 0ce16c8b by Beatriz Rizental at 2026-01-12T17:08:24+00:00 fixup! TB 43817: Add tests for Tor Browser Bug 43243: Make it possible to run all tor test just by using tags. Just adding the tag wasn't enough though, had to add it to a list of tests in integration-tests.toml. Might be an upstream bug, but I don't feel like debugging that. Also it's easy enough. - - - - - e0135559 by Beatriz Rizental at 2026-01-12T17:08:25+00:00 fixup! TB 43817: Add tests for Tor Browser Bug 43243: iInclude testing/tor directory into common test archive. - - - - - b9e408af by Beatriz Rizental at 2026-01-12T17:08:26+00:00 fixup! TB 43817: Add tests for Tor Browser Bug 43243: BUGFIX: Make it possible to run both tor browser tests in sequence. Turns out they need to explicitly close the browser, otherwise marionette doesn't do that for us unless it's the end of the whole suite. We want a restart, because we want to bootstrap before each test. - - - - - ef6aca73 by Henry Wilkes at 2026-01-12T17:08:28+00:00 fixup! TB 43901: Modify about:license for Tor Browser. TB 44420: Drop "rights" from components.conf. - - - - - 17 changed files: - browser/components/about/components.conf - build/moz.configure/windows.configure - dom/media/webrtc/libwebrtc_overrides/modules/desktop_capture/desktop_capture_types.h - python/mach/mach/sentry.py - python/mach/mach/telemetry.py - python/mozbuild/mozbuild/action/test_archive.py - testing/marionette/harness/marionette_harness/tests/integration-tests.toml - testing/moz.build - testing/tor/marionette.toml → testing/tor/manifest.toml - testing/tor/test_circuit_isolation.py - testing/tor/test_network_check.py - third_party/libwebrtc/modules/desktop_capture/win/wgc_capture_session.cc - third_party/libwebrtc/modules/desktop_capture/win/window_capture_utils.h - third_party/libwebrtc/rtc_base/cpu_info.cc - third_party/libwebrtc/rtc_base/win/create_direct3d_device.h - toolkit/content/pt_config.json - tools/base_browser/tb-dev Changes: ===================================== browser/components/about/components.conf ===================================== @@ -25,7 +25,6 @@ pages = [ 'profiling', 'reader', 'restartrequired', - 'rights', # Removed 'rights'. tor-browser#43901. # Removed 'robots'. tor-browser#42831. 'rulesets', ===================================== build/moz.configure/windows.configure ===================================== @@ -628,12 +628,13 @@ with only_when(depends(c_compiler)(lambda c: c.type == "clang-cl")): add_linker_flag("-LARGEADDRESSAWARE") add_linker_flag("-SAFESEH") - # avoid conficts with std::min/max - set_define("NOMINMAX", True) - set_define("WIN32_LEAN_AND_MEAN", True) +with only_when(depends(c_compiler)(lambda c: c.type == "clang-cl")): + # See http://support.microsoft.com/kb/143208 to use STL + set_define("NOMINMAX", True) + with only_when(target_is_windows & depends(c_compiler)(lambda c: c.type != "clang-cl")): # strsafe.h on mingw uses macros for function deprecation that pollutes namespace ===================================== dom/media/webrtc/libwebrtc_overrides/modules/desktop_capture/desktop_capture_types.h ===================================== @@ -7,11 +7,11 @@ #ifndef DOM_MEDIA_WEBRTC_LIBWEBRTCOVERRIDES_MODULES_DESKTOP_CAPTURE_DESKTOP_CAPTURE_TYPES_H_ #define DOM_MEDIA_WEBRTC_LIBWEBRTCOVERRIDES_MODULES_DESKTOP_CAPTURE_DESKTOP_CAPTURE_TYPES_H_ -// pid_t -#if !defined(XP_WIN) || defined(__MINGW32__) +#if defined(XP_WIN) && \ + !defined(__MINGW32__) // Moving this into the global namespace +typedef int pid_t; // matching what used to be in +#elif defined(XP_WIN) // video_capture_defines.h # include <sys/types.h> -#else -typedef int pid_t; #endif #include "../../third_party/libwebrtc/modules/desktop_capture/desktop_capture_types.h" ===================================== python/mach/mach/sentry.py ===================================== @@ -8,7 +8,7 @@ import sys from pathlib import Path from threading import Thread -# import sentry_sdk +import sentry_sdk from mozversioncontrol import ( InvalidRepoPath, MissingUpstreamRepo, @@ -35,8 +35,7 @@ class SentryErrorReporter(ErrorReporter): """Reports errors using Sentry.""" def report_exception(self, exception): - pass - # return sentry_sdk.capture_exception(exception) + return sentry_sdk.capture_exception(exception) class NoopErrorReporter(ErrorReporter): @@ -62,10 +61,10 @@ def register_sentry(argv, settings, topsrcdir: Path): ) _is_unmodified_mach_core_thread.start() - # sentry_sdk.init( - # _SENTRY_DSN, before_send=lambda event, _: _process_event(event, topsrcdir) - # ) - # sentry_sdk.add_breadcrumb(message="./mach {}".format(" ".join(argv))) + sentry_sdk.init( + _SENTRY_DSN, before_send=lambda event, _: _process_event(event, topsrcdir) + ) + sentry_sdk.add_breadcrumb(message="./mach {}".format(" ".join(argv))) return SentryErrorReporter() ===================================== python/mach/mach/telemetry.py ===================================== @@ -7,9 +7,11 @@ import importlib.util import os import subprocess import sys +import urllib.parse as urllib_parse from pathlib import Path from textwrap import dedent +import requests from mozbuild.base import BuildEnvironmentNotFoundException, MozbuildObject from mozbuild.telemetry import filter_args from mozfile import json @@ -90,7 +92,10 @@ def is_applicable_telemetry_environment(): def is_telemetry_enabled(settings): - return False + if os.environ.get("DISABLE_TELEMETRY") == "1": + return False + + return settings.mach_telemetry.is_enabled def arcrc_path(): @@ -127,7 +132,40 @@ def resolve_setting_from_arcconfig(topsrcdir: Path, setting): def resolve_is_employee_by_credentials(topsrcdir: Path): - return None + try: + phabricator_uri = resolve_setting_from_arcconfig(topsrcdir, "phabricator.uri") + + if not phabricator_uri: + return None + + with arcrc_path().open() as arcrc_file: + arcrc = json.load(arcrc_file) + + phabricator_token = ( + arcrc.get("hosts", {}) + .get(urllib_parse.urljoin(phabricator_uri, "api/"), {}) + .get("token") + ) + + if not phabricator_token: + return None + + bmo_uri = ( + resolve_setting_from_arcconfig(topsrcdir, "bmo_url") + or "https://bugzilla.mozilla.org" + ) + bmo_api_url = urllib_parse.urljoin(bmo_uri, "rest/whoami") + bmo_result = requests.get( + bmo_api_url, headers={"X-PHABRICATOR-TOKEN": phabricator_token} + ) + + return "mozilla-employee-confidential" in bmo_result.json().get("groups", []) + except ( + FileNotFoundError, + json.JSONDecodeError, + requests.exceptions.RequestException, + ): + return None def resolve_is_employee_by_vcs(topsrcdir: Path): ===================================== python/mozbuild/mozbuild/action/test_archive.py ===================================== @@ -229,6 +229,12 @@ ARCHIVE_FILES = { "pattern": "**", "dest": "certs", }, + { + "source": buildconfig.topsrcdir, + "base": "", + "pattern": "testing/tor", + "dest": "tor", + }, ], "cppunittest": [ {"source": STAGE, "base": "", "pattern": "cppunittest/**"}, ===================================== testing/marionette/harness/marionette_harness/tests/integration-tests.toml ===================================== @@ -56,6 +56,10 @@ ["include:../../../../../netwerk/test/marionette/manifest.toml"] +# tor tests + +["include:../../../../../testing/tor/manifest.toml"] + # toolkit tests ["include:../../../../../toolkit/components/antitracking/bouncetrackingprotection/test/marionette/manifest.toml"] ===================================== testing/moz.build ===================================== @@ -24,4 +24,4 @@ PERFTESTS_MANIFESTS += [ "performance/perftest.toml", ] -MARIONETTE_MANIFESTS += ["tor/marionette.toml"] +MARIONETTE_MANIFESTS += ["tor/manifest.toml"] ===================================== testing/tor/marionette.toml → testing/tor/manifest.toml ===================================== @@ -1,4 +1,5 @@ [DEFAULT] +tags = "tor" ["test_circuit_isolation.py"] ===================================== testing/tor/test_circuit_isolation.py ===================================== @@ -8,6 +8,9 @@ TOR_BOOTSTRAP_TIMEOUT = 30000 # 30s class TestCircuitIsolation(MarionetteTestCase): + def tearDown(self): + self.marionette.restart(in_app=False, clean=True) + super(TestCircuitIsolation, self).tearDown() def bootstrap(self): with self.marionette.using_context("chrome"): ===================================== testing/tor/test_network_check.py ===================================== @@ -14,6 +14,10 @@ class TestNetworkCheck(MarionetteTestCase): self.l10n = L10n(self.marionette) + def tearDown(self): + self.marionette.restart(in_app=False, clean=True) + super(TestNetworkCheck, self).tearDown() + def attemptConnection(self, tries=1): if tries > 3: self.assertTrue(False, "Failed to connect to Tor after 3 attempts") ===================================== third_party/libwebrtc/modules/desktop_capture/win/wgc_capture_session.cc ===================================== @@ -11,6 +11,8 @@ #include <dispatcherqueue.h> #include <windows.graphics.capture.interop.h> #include <windows.graphics.directx.direct3d11.interop.h> +#include <windows.graphics.h> +#include <wrl/client.h> #include <wrl/event.h> #include <algorithm> ===================================== third_party/libwebrtc/modules/desktop_capture/win/window_capture_utils.h ===================================== @@ -11,7 +11,7 @@ #ifndef MODULES_DESKTOP_CAPTURE_WIN_WINDOW_CAPTURE_UTILS_H_ #define MODULES_DESKTOP_CAPTURE_WIN_WINDOW_CAPTURE_UTILS_H_ -#include <shlobj_core.h> +#include <shlobj.h> #include <windows.h> #include <wrl/client.h> ===================================== third_party/libwebrtc/rtc_base/cpu_info.cc ===================================== @@ -97,7 +97,7 @@ uint64_t xgetbv(uint32_t xcr) { } #endif // WEBRTC_ENABLE_AVX2 -#ifndef _MSC_VER +#if !defined(_MSC_VER) && !defined(__MINGW32__) // Intrinsic for "cpuid". #if defined(__pic__) && defined(__i386__) static inline void __cpuid(int cpu_info[4], int info_type) { ===================================== third_party/libwebrtc/rtc_base/win/create_direct3d_device.h ===================================== @@ -12,9 +12,8 @@ #define RTC_BASE_WIN_CREATE_DIRECT3D_DEVICE_H_ #include <windows.graphics.directx.direct3d11.h> -#ifndef __MINGW32__ -# include <windows.graphics.directX.direct3d11.interop.h> -#else +#include <windows.graphics.directx.direct3d11.interop.h> +#ifdef __MINGW32__ # include <dxgi.h> # include <inspectable.h> extern "C" { @@ -23,6 +22,7 @@ HRESULT __stdcall CreateDirect3D11DeviceFromDXGIDevice( ::IDXGIDevice* dxgiDevice, ::IInspectable** graphicsDevice); } #endif + #include <winerror.h> #include <wrl/client.h> ===================================== toolkit/content/pt_config.json ===================================== @@ -25,4 +25,3 @@ ] } } - ===================================== tools/base_browser/tb-dev ===================================== @@ -6,6 +6,7 @@ Useful tools for working on tor-browser repository. import argparse import atexit +import functools import json import os import re @@ -14,8 +15,15 @@ import sys import tempfile import termios import urllib.request +from collections.abc import Callable, Iterable, Iterator +from types import ModuleType +from typing import Any, NotRequired, TypedDict, TypeVar -import argcomplete +argcomplete: None | ModuleType = None +try: + import argcomplete +except ImportError: + pass GIT_PATH = "/usr/bin/git" UPSTREAM_URLS = { @@ -36,9 +44,14 @@ class TbDevException(Exception): pass -def git_run(args, check=True, env=None): +def git_run( + args: list[str], check: bool = True, env: None | dict[str, str] = None +) -> None: """ Run a git command with output sent to stdout. + :param args: The arguments to pass to git. + :param check: Whether to check for success. + :param env: Optional environment to set. """ if env is not None: tmp_env = dict(os.environ) @@ -51,46 +64,122 @@ def git_run(args, check=True, env=None): raise TbDevException(str(err)) from err -def git_get(args): +def git_run_pager( + args: list[str] | None = None, + arg_sequence: Iterable[list[str]] | None = None, + pager_prefix: None | str = None, +) -> None: """ - Run a git command with each non-empty line returned in a list. + Run a sequence of git commands with the output concatenated and sent to the + git pager. + :param args: The arguments to pass to git, or `None` if a sequence is desired. + :param arg_sequence: A sequence representing several git commands. + :param pager_prefix: An optional text to send to the pager first. + """ + if arg_sequence is None: + if args is not None: + arg_sequence = (args,) + else: + raise ValueError("Missing `arg_sequence` or `args`") + elif args is not None: + raise ValueError("Unexpected both args and arg_sequence") + + pager = git_get(["var", "GIT_PAGER"]) + if not pager: + raise TbDevException("Missing a GIT_PAGER") + command = [pager] + if os.path.basename(pager) == "less": + # Show colours. + command.append("-R") + + pager_process = subprocess.Popen(command, stdin=subprocess.PIPE, text=True) + assert pager_process.stdin is not None + + if pager_prefix is not None: + pager_process.stdin.write(pager_prefix) + pager_process.stdin.flush() + + for git_args in arg_sequence: + subprocess.run( + [GIT_PATH, "--no-pager", *git_args], check=False, stdout=pager_process.stdin + ) + + pager_process.stdin.close() + + status = pager_process.wait() + if status != 0: + raise TbDevException(f"git pager {pager} exited with status {status}") + + +def git_get(args: list[str], strip: bool = True, check: bool = True) -> str: + """ + Return the output from a git command. + :param args: The arguments to send to git. + :param strip: Whether to strip the whitespace from the output. + :param check: Whether to check for success. + :returns: The stdout. """ try: git_process = subprocess.run( - [GIT_PATH, *args], text=True, stdout=subprocess.PIPE, check=True + [GIT_PATH, *args], text=True, stdout=subprocess.PIPE, check=check ) except subprocess.CalledProcessError as err: raise TbDevException(str(err)) from err - return [line for line in git_process.stdout.split("\n") if line] + ret = git_process.stdout + if strip: + ret = ret.strip() + return ret -local_root = None +def git_lines(args: list[str]) -> Iterator[str]: + """ + Yields the non-empty lines returned by the git command. + :param args: The arguments to send to git. + :yield: The lines. + """ + for line in git_get(args, strip=False).split("\n"): + if not line: + continue + yield line + + +def git_path_args(path_iter: Iterable[str]) -> Iterator[str]: + """ + Generate the trailing arguments to specify paths in git commands, includes + the "--" separator just before the paths. + :param path_iter: The paths that should be passed in. + :yields: The git arguments. + """ + yield "--" + for path in path_iter: + yield f":(literal){path}" -def get_local_root(): +@functools.cache +def get_local_root() -> str: """ Get the path for the tor-browser root directory. + :returns: The local root. """ - global local_root - if local_root is None: - try: - # Make sure we have a matching remote in this git repository. - if get_upstream_details()["is-browser-repo"]: - local_root = git_get(["rev-parse", "--show-toplevel"])[0] - else: - local_root = "" - except TbDevException: - local_root = "" - return local_root + try: + # Make sure we have a matching remote in this git repository. + if get_upstream_details()["is-browser-repo"] == "True": + return git_get(["rev-parse", "--show-toplevel"]) + else: + return "" + except TbDevException: + return "" -def determine_upstream_details(): +@functools.cache +def get_upstream_details() -> dict[str, str]: """ - Determine details about the upstream. + Get details about the upstream repository. + :returns: The details. """ remote_urls = { - remote: git_get(["remote", "get-url", remote])[0] - for remote in git_get(["remote"]) + remote: git_get(["remote", "get-url", remote]) + for remote in git_lines(["remote"]) } matches = { @@ -102,7 +191,7 @@ def determine_upstream_details(): } is_browser_repo = len(matches) > 0 - details = {"is-browser-repo": is_browser_repo} + details = {"is-browser-repo": str(is_browser_repo)} origin_remote_repo = matches.get("origin", None) upstream_remote_repo = matches.get("upstream", None) @@ -125,31 +214,30 @@ def determine_upstream_details(): return details -cached_upstream_details = None - - -def get_upstream_details(): - """ - Get details about the upstream repository. - """ - global cached_upstream_details - if cached_upstream_details is None: - cached_upstream_details = determine_upstream_details() - return cached_upstream_details - - class Reference: """Represents a git reference to a commit.""" - def __init__(self, name, commit): - self.name = name + _REFS_REGEX = re.compile(r"refs/[a-z]+/") + + def __init__(self, full_name: str, commit: str) -> None: + """ + :param full_name: The full reference name. E.g. "refs/tags/MyTag". + :param commit: The commit hash for the commit this reference points to. + """ + match = self.__class__._REFS_REGEX.match(full_name) + if not match: + raise ValueError(f"Invalid reference name {full_name}") + self.full_name = full_name + self.name = full_name[match.end() :] self.commit = commit -def get_refs(ref_type, name_start): +def get_refs(ref_type: str, name_start: str) -> Iterator[Reference]: """ - Get a list of references that match the given 'ref_type' ("tag" or "remote" - or "head") that starts with the given 'name_start'. + Get a list of references that match the given conditions. + :param ref_type: The ref type to search for ("tag" or "remote" or "head"). + :param name_start: The ref name start to match against. + :yield: The matching references. """ if ref_type == "tag": ref_start = "refs/tags/" @@ -163,56 +251,83 @@ def get_refs(ref_type, name_start): fstring = "%(*objectname),%(objectname),%(refname)" pattern = f"{ref_start}{name_start}**" - def line_to_ref(line): + def line_to_ref(line: str) -> Reference: [objectname_reference, objectname, ref_name] = line.split(",", 2) # For annotated tags, the objectname_reference is non-empty and points # to an actual commit. # For remotes, heads and lightweight tags, the objectname_reference will # be empty and objectname will point directly to the commit. - return Reference( - ref_name.replace(ref_start, "", 1), objectname_reference or objectname - ) + return Reference(ref_name, objectname_reference or objectname) - return [ + return ( line_to_ref(line) - for line in git_get(["for-each-ref", f"--format={fstring}", pattern]) - ] + for line in git_lines(["for-each-ref", f"--format={fstring}", pattern]) + ) -def get_nearest_ref(ref_type, name_start, search_from): +def get_firefox_ref(search_from: str) -> Reference: """ - Search backwards from the 'search_from' commit to find the first commit - that matches the given 'ref_type' that starts with the given 'name_start'. + Search for the commit that comes from firefox. + :param search_from: The commit to search backwards from. + :returns: The firefox reference. """ - ref_list = get_refs(ref_type, name_start) + # Only search a limited history that should include the FIREFOX_ tag. + search_commits = [c for c in git_lines(["rev-list", "-1000", search_from])] + + firefox_tag_prefix = "FIREFOX_" - for commit in git_get(["rev-list", "-1000", search_from]): - for ref in ref_list: + existing_tags = list(get_refs("tag", firefox_tag_prefix)) + for commit in search_commits: + for ref in existing_tags: if commit == ref.commit: return ref - raise TbDevException(f"No {name_start} commit found in the last 1000 commits") - - -def get_firefox_ref(search_from): + # Might just need to fetch tags from the remote. + upstream = get_upstream_details().get("remote", None) + if upstream: + remote_ref: None | Reference = None + search_index = len(search_commits) + # Search the remote for a tag that is in our history. + # We want to avoid triggering a long fetch, so we just want to grab the + # tag that already points to a commit in our history. + for line in git_lines( + ["ls-remote", upstream, f"refs/tags/{firefox_tag_prefix}*"] + ): + objectname, name = line.split("\t", 1) + for index in range(search_index): + if search_commits[index] == objectname: + # Remove trailing "^{}" for commits pointed to by + # annotated tags. + remote_ref = Reference(re.sub(r"\^\{\}$", "", name), objectname) + # Only continue to search for references that are even + # closer to `search_from`. + search_index = index + break + if remote_ref is not None: + # Get a local copy of just this tag. + git_run(["fetch", "--no-tags", upstream, "tag", remote_ref.name]) + return ref + + raise TbDevException("Unable to find FIREFOX_ tag") + + +def get_upstream_tracking_branch(search_from: str) -> str: """ - Search backwards from the 'search_from' commit to find the commit that comes - from firefox. + :param search_from: The commit reference. + :returns: The upstream branch reference name. """ - return get_nearest_ref("tag", "FIREFOX_", search_from) - - -def get_upstream_tracking_branch(search_from): - return git_get(["rev-parse", "--abbrev-ref", f"{search_from}@{{upstream}}"])[0] + return git_get(["rev-parse", "--abbrev-ref", f"{search_from}@{{upstream}}"]) -def get_upstream_basis_commit(search_from): +def get_upstream_basis_commit(search_from: str) -> str: """ Get the first common ancestor of search_from that is also in its upstream branch. + :param search_from: The commit reference. + :returns: The upstream commit hash. """ upstream_branch = get_upstream_tracking_branch(search_from) - commit = git_get(["merge-base", search_from, upstream_branch])[0] + commit = git_get(["merge-base", search_from, upstream_branch]) # Verify that the upstream commit shares the same firefox basis. Otherwise, # this would indicate that the upstream is on an early or later FIREFOX # base. @@ -226,26 +341,82 @@ def get_upstream_basis_commit(search_from): return commit -def get_changed_files(from_commit, staged=False): +class FileChange: + """Represents a git change to a commit.""" + + def __init__(self, status: str, path: str, new_path: str) -> None: + """ + :param status: The file change status used within git diff. E.g. "M" for + modified, or "D" for deleted. + :param path: The source file path. + :param new_path: The file path after the change. + """ + self.status = status + self.path = path + self.new_path = new_path + + +RAW_DIFF_PATH_PATTERN = r"(?P<path>[^\0]*)\0" +RAW_DIFF_LINE_REGEX = re.compile( + r":[0-7]+ [0-7]+ [0-9a-f]+ [0-9a-f]+ (?P<status>[ADMTUXRC])[0-9]*\0" + + RAW_DIFF_PATH_PATTERN +) +RAW_DIFF_PATH_REGEX = re.compile(RAW_DIFF_PATH_PATTERN) + + +def parse_raw_diff_line(raw_output: str) -> tuple[FileChange, int]: """ - Get a list of filenames relative to the current working directory that have + Parse the --raw diff output from git. + :param raw_output: The raw output. + :returns: The change for this line, and the offset for the end of the raw + diff line. + """ + match = RAW_DIFF_LINE_REGEX.match(raw_output) + if not match: + raise ValueError(f"Invalid raw output: {raw_output[:50]}...") + path = os.path.relpath(os.path.join(get_local_root(), match.group("path"))) + status = match.group("status") + if status in ("R", "C"): + match = RAW_DIFF_PATH_REGEX.match(raw_output, pos=match.end()) + if not match: + raise ValueError(f"Invalid raw output for rename: {raw_output[:50]}...") + new_path = os.path.relpath(os.path.join(get_local_root(), match.group("path"))) + else: + new_path = path + + return FileChange(status, path, new_path), match.end() + + +def get_changed_files( + from_commit: None | str = None, staged: bool = False +) -> Iterator[FileChange]: + """ + Get a list of file changes relative to the current working directory that have been changed since 'from_commit' (non-inclusive). + :param from_commit: The commit to compare against, otherwise use the git + diff default. + :param staged: Whether to limit the diff to staged changes. + :yield: The file changes. """ - args = ["diff"] + args = ["diff", "-z", "--raw"] if staged: args.append("--staged") - args.append("--name-only") - args.append(from_commit) - return [ - os.path.relpath(os.path.join(get_local_root(), filename)) - for filename in git_get(args) - ] + if from_commit: + args.append(from_commit) + raw_output = git_get(args, strip=False) + while raw_output: + file_change, end = parse_raw_diff_line(raw_output) + yield file_change + raw_output = raw_output[end:] -def file_contains(filename, regex): +def file_contains(filename: str, regex: re.Pattern[str]) -> bool: """ Return whether the file is a utf-8 text file containing the regular expression given by 'regex'. + :param filename: The file path. + :param regex: The pattern to search for. + :returns: Whether the pattern was matched. """ with open(filename, encoding="utf-8") as file: try: @@ -258,9 +429,10 @@ def file_contains(filename, regex): return False -def get_gitlab_default(): +def get_gitlab_default() -> str: """ Get the name of the default branch on gitlab. + :returns: The branch name. """ repo_name = get_upstream_details().get("repo-name", None) if repo_name is None: @@ -283,12 +455,14 @@ def get_gitlab_default(): ) with urllib.request.urlopen(gitlab_request, timeout=20) as response: - return json.load(response)["data"]["project"]["repository"]["rootRef"] + default = json.load(response)["data"]["project"]["repository"]["rootRef"] + assert isinstance(default, str) + return default -def within_browser_root(): +def within_browser_root() -> bool: """ - Whether we are with the tor browser root. + :returns: Whether we are with the tor browser root. """ root = get_local_root() if not root: @@ -301,24 +475,24 @@ def within_browser_root(): # * -------------------- * -def show_firefox_commit(_args): +def show_firefox_commit(_args: argparse.Namespace) -> None: """ Print the tag name and commit for the last firefox commit below the current HEAD. """ ref = get_firefox_ref("HEAD") - print(ref.name) + print(ref.full_name) print(ref.commit) -def show_upstream_basis_commit(_args): +def show_upstream_basis_commit(_args: argparse.Namespace) -> None: """ Print the last upstream commit for the current HEAD. """ print(get_upstream_basis_commit("HEAD")) -def show_log(args): +def show_log(args: argparse.Namespace) -> None: """ Show the git log between the current HEAD and the last firefox commit. """ @@ -326,7 +500,7 @@ def show_log(args): git_run(["log", f"{commit}..HEAD", *args.gitargs], check=False) -def show_files_containing(args): +def show_files_containing(args: argparse.Namespace) -> None: """ List all the files that that have been modified for tor browser, that also contain a regular expression. @@ -336,33 +510,32 @@ def show_files_containing(args): except re.error as err: raise TbDevException(f"{args.regex} is not a valid python regex") from err - file_list = get_changed_files(get_firefox_ref("HEAD").commit) - - for filename in file_list: - if not os.path.isfile(filename): + for file_change in get_changed_files(get_firefox_ref("HEAD").commit): + path = file_change.new_path + if not os.path.isfile(path): # deleted ofile continue - if file_contains(filename, regex): - print(filename) + if file_contains(path, regex): + print(path) -def show_changed_files(_args): +def show_changed_files(_args: argparse.Namespace) -> None: """ List all the files that have been modified relative to upstream. """ - for filename in get_changed_files(get_upstream_basis_commit("HEAD")): - print(filename) + for file_change in get_changed_files(get_upstream_basis_commit("HEAD")): + print(file_change.new_path) -def lint_changed_files(args): +def lint_changed_files(args: argparse.Namespace) -> None: """ Lint all the files that have been modified relative to upstream. """ os.chdir(get_local_root()) file_list = [ - f + f.new_path for f in get_changed_files(get_upstream_basis_commit("HEAD")) - if os.path.isfile(f) # Not deleted + if os.path.isfile(f.new_path) # Not deleted ] # We add --warnings since clang only reports whitespace issues as warnings. subprocess.run( @@ -371,10 +544,18 @@ def lint_changed_files(args): ) -def prompt_user(prompt, convert): +# TODO: replace with "prompt_user[T](..., T]) -> T" after python 3.12 is the +# minimum mach version. +T = TypeVar("T") + + +def prompt_user(prompt: str, convert: Callable[[str], T]) -> T: """ - Ask the user for some input until the given converter returns without - throwing a ValueError. + Ask the user for some input. + :param prompt: The prompt to show the user. + :param convert: A method to convert the response into a type. Should + throw `ValueError` if the user should be re-prompted for a valid input. + :returns: The first valid user response. """ while True: # Flush out stdin. @@ -388,8 +569,12 @@ def prompt_user(prompt, convert): pass -def binary_reply_default_no(value): - """Process a 'y' or 'n' reply, defaulting to 'n' if empty.""" +def binary_reply_default_no(value: str) -> bool: + """ + Process a 'y' or 'n' reply, defaulting to 'n' if empty. + :param value: The user input. + :returns: Whether the answer is yes. + """ if value == "": return False if value.lower() == "y": @@ -399,121 +584,737 @@ def binary_reply_default_no(value): raise ValueError() -def get_fixup_for_file(filename, firefox_commit): - """Find the commit the given file should fix up.""" +class FixupTarget: + """Represents a commit that can be targeted by a fixup.""" + + def __init__(self, commit: str, short_ref: str, title: str) -> None: + """ + :param commit: The commit hash for the commit. + :param short_ref: The shortened commit hash for display. + :param title: The first line of the commit message. + """ + self.commit = commit + self.short_ref = short_ref + self.title = title + self.changes: list[FileChange] = [] + self.fixups: list[FixupTarget] = [] + self.target: None | FixupTarget = None + + _FIXUP_REGEX = re.compile(r"^fixup! +") + + def trim_fixup(self) -> tuple[str, int]: + """ + Trim the "fixup!" prefixes. + :returns: The stripped commit title and the fixup depth (how many fixups + prefixes there were). + """ + title = self.title + depth = 0 + while True: + match = self.__class__._FIXUP_REGEX.match(title) + if not match: + return title, depth + title = title[match.end() :] + depth += 1 + + def touches_path( + self, path: str, filter_status: None | str = None, check_dir: bool = False + ) -> bool: + """ + Whether this target, or one of its fixups or target, touches the given + path. + :param path: The path to check. + :param filter_status: Limit the detected changes to the given status(es). + :param check_dir: Whether we should treat `path` as a directory and check for + files within it. + :returns: Whether this target matches. + """ + # NOTE: In the case of renames, we generally assume that renames occur + # in the fixup targets. E.g. "Commit 1" creates the file "file.txt", and + # "fixup! Commit 1" renames it to "new.txt". In this case, if the + # FixupTarget for "Commit 1" is passed in "file.txt" it will match. And + # if it is passed in "new.txt" it will also match via the self.fixups + # field, which will include the "fixup! Commit 1" rename. + # But the "fixup ! Commit 1" FixupTargets will only match with + # "file.txt" if they occurred before the rename fixup, and will only + # match with "new.txt" if they occur after the rename fixup. With the + # exception of the rename fixup itself, which will match both. + # + # In principle, we could identify a file across renames (have a mapping + # from each commit to what the file is called at that stage) and match + # using this file identifier. Similar to the "--follow" git diff + # argument. This would then cover cases where a rename occurs between + # the commit and its fixups, and allow fixups before the rename to also + # match. However, the former case is unexpected and the latter case + # would not be that useful. + if self._touches_path_basis(path, filter_status, check_dir): + return True + # Mark this as a valid target for the path if one of our fixups changes + # this path. + # NOTE: We use _touch_path_basis to prevent recursion. This means we + # will only check one layer up or down, but we only expect fixups of + # up to depth 1. + for fixup_target in self.fixups: + if fixup_target._touches_path_basis(path, filter_status, check_dir): + return True + # Mark this as a valid target if our target changes this path. + if self.target is not None and self.target._touches_path_basis( + path, filter_status, check_dir + ): + return True + return False + + def _touches_path_basis( + self, path: str, filter_status: None | str, check_dir: bool + ) -> bool: + """ + Whether this target touches the given path. + :param path: The path to check. + :param filter_status: Limit the detected changes to the given status. + :param check_dir: Whether we should treat `path` as a directory and check for + files within it. + :returns: Whether this target matches. + """ + for file_change in self.changes: + if filter_status is not None and file_change.status not in filter_status: + continue + for test_path in (file_change.path, file_change.new_path): + if check_dir: + if os.path.commonpath((os.path.dirname(test_path), path)) == path: + # test_path's directory matches the path or is within it. + return True + elif test_path == path: + return True + return False + + +def get_fixup_targets( + target_list: list[FixupTarget], + from_commit: str, + to_commit: str, + fixup_depth: int = 0, +) -> None: + """ + Find all the commits that can be targeted by a fixup between the given + commits. + :param target_list: The list to fill with targets. Appended in the order of + `from_commit` to `to_commit`. + :param from_commit: The commit to start from (non-inclusive). + :param to_commit: The commit to end on (inclusive). + :param fixup_depth: The maximum "depth" of fixups. I.e. how many "fixup!" + prefixes to allow. + """ + raw_output = git_get( + [ + "log", + "--pretty=format:%H,%h,%s", + "--reverse", + "--raw", + "-z", + f"{from_commit}..{to_commit}", + ], + strip=False, + ) + pretty_regex = re.compile( + r"(?P<commit>[0-9a-f]+),(?P<short_ref>[0-9a-f]+),(?P<title>[^\n\0]*)\n" + ) + excluded_regex_list = [ + re.compile(r"^Bug [0-9]+.*r="), # Backported Mozilla bug. + re.compile(r"^dropme! "), + ] + + while raw_output: + match = pretty_regex.match(raw_output) + if not match: + raise ValueError(f"Invalid pretty format: {raw_output[:100]}...") + fixup_target = FixupTarget( + match.group("commit"), match.group("short_ref"), match.group("title") + ) + raw_output = raw_output[match.end() :] + while raw_output and raw_output[0] != "\0": + file_change, end = parse_raw_diff_line(raw_output) + fixup_target.changes.append(file_change) + raw_output = raw_output[end:] + if raw_output: + # Skip over the "\0". + raw_output = raw_output[1:] + + for regex in excluded_regex_list: + if regex.match(fixup_target.title): + # Exclude from the list. + continue + + trimmed_title, depth = fixup_target.trim_fixup() + if depth: + original_target = None + for target in target_list: + if target.title == trimmed_title: + original_target = target + break + + if original_target: + original_target.fixups.append(fixup_target) + fixup_target.target = original_target + if depth > fixup_depth: + # Exclude from the list. + continue + + target_list.append(fixup_target) + + +class NewCommitBasis: + def __init__(self) -> None: + self.staged_paths: set[str] = set() + self.adding_paths: set[str] = set() + + def add(self, paths: Iterable[str], staged: bool) -> None: + """ + Add a path to include in this commit. + :param paths: The paths to add. + :param staged: Whether we are adding already staged changes. + """ + if staged: + self.staged_paths.update(paths) + return + + self.adding_paths.update(paths) + + +class NewCommit(NewCommitBasis): + """Represents a new commit that we want to create.""" - def parse_log_line(line): - [commit, short_ref, title] = line.split(",", 2) - return {"commit": commit, "short-ref": short_ref, "title": title} + def __init__(self, alias: str) -> None: + """ + :param alias: The alias name for the commit. + """ + super().__init__() + self.alias = alias - options = [ - parse_log_line(line) - for line in git_get( - [ - "log", - "--pretty=format:%H,%h,%s", - f"{firefox_commit}..HEAD", - "--", - filename, - ] + +class NewFixup(NewCommitBasis): + """Represents a new fixup commit that we want to create.""" + + def __init__(self, target: FixupTarget) -> None: + """ + :param target: The commit to target with the fixup. + """ + super().__init__() + self.target = target + + +def get_suggested_fixup_targets_for_change( + file_change: FileChange, + fixup_target_list: list[FixupTarget], + firefox_directories_lazy: Callable[[], set[str]], +) -> Iterator[FixupTarget]: + """ + Find the suggested fixup targets for the given file change. + :param file_change: The file change to get a suggestion for. + :param fixup_target_list: The list to choose from. + :param firefox_directories_lazy: Lazy method to return the firefox + directories. + :yield: The suggested fixup targets. + """ + + def filter_list( + path: str, filter_status: None | str = None, check_dir: bool = False + ) -> Iterator[FixupTarget]: + return ( + t + for t in fixup_target_list + if t.touches_path(path, filter_status=filter_status, check_dir=check_dir) ) + + if file_change.status == "D": + # Deleted. + # Find the commit that introduced this file or previously deleted it. + # I.e. added the file ("A"), renamed it ("R"), or deleted it ("D"). + yield from filter_list(file_change.path, filter_status="ARD") + return + + if file_change.status == "A": + # First check to see if this file name was actually touched before. + yielded_target = False + for target in filter_list(file_change.path): + yielded_target = True + yield target + if yielded_target: + return + # Else, find commits that introduced files in the same directory, or + # deleted in them, if they are not firefox directories. + dir_path = file_change.path + while True: + dir_path = os.path.dirname(dir_path) + if not dir_path or dir_path in firefox_directories_lazy(): + return + + yielded_target = False + for target in filter_list(dir_path, filter_status="ARD", check_dir=True): + yielded_target = True + yield target + + if yielded_target: + return + # Else, search one directory higher. + + if file_change.status == "R": + # Renamed. + # Find the commit that introduced the original name for this file. + yield from filter_list(file_change.path, filter_status="AR") + return + + # Modified. + yield from filter_list(file_change.path) + + +def ask_for_target( + file_change_list: list[FileChange], + new_commits_list: list[NewCommit | NewFixup], + suggested_fixup_target_list: list[FixupTarget], + full_fixup_target_list: list[FixupTarget], + staged: bool = False, +) -> bool: + """ + Ask the user to choose a target. + :param file_change_list: The file changes to ask for. + :param new_commits_list: The list of pending new commits, may be added to. + :param suggested_fixup_target_list: The list of suggested target fixups + to choose from. + :param staged: Whether this is for staged changes. + :returns: `True` if the operation should be aborted. + """ + + new_paths = [c.new_path for c in file_change_list] + all_paths = set(new_paths).union(c.path for c in file_change_list) + non_fixup_commits: list[NewCommit] = [ + n for n in new_commits_list if isinstance(n, NewCommit) ] - if not options: - print(f"No commit found for {filename}") - return None - def valid_index(val): + shown_list: list[NewCommit | FixupTarget] = ( + non_fixup_commits + suggested_fixup_target_list + ) + + can_skip = not staged + shown_full = False + + index_offset = 2 + + def valid_response(val: str) -> tuple[str, None | NewCommit | FixupTarget]: + val = val.strip() + + if val == "h": + return "help", None + + if val == "a": + return "abort", None + if val == "d": - return val + return "diff", None + + if val == "f": + if shown_full: + # Already done once. + raise ValueError() + return "full-list", None + is_patch_full = val.startswith("P") is_patch = val.startswith("p") - if is_patch: - val = val[1:] + if is_patch or is_patch_full: + index = int(val[1:], base=10) # Raises ValueError if not integer. + else: + index = int(val, base=10) # Raises ValueError if not integer. + if index == 0: + if not can_skip: + raise ValueError() + return "skip", None + + if index == 1: + return "new", None - # May raise a ValueError. - as_index = int(val) - if as_index < 0 or as_index > len(options): + index -= index_offset + + if index < 0 or index >= len(shown_list): raise ValueError() - if as_index == 0: - if is_patch: + selected = shown_list[index] + + if is_patch_full: + return "patch-full", selected + if is_patch: + return "patch", selected + return "target", selected + + def alias_response(val: str) -> str: + # Choose a default alias name if none is given. + val = val.strip() or f"New commit {len(non_fixup_commits)}" + for new_commit in non_fixup_commits: + if new_commit.alias == val: + # Already in use. raise ValueError() - return None + return val + + def print_index_option(index: int, description: str) -> None: + print(f" \x1b[1m{index}\x1b[0m: {description}") - return (is_patch, options[as_index - 1]["commit"]) + def in_pink(text: str) -> str: + return f"\x1b[1;38;5;212m{text}\x1b[0m" + prefix_str = "For " + (in_pink("staged") if staged else "unstaged") + " changes to" + if len(new_paths) == 1: + print(f"{prefix_str} {in_pink(new_paths[0])}:") + else: + print(f"{prefix_str}:") + for path in new_paths: + print(f" {in_pink(path)}") + print("") + + show_help = True + reshow_list = True while True: - print(f"For {filename}:\n") - print(" \x1b[1m0\x1b[0m: None") - for index, opt in enumerate(options): - print( - f" \x1b[1m{index + 1}\x1b[0m: " - + f"\x1b[1;38;5;212m{opt['short-ref']}\x1b[0m " - + opt["title"] - ) + if reshow_list: + if can_skip: + print_index_option(0, "Skip") + print_index_option(1, "New commit") + for index, target in enumerate(shown_list, start=index_offset): + if isinstance(target, NewCommit): + print_index_option(index, f"Add to new commit: {target.alias}") + else: + print_index_option( + index, f"Fixup: {in_pink(target.short_ref)} {target.title}" + ) + reshow_list = False print("") - response = prompt_user( - "Choose an <index> to fixup, or '0' to skip this file, " - "or 'd' to view the pending diff, " - "or 'p<index>' to view the patch for the index: ", - valid_index, + + response, selected = prompt_user( + ( + "Choose an <index> to target. Type 'h' for additional options: " + if show_help + else "Choose an <index> to target or an option: " + ), + valid_response, ) - if response is None: - # Skip this file. - return None - if response == "d": - git_run(["diff", "--", filename]) + if response == "help": + print("Options:") + for option, desc in ( + ("h", "show the available options."), + ("a", "abort this commit operation and all pending commits."), + ( + ("", "") + if shown_full + else ( + "f", + "show the full list of fixup targets, rather than just the suggested ones.", + ) + ), + ("d", "view the diff for the pending file changes."), + ( + "P<index>", + "view the patch for the index (including its relevant fixups).", + ), + ( + "p<index>", + "view the patch for the index (including its relevant fixups), " + "limited to the current files.", + ), + ): + if not option: + # Skip this option. + continue + print(f" \x1b[1m{option[0]}\x1b[0m{option[1:].ljust(7)}: {desc}") + # Do not show the help option again. + show_help = False + continue + + if response == "abort": + return True + + if response == "skip": + return False + + if response == "new": + new_alias = prompt_user( + "Enter an optional temporary alias for this new commit: ", + alias_response, + ) + new_commit = NewCommit(new_alias) + new_commit.add(all_paths, staged) + new_commits_list.append(new_commit) + return False + + if response == "target": + assert selected is not None + + if isinstance(selected, NewCommit): + # Adding to a new commit. + selected.add(all_paths, staged) + return False + + for new_fixup in new_commits_list: + if not isinstance(new_fixup, NewFixup): + continue + if new_fixup.target == selected: + # We already have a pending fixup commit that targets this + # selected target. Add this path to the same commit. + new_fixup.add(all_paths, staged) + return False + + new_fixup = NewFixup(selected) + new_fixup.add(all_paths, staged) + new_commits_list.append(new_fixup) + return False + + if response == "full-list": + shown_list = non_fixup_commits + full_fixup_target_list + shown_full = True + reshow_list = True continue - view_patch, commit = response - if view_patch: - git_run(["log", "-p", "-1", commit, "--", filename]) + if response == "diff": + git_args = ["diff", "--color"] + if staged: + git_args.append("--staged") + git_args.extend(git_path_args(all_paths)) + git_run_pager(git_args) continue - return commit + if response in ("patch", "patch-full"): + assert selected is not None + + filter_paths = response == "patch" + + if isinstance(selected, NewCommit): + git_sequence = [ + ["diff", "--color", "--staged", *git_path_args((path,))] + for path in selected.staged_paths + if not filter_paths or path in all_paths + ] + git_sequence.extend( + ["diff", "--color", *git_path_args((path,))] + for path in selected.adding_paths + if not filter_paths or path in all_paths + ) + + # Show what the expected patch will be for the new commit. + git_run_pager( + arg_sequence=git_sequence, pager_prefix=f"{selected.alias}\n\n" + ) + else: + # Show the log entry for the FixupTarget and each of its fixups. + # Order with the commmit closest to HEAD first. We expect + # selected.fixups to match this order. + git_sequence = [] + # If `filter_paths` is set, we want to limit the log to the + # paths, and try to track any renames in the commit history. + prev_log_paths: None | set[str] = None + # For the first commit in the sequence, we use the old path + # names (rather than `c.new_path`) since we expect the commit + # which is closest to us to use the older names. + log_paths: None | set[str] = ( + {c.path for c in file_change_list} if filter_paths else None + ) + for target in (*selected.fixups, selected): + git_args = [ + "log", + "--color", + "-p", + f"{target.commit}~1..{target.commit}", + ] + if filter_paths: + assert log_paths is not None + # Track the renamed paths. + prev_log_paths = log_paths.copy() + for file_change in target.changes: + if ( + file_change.status == "R" + and file_change.new_path in log_paths + ): + # file was renamed in this change. + # Update log_paths to the new name. + # NOTE: This should have a similar effect to the + # --follow option for git log for a single file + # NOTE: File renames will not be properly + # tracked if a rename occurs outside of + # `selected.changes` or + # `selected.fixups[].changes`, but this is + # unexpected. + log_paths.remove(file_change.new_path) + log_paths.add(file_change.path) + + # NOTE: This log entry may be empty if none of the paths + # match. + # NOTE: We include both log_paths and prev_log_paths to + # show renames in the diff output. + git_args.extend(git_path_args(log_paths | prev_log_paths)) + git_sequence.append(git_args) + # Combine all the logs into one. + git_run_pager(arg_sequence=git_sequence) + continue + + raise ValueError(f"Unexpected response: {response}") -def auto_fixup(_args): +def auto_commit(_args: argparse.Namespace) -> None: """ - Automatically find and fix up commits using the current unstaged changes. + Automatically find and fix up commits for any pending changes. """ + # Want git log and add to be run from the root. + os.chdir(get_local_root()) # Only want to search as far back as the firefox commit. firefox_commit = get_firefox_ref("HEAD").commit - staged_files = get_changed_files("HEAD", staged=True) - if staged_files: - raise TbDevException(f"Have already staged files: {staged_files}") + staged_changes = [f for f in get_changed_files(staged=True)] + if staged_changes: + print("Existing staged changes for:") + for file_change in staged_changes: + print(f" {file_change.new_path}") + if not prompt_user( + "Include staged changes? (y/\x1b[4mn\x1b[0m)", binary_reply_default_no + ): + raise TbDevException("Cannot continue with pending staged changes") + print("") - fixups = {} - for filename in get_changed_files("HEAD"): - commit = get_fixup_for_file(filename, firefox_commit) - if commit is None: + full_target_list: list[FixupTarget] = [] + # Determine if HEAD points to a branch or not and has an upstream commit. + # We choose check=False since the exit status is non-zero when we are in a + # detached state. + head_symbolic_ref = git_get(["symbolic-ref", "-q", "HEAD"], check=False) + if not head_symbolic_ref or not bool( + git_get(["for-each-ref", "--format=%(upstream)", head_symbolic_ref]) + ): + # Unexpected, but not fatal. + print("HEAD has no upstream tracking!") + # Just include all commits since firefox_commit with no fixup depth + get_fixup_targets(full_target_list, firefox_commit, "HEAD", fixup_depth=0) + else: + upstream_commit = get_upstream_basis_commit("HEAD") + # Only include "fixup!" commits that are between here and the upstream + # tracking commit. + get_fixup_targets( + full_target_list, firefox_commit, upstream_commit, fixup_depth=0 + ) + get_fixup_targets(full_target_list, upstream_commit, "HEAD", fixup_depth=1) + + # full_target_list is ordered with the earlier commits first. Reverse this. + full_target_list.reverse() + # Also reverse the fixups order to follow the same order. + for target in full_target_list: + target.fixups.reverse() + + # Lazy load the list of firefox directories since they are unlikely to be + # needed. + @functools.cache + def firefox_directories_lazy() -> set[str]: + return { + dir_name + for dir_name in git_get( + [ + "ls-tree", + "-r", + "-d", + "--name-only", + "--full-tree", + "-z", + firefox_commit, + ], + strip=False, + ).split("\0") + if dir_name + } + + # Check untracked files to be added. + for path in git_get( + ["ls-files", "--other", "--exclude-standard", "-z"], strip=False + ).split("\0"): + if not path: continue - if commit not in fixups: - fixups[commit] = [filename] - else: - fixups[commit].append(filename) + if prompt_user( + f"Start tracking file `{path}`? (y/\x1b[4mn\x1b[0m)", + binary_reply_default_no, + ): + # Include in the git diff output, but do not stage. + git_run(["add", "--intent-to-add", path]) print("") - for commit, files in fixups.items(): - print("") - git_run(["add", *files]) - git_run(["commit", f"--fixup={commit}"]) + aborted = False + new_commits_list: list[NewCommit | NewFixup] = [] + # First go through staged changes. + if staged_changes: + common_fixup_targets = None + for change in staged_changes: + target_iter = get_suggested_fixup_targets_for_change( + change, full_target_list, firefox_directories_lazy + ) + if common_fixup_targets is None: + common_fixup_targets = set(target_iter) + else: + common_fixup_targets.intersection_update(target_iter) + + assert common_fixup_targets is not None + + aborted = ask_for_target( + staged_changes, + new_commits_list, + # Sort in the same order as full_target_list. + [target for target in full_target_list if target in common_fixup_targets], + full_target_list, + staged=True, + ) print("") - if prompt_user( - "Edit fixup commit message? (y/\x1b[4mn\x1b[0m)", binary_reply_default_no - ): + if not aborted: + for file_change in get_changed_files(): + target_list = list( + get_suggested_fixup_targets_for_change( + file_change, full_target_list, firefox_directories_lazy + ) + ) + aborted = ask_for_target( + [file_change], + new_commits_list, + target_list, + full_target_list, + staged=False, + ) + print("") + if aborted: + break + + if aborted: + return + + # NOTE: Only the first commit can include staged changes. + # This should already be the case, but we want to double check. + for commit_index in range(1, len(new_commits_list)): + if new_commits_list[commit_index].staged_paths: + raise ValueError(f"Staged changes for commit {commit_index}") + + for new_commit in new_commits_list: + print("") + if new_commit.adding_paths: + git_run(["add", *git_path_args(new_commit.adding_paths)]) + if isinstance(new_commit, NewFixup): + git_run(["commit", f"--fixup={new_commit.target.commit}"]) + print("") + is_double_fixup = bool(new_commit.target.target) + if not is_double_fixup and prompt_user( + "Edit fixup commit message? (y/\x1b[4mn\x1b[0m)", + binary_reply_default_no, + ): + git_run(["commit", "--amend"]) + print("") + else: + git_run(["commit", "-m", new_commit.alias]) git_run(["commit", "--amend"]) + print("") -def clean_fixups(_args): +def clean_fixups(_args: argparse.Namespace) -> None: """ Perform an interactive rebase that automatically applies fixups, similar to --autosquash but also works on fixups of fixups. """ - user_editor = git_get(["var", "GIT_SEQUENCE_EDITOR"])[0] + user_editor = git_get(["var", "GIT_SEQUENCE_EDITOR"]) sub_editor = os.path.join( os.path.dirname(os.path.realpath(__file__)), FIXUP_PREPROCESSOR_EDITOR ) @@ -525,7 +1326,7 @@ def clean_fixups(_args): ) -def show_default(_args): +def show_default(_args: argparse.Namespace) -> None: """ Print the default branch name from gitlab. """ @@ -536,7 +1337,7 @@ def show_default(_args): print(f"{upstream}/{default_branch}") -def branch_from_default(args): +def branch_from_default(args: argparse.Namespace) -> None: """ Fetch the default gitlab branch from upstream and create a new local branch. """ @@ -557,7 +1358,7 @@ def branch_from_default(args): ) -def move_to_default(args): +def move_to_default(args: argparse.Namespace) -> None: """ Fetch the default gitlab branch from upstream and move the specified branch's commits on top. A new branch will be created tracking the default @@ -569,7 +1370,7 @@ def move_to_default(args): if branch_name is None: # Use current branch as default. try: - branch_name = git_get(["branch", "--show-current"])[0] + branch_name = git_get(["branch", "--show-current"]) except IndexError: raise TbDevException("No current branch") @@ -608,7 +1409,7 @@ def move_to_default(args): git_run(["cherry-pick", f"{current_basis}..{old_branch_name}"], check=False) -def show_range_diff(args): +def show_range_diff(args: argparse.Namespace) -> None: """ Show the range diff between two branches, from their firefox bases. """ @@ -624,21 +1425,21 @@ def show_range_diff(args): ) -def show_diff_diff(args): +def show_diff_diff(args: argparse.Namespace) -> None: """ Show the diff between the diffs of two branches, relative to their firefox bases. """ - config_res = git_get(["config", "--get", "diff.tool"]) - if not config_res: + try: + diff_tool = next(git_lines(["config", "--get", "diff.tool"])) + except StopIteration: raise TbDevException("No diff.tool configured for git") - diff_tool = config_res[0] # Filter out parts of the diff we expect to be different. index_regex = re.compile(r"index [0-9a-f]{12}\.\.[0-9a-f]{12}") lines_regex = re.compile(r"@@ -[0-9]+,[0-9]+ \+[0-9]+,[0-9]+ @@(?P<rest>.*)") - def save_diff(branch): + def save_diff(branch: str) -> str: firefox_commit = get_firefox_ref(branch).commit file_desc, file_name = tempfile.mkstemp( text=True, prefix=f'{branch.split("/")[-1]}-' @@ -653,6 +1454,7 @@ def show_diff_diff(args): ) with os.fdopen(file_desc, "w") as file: + assert diff_process.stdout is not None for line in diff_process.stdout: if index_regex.match(line): # Fake data that will match. @@ -665,7 +1467,7 @@ def show_diff_diff(args): continue file.write(line) - status = diff_process.poll() + status = diff_process.wait() if status != 0: raise TbDevException(f"git diff exited with status {status}") @@ -681,7 +1483,7 @@ def show_diff_diff(args): # * -------------------- * -def branch_complete(prefix, parsed_args, **kwargs): +def branch_complete(prefix: str, **_kwargs: Any) -> list[str]: """ Complete the argument with a branch name. """ @@ -689,7 +1491,7 @@ def branch_complete(prefix, parsed_args, **kwargs): return [] try: branches = [ref.name for ref in get_refs("head", "")] - branches.extend([ref.name for ref in get_refs("remote", "")]) + branches.extend(ref.name for ref in get_refs("remote", "")) branches.append("HEAD") except Exception: return [] @@ -699,7 +1501,20 @@ def branch_complete(prefix, parsed_args, **kwargs): parser = argparse.ArgumentParser() subparsers = parser.add_subparsers(required=True) -for name, details in { + +class ArgConfig(TypedDict): + help: str + metavar: NotRequired[str] + nargs: NotRequired[str] + completer: NotRequired[Callable[[str], list[str]]] + + +class CommandConfig(TypedDict): + func: Callable[[argparse.Namespace], None] + args: NotRequired[dict[str, ArgConfig]] + + +all_commands: dict[str, CommandConfig] = { "show-upstream-basis-commit": { "func": show_upstream_basis_commit, }, @@ -716,8 +1531,8 @@ for name, details in { }, }, }, - "auto-fixup": { - "func": auto_fixup, + "auto-commit": { + "func": auto_commit, }, "clean-fixups": { "func": clean_fixups, @@ -794,20 +1609,25 @@ for name, details in { "regex": {"help": "the regex that the files must contain"}, }, }, -}.items(): - help_message = re.sub(r"\s+", " ", details["func"].__doc__).strip() +} + +for name, command_config in all_commands.items(): + help_message = command_config["func"].__doc__ + assert isinstance(help_message, str) + help_message = re.sub(r"\s+", " ", help_message).strip() sub = subparsers.add_parser(name, help=help_message) - sub.set_defaults(func=details["func"]) - for arg, keywords in details.get("args", {}).items(): + sub.set_defaults(func=command_config["func"]) + for arg, keywords in command_config.get("args", {}).items(): completer = None if "completer" in keywords: completer = keywords["completer"] del keywords["completer"] sub_arg = sub.add_argument(arg, **keywords) - if completer is not None: - sub_arg.completer = completer + if completer is not None and argcomplete is not None: + sub_arg.completer = completer # type: ignore -argcomplete.autocomplete(parser) +if argcomplete is not None: + argcomplete.autocomplete(parser) try: if not within_browser_root(): View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/858c7a7... -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/858c7a7... You're receiving this email because of your account on gitlab.torproject.org.