feedback from PR

Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>

snapcraft_legacy/internal/project_loader/__init__.py

@@ -77,22 +77,22 @@ def _validate_replacement(attr: str, replacement: str, value: Optional[str]) ->
 
     :raises Exception: If a replacement cannot occur.
     """
+    # return early when there is a replacement value (this is the most common case)
+    if value is not None:
+        return
+
     project_variables = [
         "SNAPCRAFT_ARCH_BUILD_FOR",
         "SNAPCRAFT_ARCH_TRIPLET_BUILD_FOR",
     ]
 
     # expand to shell syntax for variables (`$item` and `${item}`)
-    expanded_project_variables = (
-        [f"${item}" for item in project_variables ] +
-        [f"${{{item}}}" for item in project_variables]
-    )
-
-    if (
-        replacement in attr
-        and replacement in expanded_project_variables
-        and value is None
-    ):
+    expanded_project_variables = [
+        *(f"${item}" for item in project_variables ),
+        *(f"${{{item}}}" for item in project_variables),
+    ]
+
+    if replacement in attr and replacement in expanded_project_variables:
         raise VariableEvaluationError(
             variable=replacement,
             reason="the build-for architecture could not be determined"

tests/spread/cross-compile/environment-errors/task.yaml

@@ -0,0 +1,22 @@
+summary: Test failing scenarios for architecture envvars
+
+environment:
+  SNAP/build_for_multi_arch_error: build-for-multi-arch-error
+  SNAP/build_for_multi_arch_triplet_error: build-for-multi-arch-triplet-error
+  SNAP/build_for_part_error: build-for-part-error
+  SNAP/build_for_part_triplet_error: build-for-part-triplet-error
+  SNAP/build_for_unknown_arch_error: build-for-unknown-arch-error
+  SNAP/build_for_unknown_arch_triplet_error: build-for-unknown-arch-triplet-error
+
+restore: |
+  cd "./snaps/$SNAP"
+  snapcraft clean --destructive-mode
+  rm -f ./*.snap
+
+execute: |
+  cd "./snaps/$SNAP"
+
+  if snapcraft --destructive-mode; then
+    echo "snapcraft did not fail but should have"
+    exit 1
+  fi

tests/spread/cross-compile/environment/snaps/build-for-multi-arch/snap/snapcraft.yaml

@@ -19,8 +19,10 @@ environment:
 parts:
   my-part:
     plugin: nil
-    override-pull:
-      echo ${SNAPCRAFT_ARCH_TRIPLET}
-      echo ${SNAPCRAFT_ARCH_BUILD_ON}
-      echo ${SNAPCRAFT_ARCH_TRIPLET_BUILD_ON}
-      echo ${SNAPCRAFT_TARGET_ARCH}
+    override-build: |
+      cat << EOF >> $SNAPCRAFT_PART_INSTALL/part-variables.txt
+      SNAPCRAFT_ARCH_TRIPLET: ${SNAPCRAFT_ARCH_TRIPLET}
+      SNAPCRAFT_ARCH_BUILD_ON: ${SNAPCRAFT_ARCH_BUILD_ON}
+      SNAPCRAFT_ARCH_TRIPLET_BUILD_ON: ${SNAPCRAFT_ARCH_TRIPLET_BUILD_ON}
+      SNAPCRAFT_TARGET_ARCH: ${SNAPCRAFT_TARGET_ARCH}
+      EOF

tests/spread/cross-compile/environment/snaps/build-for-single-arch/snap/snapcraft.yaml

@@ -21,10 +21,12 @@ environment:
 parts:
   my-part:
     plugin: nil
-    override-pull:
-      echo ${SNAPCRAFT_ARCH_TRIPLET}
-      echo ${SNAPCRAFT_ARCH_BUILD_FOR}
-      echo ${SNAPCRAFT_ARCH_TRIPLET_BUILD_FOR}
-      echo ${SNAPCRAFT_ARCH_BUILD_ON}
-      echo ${SNAPCRAFT_ARCH_TRIPLET_BUILD_ON}
-      echo ${SNAPCRAFT_TARGET_ARCH}
+    override-build: |
+      cat << EOF >> $SNAPCRAFT_PART_INSTALL/part-variables.txt
+      SNAPCRAFT_ARCH_TRIPLET: ${SNAPCRAFT_ARCH_TRIPLET}
+      SNAPCRAFT_ARCH_BUILD_FOR: ${SNAPCRAFT_ARCH_BUILD_FOR}
+      SNAPCRAFT_ARCH_TRIPLET_BUILD_FOR: ${SNAPCRAFT_ARCH_TRIPLET_BUILD_FOR}
+      SNAPCRAFT_ARCH_BUILD_ON: ${SNAPCRAFT_ARCH_BUILD_ON}
+      SNAPCRAFT_ARCH_TRIPLET_BUILD_ON: ${SNAPCRAFT_ARCH_TRIPLET_BUILD_ON}
+      SNAPCRAFT_TARGET_ARCH: ${SNAPCRAFT_TARGET_ARCH}
+      EOF

tests/spread/cross-compile/environment/snaps/build-for-unknown-arch/snap/snapcraft.yaml

@@ -19,8 +19,10 @@ environment:
 parts:
   my-part:
     plugin: nil
-    override-pull:
-      echo ${SNAPCRAFT_ARCH_TRIPLET}
-      echo ${SNAPCRAFT_ARCH_BUILD_ON}
-      echo ${SNAPCRAFT_ARCH_TRIPLET_BUILD_ON}
-      echo ${SNAPCRAFT_TARGET_ARCH}
+    override-build: |
+      cat << EOF >> $SNAPCRAFT_PART_INSTALL/part-variables.txt
+      SNAPCRAFT_ARCH_TRIPLET: ${SNAPCRAFT_ARCH_TRIPLET}
+      SNAPCRAFT_ARCH_BUILD_ON: ${SNAPCRAFT_ARCH_BUILD_ON}
+      SNAPCRAFT_ARCH_TRIPLET_BUILD_ON: ${SNAPCRAFT_ARCH_TRIPLET_BUILD_ON}
+      SNAPCRAFT_TARGET_ARCH: ${SNAPCRAFT_TARGET_ARCH}
+      EOF

tests/spread/cross-compile/environment/snaps/target-arch/snap/snapcraft.yaml

@@ -21,10 +21,12 @@ environment:
 parts:
   my-part:
     plugin: nil
-    override-pull:
-      echo ${SNAPCRAFT_ARCH_TRIPLET}
-      echo ${SNAPCRAFT_ARCH_BUILD_FOR}
-      echo ${SNAPCRAFT_ARCH_TRIPLET_BUILD_FOR}
-      echo ${SNAPCRAFT_ARCH_BUILD_ON}
-      echo ${SNAPCRAFT_ARCH_TRIPLET_BUILD_ON}
-      echo ${SNAPCRAFT_TARGET_ARCH}
+    override-build: |
+      cat << EOF >> $SNAPCRAFT_PART_INSTALL/part-variables.txt
+      SNAPCRAFT_ARCH_TRIPLET: ${SNAPCRAFT_ARCH_TRIPLET}
+      SNAPCRAFT_ARCH_BUILD_FOR: ${SNAPCRAFT_ARCH_BUILD_FOR}
+      SNAPCRAFT_ARCH_TRIPLET_BUILD_FOR: ${SNAPCRAFT_ARCH_TRIPLET_BUILD_FOR}
+      SNAPCRAFT_ARCH_BUILD_ON: ${SNAPCRAFT_ARCH_BUILD_ON}
+      SNAPCRAFT_ARCH_TRIPLET_BUILD_ON: ${SNAPCRAFT_ARCH_TRIPLET_BUILD_ON}
+      SNAPCRAFT_TARGET_ARCH: ${SNAPCRAFT_TARGET_ARCH}
+      EOF

tests/spread/cross-compile/environment/task.yaml

@@ -2,14 +2,8 @@ summary: Test envvars related to architectures
 
 environment:
   SNAP/build_for_multi_arch: build-for-multi-arch
-  SNAP/build_for_multi_arch_error: build-for-multi-arch-error
-  SNAP/build_for_multi_arch_triplet_error: build-for-multi-arch-triplet-error
-  SNAP/build_for_part_error: build-for-part-error
-  SNAP/build_for_part_triplet_error: build-for-part-triplet-error
   SNAP/build_for_single_arch: build-for-single-arch
   SNAP/build_for_unknown_arch: build-for-unknown-arch
-  SNAP/build_for_unknown_arch_error: build-for-unknown-arch-error
-  SNAP/build_for_unknown_arch_triplet_error: build-for-unknown-arch-triplet-error
   SNAP/target_arch: target-arch
 
 restore: |
@@ -20,25 +14,24 @@ restore: |
 execute: |
   cd "./snaps/$SNAP"
 
-  # test error scenarios
-  if [[ $SNAP == *error* ]]; then
-    if snapcraft --destructive-mode; then
-      echo "snapcraft did not fail but should have"
-      exit 1
-    fi
-    exit 0
-  fi
-
-  # test successful scenarios
   if [ "${SNAP}" == "target-arch" ]; then
     snapcraft prime --destructive-mode --target-arch armhf --enable-experimental-target-arch
   else
     snapcraft prime --destructive-mode
   fi
 
+  # verify variables were evaluated in the environment in the snap.yaml
   while read -r expected_envvar; do
     if ! grep -qF "$expected_envvar" prime/meta/snap.yaml; then
       echo "Did not find '$expected_envvar' in 'prime/meta/snap.yaml'."
       exit 1
     fi
   done < expected-env.txt
+
+  # verify variables were evaluated inside a part's override script
+  while read -r expected_envvar; do
+    if ! grep -qF "$expected_envvar" prime/part-variables.txt; then
+      echo "Did not find '$expected_envvar' in 'prime/part-variables.txt'."
+      exit 1
+    fi
+  done < expected-env.txt