Skip to content

Commit

Permalink
chore: merge 3.2.1 hotfix to main (#1943)
Browse files Browse the repository at this point in the history
  • Loading branch information
lengau authored Oct 9, 2024
2 parents f9d17b0 + e0003db commit d6f9d13
Show file tree
Hide file tree
Showing 14 changed files with 470 additions and 96 deletions.
1 change: 1 addition & 0 deletions .github/workflows/spread.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ jobs:
name: Run spread
env:
CHARMCRAFT_AUTH: ${{ secrets.CHARMCRAFT_AUTH }}
CHARMCRAFT_SINGLE_CHARM_AUTH: ${{ secrets.CHARMCRAFT_SINGLE_CHARM_AUTH }}
CHARM_DEFAULT_NAME: gh-ci-charmcraft-charm
BUNDLE_DEFAULT_NAME: gh-ci-charmcraft-bundle
run: |
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ jobs:
if [[ $(lsb_release --codename --short) == 'jammy' ]]; then
python3 -m pip install -U pip
fi
- name: Setup LXD
uses: canonical/[email protected]
if: ${{ runner.os == 'Linux' }}
- name: Install skopeo (mac)
# This is only necessary for Linux until skopeo >= 1.11 is in repos.
# Once we're running on Noble, we can get skopeo from apt.
Expand Down
20 changes: 10 additions & 10 deletions charmcraft/application/commands/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ class WhoamiCommand(CharmcraftCommand):
)
format_option = True

def run(self, parsed_args):
def run(self, parsed_args: argparse.Namespace) -> None:
"""Run the command."""
try:
macaroon_info = self._services.store.client.whoami()
Expand All @@ -259,7 +259,7 @@ def run(self, parsed_args):
return

human_msgs = []
prog_info = {"logged": True}
prog_info: dict[str, Any] = {"logged": True}

human_msgs.append(f"name: {macaroon_info['account']['display-name']}")
prog_info["name"] = macaroon_info["account"]["display-name"]
Expand All @@ -275,20 +275,20 @@ def run(self, parsed_args):
prog_info["permissions"] = permissions

if packages := macaroon_info.get("packages"):
grouped = {}
grouped: dict[str, list[dict[str, str]]] = {}
for package in packages:
grouped.setdefault(package.type, []).append(package)
grouped.setdefault(package["type"], []).append(package)
for package_type, title in [("charm", "charms"), ("bundle", "bundles")]:
if package_type in grouped:
human_msgs.append(f"{title}:")
pkg_info = []
for item in grouped[package_type]:
if item.name is not None:
human_msgs.append(f"- name: {item.name}")
pkg_info.append({"name": item.name})
elif item.id is not None:
human_msgs.append(f"- id: {item.id}")
pkg_info.append({"id": item.id})
if (name := item.get("name")) is not None:
human_msgs.append(f"- name: {name}")
pkg_info.append({"name": name})
elif (pkg_id := item.get("id")) is not None:
human_msgs.append(f"- id: {pkg_id}")
pkg_info.append({"id": pkg_id})
prog_info[title] = pkg_info

if channels := macaroon_info.get("channels"):
Expand Down
7 changes: 6 additions & 1 deletion charmcraft/charm_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,12 @@ def _install_dependencies(self, staging_venv_dir: pathlib.Path):
# known working version of pip.
if get_pip_version(pip_cmd) < MINIMUM_PIP_VERSION:
_process_run(
[pip_cmd, "install", "--force-reinstall", f"pip@{KNOWN_GOOD_PIP_URL}"]
[
pip_cmd,
"install",
"--force-reinstall",
f"pip@{KNOWN_GOOD_PIP_URL}",
]
)

with instrum.Timer("Installing all dependencies"):
Expand Down
91 changes: 88 additions & 3 deletions charmcraft/services/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,55 @@
"""Service class for creating providers."""
from __future__ import annotations

import contextlib
import io
from collections.abc import Generator

from craft_application.models import BuildInfo

