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

charm_builder: install binary python packages (CRAFT-601) #588

Merged
merged 6 commits into from
Nov 9, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
13 changes: 13 additions & 0 deletions .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,19 @@ jobs:
unzip -l python-packages-test_*.charm | grep "venv/bumpversion/__init__.py"
popd

mkdir -p binary-python-packages-test
pushd binary-python-packages-test
charmcraft -v init --author testuser
sed -i "s|20.04|$VERSION_ID|g" charmcraft.yaml
cat <<- EOF >> charmcraft.yaml
parts:
charm:
charm-binary-python-packages: [bump2version]
EOF
sg lxd -c "charmcraft -v pack"
unzip -l binary-python-packages-test_*.charm | grep "venv/bumpversion/__init__.py"
popd

sudo snap set charmcraft provider=lxd
sudo snap set charmcraft provider=multipass
if sudo snap set charmcraft provider=invalid; then
Expand Down
22 changes: 20 additions & 2 deletions charmcraft/charm_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,15 @@ def __init__(
builddir: pathlib.Path,
entrypoint: pathlib.Path,
allow_pip_binary: bool = None,
binary_python_packages: List[str] = None,
python_packages: List[str] = None,
requirements: List[str] = None,
):
self.charmdir = charmdir
self.buildpath = builddir
self.entrypoint = entrypoint
self.allow_pip_binary = allow_pip_binary
self.binary_python_packages = binary_python_packages
self.python_packages = python_packages
self.requirement_paths = requirements
self.ignore_rules = self._load_juju_ignore()
Expand Down Expand Up @@ -221,16 +223,23 @@ def handle_dependencies(self):
"""Handle from-directory and virtualenv dependencies."""
emit.progress("Installing dependencies")

if self.requirement_paths or self.python_packages:
if self.requirement_paths or self.binary_python_packages or self.python_packages:
# create virtualenv using the host environment python
staging_venv_dir = self.charmdir / STAGING_VENV_DIRNAME
_process_run(["python3", "-m", "venv", str(staging_venv_dir)])
pip_cmd = str(_find_venv_bin(staging_venv_dir, "pip3"))

_process_run([pip_cmd, "--version"])

if self.binary_python_packages:
# install python packages, allowing binary packages
cmd = [pip_cmd, "install", "--upgrade"] # base command
for pkg in self.binary_python_packages:
cmd.append(pkg) # the python package to install
_process_run(cmd)

if self.python_packages:
# install python packages
# install python packages from source
cmd = [pip_cmd, "install", "--upgrade", "--no-binary", ":all:"] # base command
for pkg in self.python_packages:
cmd.append(pkg) # the python package to install
Expand Down Expand Up @@ -315,6 +324,14 @@ def _parse_arguments() -> argparse.Namespace:
required=True,
help="The build destination directory",
)
parser.add_argument(
"-b",
"--binary-package",
metavar="pkg",
action="append",
default=None,
help="Binary Python package to install before requirements.",
)
parser.add_argument(
"-p",
"--package",
Expand Down Expand Up @@ -346,6 +363,7 @@ def main():
charmdir=pathlib.Path(options.charmdir),
builddir=pathlib.Path(options.builddir),
entrypoint=pathlib.Path(options.entrypoint),
binary_python_packages=options.binary_package,
python_packages=options.package,
requirements=options.requirement,
)
Expand Down
14 changes: 13 additions & 1 deletion charmcraft/parts.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class CharmPluginProperties(plugins.PluginProperties, plugins.PluginModel):

source: str = ""
charm_entrypoint: str = "" # TODO: add default after removing --entrypoint
charm_binary_python_packages: List[str] = []
charm_python_packages: List[str] = []
charm_requirements: List[str] = []

Expand Down Expand Up @@ -67,9 +68,17 @@ class CharmPlugin(plugins.Plugin):
(string)
The path to the main charm executable, relative to the charm root.

- ``charm-binary-python-packages``
(list of strings)
A list of python packages to install from PyPI before installing
requirements. Binary packages are allowed, but they may also be
installed from sources if a package is only available in source form.

- ``charm-python-packages``
(list of strings)
A list of python packages to install from PyPI before installing requirements.
A list of python packages to install from PyPI before installing
requirements. These packages will be installed from sources and built
locally at packing time.

