autoformat: change from hg fix strategy to tip commit strategy (Bug 1785004)

Remove the `hg fix` based autoformatting and replace with
running raw `mach lint` on the tip commit, creating an
autoformat commit if the linters made any changes.

Author: cgsheeh

hgext/postfix_hook.py

@@ -2,16 +2,17 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 import copy
-from contextlib import contextmanager
 import configparser
 import logging
 import os
-from pathlib import Path
 import shlex
 import shutil
+import subprocess
 import tempfile
 import uuid
 
+from contextlib import contextmanager
+from pathlib import Path
 from typing import List, Optional
 
 import hglib
@@ -101,21 +102,17 @@ class PatchConflict(PatchApplicationFailure):
     )
 
 
-def check_fix_output_for_replacements(fix_output: List[bytes]) -> Optional[List[str]]:
-    """Parses `hg fix` output.
+class AutoformattingException(HgException):
+    """Exception when autoformatting fails to format a patch stack."""
 
-    Returns:
-        A list of changeset hashes, or None if no changesets are changed.
-    """
-    for line in fix_output:
-        if not line.startswith(b"REPLACEMENTS: "):
-            continue
+    pass
 
-        replacements_list = line.split(b"REPLACEMENTS: ", maxsplit=1)[-1].split(b",")
 
-        return [element.decode("latin-1") for element in replacements_list]
+AUTOFORMAT_COMMIT_MESSAGE = """
+No bug: apply code formatting
 
-    return None
+# ignore-this-changeset
+""".strip()
 
 
 class HgRepo:
@@ -136,10 +133,6 @@ class HgRepo:
         "extensions.strip": "",
         "extensions.rebase": "",
         "extensions.set_landing_system": "/app/hgext/set_landing_system.py",
-        # Turn on fix extension for autoformatting, set to abort on failure
-        "extensions.fix": "",
-        "fix.failure": "abort",
-        "hooks.postfix": "python:/app/hgext/postfix_hook.py:postfix_hook",
     }
 
     def __init__(self, path, config=None):
@@ -368,56 +361,75 @@ def apply_patch(self, patch_io_buf):
                 + ["--logfile", f_msg.name]
             )
 
-    def format(self) -> Optional[List[str]]:
-        """Run `hg fix` to format the currently checked-out stack, reading
-        fileset patterns for each formatter from the `.lando.ini` file in-tree."""
-        # Avoid attempting to autoformat without `.lando.ini` in-tree.
-        lando_config_path = Path(self.path) / ".lando.ini"
-        if not lando_config_path.exists():
+    def read_lando_config(self) -> Optional[configparser.ConfigParser]:
+        """Attempt to read the `.lando.ini` file."""
+        try:
+            lando_ini_contents = self.read_checkout_file(".lando.ini")
+        except ValueError:
             return None
 
         # ConfigParser will use `:` as a delimeter unless told otherwise.
         # We set our keys as `formatter:pattern` so specify `=` as the delimiters.
         parser = configparser.ConfigParser(delimiters="=")
-        with lando_config_path.open() as f:
-            parser.read_file(f)
+        parser.read_string(lando_ini_contents)
 
-        # If the file doesn't have a `fix` section, exit early.
-        if not parser.has_section("fix"):
-            return None
+        return parser
 
-        fix_hg_command = []
-        for key, value in parser.items("fix"):
-            if not key.endswith(":pattern"):
-                continue
+    def format_stack_tip(self, mach_path: Path) -> Optional[str]:
+        """Add an autoformat commit to the top of the patch stack."""
+        # Update to the tip of the stack.
+        self.run_hg(["update", "-r", "tip"])
 
-            fix_hg_command += ["--config", f"fix.{key}={value}"]
+        # Run linters.
+        subprocess.run([mach_path, "lint", "-r", "stack()"], check=True)
 
-        # Exit if we didn't find any patterns.
-        if not fix_hg_command:
-            return None
+        try:
+            # Create a new commit.
+            self.run_hg(
+                ["commit"]
+                + ["--message", AUTOFORMAT_COMMIT_MESSAGE]
+                + ["--landing_system", "lando"]
+            )
+        except hglib.error.CommandError as exc:
+            if exc.out.strip() == b"nothing changed":
+                # If nothing changed after formatting we can just return.
+                return None
 
-        # Run the formatters.
-        fix_hg_command += ["fix", "-r", "stack()"]
-        fix_output = self.run_hg(fix_hg_command).splitlines()
+            raise exc
 
-        # Update the working directory to the latest change.
-        self.run_hg(["update", "-C", "-r", "tip"])
+        return self.get_current_node().decode("utf-8")
 
-        # Exit if no revisions were reformatted.
-        pre_formatting_hashes = check_fix_output_for_replacements(fix_output)
-        if not pre_formatting_hashes:
+    def format_stack(self) -> Optional[List[str]]:
+        """Format the patch stack for landing."""
+        # Disable autoformatting if `.lando.ini` is missing or not enabled.
+        landoini_config = self.read_lando_config()
+        if (
+            not landoini_config
+            or not landoini_config.has_section("autoformat")
+            or not landoini_config.getboolean("autoformat", "enabled")
+        ):
             return None
 
