Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: flow for plugin package file mapping and xattr tagging #979

Merged
merged 12 commits into from
Jan 20, 2025
18 changes: 17 additions & 1 deletion craft_parts/executor/part_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

from typing_extensions import Protocol

from craft_parts import callbacks, errors, overlays, packages, plugins, sources
from craft_parts import callbacks, errors, overlays, packages, plugins, sources, xattrs
from craft_parts.actions import Action, ActionType
from craft_parts.infos import PartInfo, StepInfo
from craft_parts.overlays import LayerHash, OverlayManager
Expand All @@ -47,6 +47,8 @@

logger = logging.getLogger(__name__)

_ORIGIN_PACKAGE_FORMAT = "origin_{plugin_name}_package"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit strange in isolate form but I see it's following the xattr pattern already used for stage packages.



# pylint: disable=too-many-lines

Expand Down Expand Up @@ -315,6 +317,10 @@ def _run_build(
stderr=stderr,
)

# The plugin has by now built/unpacked packages, now get that file list mapping
# and xattr the files.
self._annotate_plugin_files()

# Organize the installed files as requested. We do this in the build step for
# two reasons:
#
Expand Down Expand Up @@ -982,6 +988,16 @@ def _unpack_stage_snaps(self) -> None:
for snap_source in snap_sources:
snap_source.provision(install_dir, keep=True)

def _annotate_plugin_files(self) -> None:
"""Get the PackageFiles from the plugin and tag all the files it includes."""
for pkg, pkg_files in self._plugin.get_package_files().items():
for pkg_file in pkg_files:
xattrs.write_xattr(
pkg_file,
_ORIGIN_PACKAGE_FORMAT.format(plugin_name=pkg.plugin),
f"{pkg.name}={pkg.version}",
)


def _remove(filename: Path) -> None:
"""Remove the given directory entry.
Expand Down
2 changes: 2 additions & 0 deletions craft_parts/packages/platform.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# noqa: A005
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this will render the editor modeline below useless.


# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
#
# Copyright 2017-2023 Canonical Ltd.
Expand Down
8 changes: 6 additions & 2 deletions craft_parts/plugins/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
class Package:
"""A dataclass that uniquely identifies a package."""

plugin: str
name: str
version: str

Expand Down Expand Up @@ -103,8 +104,11 @@ def set_action_properties(self, action_properties: ActionProperties) -> None:
"""
self._action_properties = deepcopy(action_properties)

def get_files(self) -> PackageFiles:
"""Get a mapping of (package name, package version) -> installed file."""
def get_package_files(self) -> PackageFiles:
"""Get a mapping of Package -> files, if the plugin supports this.

Used for xattr annotation, for reduced SBOM generation.
"""
return {}


Expand Down
4 changes: 2 additions & 2 deletions craft_parts/plugins/npm_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ def get_build_commands(self) -> list[str]:
return cmd

