diff --git a/charmcraft/charm_builder.py b/charmcraft/charm_builder.py index 1508d8898..9022ab810 100644 --- a/charmcraft/charm_builder.py +++ b/charmcraft/charm_builder.py @@ -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" @@ -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} """ @@ -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): diff --git a/charmcraft/commands/build.py b/charmcraft/commands/build.py index 702aad727..ec172ff76 100644 --- a/charmcraft/commands/build.py +++ b/charmcraft/commands/build.py @@ -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", @@ -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) diff --git a/tests/commands/test_build.py b/tests/commands/test_build.py index 71921c1f2..b68d64607 100644 --- a/tests/commands/test_build.py +++ b/tests/commands/test_build.py @@ -1257,6 +1257,7 @@ def test_build_part_from_config(basic_project, monkeypatch): "prime": [ "src", "venv", + "site-packages", "metadata.yaml", "dispatch", "hooks", @@ -1332,6 +1333,7 @@ def test_build_part_include_venv_pydeps(basic_project, monkeypatch): "prime": [ "src", "venv", + "site-packages", "metadata.yaml", "dispatch", "hooks", diff --git a/tests/spread/smoketests/basic/task.yaml b/tests/spread/smoketests/basic/task.yaml index d713e4361..5d16d340d 100644 --- a/tests/spread/smoketests/basic/task.yaml +++ b/tests/spread/smoketests/basic/task.yaml @@ -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 diff --git a/tests/spread/smoketests/different-dependencies/task.yaml b/tests/spread/smoketests/different-dependencies/task.yaml index 78c99ec55..3295a32d6 100644 --- a/tests/spread/smoketests/different-dependencies/task.yaml +++ b/tests/spread/smoketests/different-dependencies/task.yaml @@ -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" diff --git a/tests/spread/smoketests/full-lifecycle/task.yaml b/tests/spread/smoketests/full-lifecycle/task.yaml index 246cc1559..f0b1ab010 100644 --- a/tests/spread/smoketests/full-lifecycle/task.yaml +++ b/tests/spread/smoketests/full-lifecycle/task.yaml @@ -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 diff --git a/tests/test_charm_builder.py b/tests/test_charm_builder.py index 2a89be62f..7d25597ac 100644 --- a/tests/test_charm_builder.py +++ b/tests/test_charm_builder.py @@ -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") @@ -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") @@ -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") @@ -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") @@ -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") @@ -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() @@ -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): @@ -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() @@ -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): @@ -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 @@ -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( @@ -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: @@ -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): @@ -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: @@ -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