From 6abfa63092e83a2ebc220dac98a8befbe3ccf9e6 Mon Sep 17 00:00:00 2001 From: Callahan Kovacs Date: Tue, 24 May 2022 16:31:02 -0500 Subject: [PATCH 1/2] environment: get LD_LIBRARY_PATH Signed-off-by: Callahan Kovacs --- snapcraft/meta/snap_yaml.py | 120 ++++++++- snapcraft/projects.py | 36 --- .../appstream-desktop/expected_snap.yaml | 2 +- .../spread/general/unicode-metadata/task.yaml | 2 +- tests/unit/meta/test_snap_yaml.py | 241 +++++++++++++----- tests/unit/test_projects.py | 59 +---- 6 files changed, 302 insertions(+), 158 deletions(-) diff --git a/snapcraft/meta/snap_yaml.py b/snapcraft/meta/snap_yaml.py index e3f8527624..5e079f247b 100644 --- a/snapcraft/meta/snap_yaml.py +++ b/snapcraft/meta/snap_yaml.py @@ -16,6 +16,9 @@ """Create snap.yaml metadata file.""" +import glob +import os +import re from pathlib import Path from typing import Any, Dict, List, Optional, Set, Union, cast @@ -24,6 +27,17 @@ from snapcraft.projects import Project +_ARCH_TO_TRIPLET: Dict[str, str] = { + "arm64": "aarch64-linux-gnu", + "armhf": "arm-linux-gnueabihf", + "i386": "i386-linux-gnu", + "powerpc": "powerpc-linux-gnu", + "ppc64el": "powerpc64le-linux-gnu", + "riscv64": "riscv64-linux-gnu", + "s390x": "s390x-linux-gnu", + "amd64": "x86_64-linux-gnu", +} + class Socket(YamlModel): """snap.yaml app socket entry.""" @@ -188,7 +202,7 @@ def write(project: Project, prime_dir: Path, *, arch: str): apps=snap_apps or None, confinement=project.confinement, grade=project.grade or "stable", - environment=project.environment, + environment=_populate_environment(project.environment, prime_dir, arch), plugs=project.plugs, slots=project.slots, hooks=project.hooks, @@ -214,3 +228,107 @@ def _repr_str(dumper, data): if "\n" in data: return dumper.represent_scalar("tag:yaml.org,2002:str", data, style="|") return dumper.represent_scalar("tag:yaml.org,2002:str", data) + + +def _populate_environment(environment, prime_dir, arch): + """Populate default app environmental variables. + + 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. + """ + if environment is None: + return { + "LD_LIBRARY_PATH": _get_ld_library_path(prime_dir, arch), + "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: + environment["LD_LIBRARY_PATH"] = _get_ld_library_path(prime_dir, arch) + + try: + if not environment["PATH"]: + environment.pop("PATH") + except KeyError: + environment["PATH"] = "$SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH" + + if len(environment): + return environment + + # if the environment only contained a null LD_LIBRARY_PATH and a null PATH, return None + return None + + +def _get_ld_library_path(prime_dir, arch) -> str: + """Get LD_LIBRARY_PATH variable.""" + paths = ["${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}"] + # Add the default LD_LIBRARY_PATH + paths += _get_common_ld_library_paths(prime_dir, arch) + # Add more specific LD_LIBRARY_PATH from staged packages if necessary + paths += _get_configured_ld_library_paths(prime_dir) + + ld_library_path = ":".join(paths) + + return re.sub(str(prime_dir), "$SNAP", ld_library_path) + + +def _get_common_ld_library_paths(prime_dir, arch) -> List[str]: + """Return common library paths for a snap. + + If existing_only is set the paths returned must exist for + the root that was set. + """ + triplet = _ARCH_TO_TRIPLET[arch] + paths = [ + os.path.join(prime_dir, "lib"), + os.path.join(prime_dir, "usr", "lib"), + os.path.join(prime_dir, "lib", triplet), + os.path.join(prime_dir, "usr", "lib", triplet), + ] + + return [p for p in paths if os.path.exists(p)] + + +def _get_configured_ld_library_paths(prime_dir: str) -> List[str]: + """Determine additional library paths needed for the linker loader. + + This is a workaround until full library searching is implemented which + works by searching for ld.so.conf in specific hard coded locations + within root. + + :param prime_dir str: the directory to search for specific ld.so.conf + entries. + :returns: a list of strings of library paths where relevant libraries + can be found within prime_dir. + """ + # If more ld.so.conf files need to be supported, add them here. + ld_config_globs = {f"{prime_dir}/usr/lib/*/mesa*/ld.so.conf"} + + ld_library_paths = [] + for this_glob in ld_config_globs: + for ld_conf_file in glob.glob(this_glob): + ld_library_paths.extend(_extract_ld_library_paths(ld_conf_file)) + + return [prime_dir + path for path in ld_library_paths] + + +def _extract_ld_library_paths(ld_conf_file: str) -> List[str]: + # From the ldconfig manpage, paths can be colon-, space-, tab-, newline-, + # or comma-separated. + path_delimiters = re.compile(r"[:\s,]") + comments = re.compile(r"#.*$") + + paths = [] + with open(ld_conf_file, "r", encoding="utf-8") as ld_config: + for line in ld_config: + # Remove comments from line + line = comments.sub("", line).strip() + + if line: + paths.extend(path_delimiters.split(line)) + + return paths diff --git a/snapcraft/projects.py b/snapcraft/projects.py index 00ed168843..98f86d71fa 100644 --- a/snapcraft/projects.py +++ b/snapcraft/projects.py @@ -388,42 +388,6 @@ def _validate_epoch(cls, epoch): return epoch - @pydantic.validator("environment", always=True) - @classmethod - def _validate_environment(cls, environment): - """Validate app environmental variables. - - 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. - """ - if environment is None: - return { - "LD_LIBRARY_PATH": "$SNAP_LIBRARY_PATH:$LD_LIBRARY_PATH", - "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: - environment["LD_LIBRARY_PATH"] = "$SNAP_LIBRARY_PATH:$LD_LIBRARY_PATH" - - try: - if not environment["PATH"]: - environment.pop("PATH") - except KeyError: - environment[ - "PATH" - ] = "$SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH" - - if len(environment): - return environment - - # if the environment only contained a null LD_LIBRARY_PATH and a null PATH, return None - return None - @classmethod def unmarshal(cls, data: Dict[str, Any]) -> "Project": """Create and populate a new ``Project`` object from dictionary data. diff --git a/tests/spread/core22/appstream-desktop/expected_snap.yaml b/tests/spread/core22/appstream-desktop/expected_snap.yaml index 798303b591..dcb6deddc0 100644 --- a/tests/spread/core22/appstream-desktop/expected_snap.yaml +++ b/tests/spread/core22/appstream-desktop/expected_snap.yaml @@ -23,5 +23,5 @@ apps: confinement: strict grade: devel environment: - LD_LIBRARY_PATH: $SNAP_LIBRARY_PATH:$LD_LIBRARY_PATH + LD_LIBRARY_PATH: ${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH diff --git a/tests/spread/general/unicode-metadata/task.yaml b/tests/spread/general/unicode-metadata/task.yaml index babb44cd2e..bcc4cfc19c 100644 --- a/tests/spread/general/unicode-metadata/task.yaml +++ b/tests/spread/general/unicode-metadata/task.yaml @@ -12,7 +12,7 @@ prepare: | sed -e "s/base: {{BASE}}/base: ${base}/g" expected_snap_tmpl.yaml > expected_snap.yaml if [[ "$base" =~ "core22" ]]; then # shellcheck disable=SC2016 - echo -e 'environment:\n LD_LIBRARY_PATH: $SNAP_LIBRARY_PATH:$LD_LIBRARY_PATH\n PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH' >> expected_snap.yaml + echo -e 'environment:\n LD_LIBRARY_PATH: ${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}\n PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH' >> expected_snap.yaml fi restore: | diff --git a/tests/unit/meta/test_snap_yaml.py b/tests/unit/meta/test_snap_yaml.py index b8930b2dd7..14f96c49f8 100644 --- a/tests/unit/meta/test_snap_yaml.py +++ b/tests/unit/meta/test_snap_yaml.py @@ -26,38 +26,36 @@ @pytest.fixture def simple_project(): - snapcraft_yaml = textwrap.dedent( - """\ - name: mytest - version: 1.29.3 - base: core22 - summary: Single-line elevator pitch for your amazing snap - description: | - This is my-snap's description. You have a paragraph or two to tell the - most important story about your snap. Keep it under 100 words though, - we live in tweetspace and your description wants to look good in the snap - store. + def _simple_project(**kwargs): + snapcraft_config = { + "name": "mytest", + "version": "1.29.3", + "base": "core22", + "summary": "Single-line elevator pitch for your amazing snap", + "description": "test-description", + "confinement": "strict", + "parts": { + "part1": { + "plugin": "nil", + }, + }, + "apps": { + "app1": { + "command": "bin/mytest", + }, + }, + **kwargs, + } + return Project.unmarshal(snapcraft_config) - confinement: strict - - parts: - part1: - plugin: nil - - apps: - app1: - command: bin/mytest - """ - ) - data = yaml.safe_load(snapcraft_yaml) - yield Project.unmarshal(data) + yield _simple_project def test_simple_snap_yaml(simple_project, new_dir): snap_yaml.write( - simple_project, + simple_project(), prime_dir=Path(new_dir), - arch="arch", + arch="amd64", ) yaml_file = Path("meta/snap.yaml") assert yaml_file.is_file() @@ -68,13 +66,9 @@ def test_simple_snap_yaml(simple_project, new_dir): name: mytest version: 1.29.3 summary: Single-line elevator pitch for your amazing snap - description: | - This is my-snap's description. You have a paragraph or two to tell the - most important story about your snap. Keep it under 100 words though, - we live in tweetspace and your description wants to look good in the snap - store. + description: test-description architectures: - - arch + - amd64 base: core22 apps: app1: @@ -82,7 +76,7 @@ def test_simple_snap_yaml(simple_project, new_dir): confinement: strict grade: stable environment: - LD_LIBRARY_PATH: $SNAP_LIBRARY_PATH:$LD_LIBRARY_PATH + LD_LIBRARY_PATH: ${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH """ ) @@ -207,7 +201,7 @@ def test_complex_snap_yaml(complex_project, new_dir): snap_yaml.write( complex_project, prime_dir=Path(new_dir), - arch="arch", + arch="amd64", ) yaml_file = Path("meta/snap.yaml") assert yaml_file.is_file() @@ -225,7 +219,7 @@ def test_complex_snap_yaml(complex_project, new_dir): store. type: app architectures: - - arch + - amd64 base: core22 assumes: - command-chain @@ -274,7 +268,7 @@ def test_complex_snap_yaml(complex_project, new_dir): grade: devel environment: GLOBAL_VARIABLE: test-global-variable - LD_LIBRARY_PATH: $SNAP_LIBRARY_PATH:$LD_LIBRARY_PATH + LD_LIBRARY_PATH: ${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH plugs: empty-plug: null @@ -325,54 +319,179 @@ def test_complex_snap_yaml(complex_project, new_dir): ) -def test_hook_command_chain_assumes(new_dir): - snapcraft_yaml = textwrap.dedent( +def test_hook_command_chain_assumes(simple_project, new_dir): + hooks = { + "hook": { + "command-chain": ["c1"], + }, + } + + snap_yaml.write( + simple_project(hooks=hooks), + 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" + version: 1.29.3 + summary: Single-line elevator pitch for your amazing snap + description: test-description + architectures: + - amd64 base: core22 - summary: Summary - description: Description + assumes: + - command-chain + apps: + app1: + command: bin/mytest confinement: strict - - parts: - part1: - plugin: nil - + grade: stable + environment: + LD_LIBRARY_PATH: ${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} + PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH hooks: hook: - command-chain: ["c1"] + command-chain: + - c1 + """ + ) + + +def test_project_environment_ld_library_path_and_path_defined(simple_project, new_dir): + """Test behavior of defining LD_LIBRARY_PATH and PATH variables.""" + environment = { + "LD_LIBRARY_PATH": "test-1", + "PATH": "test-2", + } + snap_yaml.write( + simple_project(environment=environment), + 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: strict + grade: stable + environment: + LD_LIBRARY_PATH: test-1 + PATH: test-2 + """ + ) + + +def test_project_environment_ld_library_path_defined(simple_project, new_dir): + """LD_LIBRARY_PATH can be overridden without affecting PATH.""" + environment = {"LD_LIBRARY_PATH": "test-1"} + + snap_yaml.write( + simple_project(environment=environment), + 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: strict + grade: stable + environment: + LD_LIBRARY_PATH: test-1 + PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH """ ) - project = Project.unmarshal(yaml.safe_load(snapcraft_yaml)) + +def test_project_environment_path_defined(simple_project, new_dir): + """PATH can be overridden without affecting LD_LIBRARY_PATH.""" + environment = {"PATH": "test-2"} snap_yaml.write( - project, + simple_project(environment=environment), prime_dir=Path(new_dir), - arch="arch", + 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: strict + grade: stable + environment: + PATH: test-2 + LD_LIBRARY_PATH: ${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} + """ + ) + +def test_project_environment_ld_library_path_null(simple_project, new_dir): + """LD_LIBRARY_PATH can be overridden without affecting PATH.""" + environment = {"LD_LIBRARY_PATH": None} + snap_yaml.write( + simple_project(environment=environment), + 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' - summary: Summary - description: Description + version: 1.29.3 + summary: Single-line elevator pitch for your amazing snap + description: test-description architectures: - - arch + - amd64 base: core22 - assumes: - - command-chain + apps: + app1: + command: bin/mytest confinement: strict grade: stable environment: - LD_LIBRARY_PATH: $SNAP_LIBRARY_PATH:$LD_LIBRARY_PATH PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH - hooks: - hook: - command-chain: - - c1 """ ) diff --git a/tests/unit/test_projects.py b/tests/unit/test_projects.py index 927ed469b0..4bf265b841 100644 --- a/tests/unit/test_projects.py +++ b/tests/unit/test_projects.py @@ -97,10 +97,7 @@ def test_project_defaults(self, project_yaml_data): assert project.plugs is None assert project.slots is None assert project.epoch is None - assert project.environment == { - "LD_LIBRARY_PATH": "$SNAP_LIBRARY_PATH:$LD_LIBRARY_PATH", - "PATH": "$SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH", - } + assert project.environment is None assert project.adopt_info is None def test_app_defaults(self, project_yaml_data): @@ -431,60 +428,6 @@ def test_project_environment_invalid(self, environment, project_yaml_data): with pytest.raises(errors.ProjectValidationError, match=error): Project.unmarshal(project_yaml_data(environment=environment)) - def test_project_environment_ld_library_path_and_path_defined( - self, project_yaml_data - ): - """Test behavior of defining LD_LIBRARY_PATH and PATH variables.""" - environment = { - "LD_LIBRARY_PATH": "test-1", - "PATH": "test-2", - } - data = project_yaml_data(environment=environment) - project = Project.unmarshal(data) - - assert project.environment is not None - assert project.environment == environment - - def test_project_environment_ld_library_path_defined(self, project_yaml_data): - """LD_LIBRARY_PATH can be overridden without affecting PATH.""" - environment = {"LD_LIBRARY_PATH": "test-1"} - data = project_yaml_data(environment=environment) - project = Project.unmarshal(data) - - assert project.environment is not None - assert project.environment["LD_LIBRARY_PATH"] == "test-1" - assert ( - project.environment["PATH"] - == "$SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH" - ) - - def test_project_environment_path_defined(self, project_yaml_data): - """PATH can be overridden without affecting LD_LIBRARY_PATH.""" - environment = {"PATH": "test-2"} - data = project_yaml_data(environment=environment) - project = Project.unmarshal(data) - - assert project.environment is not None - assert project.environment is not None - assert ( - project.environment["LD_LIBRARY_PATH"] - == "$SNAP_LIBRARY_PATH:$LD_LIBRARY_PATH" - ) - assert project.environment["PATH"] == "test-2" - - def test_project_environment_ld_library_path_null(self, project_yaml_data): - """LD_LIBRARY_PATH can be overridden without affecting PATH.""" - environment = {"LD_LIBRARY_PATH": None} - data = project_yaml_data(environment=environment) - project = Project.unmarshal(data) - - assert project.environment is not None - assert project.environment.get("LD_LIBRARY_PATH") is None - assert ( - project.environment["PATH"] - == "$SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH" - ) - @pytest.mark.parametrize( "plugs", [ From 08ee32178673171ce90eb222ed9ecf7ad247da8b Mon Sep 17 00:00:00 2001 From: Sergio Schvezov Date: Tue, 24 May 2022 23:43:29 -0300 Subject: [PATCH 2/2] utils: refactor library methods out of snap_yaml - move the library methods out of snap_yaml to utils - add unit tests for the library methods - add proper type hints - remove logic for arch triplet in snap_yaml and use the lifecycle instead Signed-off-by: Sergio Schvezov --- snapcraft/meta/snap_yaml.py | 100 +++--------------------------- snapcraft/parts/lifecycle.py | 5 +- snapcraft/parts/parts.py | 5 ++ snapcraft/utils.py | 72 ++++++++++++++++++++- tests/unit/meta/test_snap_yaml.py | 7 +++ tests/unit/test_utils.py | 79 +++++++++++++++++++++++ 6 files changed, 175 insertions(+), 93 deletions(-) diff --git a/snapcraft/meta/snap_yaml.py b/snapcraft/meta/snap_yaml.py index 5e079f247b..e60ec473e0 100644 --- a/snapcraft/meta/snap_yaml.py +++ b/snapcraft/meta/snap_yaml.py @@ -16,9 +16,6 @@ """Create snap.yaml metadata file.""" -import glob -import os -import re from pathlib import Path from typing import Any, Dict, List, Optional, Set, Union, cast @@ -26,17 +23,7 @@ from pydantic_yaml import YamlModel from snapcraft.projects import Project - -_ARCH_TO_TRIPLET: Dict[str, str] = { - "arm64": "aarch64-linux-gnu", - "armhf": "arm-linux-gnueabihf", - "i386": "i386-linux-gnu", - "powerpc": "powerpc-linux-gnu", - "ppc64el": "powerpc64le-linux-gnu", - "riscv64": "riscv64-linux-gnu", - "s390x": "s390x-linux-gnu", - "amd64": "x86_64-linux-gnu", -} +from snapcraft.utils import get_ld_library_paths class Socket(YamlModel): @@ -134,7 +121,7 @@ class Config: # pylint: disable=too-few-public-methods alias_generator = lambda s: s.replace("_", "-") # noqa: E731 -def write(project: Project, prime_dir: Path, *, arch: str): +def write(project: Project, prime_dir: Path, *, arch: str, arch_triplet: str): """Create a snap.yaml file.""" meta_dir = prime_dir / "meta" meta_dir.mkdir(parents=True, exist_ok=True) @@ -187,6 +174,8 @@ def write(project: Project, prime_dir: Path, *, arch: str): if project.hooks and any(h for h in project.hooks.values() if h.command_chain): assumes.add("command-chain") + environment = _populate_environment(project.environment, prime_dir, arch_triplet) + snap_metadata = SnapMetadata( name=project.name, title=project.title, @@ -202,7 +191,7 @@ def write(project: Project, prime_dir: Path, *, arch: str): apps=snap_apps or None, confinement=project.confinement, grade=project.grade or "stable", - environment=_populate_environment(project.environment, prime_dir, arch), + environment=environment, plugs=project.plugs, slots=project.slots, hooks=project.hooks, @@ -230,7 +219,9 @@ def _repr_str(dumper, data): return dumper.represent_scalar("tag:yaml.org,2002:str", data) -def _populate_environment(environment, prime_dir, arch): +def _populate_environment( + environment: Optional[Dict[str, Optional[str]]], prime_dir: Path, arch_triplet: str +): """Populate default app environmental variables. Three cases for LD_LIBRARY_PATH and PATH variables: @@ -240,7 +231,7 @@ def _populate_environment(environment, prime_dir, arch): """ if environment is None: return { - "LD_LIBRARY_PATH": _get_ld_library_path(prime_dir, arch), + "LD_LIBRARY_PATH": get_ld_library_paths(prime_dir, arch_triplet), "PATH": "$SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH", } @@ -248,7 +239,7 @@ def _populate_environment(environment, prime_dir, arch): if not environment["LD_LIBRARY_PATH"]: environment.pop("LD_LIBRARY_PATH") except KeyError: - environment["LD_LIBRARY_PATH"] = _get_ld_library_path(prime_dir, arch) + environment["LD_LIBRARY_PATH"] = get_ld_library_paths(prime_dir, arch_triplet) try: if not environment["PATH"]: @@ -261,74 +252,3 @@ def _populate_environment(environment, prime_dir, arch): # if the environment only contained a null LD_LIBRARY_PATH and a null PATH, return None return None - - -def _get_ld_library_path(prime_dir, arch) -> str: - """Get LD_LIBRARY_PATH variable.""" - paths = ["${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}"] - # Add the default LD_LIBRARY_PATH - paths += _get_common_ld_library_paths(prime_dir, arch) - # Add more specific LD_LIBRARY_PATH from staged packages if necessary - paths += _get_configured_ld_library_paths(prime_dir) - - ld_library_path = ":".join(paths) - - return re.sub(str(prime_dir), "$SNAP", ld_library_path) - - -def _get_common_ld_library_paths(prime_dir, arch) -> List[str]: - """Return common library paths for a snap. - - If existing_only is set the paths returned must exist for - the root that was set. - """ - triplet = _ARCH_TO_TRIPLET[arch] - paths = [ - os.path.join(prime_dir, "lib"), - os.path.join(prime_dir, "usr", "lib"), - os.path.join(prime_dir, "lib", triplet), - os.path.join(prime_dir, "usr", "lib", triplet), - ] - - return [p for p in paths if os.path.exists(p)] - - -def _get_configured_ld_library_paths(prime_dir: str) -> List[str]: - """Determine additional library paths needed for the linker loader. - - This is a workaround until full library searching is implemented which - works by searching for ld.so.conf in specific hard coded locations - within root. - - :param prime_dir str: the directory to search for specific ld.so.conf - entries. - :returns: a list of strings of library paths where relevant libraries - can be found within prime_dir. - """ - # If more ld.so.conf files need to be supported, add them here. - ld_config_globs = {f"{prime_dir}/usr/lib/*/mesa*/ld.so.conf"} - - ld_library_paths = [] - for this_glob in ld_config_globs: - for ld_conf_file in glob.glob(this_glob): - ld_library_paths.extend(_extract_ld_library_paths(ld_conf_file)) - - return [prime_dir + path for path in ld_library_paths] - - -def _extract_ld_library_paths(ld_conf_file: str) -> List[str]: - # From the ldconfig manpage, paths can be colon-, space-, tab-, newline-, - # or comma-separated. - path_delimiters = re.compile(r"[:\s,]") - comments = re.compile(r"#.*$") - - paths = [] - with open(ld_conf_file, "r", encoding="utf-8") as ld_config: - for line in ld_config: - # Remove comments from line - line = comments.sub("", line).strip() - - if line: - paths.extend(path_delimiters.split(line)) - - return paths diff --git a/snapcraft/parts/lifecycle.py b/snapcraft/parts/lifecycle.py index 74f6dc7f21..1148d5e20f 100644 --- a/snapcraft/parts/lifecycle.py +++ b/snapcraft/parts/lifecycle.py @@ -251,6 +251,7 @@ def _run_command( project, lifecycle.prime_dir, arch=lifecycle.target_arch, + arch_triplet=lifecycle.target_arch_triplet, ) emit.message("Generated snap metadata", intermediate=True) @@ -378,7 +379,9 @@ def _set_step_environment(step_info: StepInfo) -> bool: return True -def _expand_environment(snapcraft_yaml: Dict[str, Any], *, parallel_build_count: int) -> None: +def _expand_environment( + snapcraft_yaml: Dict[str, Any], *, parallel_build_count: int +) -> None: """Expand global variables in the provided dictionary values. :param snapcraft_yaml: A dictionary containing the contents of the diff --git a/snapcraft/parts/parts.py b/snapcraft/parts/parts.py index 5e26534ac1..059f18ea2c 100644 --- a/snapcraft/parts/parts.py +++ b/snapcraft/parts/parts.py @@ -108,6 +108,11 @@ def target_arch(self) -> str: """Return the parts project target architecture.""" return self._lcm.project_info.target_arch + @property + def target_arch_triplet(self) -> str: + """Return the parts project target architecture.""" + return self._lcm.project_info.arch_triplet + @property def project_vars(self) -> Dict[str, str]: """Return the value of project variable ``version``.""" diff --git a/snapcraft/utils.py b/snapcraft/utils.py index ee50fb0483..e778dab44d 100644 --- a/snapcraft/utils.py +++ b/snapcraft/utils.py @@ -15,15 +15,17 @@ # along with this program. If not, see . """Utilities for snapcraft.""" - +import glob import multiprocessing import os import pathlib import platform +import re import sys from dataclasses import dataclass from getpass import getpass -from typing import Iterable, Optional +from pathlib import Path +from typing import Iterable, List, Optional from craft_cli import emit @@ -276,3 +278,69 @@ def humanize_list( humanized += "," return f"{humanized} {conjunction} {quoted_items[-1]}" + + +def _extract_ld_library_paths(ld_conf_file: str) -> List[str]: + # From the ldconfig manpage, paths can be colon-, space-, tab-, newline-, + # or comma-separated. + path_delimiters = re.compile(r"[:\s,]") + comments = re.compile(r"#.*$") + + paths = [] + with open(ld_conf_file, "r", encoding="utf-8") as ld_config: + for line in ld_config: + # Remove comments from line + line = comments.sub("", line).strip() + + if line: + paths.extend(path_delimiters.split(line)) + + return paths + + +def _get_configured_ld_library_paths(prime_dir: Path) -> List[str]: + """Determine additional library paths needed for the linker loader. + + This is a workaround until full library searching is implemented which + works by searching for ld.so.conf in specific hard coded locations + within root. + + :param prime_dir str: the directory to search for specific ld.so.conf + entries. + :returns: a list of strings of library paths where relevant libraries + can be found within prime_dir. + """ + # If more ld.so.conf files need to be supported, add them here. + ld_config_globs = {f"{str(prime_dir)}/usr/lib/*/mesa*/ld.so.conf"} + + ld_library_paths = [] + for this_glob in ld_config_globs: + for ld_conf_file in glob.glob(this_glob): + ld_library_paths.extend(_extract_ld_library_paths(ld_conf_file)) + + return [str(prime_dir / path.lstrip("/")) for path in ld_library_paths] + + +def _get_common_ld_library_paths(prime_dir: Path, arch_triplet: str) -> List[str]: + """Return common existing PATH entries for a snap.""" + paths = [ + prime_dir / "lib", + prime_dir / "usr" / "lib", + prime_dir / "lib" / arch_triplet, + prime_dir / "usr" / "lib" / arch_triplet, + ] + + return [str(p) for p in paths if p.exists()] + + +def get_ld_library_paths(prime_dir: Path, arch_triplet: str) -> str: + """Return a usable in-snap LD_LIBRARY_PATH variable.""" + paths = ["${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}"] + # Add the default LD_LIBRARY_PATH + paths += _get_common_ld_library_paths(prime_dir, arch_triplet) + # Add more specific LD_LIBRARY_PATH from staged packages if necessary + paths += _get_configured_ld_library_paths(prime_dir) + + ld_library_path = ":".join(paths) + + return re.sub(str(prime_dir), "$SNAP", ld_library_path) diff --git a/tests/unit/meta/test_snap_yaml.py b/tests/unit/meta/test_snap_yaml.py index 14f96c49f8..8cffb55be6 100644 --- a/tests/unit/meta/test_snap_yaml.py +++ b/tests/unit/meta/test_snap_yaml.py @@ -56,6 +56,7 @@ def test_simple_snap_yaml(simple_project, new_dir): simple_project(), prime_dir=Path(new_dir), arch="amd64", + arch_triplet="x86_64-linux-gnu", ) yaml_file = Path("meta/snap.yaml") assert yaml_file.is_file() @@ -202,6 +203,7 @@ def test_complex_snap_yaml(complex_project, new_dir): complex_project, prime_dir=Path(new_dir), arch="amd64", + arch_triplet="x86_64-linux-gnu", ) yaml_file = Path("meta/snap.yaml") assert yaml_file.is_file() @@ -330,6 +332,7 @@ def test_hook_command_chain_assumes(simple_project, new_dir): simple_project(hooks=hooks), prime_dir=Path(new_dir), arch="amd64", + arch_triplet="x86_64-linux-gnu", ) yaml_file = Path("meta/snap.yaml") assert yaml_file.is_file() @@ -372,6 +375,7 @@ def test_project_environment_ld_library_path_and_path_defined(simple_project, ne simple_project(environment=environment), prime_dir=Path(new_dir), arch="amd64", + arch_triplet="x86_64-linux-gnu", ) yaml_file = Path("meta/snap.yaml") assert yaml_file.is_file() @@ -406,6 +410,7 @@ def test_project_environment_ld_library_path_defined(simple_project, new_dir): simple_project(environment=environment), prime_dir=Path(new_dir), arch="amd64", + arch_triplet="x86_64-linux-gnu", ) yaml_file = Path("meta/snap.yaml") assert yaml_file.is_file() @@ -439,6 +444,7 @@ def test_project_environment_path_defined(simple_project, new_dir): simple_project(environment=environment), prime_dir=Path(new_dir), arch="amd64", + arch_triplet="x86_64-linux-gnu", ) yaml_file = Path("meta/snap.yaml") assert yaml_file.is_file() @@ -472,6 +478,7 @@ def test_project_environment_ld_library_path_null(simple_project, new_dir): simple_project(environment=environment), prime_dir=Path(new_dir), arch="amd64", + arch_triplet="x86_64-linux-gnu", ) yaml_file = Path("meta/snap.yaml") assert yaml_file.is_file() diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 1018f17a13..2b612f3ddf 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -275,3 +275,82 @@ def test_get_parallel_build_count_limited(mocker, max_count, count): ) def test_humanize_list(items, conjunction, expected): assert utils.humanize_list(items, conjunction) == expected + + +################# +# Library Paths # +################# + + +@pytest.fixture +def library_config_file(tmp_path): + config_file = tmp_path / "usr/lib/i286-other-other/mesa/ld.so.conf" + config_file.parent.mkdir(parents=True) + config_file.write_text( + dedent( + """\ + # This is a comment + /foo/bar + /colon:/separated,/comma\t/tab /space # This is another comment + /baz""" + ) + ) + + return config_file + + +def test_extract_ld_library_paths(tmp_path, library_config_file): + assert utils._extract_ld_library_paths(library_config_file) == [ + "/foo/bar", + "/colon", + "/separated", + "/comma", + "/tab", + "/space", + "/baz", + ] + + +@pytest.mark.parametrize( + "lib_dirs,expected_env", + [ + (["lib"], "$SNAP/lib"), + (["lib", "usr/lib"], "$SNAP/lib:$SNAP/usr/lib"), + ( + ["lib/i286-none-none", "usr/lib/i286-none-none"], + "$SNAP/lib:$SNAP/usr/lib:$SNAP/lib/i286-none-none:$SNAP/usr/lib/i286-none-none", + ), + ], +) +def test_get_ld_library_paths(tmp_path, lib_dirs, expected_env): + for d in lib_dirs: + (tmp_path / d).mkdir(parents=True) + + expected_env = ( + f"${{SNAP_LIBRARY_PATH}}${{LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}}:{expected_env}" + ) + assert utils.get_ld_library_paths(tmp_path, "i286-none-none") == expected_env + + +@pytest.mark.parametrize( + "lib_dirs,expected_env", + [ + (["lib", "usr/lib"], "$SNAP/lib:$SNAP/usr/lib"), + ( + ["lib/i286-none-none", "usr/lib/i286-none-none"], + "$SNAP/lib:$SNAP/usr/lib:$SNAP/lib/i286-none-none:$SNAP/usr/lib/i286-none-none", + ), + ], +) +def test_get_ld_library_paths_with_conf( + tmp_path, lib_dirs, expected_env, library_config_file +): + for d in lib_dirs: + (tmp_path / d).mkdir(parents=True, exist_ok=True) + + expected_env = ( + "${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}:" + f"{expected_env}:" + "$SNAP/foo/bar:$SNAP/colon:$SNAP/separated:$SNAP/comma:$SNAP/tab:$SNAP/space:$SNAP/baz" + ) + assert utils.get_ld_library_paths(tmp_path, "i286-none-none") == expected_env