meta: omit LD_LIBRARY_PATH and PATH in classic confinement (#4216)

Signed-off-by: Callahan Kovacs <[email protected]>

snapcraft/meta/snap_yaml.py

@@ -397,7 +397,9 @@ def write(project: Project, prime_dir: Path, *, arch: str):
     # if arch is "all", do not include architecture-specific paths in the environment
     arch_triplet = None if arch == "all" else project.get_build_for_arch_triplet()
 
-    environment = _populate_environment(project.environment, prime_dir, arch_triplet)
+    environment = _populate_environment(
+        project.environment, prime_dir, arch_triplet, project.confinement
+    )
     version = process_version(project.version)
 
     # project provided assumes and computed assumes
@@ -454,39 +456,44 @@ def _populate_environment(
     environment: Optional[Dict[str, Optional[str]]],
     prime_dir: Path,
     arch_triplet: Optional[str],
-):
-    """Populate default app environmental variables.
+    confinement: str,
+) -> Optional[Dict[str, Optional[str]]]:
+    """Populate default app environment variables, LD_LIBRARY_PATH and PATH.
 
-    Three cases for LD_LIBRARY_PATH and PATH variables:
-        - If LD_LIBRARY_PATH or PATH are defined, keep user-defined values.
-        - If LD_LIBRARY_PATH or PATH are not defined, set to default values.
-        - If LD_LIBRARY_PATH or PATH are null, do not use default values.
+    Three cases for environment variables:
+      - If defined, keep user-defined value.
+      - If not defined, set to default value.
+      - If null, do not use default value.
 
     :param environment: Dictionary of environment variables from the project.
     :param prime_dir: The directory containing the content to be snapped.
     :param arch_triplet: Architecture triplet of the target arch. If None, the
     environment will not contain architecture-specific paths.
+    :param confinement: If classically confined, then no default values will be used.
+
+    :returns: Dictionary of environment variables or None if all envvars are null or
+    confinement is classic.
     """
     if environment is None:
+        if confinement == "classic":
+            return None
         return {
             "LD_LIBRARY_PATH": get_ld_library_paths(prime_dir, arch_triplet),
             "PATH": "$SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH",
         }
 
-    try:
-        if not environment["LD_LIBRARY_PATH"]:
-            environment.pop("LD_LIBRARY_PATH")
-    except KeyError:
+    # if LD_LIBRARY_PATH is not defined, use default value when not classic
+    if "LD_LIBRARY_PATH" not in environment and confinement != "classic":
         environment["LD_LIBRARY_PATH"] = get_ld_library_paths(prime_dir, arch_triplet)
+    # else if null, then remove from environment
+    elif "LD_LIBRARY_PATH" in environment and not environment["LD_LIBRARY_PATH"]:
+        del environment["LD_LIBRARY_PATH"]
 
-    try:
-        if not environment["PATH"]:
-            environment.pop("PATH")
-    except KeyError:
+    # if PATH is not defined, use default value when not classic
+    if "PATH" not in environment and confinement != "classic":
         environment["PATH"] = "$SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH"
+    # else if null, then remove from environment
+    elif "PATH" in environment and not environment["PATH"]:
+        del environment["PATH"]
 
-    if len(environment):
-        return environment
-
-    # if the environment only contained a null LD_LIBRARY_PATH and a null PATH, return None
-    return None
+    return environment if environment else None

tests/unit/meta/test_snap_yaml.py

@@ -593,6 +593,62 @@ def test_project_environment_ld_library_path_null(simple_project, new_dir):
     )
 
 
+@pytest.mark.parametrize(
+    "ld_library_path",
+    [{}, {"LD_LIBRARY_PATH": None}, {"LD_LIBRARY_PATH": "test-ld-library-path"}],
+)
+@pytest.mark.parametrize("path", [{}, {"PATH": None}, {"PATH": "test-path"}])
+@pytest.mark.parametrize(
+    "other_var", [{}, {"OTHER_VAR": None}, {"OTHER_VAR": "test-foo"}]
+)
+def test_project_environment_classic_confinement(
+    ld_library_path, path, other_var, simple_project, new_dir
+):
+    """Verify environment when confinement is classic."""
+    # create expected environment in meta/snap.yaml
+    environment = ld_library_path.update(path)
+    if environment:
+        expected_environment = "environment\n"
+        if ld_library_path:
+            expected_environment += f"  LD_LIBRARY_PATH: {ld_library_path}"
+        if path:
+            expected_environment += f"  PATH: {path}"
+        if other_var:
+            expected_environment += f"  OTHER_VAR: {other_var}"
+    else:
+        expected_environment = ""
+
+    snap_yaml.write(
+        simple_project(environment=environment, confinement="classic"),
+        prime_dir=Path(new_dir),
+        arch="amd64",
+    )
+    yaml_file = Path("meta/snap.yaml")
+    assert yaml_file.is_file()
+
+    content = yaml_file.read_text()
+    assert (
+        content
+        == textwrap.dedent(
+            """\
+        name: mytest
+        version: 1.29.3
+        summary: Single-line elevator pitch for your amazing snap
+        description: test-description
+        architectures:
+        - amd64
+        base: core22
+        apps:
+          app1:
+            command: bin/mytest
+        confinement: classic
+        grade: stable
+        """
+        )
+        + expected_environment
+    )
+
+
 def test_version_git(simple_project, new_dir, mocker):
     """Version in projects with ``version:git`` must be correctly handled."""
     mocker.patch(