Skip to content

Commit

Permalink
fix(charm_builder): force-install pip
Browse files Browse the repository at this point in the history
The previous version wasn't installing pip if it was already in the venv

Fixes #1456
CRAFT-2538
  • Loading branch information
lengau committed Sep 11, 2024
1 parent 2fa3602 commit 96bfdfd
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 7 deletions.
4 changes: 3 additions & 1 deletion charmcraft/charm_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,9 @@ def _install_dependencies(self, staging_venv_dir):
# common charm dependencies (e.g. ops). Resolve this by updating to a
# known working version of pip.
if get_pip_version(pip_cmd) < MINIMUM_PIP_VERSION:
_process_run([pip_cmd, "install", f"pip@{KNOWN_GOOD_PIP_URL}"])
_process_run(
[pip_cmd, "install", "--force-reinstall", f"pip@{KNOWN_GOOD_PIP_URL}"]
)

with instrum.Timer("Installing all dependencies"):
if self.strict_dependencies:
Expand Down
10 changes: 5 additions & 5 deletions tests/test_charm_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ def test_build_dependencies_virtualenv_simple(tmp_path, assert_output):

assert mock.mock_calls == [
call(["python3", "-m", "venv", str(tmp_path / STAGING_VENV_DIRNAME)]),
call([pip_cmd, "install", f"pip@{KNOWN_GOOD_PIP_URL}"]),
call([pip_cmd, "install", "--force-reinstall", f"pip@{KNOWN_GOOD_PIP_URL}"]),
call([pip_cmd, "install", "--no-binary=:all:", f"--requirement={reqs_file}"]),
]

Expand Down Expand Up @@ -652,7 +652,7 @@ def test_build_dependencies_virtualenv_multiple(tmp_path, assert_output):
pip_cmd = str(charm_builder._find_venv_bin(tmp_path / STAGING_VENV_DIRNAME, "pip"))
assert mock.mock_calls == [
call(["python3", "-m", "venv", str(tmp_path / STAGING_VENV_DIRNAME)]),
call([pip_cmd, "install", f"pip@{KNOWN_GOOD_PIP_URL}"]),
call([pip_cmd, "install", "--force-reinstall", f"pip@{KNOWN_GOOD_PIP_URL}"]),
call(
[
pip_cmd,
Expand Down Expand Up @@ -714,7 +714,7 @@ def test_build_dependencies_virtualenv_packages(tmp_path, assert_output):

assert mock.mock_calls == [
call(["python3", "-m", "venv", str(tmp_path / STAGING_VENV_DIRNAME)]),
call([pip_cmd, "install", f"pip@{KNOWN_GOOD_PIP_URL}"]),
call([pip_cmd, "install", "--force-reinstall", f"pip@{KNOWN_GOOD_PIP_URL}"]),
call([pip_cmd, "install", "--no-binary=:all:", "pkg1", "pkg2"]),
]

Expand Down Expand Up @@ -747,7 +747,7 @@ def test_build_dependencies_virtualenv_binary_packages(tmp_path, assert_output):

assert mock.mock_calls == [
call(["python3", "-m", "venv", str(tmp_path / STAGING_VENV_DIRNAME)]),
call([pip_cmd, "install", f"pip@{KNOWN_GOOD_PIP_URL}"]),
call([pip_cmd, "install", "--force-reinstall", f"pip@{KNOWN_GOOD_PIP_URL}"]),
call([pip_cmd, "install", "pkg1", "pkg2"]),
]

Expand Down Expand Up @@ -786,7 +786,7 @@ def test_build_dependencies_virtualenv_all(tmp_path, assert_output):

assert mock.mock_calls == [
call(["python3", "-m", "venv", str(tmp_path / STAGING_VENV_DIRNAME)]),
call([pip_cmd, "install", f"pip@{KNOWN_GOOD_PIP_URL}"]),
call([pip_cmd, "install", "--force-reinstall", f"pip@{KNOWN_GOOD_PIP_URL}"]),
call(
[
pip_cmd,
Expand Down
5 changes: 4 additions & 1 deletion tests/unit/utils/test_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,10 @@ def test_get_pip_command(

@pytest.mark.parametrize(
("pip_cmd", "stdout", "expected"),
[("pip", "pip 22.0.2 from /usr/lib/python3/dist-packages/pip (python 3.10)\n", (22, 0, 2))],
[
("pip", "pip 22.0.2 from /usr/lib/python3/dist-packages/pip (python 3.10)\n", (22, 0, 2)),
("venv/bin/pip", "pip 20.0.2 from /root/venv/lib/python3.8/site-packages/pip (python 3.8)", (20, 0, 2)),
],
)
def test_get_pip_version_success(
fake_process,
Expand Down

0 comments on commit 96bfdfd

Please sign in to comment.