try:
import fcntl
except ModuleNotFoundError: # Not available on Windows.
fcntl = None # type: ignore[assignment]
import os
import pathlib
from typing import cast

import craft_application
import craft_providers
from craft_application import services
from craft_cli import emit
from craft_providers import bases

from charmcraft import env
from charmcraft import env, models


class ProviderService(services.ProviderService):
"""Business logic for getting providers."""

def __init__(
self,
app: craft_application.AppMetadata,
services: craft_application.ServiceFactory,
*,
project: models.CharmcraftProject,
work_dir: pathlib.Path,
build_plan: list[BuildInfo],
provider_name: str | None = None,
install_snap: bool = True,
) -> None:
super().__init__(
app,
services,
project=project,
work_dir=work_dir,
build_plan=build_plan,
provider_name=provider_name,
install_snap=install_snap,
)
self._cache_path: pathlib.Path | None = None
self._lock: io.TextIOBase | None = None

def setup(self) -> None:
"""Set up the provider service for Charmcraft."""
super().setup()
Expand All @@ -56,12 +92,61 @@ def get_base(
If no cache_path is included, adds one.
"""
self._cache_path = cast(
pathlib.Path, kwargs.get("cache_path", env.get_host_shared_cache_path())
)
self._lock = _maybe_lock_cache(self._cache_path)

# Forward the shared cache path.
if "cache_path" not in kwargs:
kwargs["cache_path"] = env.get_host_shared_cache_path()
kwargs["cache_path"] = self._cache_path if self._lock else None
return super().get_base(
base_name,
instance_name=instance_name,
# craft-application annotation is incorrect
**kwargs, # type: ignore[arg-type]
)

@contextlib.contextmanager
def instance(
self,
build_info: BuildInfo,
*,
work_dir: pathlib.Path,
allow_unstable: bool = True,
**kwargs: bool | str | None,
) -> Generator[craft_providers.Executor, None, None]:
"""Instance override for Charmcraft."""
with super().instance(
build_info, work_dir=work_dir, allow_unstable=allow_unstable, **kwargs
) as instance:
try:
yield instance
finally:
if fcntl is not None and self._lock:
fcntl.flock(self._lock, fcntl.LOCK_UN)
self._lock.close()


def _maybe_lock_cache(path: pathlib.Path) -> io.TextIOBase | None:
"""Lock the cache so we only have one copy of Charmcraft using it at a time."""
if fcntl is None: # Don't lock on Windows - just don't cache.
return None
cache_lock_path = path / "charmcraft.lock"

emit.trace("Attempting to lock the cache path")
lock_file = cache_lock_path.open("w+")
try:
# Exclusive lock, but non-blocking.
fcntl.flock(lock_file, fcntl.LOCK_EX | fcntl.LOCK_NB)
except OSError:
emit.progress(
"Shared cache locked by another process; running without cache.", permanent=True
)
return None
else:
pid = str(os.getpid())
lock_file.write(pid)
lock_file.flush()
os.fsync(lock_file.fileno())
emit.trace(f"Cache path locked by this process ({pid})")
return lock_file
30 changes: 25 additions & 5 deletions docs/reference/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,28 @@ Command line
The pack command now updates charm the libs in the project directory if they don't meet
the requirements in the ``charm-libs`` key of ``charmcraft.yaml``.

3.2.1 (2024-09-16)
------------------

This is a bugfix release for 3.2, bringing in two fixes:

Core
====

The shared cache directory now gets locked. Builds that run while another copy of
Charmcraft has the cache directory locked will run without a shared cache.

Plugins
#######

charm
"""""

The charm plugin will now force-install pip if the installed venv version is older
than the minimum version, guaranteeing that pip gets updated correctly.

For a complete list of commits, see the `3.2.1`_ release on GitHub.

2.7.4 (2024-10-07)
------------------

Expand All @@ -99,11 +121,8 @@ For a complete list of commits, see the `2.7.4`_ release on GitHub.
Core
====

Plugins
#######

charm
"""""
The shared cache directory now gets locked. Builds that run while another copy of
Charmcraft has the cache directory locked will run without a shared cache.

The charm plugin now force-reinstalls pip when necessary, guaranteeing a correct
version of pip.
Expand Down Expand Up @@ -348,3 +367,4 @@ page.
.. _3.1.1: https://github.com/canonical/charmcraft/releases/tag/3.1.1
.. _3.1.2: https://github.com/canonical/charmcraft/releases/tag/3.1.2
.. _3.2.0: https://github.com/canonical/charmcraft/releases/tag/3.2.0
.. _3.2.1: https://github.com/canonical/charmcraft/releases/tag/3.2.1
8 changes: 3 additions & 5 deletions spread.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,9 @@ suites:
# https://github.com/canonical/lxd-cloud/blob/f20a64a8af42485440dcbfd370faf14137d2f349/test/includes/lxd.sh#L13-L23
iptables -P FORWARD ACCEPT
# Ensure that the reused charms and bundles are registered if necessary.
# Ensure that the reused charms are registered if necessary.
if ! charmcraft status "${CHARM_DEFAULT_NAME}"; then
charmcraft register $CHARM_DEFAULT_NAME || ERROR Charm $CHARM_DEFAULT_NAME cannot be registered to this account.
fi
if ! charmcraft status $BUNDLE_DEFAULT_NAME; then
charmcraft register-bundle $BUNDLE_DEFAULT_NAME || ERROR Charm $BUNDLE_DEFAULT_NAME cannot be registered to this account.
charmcraft register $CHARM_DEFAULT_NAME
fi
rm -f charmcraft.yaml
Expand All @@ -226,6 +223,7 @@ suites:
# should be part of the environment (when running spread locally just define it,
# for GH actions set it in Settings -> Security -> Actions -> Repository secrets)
CHARMCRAFT_AUTH: "$(HOST: echo $CHARMCRAFT_AUTH)"
CHARMCRAFT_SINGLE_CHARM_AUTH: "$(HOST: echo $CHARMCRAFT_SINGLE_CHARM_AUTH)"

# to not flood Charmhub with names the same two are always used in the Store related
# tests (except in the names registration tests, of course); register them manually
Expand Down
21 changes: 12 additions & 9 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,19 @@ def service_factory(


@pytest.fixture
def default_build_plan():
def default_build_info() -> models.BuildInfo:
arch = util.get_host_architecture()
return [
models.BuildInfo(
base=bases.BaseName("ubuntu", "22.04"),
build_on=arch,
build_for="arm64",
platform="distro-1-test64",
)
]
return models.BuildInfo(
base=bases.BaseName("ubuntu", "22.04"),
build_on=arch,
build_for="arm64",
platform="distro-1-test64",
)


@pytest.fixture
def default_build_plan(default_build_info: models.BuildInfo):
return [default_build_info]


@pytest.fixture
Expand Down
17 changes: 2 additions & 15 deletions tests/integration/services/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,31 +14,18 @@
#
# For further info, check https://github.com/canonical/charmcraft
"""Configuration for services integration tests."""
import contextlib
import sys

import pyfakefs.fake_filesystem
import pytest

from charmcraft import services
from charmcraft.application.main import APP_METADATA, Charmcraft


@pytest.fixture
def service_factory(
fs: pyfakefs.fake_filesystem.FakeFilesystem, fake_path, simple_charm
) -> services.CharmcraftServiceFactory:
fake_project_dir = fake_path / "project"
def service_factory(simple_charm, new_path) -> services.CharmcraftServiceFactory:
fake_project_dir = new_path / "project"
fake_project_dir.mkdir()

# Allow access to the real venv library path.
# This is necessary because certifi lazy-loads the certificate file.
for python_path in sys.path:
if not python_path:
continue
with contextlib.suppress(OSError):
fs.add_real_directory(python_path)

factory = services.CharmcraftServiceFactory(app=APP_METADATA)

app = Charmcraft(app=APP_METADATA, services=factory)
Expand Down
Loading

0 comments on commit d6f9d13

Please sign in to comment.