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: python plugin file listing #943

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
18deeb0
feat: add plugin code from prototype with signature from npm PR
mattculler Dec 16, 2024
9926a0d
feat: make it work
mattculler Dec 17, 2024
9a53d08
fix: make paths correct and compact
mattculler Dec 17, 2024
e967b15
chore: autoformat
mattculler Dec 17, 2024
62c034c
fix: accidentally committed change to sortableize Package, let it ride
mattculler Dec 17, 2024
befec8f
feat: add unit test
mattculler Dec 18, 2024
deca1a2
fix: name of tested function changed
mattculler Dec 18, 2024
d24a5a3
feat: add integration test
mattculler Dec 18, 2024
684729c
fix: test failed on CI due to differing pip versions
mattculler Dec 18, 2024
be6ec70
fix: test depended on pathlib changes from 3.12, rewrote for 3.10
mattculler Dec 18, 2024
5db00b1
fix: flask has a different number of files on py3.10
mattculler Dec 18, 2024
e17057b
fix: py3.10 installs a different number of packages - adds setuptools
mattculler Dec 18, 2024
6a8e8ce
chore: code review changes
mattculler Dec 18, 2024
344dc5a
fix: code review: make test not break when flask's dependencies update
mattculler Dec 18, 2024
a3e158f
chore: code review: rewrite test to be more explicit
mattculler Dec 18, 2024
1528104
chore: code review: ensure plugin file listing is stateless
mattculler Dec 18, 2024
7968c83
fix: remove now-unnecessary check
mattculler Dec 19, 2024
a23b436
chore: refactor test to put file tree on the FS
mattculler Dec 19, 2024
737c0ca
fix: remove indirect dependency and reliance on specific versions
mattculler Dec 19, 2024
5fbace5
Update tests/integration/plugins/test_python.py
mattculler Dec 19, 2024
e2bbdfd
Update tests/integration/plugins/test_python.py
mattculler Dec 19, 2024
a68e014
fix: can't rely on package version so can't expect a certain number o…
mattculler Dec 19, 2024
031cf84
chore: autoformat
mattculler Dec 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion craft_parts/plugins/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
from craft_parts import infos


@dataclass(frozen=True, slots=True)
@dataclass(frozen=True, slots=True, order=True)
class Package:
"""A dataclass that uniquely identifies a package."""

Expand Down
37 changes: 36 additions & 1 deletion craft_parts/plugins/python_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,14 @@

"""The python plugin."""

import csv
import shlex
from email.parser import HeaderParser
from typing import Literal

from .base import BasePythonPlugin
from overrides import override

from .base import BasePythonPlugin, Package, PackageFiles
from .properties import PluginProperties


Expand Down Expand Up @@ -73,3 +77,34 @@ def _get_package_install_commands(self) -> list[str]:
)

return commands

@override
def get_files(self) -> PackageFiles:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll bet we can put this in the base Python plugin. In all three cases they generate PEP 405 compatible virtual environments in self._get_venv_directory(). (I'm fine if you don't do so in this PR, but when we get to the point of implementing it for poetry and uv it probably comes roughly for free)

# https://packaging.python.org/en/latest/specifications/binary-distribution-format/
# Could also add the pkginfo library for this

venvdir = self._get_venv_directory()
python_path = venvdir / "bin/python"
python_version = python_path.resolve().name
site_pkgs_dir = venvdir / "lib" / python_version / "site-packages"

ret = {}
for pkg_dir in site_pkgs_dir.glob("*.dist-info"):
# Get package name and version from from METADATA file.
# https://packaging.python.org/en/latest/specifications/core-metadata/
parser = HeaderParser()
with open(pkg_dir / "METADATA") as f:
pkg_metadata = parser.parse(f)
pkg_name = pkg_metadata["Name"]
pkg_version = pkg_metadata["Version"]

# Read the RECORD file
record_file = pkg_dir / "RECORD"
with open(record_file) as record_file_obj:
csvreader = csv.reader(record_file_obj)

