From 7d3ceb27b645ba5e94fc47f7306337701f325fa2 Mon Sep 17 00:00:00 2001
From: Erica Portnoy <erica@eff.org>
Date: Wed, 7 Aug 2024 11:05:06 -0700
Subject: [PATCH] add comments and explanations

---
 snap/snapcraft.yaml        |  4 +++-
 tools/snap/build_remote.py | 15 ++++++++++-----
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml
index cc811632a..eaf5870c9 100644
--- a/snap/snapcraft.yaml
+++ b/snap/snapcraft.yaml
@@ -18,6 +18,8 @@ base: core24
 grade: stable
 adopt-info: certbot
 
+# Workaround for misconfigured pyvenv.cfg; see discussion on snapcraft forum for more info
+# https://forum.snapcraft.io/t/upgrading-classic-snap-to-core24-using-snapcraft-8-3-causes-python-3-12-errors-at-runtime/
 environment:
   PYTHONPATH: "$SNAP/lib/python3.12/site-packages:${PYTHONPATH}"
 
@@ -51,7 +53,7 @@ parts:
       - ./certbot-nginx
     stage:
       - -usr/lib/python3.12/sitecustomize.py # maybe unnecessary
-      - -pyvenv.cfg
+      - -pyvenv.cfg # Workaround for misconfiguration, as noted above
       # Old versions of this file used to unstage
       # lib/python3.8/site-packages/augeas.py to avoid conflicts between
       # python-augeas 0.5.0 which was pinned in snap-constraints.txt and
diff --git a/tools/snap/build_remote.py b/tools/snap/build_remote.py
index 9935d7ab4..e28976289 100755
--- a/tools/snap/build_remote.py
+++ b/tools/snap/build_remote.py
@@ -42,10 +42,6 @@ PLUGINS = [basename(path) for path in glob.glob(join(CERTBOT_DIR, 'certbot-dns-*
 print = functools.partial(print, flush=True)
 
 
-def _snap_log_name(target: str, arch: str):
-    return f'{target}_{arch}.txt'
-
-
 def _execute_build(
         target: str, archs: Set[str], status: Dict[str, Dict[str, str]],
         workspace: str, output_lock: Lock) -> Tuple[int, List[str]]:
@@ -109,7 +105,9 @@ def _build_snap(
         workspace = CERTBOT_DIR
     else:
         workspace = join(CERTBOT_DIR, target)
-        # init and commit git repo in workspace
+        # Init and commit git repo in workspace. This is necessary starting in core24
+        # as "Projects must be at the top level of a git repository"
+        # https://snapcraft.io/docs/migrate-core24#remote-build
         subprocess.run(['git', 'init'], capture_output=True, check=True, cwd=workspace)
         subprocess.run(['git', 'add', '-A'], capture_output=True, check=True, cwd=workspace)
         subprocess.run(['git', 'commit', '-m', 'init'], capture_output=True, check=True, cwd=workspace)
@@ -125,6 +123,8 @@ def _build_snap(
             print(f'Build {target} for {",".join(archs)} (attempt {4-retry}/3) ended with '
                   f'exit code {exit_code}.')
 
+            # This output may change, and is set by
+            # https://github.com/canonical/snapcraft/blob/8ab7fd0c8a1d3f13045bec41a6e0158c063faa9b/snapcraft/commands/remote.py#L278
             failed_archs = [arch for arch in archs if status[target][arch] != 'Succeeded']
             # If the command failed or any architecture wasn't built
             # successfully, let's try to print all the output about the problem
@@ -157,6 +157,8 @@ def _build_snap(
 def _extract_state(project: str, output: str, status: Dict[str, Dict[str, str]]) -> None:
     state = status[project]
 
+    # This output may change, and is set by
+    # https://github.com/canonical/snapcraft/blob/8ab7fd0c8a1d3f13045bec41a6e0158c063faa9b/snapcraft/commands/remote.py#L218
     if "Starting new build" in output:
         for arch in state.keys():
             state[arch] = "Starting new build"
@@ -204,6 +206,9 @@ def _dump_failed_build_logs(
         if result != 'Succeeded':
             failures = True
 
+            # log name is no longer set deterministically as target_arch.txt
+            # this will still result in a single output though, as we're filtering
+            # by target above and arch here
             build_output_path = [log_name for log_name in logs_list if arch in log_name]
             if not build_output_path:
                 build_output = f'No output has been dumped by snapcraft remote-build.'