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.

Sign up for GitHub

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

Copy full virtual environment to packed charm #1105

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

Copy full virtual environment to packed charm #1105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
adbd2cd
Copy full virtual environment to packed charm
carlcsaposs-canonical May 18, 2023
90e3d19
Update tests
carlcsaposs-canonical May 18, 2023
46a379c
format
carlcsaposs-canonical May 18, 2023
25d18cb
Remove existing symlink if present
carlcsaposs-canonical May 18, 2023
c98c7a6
Fix smoketests
carlcsaposs-canonical May 19, 2023
00b02e0
Add symlink to PRIME
carlcsaposs-canonical May 19, 2023
596751f
Fix test
carlcsaposs-canonical May 19, 2023
fb6189f
Use relative path instead of absolute path for symlink
carlcsaposs-canonical May 19, 2023
8b3df60
Remove unused variable
carlcsaposs-canonical May 19, 2023
76976c2
Use relative basedir path to fix unit tests
carlcsaposs-canonical May 19, 2023
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
14 changes: 9 additions & 5 deletions charmcraft/charm_builder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
# Some constants that are used through the code.
WORK_DIRNAME = "work_dir"
VENV_DIRNAME = "venv"
SITE_PACKAGES_SYMLINK_NAME = "site-packages"
STAGING_VENV_DIRNAME = "staging-venv"
DEPENDENCIES_HASH_FILENAME = "charmcraft-dependencies-hash.txt"

Expand All @@ -50,7 +51,7 @@
# to be the value it would've otherwise been.
DISPATCH_CONTENT = """#!/bin/sh

JUJU_DISPATCH_PATH="${{JUJU_DISPATCH_PATH:-$0}}" PYTHONPATH=lib:venv \\
JUJU_DISPATCH_PATH="${{JUJU_DISPATCH_PATH:-$0}}" PYTHONPATH=lib:site-packages \\
exec ./{entrypoint_relative_path}
"""

Expand Down Expand Up @@ -319,10 +320,13 @@ def handle_dependencies(self):
# save the hash file after all successful installations
hash_file.write_text(current_deps_hash, encoding="utf8")

# always copy the virtualvenv site-packages directory to /venv in charm
basedir = pathlib.Path(STAGING_VENV_DIRNAME)
site_packages_dir = _find_venv_site_packages(basedir)
shutil.copytree(site_packages_dir, self.installdir / VENV_DIRNAME)
# copy the virtual environment directory to /venv in charm
shutil.copytree(pathlib.Path(STAGING_VENV_DIRNAME), self.installdir / VENV_DIRNAME)
# symlink /site-packages directory in charm to virtual environment site-packages directory
# (backwards compatability for charms that do not use the full virtual environment)
site_packages_symlink_path = self.installdir / SITE_PACKAGES_SYMLINK_NAME
site_packages_symlink_path.unlink(missing_ok=True)
site_packages_symlink_path.symlink_to(_find_venv_site_packages(pathlib.Path(VENV_DIRNAME)))


def _find_venv_bin(basedir, exec_base):
Expand Down
2 changes: 2 additions & 0 deletions charmcraft/commands/build.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
# Some constants that are used through the code.
BUILD_DIRNAME = "build"
VENV_DIRNAME = "venv"
SITE_PACKAGES_SYMLINK_NAME = "site-packages"

CHARM_FILES = [
"metadata.yaml",
Expand Down Expand Up @@ -233,6 +234,7 @@ def _set_prime_filter(self):
or charmlib_pydeps
):
charm_part_prime.append(VENV_DIRNAME)
charm_part_prime.append(SITE_PACKAGES_SYMLINK_NAME)

# add mandatory and optional charm files
charm_part_prime.extend(CHARM_FILES)
Expand Down
2 changes: 2 additions & 0 deletions tests/commands/test_build.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1257,6 +1257,7 @@ def test_build_part_from_config(basic_project, monkeypatch):
"prime": [
"src",
"venv",
"site-packages",
"metadata.yaml",
"dispatch",
"hooks",
Expand Down Expand Up @@ -1332,6 +1333,7 @@ def test_build_part_include_venv_pydeps(basic_project, monkeypatch):
"prime": [
"src",
"venv",
"site-packages",
"metadata.yaml",
"dispatch",
"hooks",
Expand Down
2 changes: 1 addition & 1 deletion tests/spread/smoketests/basic/task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ execute: |
cd charm
charmcraft pack --verbose
test -f charm*.charm
unzip -l charm*.charm | grep "venv/ops/charm.py"
unzip -l charm*.charm | grep "site-packages/ops/charm.py"
test ! -d build
6 changes: 3 additions & 3 deletions tests/spread/smoketests/different-dependencies/task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ execute: |
cd charm
charmcraft pack --verbose
test -f charm*.charm
unzip -l charm*.charm | grep "venv/pytest/__init__.py"
unzip -l charm*.charm | grep "venv/bumpversion/__init__.py"
unzip -l charm*.charm | grep "venv/fades/__init__.py"
unzip -l charm*.charm | grep "site-packages/pytest/__init__.py"
unzip -l charm*.charm | grep "site-packages/bumpversion/__init__.py"
unzip -l charm*.charm | grep "site-packages/fades/__init__.py"
2 changes: 1 addition & 1 deletion tests/spread/smoketests/full-lifecycle/task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,6 @@ execute: |
charmcraft pack --verbose
test -f charm*.charm
unzip -c charm*.charm hello.txt | grep "^Hello, world!"
unzip -l charm*.charm | grep "venv/ops/charm.py"
unzip -l charm*.charm | grep "site-packages/ops/charm.py"
unzip -l charm*.charm | grep extra_file.txt
! unzip -l charm*.charm | grep secrets.txt
75 changes: 45 additions & 30 deletions tests/test_charm_builder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,8 +619,9 @@ def test_build_dependencies_virtualenv_simple(tmp_path, assert_output):
),
]

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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]
assert_output("Handling dependencies", "Installing dependencies")