@override
def get_files(self) -> PackageFiles:
def get_package_files(self) -> PackageFiles:
root_modules_dir = (
self._part_info.part_install_dir / "lib/node_modules"
).absolute()
Expand Down Expand Up @@ -365,7 +365,7 @@ def _append_package_dir(
for file_or_dir in dirnames + filenames:
pkg_contents.add(walk_iteration_root / file_or_dir)

key = Package(pkg_name, pkg_version)
key = Package("npm", pkg_name, pkg_version)
if key in file_list:
# It appears we have two installs of the same package and version
# at different points in the tree. This is fine. If we ever care
Expand Down
4 changes: 2 additions & 2 deletions craft_parts/plugins/python_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def _get_package_install_commands(self) -> list[str]:
return commands

@override
def get_files(self) -> PackageFiles:
def get_package_files(self) -> PackageFiles:
# https://packaging.python.org/en/latest/specifications/binary-distribution-format/
# Could also add the pkginfo library for this

Expand All @@ -106,5 +106,5 @@ def get_files(self) -> PackageFiles:
# First column is files. These are relative, resolve() to get
# rid of all the ".." that leads up to the bin dir.
pkg_files = {(site_pkgs_dir / f[0]).resolve() for f in csvreader}
ret[Package(pkg_name, pkg_version)] = pkg_files
ret[Package("python", pkg_name, pkg_version)] = pkg_files
return ret
7 changes: 5 additions & 2 deletions craft_parts/xattrs.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@
import logging
import os
import sys
from pathlib import Path

from craft_parts import errors

logger = logging.getLogger(__name__)


def read_xattr(path: str, key: str) -> str | None:
def read_xattr(path: str | Path, key: str) -> str | None:
"""Get extended attribute metadata from a file.

:param path: The file to get metadata from.
Expand All @@ -35,6 +36,7 @@ def read_xattr(path: str, key: str) -> str | None:
"""
if sys.platform != "linux":
raise RuntimeError("xattr support only available for Linux")
path = str(path)

# Extended attributes do not apply to symlinks.
if os.path.islink(path):
Expand All @@ -58,7 +60,7 @@ def read_xattr(path: str, key: str) -> str | None:
return value.decode().strip()


def write_xattr(path: str, key: str, value: str) -> None:
def write_xattr(path: str | Path, key: str, value: str) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For general API consistency it would be better to use only Path here, unless convenience suffers too much. Then it would be simpler to test this only with Path instead of testing with str and Path.

"""Add extended attribute metadata to a file.

:param path: The file to add metadata to.
Expand All @@ -67,6 +69,7 @@ def write_xattr(path: str, key: str, value: str) -> None:
"""
if sys.platform != "linux":
raise RuntimeError("xattr support only available for Linux")
path = str(path)

# Extended attributes do not apply to symlinks.
if os.path.islink(path):
Expand Down
1 change: 1 addition & 0 deletions docs/common/craft-parts/craft-parts.wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ RuntimeIdentifier
RustPlugin
RustPluginEnvironmentValidator
RustPluginProperties
SBOM
SCons
SConsPlugin
SConsPluginEnvironmentValidator
Expand Down
4 changes: 4 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ dependencies = [
apt = [
"python-apt>=2.4.0;sys_platform=='linux'"
]
apt-manual = [
# Install this with "pip install .[apt-manual]" to get a functioning test environment
"python-apt @ https://launchpad.net/ubuntu/+archive/primary/+sourcefiles/python-apt/2.4.0ubuntu2/python-apt_2.4.0ubuntu2.tar.xz ; sys_platform == 'linux'"
]
docs = [
"canonical-sphinx",
"sphinx",
Expand Down
25 changes: 16 additions & 9 deletions tests/integration/plugins/test_npm.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,9 @@ def test_npm_plugin_include_node(create_fake_package_with_node, new_dir, partiti


@pytest.mark.slow
def test_npm_plugin_get_files(create_fake_package_with_node, new_dir, partitions):
def test_npm_plugin_get_package_files(
create_fake_package_with_node, new_dir, partitions
):
parts = create_fake_package_with_node()
lifecycle = LifecycleManager(
parts,
Expand All @@ -212,7 +214,9 @@ def test_npm_plugin_get_files(create_fake_package_with_node, new_dir, partitions
ctx.execute(actions)

part_name = list(parts["parts"].keys())[0]
actual_file_list = lifecycle._executor._handler[part_name]._plugin.get_files()
actual_file_list = lifecycle._executor._handler[
part_name
]._plugin.get_package_files()
part_install_dir = lifecycle._executor._part_list[0].part_install_dir

# This example bundles in node, which brings a ton of other dependencies -
Expand Down Expand Up @@ -251,7 +255,7 @@ def test_npm_plugin_get_files(create_fake_package_with_node, new_dir, partitions
# dependencies (cli-columns and gauge) both depend on 5.0.1, which means
# their files get collapsed under a single key (which doesn't matter for
# our purposes.)
ar211 = Package("ansi-regex", "2.1.1")
ar211 = Package("npm", "ansi-regex", "2.1.1")
assert ar211 in actual_file_list
assert len(actual_file_list[ar211]) == 4, ar211
assert _paths_relative_to_install_dir(
Expand All @@ -263,18 +267,18 @@ def test_npm_plugin_get_files(create_fake_package_with_node, new_dir, partitions
"lib/node_modules/npm/node_modules/ansi-regex/readme.md",
}

ar300 = Package("ansi-regex", "3.0.0")
ar300 = Package("npm", "ansi-regex", "3.0.0")
assert ar300 in actual_file_list
assert len(actual_file_list[ar300]) == 4, ar300

# Added "index.d.ts" file, absent from previous versions
ar500 = Package("ansi-regex", "5.0.0")
ar500 = Package("npm", "ansi-regex", "5.0.0")
assert ar500 in actual_file_list
assert len(actual_file_list[ar500]) == 5, ar500

# Between 5.0.0 and 5.0.1 they seem to have stopped packaging the readme;
# back to 4 files per install, and there are two installs of this version.
ar501 = Package("ansi-regex", "5.0.1")
ar501 = Package("npm", "ansi-regex", "5.0.1")
assert ar501 in actual_file_list
assert len(actual_file_list[ar501]) == 8, ar501
assert _paths_relative_to_parent_modules_dir(actual_file_list[ar501]) == {
Expand All @@ -285,6 +289,9 @@ def test_npm_plugin_get_files(create_fake_package_with_node, new_dir, partitions
}

# Verify scoped names work properly:
assert Package("@npmcli/installed-package-contents", "1.0.7") in actual_file_list
assert Package("@tootallnate/once", "1.1.2") in actual_file_list
assert Package("@tootallnate/once", "2.0.0") in actual_file_list
assert (
Package("npm", "@npmcli/installed-package-contents", "1.0.7")
in actual_file_list
)
assert Package("npm", "@tootallnate/once", "1.1.2") in actual_file_list
assert Package("npm", "@tootallnate/once", "2.0.0") in actual_file_list
15 changes: 10 additions & 5 deletions tests/integration/plugins/test_python.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ def _get_package_install_commands(self) -> list[str]:


@pytest.mark.slow
def test_python_plugin_get_files(new_dir, partitions):
def test_python_plugin_get_package_files(new_dir, partitions):
parts_yaml = textwrap.dedent(
"""\
parts:
Expand All @@ -438,7 +438,9 @@ def test_python_plugin_get_files(new_dir, partitions):
ctx.execute(actions)

part_name = list(parts["parts"].keys())[0]
actual_file_list = lifecycle._executor._handler[part_name]._plugin.get_files()
actual_file_list = lifecycle._executor._handler[
part_name
]._plugin.get_package_files()
part_install_dir = lifecycle._executor._part_list[0].part_install_dir

# Real quick instantiate another copy of the plugin to ensure statelessness.
Expand All @@ -450,15 +452,18 @@ def test_python_plugin_get_files(new_dir, partitions):
part=Part("foo", {"source": "."}, partitions=partitions),
)
plugin2 = PythonPlugin(properties=properties2, part_info=part_info)
assert plugin2.get_files() == actual_file_list
assert plugin2.get_package_files() == actual_file_list

# Make sure all the expected packages were installed.
# We can't assert the exact set of keys because the pip version will change
# over time. And we can't assert the number of keys because py3.10 installs
# setuptools as a separate package.

assert Package(name="Flask", version="3.1.0") in actual_file_list
assert part_install_dir / "bin/flask" in actual_file_list[Package("Flask", "3.1.0")]
assert Package("python", name="Flask", version="3.1.0") in actual_file_list
assert (
part_install_dir / "bin/flask"
in actual_file_list[Package("python", "Flask", "3.1.0")]
)

# Can't assert specific versions here because flask has >= versions for its
# dependencies.
Expand Down
42 changes: 42 additions & 0 deletions tests/unit/executor/test_part_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from craft_parts.infos import PartInfo, ProjectInfo, StepInfo
from craft_parts.overlays import OverlayManager
from craft_parts.parts import Part
from craft_parts.plugins.base import Package
from craft_parts.state_manager import states
from craft_parts.steps import Step
from craft_parts.utils import os_utils
Expand Down Expand Up @@ -821,6 +822,47 @@ def test_unpack_stage_snaps(self, mocker, new_dir, partitions):
keep=True,
)

def test_annotate_plugin_files(self, mocker):
mock_write_xattr = mocker.patch("craft_parts.xattrs.write_xattr")

package_files = {
Package("foo", "bar", "41"): {
Path("hehe/haha/hoho"),
Path("woop/de/doo"),
Path("/tmp"),
},
Package("foo", "bar", "41.0000001"): {
Path("/tmp"),
Path("something_else"),
},
Package("foo", "quz", "17million"): {
Path("/tmp"),
},
Package("wef", "few", "0.0"): {
Path("/tmp"),
},
}

part_handler = mocker.MagicMock(spec=PartHandler)
part_handler._plugin = mocker.MagicMock()
part_handler._plugin.get_package_files = mocker.MagicMock(
return_value=package_files
)
PartHandler._annotate_plugin_files(part_handler)

mock_write_xattr.assert_has_calls(
[
call(Path("hehe/haha/hoho"), "origin_foo_package", "bar=41"),
call(Path("woop/de/doo"), "origin_foo_package", "bar=41"),
call(Path("/tmp"), "origin_foo_package", "bar=41"),
call(Path("/tmp"), "origin_foo_package", "bar=41.0000001"),
call(Path("something_else"), "origin_foo_package", "bar=41.0000001"),
call(Path("/tmp"), "origin_foo_package", "quz=17million"),
call(Path("/tmp"), "origin_wef_package", "few=0.0"),
],
any_order=True,
)

def test_get_build_packages(self, new_dir, partitions):
p1 = Part(
"p1",
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/packages/test_normalize.py
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ def test_fix_pkg_config_is_dir(self, tmpdir):


@pytest.mark.parametrize(
"src,dst,result",
("src", "dst", "result"),
[
("a", "rel-to-a", "a"),
("/a", "abs-to-a", "a"),
Expand Down
Loading
Loading