Skip to content

Commit

Permalink
fix(charm_builder): force-install pip (#1891)
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.

This is the hotfix for 2.x, while
#1892 is the hotfix for 3.x

Fixes #1456
CRAFT-2538
  • Loading branch information
lengau authored Sep 12, 2024
1 parent 2fa3602 commit df6a92c
Show file tree
Hide file tree
Showing 3 changed files with 16 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
9 changes: 8 additions & 1 deletion tests/unit/utils/test_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,14 @@ 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 df6a92c

Please sign in to comment.