Expand Down Expand Up @@ -664,8 +665,9 @@ def test_build_dependencies_virtualenv_multiple(tmp_path, assert_output):
),
]

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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]
assert_output("Handling dependencies", "Installing dependencies")


Expand Down Expand Up @@ -716,8 +718,9 @@ def test_build_dependencies_virtualenv_packages(tmp_path, assert_output):
call([pip_cmd, "install", "--upgrade", "--no-binary", ":all:", "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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]
assert_output("Handling dependencies", "Installing dependencies")


Expand Down Expand Up @@ -747,8 +750,9 @@ def test_build_dependencies_virtualenv_binary_packages(tmp_path, assert_output):
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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]
assert_output("Handling dependencies", "Installing dependencies")


Expand Down Expand Up @@ -797,8 +801,9 @@ def test_build_dependencies_virtualenv_all(tmp_path, assert_output):
call([pip_cmd, "install", "--upgrade", "--no-binary", ":all:", "pkg5", "pkg6"]),
]

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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]
assert_output("Handling dependencies", "Installing dependencies")


Expand Down Expand Up @@ -832,8 +837,9 @@ def test_build_dependencies_no_reused_missing_venv(tmp_path, assert_output):
assert staging_venv_dir.exists()

# installation directory copied to the build directory
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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]

# remove the site venv directory
staging_venv_dir.rmdir()
Expand All @@ -849,8 +855,9 @@ def test_build_dependencies_no_reused_missing_venv(tmp_path, assert_output):
assert staging_venv_dir.exists()

# installation directory copied *again* to the build directory
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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]


def test_build_dependencies_no_reused_missing_hash_file(tmp_path, assert_output):
Expand Down Expand Up @@ -883,8 +890,9 @@ def test_build_dependencies_no_reused_missing_hash_file(tmp_path, assert_output)
assert staging_venv_dir.exists()

# installation directory copied to the build directory
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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]

# remove the hash file
(tmp_path / DEPENDENCIES_HASH_FILENAME).unlink()
Expand All @@ -900,8 +908,9 @@ def test_build_dependencies_no_reused_missing_hash_file(tmp_path, assert_output)
assert staging_venv_dir.exists()

# installation directory copied *again* to the build directory
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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]


def test_build_dependencies_no_reused_problematic_hash_file(tmp_path, assert_output):
Expand Down Expand Up @@ -934,8 +943,9 @@ def test_build_dependencies_no_reused_problematic_hash_file(tmp_path, assert_out
assert staging_venv_dir.exists()

# installation directory copied to the build directory
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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]

# avoid the file to be read succesfully
(tmp_path / DEPENDENCIES_HASH_FILENAME).write_bytes(b"\xc3\x28") # invalid UTF8
Expand All @@ -954,8 +964,9 @@ def test_build_dependencies_no_reused_problematic_hash_file(tmp_path, assert_out
assert staging_venv_dir.exists()

# installation directory copied *again* to the build directory
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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]


@pytest.mark.parametrize(
Expand Down Expand Up @@ -1008,8 +1019,9 @@ def test_build_dependencies_no_reused_different_dependencies(
assert staging_venv_dir.exists()

# installation directory copied to the build directory
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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]

# for the second call, default new dependencies to first ones so only one is changed at a time
if new_reqs_content is not None:
Expand All @@ -1033,8 +1045,9 @@ def test_build_dependencies_no_reused_different_dependencies(
assert staging_venv_dir.exists()

# installation directory copied *again* to the build directory
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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]


def test_build_dependencies_reused(tmp_path, assert_output):
Expand Down Expand Up @@ -1071,8 +1084,9 @@ def test_build_dependencies_reused(tmp_path, assert_output):
assert staging_venv_dir.exists()

# installation directory copied to the build directory
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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]

# second run!
with patch("shutil.copytree") as mock_copytree:
Expand All @@ -1083,8 +1097,9 @@ def test_build_dependencies_reused(tmp_path, assert_output):

# installation directory copied *again* to the build directory (this is always done as
# buildpath is cleaned)
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)]
assert mock_copytree.mock_calls == [
call(pathlib.Path(STAGING_VENV_DIRNAME), build_dir / VENV_DIRNAME)
]


# -- tests about juju ignore
Expand Down