- ``charm-requirements``
(list of strings)
Expand Down Expand Up @@ -138,6 +147,9 @@ def get_build_commands(self) -> List[str]:
entrypoint = self._part_info.part_build_dir / options.charm_entrypoint
build_cmd.extend(["--entrypoint", str(entrypoint)])

for pkg in options.charm_binary_python_packages:
build_cmd.extend(["-b", pkg])

for pkg in options.charm_python_packages:
build_cmd.extend(["-p", pkg])

Expand Down
4 changes: 2 additions & 2 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ python-dateutil==2.8.2
pytz==2021.3
pyxdg==0.27
PyYAML==6.0
regex==2021.10.23
regex==2021.11.2
requests==2.26.0
requests-toolbelt==0.9.1
requests-unixsocket==0.2.0
Expand All @@ -61,4 +61,4 @@ tomli==1.2.2
typing-extensions==3.10.0.2
urllib3==1.26.7
zipp==3.6.0
git+git://github.com/canonical/craft-cli.git@4af19f9c0da733321dc754be1180aea28f3feeb1
git+https://github.com/canonical/craft-cli.git@4af19f9c0da733321dc754be1180aea28f3feeb1
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

@cmatsuoka cmatsuoka Nov 3, 2021

Choose a reason for hiding this comment

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

Github is deprecating access using the unencrypted git protocol (see https://github.blog/2021-09-01-improving-git-protocol-security-github/). This is not effective yet but there was a brownout on Nov 02 when this PR was submitted. Also note that all entries using non-published packages will be removed before the next release.

2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,4 @@ tabulate==0.8.9
typing-extensions==3.10.0.2
urllib3==1.26.7
zipp==3.6.0
git+git://github.com/canonical/craft-cli.git@4af19f9c0da733321dc754be1180aea28f3feeb1
git+https://github.com/canonical/craft-cli.git@4af19f9c0da733321dc754be1180aea28f3feeb1
1 change: 1 addition & 0 deletions snap/snapcraft.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ parts:
plugin: nil
build-packages:
- python3-dev
- git
Copy link
Contributor

Choose a reason for hiding this comment

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

What this is needed for?

Copy link
Contributor Author

@cmatsuoka cmatsuoka Nov 3, 2021

Choose a reason for hiding this comment

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

This was needed to be able to install packages using git+https and can be removed when those lines are removed. I will revert these changes since this brownout period ended (if we leave this to be changed later we'll forget to remove the git entry from snapcraft.yaml).

stage-packages:
- libpython3-stdlib
- libpython3.8-minimal
Expand Down
44 changes: 41 additions & 3 deletions tests/test_charm_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,7 @@ def test_build_dependencies_virtualenv_simple(tmp_path):
charmdir=tmp_path,
builddir=build_dir,
entrypoint=pathlib.Path("whatever"),
binary_python_packages=[],
python_packages=[],
requirements=["reqs.txt"],
)
Expand Down Expand Up @@ -620,6 +621,7 @@ def test_build_dependencies_virtualenv_multiple(tmp_path):
charmdir=tmp_path,
builddir=build_dir,
entrypoint=pathlib.Path("whatever"),
binary_python_packages=[],
python_packages=[],
requirements=["reqs1.txt", "reqs2.txt"],
)
Expand Down Expand Up @@ -660,6 +662,7 @@ def test_build_dependencies_virtualenv_none(tmp_path):
charmdir=tmp_path,
builddir=build_dir,
entrypoint=pathlib.Path("whatever"),
binary_python_packages=[],
python_packages=[],
requirements=[],
)
Expand All @@ -681,6 +684,7 @@ def test_build_dependencies_virtualenv_packages(tmp_path):
charmdir=tmp_path,
builddir=build_dir,
entrypoint=pathlib.Path("whatever"),
binary_python_packages=[],
python_packages=["pkg1", "pkg2"],
requirements=[],
)
Expand All @@ -701,7 +705,7 @@ def test_build_dependencies_virtualenv_packages(tmp_path):
assert mock_copytree.mock_calls == [call(site_packages_dir, build_dir / VENV_DIRNAME)]


def test_build_dependencies_virtualenv_both(tmp_path):
def test_build_dependencies_virtualenv_binary_packages(tmp_path):
"""A virtualenv is created with the specified packages."""
metadata = tmp_path / CHARM_METADATA
metadata.write_text("name: crazycharm")
Expand All @@ -712,7 +716,40 @@ def test_build_dependencies_virtualenv_both(tmp_path):
charmdir=tmp_path,
builddir=build_dir,
entrypoint=pathlib.Path("whatever"),
python_packages=["pkg1", "pkg2"],
binary_python_packages=["pkg1", "pkg2"],
python_packages=[],
requirements=[],
)

