2026 11 12 tjr issue fixes

apis/phabricator.py

@@ -47,9 +47,9 @@ def _call_conduit(self, method, params):
         cmd += " | arc call-conduit --conduit-uri=%s %s --" % (self.url, method)
         ret = self.run(cmd, shell=True)
         try:
-            result = json.loads(ret.stdout.decode())
+            result = json.loads(ret.stdout)
         except Exception:
-            raise Exception("Could not decode response as JSON: %s" % ret.stdout.decode())
+            raise Exception("Could not decode response as JSON: %s" % ret.stdout)
         if result['error']:
             raise ConduitError(method, result)
         return result['response']
@@ -90,7 +90,7 @@ def submit_to_phabricator(rev_id, retry_attempt=None):
             cmd.append("--")
 
             ret = self.run(cmd, clean_return=False)
-            too_large = ARC_DIFF_TOO_LARGE.search(ret.stdout.decode())
+            too_large = ARC_DIFF_TOO_LARGE.search(ret.stdout)
             if too_large:
                 raise DiffTooLargeError(
                     "arc diff will not submit %s because its diff is %s bytes, over arc's limit of 4 MiB for a single file."
@@ -100,7 +100,7 @@ def submit_to_phabricator(rev_id, retry_attempt=None):
             else:
                 ret.check_returncode()
 
-            output = ret.stdout.decode()
+            output = ret.stdout
 
             phab_revision = False
             r = re.compile(self.url + "D([0-9]+)")

apis/taskcluster.py

@@ -21,6 +21,9 @@
 # hghooks/mozhghooks/treeclosure.py).
 TREE_CLOSED = re.compile(r"\S+ is CLOSED! Reason: (.*)")
 TREE_STATUS_UNAVAILABLE = "Unable to check if the tree is open - treating as if CLOSED."
+# mach try refuses to push with uncommitted changes ("ERROR please commit changes before
+# continuing" in older versions, "error: commit changes before continuing" now)
+UNCOMMITTED_CHANGES = "commit changes before continuing"
 
 
 class TryTreeClosedError(Exception):
@@ -177,14 +180,19 @@ def submit_to_try(self, library, recursed=0):
             try_arguments.append("--artifact")
 
         ret = self.run(try_arguments, clean_return=False)
-        output = ret.stdout.decode()
+        output = ret.stdout
         if ret.returncode:
-            stderr = ret.stderr.decode() if getattr(ret, "stderr", None) else ""
+            stderr = ret.stderr if getattr(ret, "stderr", None) else ""
             closed = TREE_CLOSED.search(output + stderr)
             if closed:
                 raise TryTreeClosedError(closed.group(1).strip(), shlex.join(try_arguments))
             if TREE_STATUS_UNAVAILABLE in output + stderr:
                 raise TryTreeClosedError("its tree status couldn't be checked, so the push was refused as if it were closed", shlex.join(try_arguments))
+            if UNCOMMITTED_CHANGES in output + stderr:
+                # Our commits succeeded, so something was changed or deleted without being
+                # registered with hg (#392). Record what, so the cause can be found.
+                status = self.run(["hg", "status"], clean_return=False).stdout
+                self.logger.log("./mach try refused to push because of uncommitted changes. hg status shows:\n%s" % status, level=LogLevel.Error)
             ret.check_returncode()
 
         isNext = False

automation.py

@@ -202,7 +202,7 @@ def _validate(self, config_dictionary):
     def run(self, library_filter=""):
         try:
             start_time = time.time()
-            updatebot_version = self.cmdProvider.run(["git", "log", "-1", "--oneline"], shell=False, clean_return=True).stdout.decode().strip()
+            updatebot_version = self.cmdProvider.run(["git", "log", "-1", "--oneline"], shell=False, clean_return=True).stdout.strip()
             python_version = sys.version.replace("\n", " ")
             self.logger.log("Running Updatebot version: {0} on Python {1}".format(updatebot_version, python_version), level=LogLevel.Info)
             self.logger.log("Python prefix: {0}".format(sys.prefix), level=LogLevel.Debug)

components/aiprovider.py

@@ -112,7 +112,7 @@ def _write(suffix, contents):
             self.logger.log("Could not write AI debug output: %s" % e, level=LogLevel.Warning)
 
     def _parse_result(self, ret):
-        stdout = ret.stdout.decode() if isinstance(ret.stdout, bytes) else ret.stdout
+        stdout = ret.stdout
         try:
             data = json.loads(stdout)
         except ValueError:

components/bugzilla.py

@@ -32,6 +32,14 @@ def UPDATE_DETAILS_WITHOUT_COMMITS(library, commit_stats, error):
 %s
 """ % (library.repo_url, error.strip().replace("\n", "\n> "), commit_stats)
 
+    @staticmethod
+    def LICENSE_FILES_CHANGED(files):
+        return """
+This update changes license files, so please check that the license in moz.yaml is still correct:
+
+%s
+""" % "\n".join("- " + f for f in files)
+
     @staticmethod
     def UPDATE_DETAILS(num_commits, num_new_commits, commit_stats, commit_details, author_summary=""):
         new_commit_str = ""

components/commandprovider.py

@@ -35,7 +35,7 @@ def run(self, args, shell=False, clean_return=True, cwd=None, stdin_path=None, t
         return ret
 
     def _report_traceback_in_stderr(self, args, stderr):
-        stderr = stderr.decode(errors="replace") if stderr else ""
+        stderr = stderr or ""
         if "Traceback (most recent call last):" not in stderr:
             return
         lines = [line for line in stderr.strip().splitlines() if line.strip()]

components/commandrunner.py

@@ -45,19 +45,22 @@ def _run(args, shell, clean_return, errorlog=do_nothing, infolog=do_nothing, deb
     # Merge any extra env (e.g. secrets) over the inherited environment.
     run_env = {**os.environ, **env} if env else None
     try:
+        # Output that isn't valid UTF-8 (e.g. a commit stored in another encoding)
+        # has the bad bytes replaced, rather than making callers decode it.
         ret = subprocess.run(
             args, shell=shell, stdout=PIPE, stderr=PIPE, timeout=timeout,
-            cwd=cwd, stdin=stdin_handle or subprocess.DEVNULL, env=run_env)
+            cwd=cwd, stdin=stdin_handle or subprocess.DEVNULL, env=run_env,
+            encoding="utf-8", errors="replace")
     except subprocess.TimeoutExpired as e:
         ran_to_completion = False
-        stdout = e.stdout
-        stderr = e.stderr
+        # TimeoutExpired carries bytes even when the output is text
+        stdout = e.stdout.decode(errors="replace") if isinstance(e.stdout, bytes) else e.stdout
+        stderr = e.stderr.decode(errors="replace") if isinstance(e.stderr, bytes) else e.stderr
         exception = e
     else:
         ran_to_completion = True
-        # Only for the log: callers get the raw output, which may not be UTF-8
-        stdout = ret.stdout.decode(errors="replace")
-        stderr = ret.stderr.decode(errors="replace")
+        stdout = ret.stdout
+        stderr = ret.stderr
     finally:
         if stdin_handle:
             stdin_handle.close()

components/hg.py

@@ -26,7 +26,7 @@ def reset_repository(cmdrunner):
     else:
         ret = cmdrunner.run(["hg", "strip", "roots(outgoing())", "--no-backup"], clean_return=False)
         if ret.returncode == 255:
-            if "abort: empty revision set" not in ret.stderr.decode():
+            if "abort: empty revision set" not in ret.stderr:
                 ret.check_returncode()
 
 
@@ -76,10 +76,18 @@ def changed_files(self, exclude=()):
         # deleted files, with paths relative to the root of the checkout.
         self.run(["hg", "addremove", "-q"])
         excluded = set(os.path.normpath(p) for p in exclude)
-        files = self.run(["hg", "--config", "ui.relative-paths=no", "status", "--no-status"]).stdout.decode().splitlines()
+        files = self.run(["hg", "--config", "ui.relative-paths=no", "status", "--no-status"]).stdout.splitlines()
         return [f for f in files if f and os.path.normpath(f) not in excluded]
 
+    @logEntryExit
+    def changed_files_in(self, directory):
+        # Modified, added, removed, missing and untracked files under directory, with
+        # paths relative to the root of the checkout. Unlike changed_files, this doesn't
+        # change what's registered with hg.
+        files = self.run(["hg", "--config", "ui.relative-paths=no", "status", "--no-status", directory]).stdout.splitlines()
+        return [f for f in files if f]
+
     @logEntryExit
     def diff_stats(self):
         ret = self.run(["hg", "diff", "--stat"])
-        return ret.stdout.decode().rstrip()
+        return ret.stdout.rstrip()

components/libraryprovider.py

@@ -51,6 +51,8 @@ def __init__(self, dict):
         self.fuzzy_query = dict.get('fuzzy_query', None)
         self.fuzzy_paths = dict.get('fuzzy_paths', None)
         self.artifact_build = dict.get('artifact_build', False)
+        # Relative to the vendor directory, as in moz.yaml
+        self.license_file = dict.get('license_file') or None
         self.yaml_path = dict['yaml_path']
         # Paths are relative to the root of the gecko checkout, like yaml_path
         self.vendor_directory = dict.get('vendor_directory') or os.path.dirname(self.yaml_path)
@@ -176,7 +178,7 @@ def get_libraries(self, gecko_path):
         if self._libraries is None:
             libraries = []
             command = ["find", gecko_path, "-type", "f", "-name", "moz.yaml"]
-            mozilla_central_yamls = self.run(command).stdout.decode()
+            mozilla_central_yamls = self.run(command).stdout
 
             mozilla_central_yamls = mozilla_central_yamls.strip().split("\n")
             for file in mozilla_central_yamls:
@@ -225,6 +227,7 @@ def validate_library(yaml_contents, yaml_path):
             'fuzzy_query': '',
             'fuzzy_paths': [],
             'artifact_build': False,
+            'license_file': '',
             'tasks': [],
             'yaml_path': '',
             'vendor_directory': '',
@@ -247,6 +250,9 @@ def validate_library(yaml_contents, yaml_path):
         else:
             validated_library['revision'] = None
 
+        if 'origin' in library and 'license-file' in library['origin']:
+            validated_library['license_file'] = library['origin']['license-file']
+
         # Attempt to get the repository url (not required by moz.yaml) if present
         if 'vendoring' in library and 'url' in library['vendoring']:
             validated_library['repo_url'] = library['vendoring']['url']

components/logging.py

@@ -223,13 +223,13 @@ def __init__(self, config):
 
     def _update_config(self, additional_config):
         version = "updatebot-"
-        version += _run(["git", "rev-parse", "HEAD"], shell=False, clean_return=True).stdout.decode().strip()
+        version += _run(["git", "rev-parse", "HEAD"], shell=False, clean_return=True).stdout.strip()
 
         environment = ""
         if "TASK_ID" in os.environ:
             environment = "taskcluster"
         else:
-            environment = _run(["hostname"], shell=False, clean_return=True).stdout.decode().strip()
+            environment = _run(["hostname"], shell=False, clean_return=True).stdout.strip()
 
         sentry_init(
             dsn=self.config['sentry_config']['url'],

components/mach_vendor.py

@@ -23,7 +23,7 @@ def __init__(self, config):
     @logEntryExit
     @retry
     def check_for_update(self, library):
-        result = self.run(["./mach", "vendor", "--check-for-update", library.yaml_path]).stdout.decode().strip()
+        result = self.run(["./mach", "vendor", "--check-for-update", library.yaml_path]).stdout.strip()
 
         if "Creating " in result and " state directory" in result:
             # If no ~/.mozbuild directory was present this gets output unfortunately.
@@ -51,8 +51,8 @@ def vendor(self, library, revision):
             if ret.returncode == 0:
                 return (VendorResult.SUCCESS, "")
 
-            msg = ret.stderr.decode().rstrip() + "\n\n" if ret.stderr else ""
-            msg += ret.stdout.decode().rstrip()
+            msg = ret.stderr.rstrip() + "\n\n" if ret.stderr else ""
+            msg += (ret.stdout or "").rstrip()
 
             if ret.returncode == 255:
                 return (VendorResult.MOZBUILD_ERROR, msg)
@@ -62,8 +62,8 @@ def vendor(self, library, revision):
                 return (VendorResult.GENERAL_ERROR, msg)
         except Exception as e:
             if isinstance(e, subprocess.CalledProcessError):
-                msg = e.stderr.decode().rstrip() + "\n\n" if e.stderr else ""
-                msg += e.stdout.decode().rstrip()
+                msg = e.stderr.rstrip() + "\n\n" if e.stderr else ""
+                msg += (e.stdout or "").rstrip()
             else:
                 msg = str(e)
             return (VendorResult.GENERAL_ERROR, msg)

components/scmprovider.py

@@ -66,9 +66,9 @@ def populate_details(self, repo, run):
             return
 
         rev_range = [self.revision + "^", self.revision]
-        self.parent_revision = run(["git", "rev-parse", self.revision + "^"]).stdout.decode().strip()
+        self.parent_revision = run(["git", "rev-parse", self.revision + "^"]).stdout.strip()
 
-        files_changed = run(["git", "diff", "--name-status"] + rev_range).stdout.decode().split("\n")
+        files_changed = run(["git", "diff", "--name-status"] + rev_range).stdout.split("\n")
         for f in files_changed:
             f = f.strip()
             if not f:
@@ -84,14 +84,14 @@ def populate_details(self, repo, run):
             else:
                 self.files_other.append(parts[0] + " " + parts[1])
 
-        self.summary = run(["git", "log", "--pretty=%s", "-1", self.revision]).stdout.decode()
-        self.author = run(["git", "log", "--pretty=%an <%ae>", "-1", self.revision]).stdout.decode()
-        self.description = run(["git", "log", "--pretty=%b", "-1", self.revision]).stdout.decode()
+        self.summary = run(["git", "log", "--pretty=%s", "-1", self.revision]).stdout
+        self.author = run(["git", "log", "--pretty=%an <%ae>", "-1", self.revision]).stdout
+        self.description = run(["git", "log", "--pretty=%b", "-1", self.revision]).stdout
         # A signed commit (with GPG or SSH) has a gpgsig header. We can't verify the
         # signature without the signer's key, only tell whether there is one.
         # cat-file prints the commit as stored, which may not be UTF-8 (unlike git log,
         # which converts it), but the header names we look for are ASCII.
-        headers = run(["git", "cat-file", "commit", self.revision]).stdout.decode("utf-8", "replace").split("\n\n", 1)[0]
+        headers = run(["git", "cat-file", "commit", self.revision]).stdout.split("\n\n", 1)[0]
         self.signed = any(line.startswith(("gpgsig ", "gpgsig-sha256 ")) for line in headers.split("\n"))
         self.revision_link = repo_and_commit_to_url(repo, self.revision)
         self.populated = True
@@ -241,7 +241,7 @@ def check_for_update(self, library, task, new_version, most_recent_job_version):
             self._ensure_checkout(library.repo_url, task.branch, library.name)
 
             # Step 1: Find the common ancestor between the commit we are and new_version
-            common_ancestor = self.run(["git", "merge-base", library.revision, new_version]).stdout.decode().strip()
+            common_ancestor = self.run(["git", "merge-base", library.revision, new_version]).stdout.strip()
             self.logger.log("Our common ancestor is %s." % (common_ancestor), level=LogLevel.Debug)
 
             # Step 2: Get the list of commits between the common ancestor and new_version
@@ -333,7 +333,7 @@ def _is_ancestor(self, ancestor, descendant):
         if ret.returncode not in (0, 1):
             # e.g. a revision that doesn't exist upstream (any more)
             self.logger.log("Could not determine whether %s is an ancestor of %s: %s" % (
-                ancestor, descendant, ret.stderr.decode().strip()), level=LogLevel.Warning)
+                ancestor, descendant, ret.stderr.strip()), level=LogLevel.Warning)
         return ret.returncode == 0
 
     def _commits_between(self, revision1, revision2):
@@ -342,7 +342,7 @@ def _commits_between(self, revision1, revision2):
         If rev1 == rev2, returns an empty list.
         """
         ret = self.run(["git", "log", "--pretty=%H|%ai|%ci", "--no-merges", "%s..%s" % (revision1, revision2)])
-        commits = [line.strip() for line in ret.stdout.decode().split("\n")]
+        commits = [line.strip() for line in ret.stdout.split("\n")]
         # Put them in order of oldest to newest
         commits.reverse()
         # Populate them into a class but don't get details just yet.

tasktypes/vendoring.py

@@ -6,6 +6,8 @@
 
 import copy
 import functools
+import os
+import re
 import subprocess
 from collections import defaultdict
 
@@ -18,11 +20,15 @@
 from components.logging import LogLevel, logEntryExit, logEntryExitNoArgs
 from components.hg import reset_repository, SPURIOUS_CHECK_SHELF
 
+# Upstream's license files are usually named like these (LICENSE.txt, COPYING.LIB,
+# LICENSE_FOXIT, ...); moz.yaml's license-file covers any that aren't.
+LICENSE_FILE_NAME = re.compile(r"^(LICEN[CS]E|COPYING|COPYRIGHT|NOTICE|UNLICENSE)", re.IGNORECASE)
+
 
 def _patch_exception_message(e):
     if isinstance(e, subprocess.CalledProcessError):
-        msg = ("stderr:\n" + e.stderr.decode().rstrip() + "\n\n") if e.stderr else ""
-        msg += ("stdout:\n" + e.stdout.decode().rstrip()) if e.stdout else ""
+        msg = ("stderr:\n" + e.stderr.rstrip() + "\n\n") if e.stderr else ""
+        msg += ("stdout:\n" + e.stdout.rstrip()) if e.stdout else ""
         return msg
     return str(e)
 
@@ -96,6 +102,13 @@ def process_task(self, library, task):
         self._process_new_job(library, task, new_version, timestamp, most_recent_job)
         self._reset_for_new_job()
 
+    # ====================================================================
+    @logEntryExit
+    def _changed_license_files(self, library):
+        license_file = os.path.normpath(os.path.join(library.vendor_directory, library.license_file)) if library.license_file else None
+        return [f for f in self.mercurialProvider.changed_files_in(library.vendor_directory)
+                if LICENSE_FILE_NAME.match(os.path.basename(f)) or os.path.normpath(f) == license_file]
+
     # ====================================================================
     @logEntryExit
     def _only_patches_changed(self, library):
@@ -198,11 +211,15 @@ def _process_new_job(self, library, task, new_version, timestamp, most_recent_jo
 
         # File the bug ------------------------
         commit_stats = self.mercurialProvider.diff_stats()
+        license_files = self._changed_license_files(library)
+        if license_files:
+            self.logger.log("This update changes the license files %s." % ", ".join(license_files), level=LogLevel.Info)
+        license_note = CommentTemplates.LICENSE_FILES_CHANGED(license_files) if license_files else ""
         try:
             all_upstream_commits, unseen_upstream_commits = self.scmProvider.check_for_update(library, task, new_version, most_recent_job.version if most_recent_job else None)
             if library.should_show_commit_details:
                 author_summary = self.scmProvider.build_author_summary(all_upstream_commits)
-                commit_chunks = self.scmProvider.build_bug_description(all_upstream_commits, 65534 - len(commit_stats) - len(author_summary) - 220, library.repo_url, options=task.options, new_commits=unseen_upstream_commits, compare_range=(library.revision, new_version))
+                commit_chunks = self.scmProvider.build_bug_description(all_upstream_commits, 65534 - len(commit_stats) - len(author_summary) - len(license_note) - 220, library.repo_url, options=task.options, new_commits=unseen_upstream_commits, compare_range=(library.revision, new_version))
             else:
                 author_summary = ""
                 commit_chunks = [""]
@@ -214,6 +231,7 @@ def _process_new_job(self, library, task, new_version, timestamp, most_recent_jo
             self.logger.log("Could not get the upstream commits for %s; filing the bug without them: %s" % (library.name, e), level=LogLevel.Warning)
             commit_chunks = [""]
             bug_description = CommentTemplates.UPDATE_DETAILS_WITHOUT_COMMITS(library, commit_stats, _patch_exception_message(e))
+        bug_description = license_note + bug_description
 
         created_job.bugzilla_id = self.bugzillaProvider.file_bug(library, CommentTemplates.UPDATE_SUMMARY(library, new_version, timestamp), bug_description, task.cc, blocks=task.blocking)
         self.dbProvider.update_job_add_bug_id(created_job, created_job.bugzilla_id)