-        post_formatting_hashes = (
-            self.run_hg(["log", "-r", "stack()", "-T", "{node}\n"])
-            .decode("utf-8")
-            .splitlines()[len(pre_formatting_hashes) - 1 :]
-        )
+        # If `mach` is not at the root of the repo, we can't autoformat.
+        mach_path = Path(self.path) / "mach"
+        if not mach_path.exists():
+            logger.info("No `./mach` in the repo - skipping autoformat.")
+            return None
+
+        try:
+            # If formatting each commit has failed, create an autoformatting
+            # commit that fixes each changed file in the stack.
+            formatting_changeset = self.format_stack_tip(mach_path)
 
-        logger.info(f"revisions were reformatted: {', '.join(post_formatting_hashes)}")
+            if formatting_changeset is None:
+                return None
 
-        return post_formatting_hashes
+            return [formatting_changeset]
+        except (HgException, subprocess.CalledProcessError) as tip_exc:
+            logger.warning("Failed to create an autoformat commit.")
+            logger.exception(tip_exc)
+
+        raise AutoformattingException("Could not autoformat the patch.")
 
     def push(self, target, bookmark=None):
         if not os.getenv(REQUEST_USER_ENV_VAR):
@@ -472,6 +484,10 @@ def get_remote_head(self, source: str) -> bytes:
         assert len(cset) == 12, cset
         return cset
 
+    def get_current_node(self) -> bytes:
+        """Return the currently checked out node."""
+        return self.run_hg(["identify", "-r", ".", "-i"])
+
     def update_from_upstream(self, source, remote_rev):
         # Pull and update to remote tip.
         cmds = [

landoapi/hg.py

@@ -12,11 +12,11 @@
 import subprocess
 import time
 
-import hglib
 from flask import current_app
 
 from landoapi import patches
 from landoapi.hg import (
+    AutoformattingException,
     HgRepo,
     LostPushRace,
     NoDiffStartLine,
@@ -432,16 +432,16 @@ def run_job(
                     self.notify_user_of_landing_failure(job)
                     return True
 
-            # Run `hg fix` configured formatters if enabled
+            # Run automated code formatters if enabled.
             if repo.autoformat_enabled:
                 try:
-                    replacements = hgrepo.format()
+                    replacements = hgrepo.format_stack()
 
-                    # If autoformatting changed any changesets, note those in the job.
+                    # If autoformatting added any changesets, note those in the job.
                     if replacements:
                         job.formatted_replacements = replacements
 
-                except hglib.error.CommandError as exc:
+                except AutoformattingException as exc:
                     message = (
                         "Lando failed to format your patch for conformity with our "
                         "formatting policy. Please see the details below.\n\n"

landoapi/landing_worker.py

@@ -70,6 +70,7 @@ class Repo:
     short_name: str = ""
     legacy_transplant: bool = False
     approval_required: bool = False
+    autoformat_enabled: bool = False
     commit_flags: list[tuple[str, str]] = field(default_factory=list)
     config_override: dict = field(default_factory=dict)
 
@@ -88,15 +89,6 @@ def __post_init__(self):
         if not self.short_name:
             self.short_name = self.tree
 
-    @property
-    def autoformat_enabled(self) -> bool:
-        """Return `True` if formatting is enabled for the repo."""
-        if not self.config_override:
-            # Empty config override always indicates no autoformat.
-            return False
-
-        return any(config.startswith("fix") for config in self.config_override.keys())
-
 
 SCM_ALLOW_DIRECT_PUSH = AccessGroup(
     active_group="active_scm_allow_direct_push",
@@ -182,7 +174,7 @@ def autoformat_enabled(self) -> bool:
             access_group=SCM_LEVEL_1,
             push_path="ssh://autoland.hg//repos/third-repo",
             pull_path="http://hg.test/third-repo",
-            config_override={"fix.black:command": "black -- -"},
+            autoformat_enabled=True,
             approval_required=True,
         ),
         # Approval is required for the uplift dev repo
@@ -205,15 +197,15 @@ def autoformat_enabled(self) -> bool:
             url="https://hg.mozilla.org/conduit-testing/m-c",
             access_group=SCM_CONDUIT,
             commit_flags=[DONTBUILD],
-            config_override={"fix.black:command": "black -- -"},
+            autoformat_enabled=True,
             approval_required=True,
         ),
         "vct": Repo(
             tree="vct",
             url="https://hg.mozilla.org/conduit-testing/vct",
             access_group=SCM_CONDUIT,
             push_bookmark="@",
-            config_override={"fix.black:command": "black -- -"},
+            autoformat_enabled=True,
         ),
     },
     "devsvcprod": {
@@ -254,7 +246,7 @@ def autoformat_enabled(self) -> bool:
             access_group=SCM_LEVEL_3,
             short_name="mozilla-central",
             commit_flags=[DONTBUILD],
-            config_override={"fix.black:command": "black -- -"},
+            autoformat_enabled=True,
         ),
         "comm-central": Repo(
             tree="comm-central",