with patch("charmcraft.charm_builder._process_run") as mock:
with patch("shutil.copytree") as mock_copytree:
builder.handle_dependencies()

pip_cmd = str(charm_builder._find_venv_bin(tmp_path / STAGING_VENV_DIRNAME, "pip3"))

assert mock.mock_calls == [
call(["python3", "-m", "venv", str(tmp_path / STAGING_VENV_DIRNAME)]),
call([pip_cmd, "--version"]),
call([pip_cmd, "install", "--upgrade", "pkg1", "pkg2"]),
]

site_packages_dir = charm_builder._find_venv_site_packages(pathlib.Path(STAGING_VENV_DIRNAME))
assert mock_copytree.mock_calls == [call(site_packages_dir, build_dir / VENV_DIRNAME)]


def test_build_dependencies_virtualenv_all(tmp_path):
"""A virtualenv is created with the specified packages."""
metadata = tmp_path / CHARM_METADATA
metadata.write_text("name: crazycharm")
build_dir = tmp_path / BUILD_DIRNAME
build_dir.mkdir()

builder = CharmBuilder(
charmdir=tmp_path,
builddir=build_dir,
entrypoint=pathlib.Path("whatever"),
binary_python_packages=["pkg1", "pkg2"],
python_packages=["pkg3", "pkg4"],
requirements=["reqs1.txt", "reqs2.txt"],
)

Expand All @@ -725,7 +762,8 @@ def test_build_dependencies_virtualenv_both(tmp_path):
assert mock.mock_calls == [
call(["python3", "-m", "venv", str(tmp_path / STAGING_VENV_DIRNAME)]),
call([pip_cmd, "--version"]),
call([pip_cmd, "install", "--upgrade", "--no-binary", ":all:", "pkg1", "pkg2"]),
call([pip_cmd, "install", "--upgrade", "pkg1", "pkg2"]),
call([pip_cmd, "install", "--upgrade", "--no-binary", ":all:", "pkg3", "pkg4"]),
call(
[
pip_cmd,
Expand Down
9 changes: 6 additions & 3 deletions tests/test_parts.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ def setup_method_fixture(self, tmp_path):
spec = {
"plugin": "charm",
"charm-entrypoint": "entrypoint",
"charm-python-packages": ["pkg1", "pkg2"],
"charm-binary-python-packages": ["pkg1", "pkg2"],
"charm-python-packages": ["pkg3", "pkg4"],
"charm-requirements": ["reqs1.txt", "reqs2.txt"],
}
plugin_properties = parts.CharmPluginProperties.unmarshal(spec)
Expand Down Expand Up @@ -94,8 +95,10 @@ def test_get_build_commands(self, tmp_path, monkeypatch):
"--charmdir {work_dir}/parts/foo/build "
"--builddir {work_dir}/parts/foo/install "
"--entrypoint {work_dir}/parts/foo/build/entrypoint "
"-p pkg1 "
"-p pkg2 "
"-b pkg1 "
"-b pkg2 "
"-p pkg3 "
"-p pkg4 "
"-r reqs1.txt "
"-r reqs2.txt".format(
python=sys.executable,
Expand Down
4 changes: 2 additions & 2 deletions tools/freeze-requirements.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ popd

pip install -e .
pip freeze --exclude-editable > requirements.txt
echo "git+git://github.com/canonical/craft-cli.git@4af19f9c0da733321dc754be1180aea28f3feeb1" >> requirements.txt
echo "git+https://github.com/canonical/craft-cli.git@4af19f9c0da733321dc754be1180aea28f3feeb1" >> requirements.txt
requirements_fixups "requirements.txt"

pip install -e .[dev]
pip freeze --exclude-editable > requirements-dev.txt
echo "git+git://github.com/canonical/craft-cli.git@4af19f9c0da733321dc754be1180aea28f3feeb1" >> requirements-dev.txt
echo "git+https://github.com/canonical/craft-cli.git@4af19f9c0da733321dc754be1180aea28f3feeb1" >> requirements-dev.txt
requirements_fixups "requirements-dev.txt"

rm -rf "$venv_dir"