# 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
return ret
2 changes: 1 addition & 1 deletion tests/integration/plugins/test_npm.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ def test_npm_plugin_include_node(create_fake_package_with_node, new_dir, partiti


@pytest.mark.slow
def test_npm_plugin_get_file_list(create_fake_package_with_node, new_dir, partitions):
def test_npm_plugin_get_files(create_fake_package_with_node, new_dir, partitions):
parts = create_fake_package_with_node()
lifecycle = LifecycleManager(
parts,
Expand Down
90 changes: 83 additions & 7 deletions tests/integration/plugins/test_python.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@
import textwrap
from pathlib import Path

import craft_parts.plugins.plugins
import pytest
import yaml
from craft_parts import LifecycleManager, Step, errors, plugins
from craft_parts.infos import PartInfo, ProjectInfo
from craft_parts.parts import Part
from craft_parts.plugins.base import Package
from craft_parts.plugins.python_plugin import PythonPlugin
from overrides import override


Expand Down Expand Up @@ -123,7 +126,7 @@ def test_python_plugin_symlink(new_dir, partitions):
def test_python_plugin_override_get_system_interpreter(new_dir, partitions):
"""Override the system interpreter, link should use it."""

class MyPythonPlugin(craft_parts.plugins.plugins.PythonPlugin):
class MyPythonPlugin(PythonPlugin):
@override
def _get_system_python_interpreter(self) -> str | None:
return "use-this-python"
Expand Down Expand Up @@ -159,7 +162,7 @@ def test_python_plugin_no_system_interpreter(
):
"""Check that the build fails if a payload interpreter is needed but not found."""

class MyPythonPlugin(craft_parts.plugins.plugins.PythonPlugin):
class MyPythonPlugin(PythonPlugin):
@override
def _get_system_python_interpreter(self) -> str | None:
return None
Expand Down Expand Up @@ -194,7 +197,7 @@ def _should_remove_symlinks(self) -> bool:
def test_python_plugin_remove_symlinks(new_dir, partitions):
"""Override symlink removal."""

class MyPythonPlugin(craft_parts.plugins.plugins.PythonPlugin):
class MyPythonPlugin(PythonPlugin):
@override
def _should_remove_symlinks(self) -> bool:
return True
Expand Down Expand Up @@ -250,7 +253,7 @@ def test_python_plugin_fix_shebangs(new_dir, partitions):
def test_python_plugin_override_shebangs(new_dir, partitions):
"""Override what we want in script shebang lines."""

class MyPythonPlugin(craft_parts.plugins.plugins.PythonPlugin):
class MyPythonPlugin(PythonPlugin):
@override
def _get_script_interpreter(self) -> str:
return "#!/my/script/interpreter"
Expand Down Expand Up @@ -298,7 +301,7 @@ def test_find_payload_python_bad_version(new_dir, partitions):
"""Test that the build fails if a payload interpreter is needed but it's the
wrong Python version."""

class MyPythonPlugin(craft_parts.plugins.plugins.PythonPlugin):
class MyPythonPlugin(PythonPlugin):
@override
def _get_system_python_interpreter(self) -> str | None:
# To have the build fail after failing to find the payload interpreter
Expand Down Expand Up @@ -374,7 +377,7 @@ def test_find_payload_python_good_version(new_dir, partitions):
def test_no_shebangs(new_dir, partitions):
"""Test that building a Python part with no scripts works."""

class ScriptlessPlugin(craft_parts.plugins.plugins.PythonPlugin):
class ScriptlessPlugin(PythonPlugin):
@override
def _get_package_install_commands(self) -> list[str]:
return [
Expand Down Expand Up @@ -408,3 +411,76 @@ def _get_package_install_commands(self) -> list[str]:

primed_script = lf.project_info.prime_dir / "bin/mytest"
assert not primed_script.exists()


@pytest.mark.slow
def test_python_plugin_get_files(new_dir, partitions):
parts_yaml = textwrap.dedent(
"""\
parts:
foo:
plugin: python
source: .
python-packages: [flask==3.1.0]
"""
)
parts = yaml.safe_load(parts_yaml)

lifecycle = LifecycleManager(
parts,
application_name="test_python",
cache_dir=new_dir,
partitions=partitions,
)
actions = lifecycle.plan(Step.BUILD)

with lifecycle.action_executor() as ctx:
ctx.execute(actions)

part_name = list(parts["parts"].keys())[0]
actual_file_list = lifecycle._executor._handler[part_name]._plugin.get_files()
lengau marked this conversation as resolved.
Show resolved Hide resolved
part_install_dir = lifecycle._executor._part_list[0].part_install_dir

# Real quick instantiate another copy of the plugin to ensure statelessness.
properties2 = PythonPlugin.properties_class.unmarshal(parts["parts"]["foo"])
part_info = PartInfo(
project_info=ProjectInfo(
application_name="test", cache_dir=new_dir, partitions=partitions
),
part=Part("foo", {"source": "."}, partitions=partitions),
)
plugin2 = PythonPlugin(properties=properties2, part_info=part_info)
assert plugin2.get_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")]

# Can't assert specific versions here because flask has >= versions for its
# dependencies.
seeking_pkgs = {
pkgname: False
for pkgname in [
"Jinja2",
"Werkzeug",
"blinker",
"click",
"itsdangerous",
]
}
for found_pkg in actual_file_list:
# Make sure we got some contents.
# Wheels have at least a METADATA, a RECORD, and we can assume at least
# one actual source file.
if found_pkg.name in seeking_pkgs:
assert len(actual_file_list[found_pkg]) >= 3

if found_pkg.name in seeking_pkgs:
seeking_pkgs[found_pkg.name] = True
assert all(
seeking_pkgs.values()
), f"Didn't find one or more expected packages:\n{seeking_pkgs}"
37 changes: 37 additions & 0 deletions tests/unit/plugins/test_python_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@
# You should have received a copy of the GNU Lesser General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import shutil
from pathlib import Path
from textwrap import dedent

import pytest
from craft_parts import Part, PartInfo, ProjectInfo
from craft_parts.plugins.base import Package
from craft_parts.plugins.python_plugin import PythonPlugin
from pydantic import ValidationError

Expand Down Expand Up @@ -193,3 +195,38 @@ def test_call_should_remove_symlinks(plugin, new_dir, mocker):
f"[ -f setup.py ] || [ -f pyproject.toml ] && {new_dir}/parts/p1/install/bin/pip install -U .",
*get_build_commands(new_dir, should_remove_symlinks=True),
]


def test_get_files(new_dir):
part_info = PartInfo(
project_info=ProjectInfo(application_name="test", cache_dir=new_dir),
part=Part("my-part", {}),
)
properties = PythonPlugin.properties_class.unmarshal({"source": "."})
plugin = PythonPlugin(properties=properties, part_info=part_info)

root = plugin._part_info.part_install_dir
bins_dir = root / "bin"
pkgs_install_dir = root / "lib/python/site-packages"

# Copy in a fake file tree that emulates a real package installs.
# (Integration tests actually install stuff and check a subset of the
# large installed trees.)
shutil.copytree(Path(__file__).parent / "testfiles/python/install", root)

expected = {
Package("fakeee", "1.2.3-deb_ian"): {
bins_dir / "doit",
pkgs_install_dir / "fakeee/a_file.py",
pkgs_install_dir / "fakeee/things/stuff.py",
pkgs_install_dir / "fakeee/things/nothing.py",
pkgs_install_dir / "fakeee-1.2.3-deb_ian.dist-info/LICENSE.txt",
pkgs_install_dir / "fakeee-1.2.3-deb_ian.dist-info/METADATA",
pkgs_install_dir / "fakeee-1.2.3-deb_ian.dist-info/RECORD",
pkgs_install_dir / "fakeee-1.2.3-deb_ian.dist-info/REQUESTED",
},
}

actual = plugin.get_files()

assert expected == actual
2 changes: 2 additions & 0 deletions tests/unit/plugins/testfiles/python/install/bin/doit
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#!/bin/sh
echo it is done
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This software is covered by the fake license. The terms of this license do not apply to the software.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Name: fakeee
Version: 1.2.3-deb_ian
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
../../../bin/doit,,
fakeee/a_file.py,,
fakeee/things/stuff.py,,
fakeee/things/nothing.py,,
fakeee-1.2.3-deb_ian.dist-info/LICENSE.txt,,
fakeee-1.2.3-deb_ian.dist-info/METADATA,,
fakeee-1.2.3-deb_ian.dist-info/RECORD,,
fakeee-1.2.3-deb_ian.dist-info/REQUESTED,,
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def func():
return True
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def lying_function():
return "lying_function was not executed"
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def dontdoit(a):
print(f"Not done: {a}")
Loading