From 787b0e4f4e4c9233616e97570a01462bbc3b244d Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Wed, 6 Nov 2024 15:13:17 -0500 Subject: [PATCH 01/15] feat: add parse_describe alongside UTs Signed-off-by: Dariusz Duda --- craft_application/git/__init__.py | 3 +- craft_application/git/_git_repo.py | 60 ++++++++++++++++++++++++++++++ tests/unit/git/test_git.py | 23 +++++++++++- 3 files changed, 84 insertions(+), 2 deletions(-) diff --git a/craft_application/git/__init__.py b/craft_application/git/__init__.py index 1d055527..070e7dfc 100644 --- a/craft_application/git/__init__.py +++ b/craft_application/git/__init__.py @@ -16,7 +16,7 @@ from ._errors import GitError from ._models import GitType -from ._git_repo import GitRepo, get_git_repo_type, is_repo +from ._git_repo import GitRepo, get_git_repo_type, is_repo, parse_describe __all__ = [ "GitError", @@ -24,4 +24,5 @@ "GitType", "get_git_repo_type", "is_repo", + "parse_describe", ] diff --git a/craft_application/git/_git_repo.py b/craft_application/git/_git_repo.py index 9c4dec12..5fcdf51d 100644 --- a/craft_application/git/_git_repo.py +++ b/craft_application/git/_git_repo.py @@ -22,6 +22,7 @@ import time from pathlib import Path from shlex import quote +from typing import cast from craft_parts.utils import os_utils from typing_extensions import Self @@ -91,6 +92,31 @@ def get_git_repo_type(path: Path) -> GitType: return GitType.INVALID +def parse_describe(describe_str: str) -> str: + """Parse git describe string to get a human-readable version. + + Examples (git describe -> parse_describe): + 4.1.1-0-gad012482d -> 4.1.1 + 4.1.1-16-g2d8943dbc -> 4.1.1.post16+git2d8943dbc + + For shallow clones or repositories missing tags: + 0ae7c04 -> 0ae7c04 + """ + if "-" not in describe_str: + return describe_str + splitted_describe = describe_str.split("-") + number_of_elements = 3 + if len(splitted_describe) != number_of_elements: + logger.warning("Cannot determine version basing on describe result.") + return describe_str + + version, distance, commit = splitted_describe + + if distance == "0": + return version + return f"{version}.post{distance}+git{commit[1:]}" + + class GitRepo: """Git repository class.""" @@ -353,6 +379,40 @@ def push_url( # noqa: PLR0912 (too-many-branches) f"for the git repository in {str(self.path)!r}." ) + def describe( + self, + *, + committish: str | None = None, + abbreviated_size: int | None = None, + always_use_long_format: bool | None = None, + show_commit_oid_as_fallback: bool | None = None, + ) -> str: + """Return a human readable name base on an available ref. + + :param committish: Commit-ish object name to describe. If None, HEAD will be + described + :param abbreviated_size: The same as --abbrev of ``git describe`` command + :param always_use_long_format: Always use the long format + :param show_commit_oid_as_fallback: Show uniquely abbrevaited commit as fallback + + :returns: String that describes given object. + + raises GitError: if object cannot be described + """ + logger.debug(f"Trying to describe {committish or 'HEAD'!r}.") + try: + return cast( + str, + self._repo.describe( + committish=committish, + abbreviated_size=abbreviated_size, + always_use_long_format=always_use_long_format, + show_commit_oid_as_fallback=show_commit_oid_as_fallback, + ), + ) + except pygit2.GitError as err: + raise GitError("Could not describe given object") from err + def _resolve_ref(self, ref: str) -> str: """Get a full reference name for a shorthand ref. diff --git a/tests/unit/git/test_git.py b/tests/unit/git/test_git.py index e9de637a..0a207046 100644 --- a/tests/unit/git/test_git.py +++ b/tests/unit/git/test_git.py @@ -27,7 +27,14 @@ import pygit2 import pygit2.enums import pytest -from craft_application.git import GitError, GitRepo, GitType, get_git_repo_type, is_repo +from craft_application.git import ( + GitError, + GitRepo, + GitType, + get_git_repo_type, + is_repo, + parse_describe, +) from craft_application.remote import ( RemoteBuildInvalidGitRepoError, check_git_repo_for_remote_build, @@ -137,6 +144,20 @@ def test_is_repo_error(empty_working_directory, mocker): ) +@pytest.mark.parametrize( + ("describe", "expected"), + [ + ("cdaea14", "cdaea14"), + ("4.1.1-0-gad012482d", "4.1.1"), + ("4.1.1-16-g2d8943dbc", "4.1.1.post16+git2d8943dbc"), + ("0ae7c04", "0ae7c04"), + ], +) +def test_parsing_describe(describe: str, expected: str) -> None: + """Check if describe result is correctly parsed.""" + assert parse_describe(describe) == expected + + def test_init_repo(empty_working_directory): """Initialize a GitRepo object.""" repo = GitRepo(empty_working_directory) From 7be73838962f07afbe605d5c9ce3c69bdec8f73e Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Wed, 6 Nov 2024 15:40:36 -0500 Subject: [PATCH 02/15] feat: add describe alongside examples Signed-off-by: Dariusz Duda --- craft_application/git/_git_repo.py | 2 +- tests/unit/git/test_git.py | 113 +++++++++++++++++++++++++---- 2 files changed, 101 insertions(+), 14 deletions(-) diff --git a/craft_application/git/_git_repo.py b/craft_application/git/_git_repo.py index 5fcdf51d..22674785 100644 --- a/craft_application/git/_git_repo.py +++ b/craft_application/git/_git_repo.py @@ -410,7 +410,7 @@ def describe( show_commit_oid_as_fallback=show_commit_oid_as_fallback, ), ) - except pygit2.GitError as err: + except (pygit2.GitError, KeyError) as err: raise GitError("Could not describe given object") from err def _resolve_ref(self, ref: str) -> str: diff --git a/tests/unit/git/test_git.py b/tests/unit/git/test_git.py index 0a207046..544523d0 100644 --- a/tests/unit/git/test_git.py +++ b/tests/unit/git/test_git.py @@ -15,13 +15,10 @@ """Tests for the pygit2 wrapper class.""" -import os -import pathlib import re import subprocess -from collections.abc import Iterator +from dataclasses import dataclass from pathlib import Path -from typing import cast from unittest.mock import ANY import pygit2 @@ -42,21 +39,62 @@ @pytest.fixture -def empty_working_directory(tmp_path) -> Iterator[Path]: - cwd = pathlib.Path.cwd() - +def empty_working_directory( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> Path: repo_dir = Path(tmp_path, "test-repo") repo_dir.mkdir() - os.chdir(repo_dir) - yield repo_dir - - os.chdir(cwd) + monkeypatch.chdir(repo_dir) + return repo_dir @pytest.fixture -def empty_repository(empty_working_directory) -> Path: +def empty_repository(empty_working_directory: Path) -> Path: subprocess.run(["git", "init"], check=True) - return cast(Path, empty_working_directory) + return empty_working_directory + + +@dataclass +class RepositoryDefinition: + repository_path: Path + commit: str + tag: str | None = None + + @property + def short_commit(self) -> str: + """Return abbreviated commit.""" + return self.commit[:7] + + +@pytest.fixture +def repository_with_commit(empty_repository: Path) -> RepositoryDefinition: + repo = GitRepo(empty_repository) + (empty_repository / "Some file").touch() + repo.add_all() + commit_sha = repo.commit("1") + return RepositoryDefinition( + repository_path=empty_repository, + commit=commit_sha, + ) + + +@pytest.fixture +def repository_with_anotated_tag( + repository_with_commit: RepositoryDefinition, +) -> RepositoryDefinition: + repo = GitRepo(repository_with_commit.repository_path) + test_tag = "v3.2.1" + subprocess.run( + ["git", "config", "--local", "user.name", "Testcraft", test_tag], check=True + ) + subprocess.run( + ["git", "config", "--local", "user.email", "testcraft@canonical.com", test_tag], + check=True, + ) + subprocess.run(["git", "tag", "-a", "-m", "testcraft tag", test_tag], check=True) + repository_with_commit.tag = test_tag + return repository_with_commit def test_is_repo(empty_working_directory): @@ -918,3 +956,52 @@ def test_check_git_repo_for_remote_build_shallow(empty_working_directory): match="Remote builds for shallow cloned git repos are not supported", ): check_git_repo_for_remote_build(git_shallow_path) + + +@pytest.mark.parametrize( + "always_use_long_format", + [True, False, None], + ids=lambda p: f"use_long_format={p!s}", +) +def test_describing_commit( + repository_with_commit: RepositoryDefinition, always_use_long_format: bool | None +): + """Describe an empty repository.""" + repo = GitRepo(repository_with_commit.repository_path) + + assert ( + repo.describe( + show_commit_oid_as_fallback=True, + always_use_long_format=always_use_long_format, + ) + == repository_with_commit.short_commit + ) + + +def test_describing_repo_fails_in_empty_repo(empty_repository: Path): + """Cannot describe an empty repository.""" + repo = GitRepo(empty_repository) + + with pytest.raises(GitError): + repo.describe(show_commit_oid_as_fallback=True) + + +def test_describing_tags(repository_with_anotated_tag: RepositoryDefinition): + """Git should be able to describe tags.""" + repo = GitRepo(repository_with_anotated_tag.repository_path) + assert repo.describe() == repository_with_anotated_tag.tag + + +def test_describing_commits_following_tags( + repository_with_anotated_tag: RepositoryDefinition, +): + """Git should be able to describe commit after tags.""" + repo = GitRepo(repository_with_anotated_tag.repository_path) + (repository_with_anotated_tag.repository_path / "another_file").touch() + tag = repository_with_anotated_tag.tag + repo.add_all() + new_commit = repo.commit("commit after tag") + short_new_commit = new_commit[:7] + describe_result = repo.describe() + assert describe_result == f"{tag}-1-g{short_new_commit}" + assert parse_describe(describe_result) == f"{tag}.post1+git{short_new_commit}" From 227dafdfaa9b1485e0203f8b36cb1c1e57d45e64 Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Wed, 6 Nov 2024 15:51:41 -0500 Subject: [PATCH 03/15] test: add tests for unannotated tags Signed-off-by: Dariusz Duda --- tests/unit/git/test_git.py | 59 ++++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/tests/unit/git/test_git.py b/tests/unit/git/test_git.py index 544523d0..43226175 100644 --- a/tests/unit/git/test_git.py +++ b/tests/unit/git/test_git.py @@ -80,10 +80,9 @@ def repository_with_commit(empty_repository: Path) -> RepositoryDefinition: @pytest.fixture -def repository_with_anotated_tag( +def repository_with_annotated_tag( repository_with_commit: RepositoryDefinition, ) -> RepositoryDefinition: - repo = GitRepo(repository_with_commit.repository_path) test_tag = "v3.2.1" subprocess.run( ["git", "config", "--local", "user.name", "Testcraft", test_tag], check=True @@ -97,6 +96,21 @@ def repository_with_anotated_tag( return repository_with_commit +@pytest.fixture +def repository_with_unannotated_tag( + repository_with_commit: RepositoryDefinition, +) -> RepositoryDefinition: + subprocess.run(["git", "config", "--local", "user.name", "Testcraft"], check=True) + subprocess.run( + ["git", "config", "--local", "user.email", "testcraft@canonical.com"], + check=True, + ) + test_tag = "non-annotated" + subprocess.run(["git", "tag", test_tag], check=True) + repository_with_commit.tag = test_tag + return repository_with_commit + + def test_is_repo(empty_working_directory): """Check if directory is a repo.""" GitRepo(empty_working_directory) @@ -986,22 +1000,43 @@ def test_describing_repo_fails_in_empty_repo(empty_repository: Path): repo.describe(show_commit_oid_as_fallback=True) -def test_describing_tags(repository_with_anotated_tag: RepositoryDefinition): - """Git should be able to describe tags.""" - repo = GitRepo(repository_with_anotated_tag.repository_path) - assert repo.describe() == repository_with_anotated_tag.tag +def test_describing_tags(repository_with_annotated_tag: RepositoryDefinition): + """Describe should be able to handle annotated tags.""" + repo = GitRepo(repository_with_annotated_tag.repository_path) + assert repo.describe() == repository_with_annotated_tag.tag def test_describing_commits_following_tags( - repository_with_anotated_tag: RepositoryDefinition, + repository_with_annotated_tag: RepositoryDefinition, ): - """Git should be able to describe commit after tags.""" - repo = GitRepo(repository_with_anotated_tag.repository_path) - (repository_with_anotated_tag.repository_path / "another_file").touch() - tag = repository_with_anotated_tag.tag + """Describe should be able to discover commits after tags.""" + repo = GitRepo(repository_with_annotated_tag.repository_path) + (repository_with_annotated_tag.repository_path / "another_file").touch() + tag = repository_with_annotated_tag.tag repo.add_all() new_commit = repo.commit("commit after tag") short_new_commit = new_commit[:7] - describe_result = repo.describe() + describe_result = repo.describe( + show_commit_oid_as_fallback=True, + always_use_long_format=True, + ) assert describe_result == f"{tag}-1-g{short_new_commit}" assert parse_describe(describe_result) == f"{tag}.post1+git{short_new_commit}" + + +def test_describing_unanotated_tags( + repository_with_unannotated_tag: RepositoryDefinition, +): + """Describe should error out if trying to describe repo without annotated tags.""" + repo = GitRepo(repository_with_unannotated_tag.repository_path) + with pytest.raises(GitError): + repo.describe() + + +def test_describing_fallback_to_commit_for_unannotated_tags( + repository_with_unannotated_tag: RepositoryDefinition, +): + """Describe should fallback to commit if trying to describe repo without annotated tags.""" + repo = GitRepo(repository_with_unannotated_tag.repository_path) + describe_result = repo.describe(show_commit_oid_as_fallback=True) + assert describe_result == repository_with_unannotated_tag.short_commit From 35b497a7f051fe6aa0d3b88b6196e31dc2ab0f53 Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Wed, 6 Nov 2024 16:27:06 -0500 Subject: [PATCH 04/15] feat: add version to InitService context Signed-off-by: Dariusz Duda --- craft_application/git/__init__.py | 5 ++- craft_application/git/_const.py | 19 +++++++++++ craft_application/git/_consts.py | 19 +++++++++++ craft_application/git/_models.py | 7 ++++ craft_application/services/init.py | 31 +++++++++++++++-- tests/unit/git/test_git.py | 5 +-- tests/unit/services/test_init.py | 55 ++++++++++++++++++++++++++++-- 7 files changed, 132 insertions(+), 9 deletions(-) create mode 100644 craft_application/git/_const.py create mode 100644 craft_application/git/_consts.py diff --git a/craft_application/git/__init__.py b/craft_application/git/__init__.py index 070e7dfc..6b37afd5 100644 --- a/craft_application/git/__init__.py +++ b/craft_application/git/__init__.py @@ -14,8 +14,9 @@ """Git repository utilities.""" +from ._consts import COMMIT_SHORT_SHA_LEN from ._errors import GitError -from ._models import GitType +from ._models import GitType, short_commit_sha from ._git_repo import GitRepo, get_git_repo_type, is_repo, parse_describe __all__ = [ @@ -25,4 +26,6 @@ "get_git_repo_type", "is_repo", "parse_describe", + "short_commit_sha", + "COMMIT_SHORT_SHA_LEN", ] diff --git a/craft_application/git/_const.py b/craft_application/git/_const.py new file mode 100644 index 00000000..1c45290c --- /dev/null +++ b/craft_application/git/_const.py @@ -0,0 +1,19 @@ +# Copyright 2024 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License version 3 as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program. If not, see . + +"""Git repository consts.""" + +from typing import Final + +COMMIT_SHORT_SHA_LEN: Final[int] = 7 diff --git a/craft_application/git/_consts.py b/craft_application/git/_consts.py new file mode 100644 index 00000000..1c45290c --- /dev/null +++ b/craft_application/git/_consts.py @@ -0,0 +1,19 @@ +# Copyright 2024 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License version 3 as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program. If not, see . + +"""Git repository consts.""" + +from typing import Final + +COMMIT_SHORT_SHA_LEN: Final[int] = 7 diff --git a/craft_application/git/_models.py b/craft_application/git/_models.py index 4f9cefc4..8af52271 100644 --- a/craft_application/git/_models.py +++ b/craft_application/git/_models.py @@ -16,6 +16,13 @@ from enum import Enum +from ._consts import COMMIT_SHORT_SHA_LEN + + +def short_commit_sha(commit_sha: str) -> str: + """Return shortened version of the commit.""" + return commit_sha[:COMMIT_SHORT_SHA_LEN] + class GitType(Enum): """Type of git repository.""" diff --git a/craft_application/services/init.py b/craft_application/services/init.py index 33ee2c88..e4e08f38 100644 --- a/craft_application/services/init.py +++ b/craft_application/services/init.py @@ -25,6 +25,7 @@ from craft_cli import emit from craft_application.errors import InitError +from craft_application.git import GitError, GitRepo, is_repo, parse_describe from . import base @@ -53,9 +54,9 @@ def initialise_project( f"Initialising project {project_name!r} in {str(project_dir)!r} from " f"template in {str(template_dir)!r}." ) - context = self._get_context(name=project_name) environment = self._get_templates_environment(template_dir) self._create_project_dir(project_dir=project_dir) + context = self._get_context(name=project_name, project_dir=project_dir) self._render_project(environment, project_dir, template_dir, context) def check_for_existing_files( @@ -92,6 +93,11 @@ def check_for_existing_files( retcode=os.EX_CANTCREAT, ) + @property + def default_version(self) -> str: + """Return default version that should be used for the InitService context.""" + return "0.0" + def _copy_template_file( self, template_name: str, @@ -150,14 +156,18 @@ def _render_project( shutil.copystat((template_dir / template_name), path) emit.progress("Rendered project.") - def _get_context(self, name: str) -> dict[str, Any]: + def _get_context(self, name: str, *, project_dir: pathlib.Path) -> dict[str, Any]: """Get context to render templates with. :returns: A dict of context variables. """ emit.debug(f"Set project name to '{name}'") - return {"name": name} + version = self._get_version(project_dir=project_dir) + if version is not None: + emit.debug(f"Discovered project version: {version!r}") + + return {"name": name, "version": version or self.default_version} @staticmethod def _create_project_dir(project_dir: pathlib.Path) -> None: @@ -165,6 +175,21 @@ def _create_project_dir(project_dir: pathlib.Path) -> None: emit.debug(f"Creating project directory {str(project_dir)!r}.") project_dir.mkdir(parents=True, exist_ok=True) + def _get_version(self, *, project_dir: pathlib.Path) -> str | None: + """Try to determine version if project is the git repository.""" + try: + if is_repo(project_dir): + git_repo = GitRepo(project_dir) + described = git_repo.describe( + always_use_long_format=True, + show_commit_oid_as_fallback=True, + ) + return parse_describe(described) + except GitError as error: + emit.debug(f"cannot determine project version: {error.details}") + + return None + def _get_loader(self, template_dir: pathlib.Path) -> jinja2.BaseLoader: """Return a Jinja loader for the given template directory. diff --git a/tests/unit/git/test_git.py b/tests/unit/git/test_git.py index 43226175..71384bef 100644 --- a/tests/unit/git/test_git.py +++ b/tests/unit/git/test_git.py @@ -31,6 +31,7 @@ get_git_repo_type, is_repo, parse_describe, + short_commit_sha, ) from craft_application.remote import ( RemoteBuildInvalidGitRepoError, @@ -64,7 +65,7 @@ class RepositoryDefinition: @property def short_commit(self) -> str: """Return abbreviated commit.""" - return self.commit[:7] + return short_commit_sha(self.commit) @pytest.fixture @@ -1015,7 +1016,7 @@ def test_describing_commits_following_tags( tag = repository_with_annotated_tag.tag repo.add_all() new_commit = repo.commit("commit after tag") - short_new_commit = new_commit[:7] + short_new_commit = short_commit_sha(new_commit) describe_result = repo.describe( show_commit_oid_as_fallback=True, always_use_long_format=True, diff --git a/tests/unit/services/test_init.py b/tests/unit/services/test_init.py index bd9cdca1..896b0fda 100644 --- a/tests/unit/services/test_init.py +++ b/tests/unit/services/test_init.py @@ -23,7 +23,9 @@ import jinja2 import pytest import pytest_check +from craft_cli.pytest_plugin import RecordingEmitter from craft_application import errors, services +from craft_application.git import GitRepo, short_commit_sha @pytest.fixture @@ -42,10 +44,57 @@ def mock_loader(mocker, tmp_path): ) -def test_get_context(init_service): - context = init_service._get_context(name="my-project") +def test_get_context(init_service, tmp_path: pathlib.Path): + project_dir = tmp_path / "my-project" + project_dir.mkdir() + context = init_service._get_context(name="my-project", project_dir=project_dir) + + assert context == {"name": "my-project", "version": init_service.default_version} + + +@pytest.fixture +def empty_git_repository(tmp_path: pathlib.Path) -> GitRepo: + repository = tmp_path / "my-project-git" + repository.mkdir() + return GitRepo(repository) + - assert context == {"name": "my-project"} +@pytest.fixture +def git_repository_with_commit(tmp_path: pathlib.Path) -> tuple[GitRepo, str]: + repository = tmp_path / "my-project-git" + repository.mkdir() + git_repo = GitRepo(repository) + (repository / "some_file").touch() + git_repo.add_all() + commit_sha = git_repo.commit("feat: initialize repo") + + return git_repo, commit_sha + + +def test_get_context_of_empty_git_repository( + init_service, empty_git_repository: GitRepo +): + context = init_service._get_context( + name="my-project", + project_dir=empty_git_repository.path, + ) + + assert context == {"name": "my-project", "version": init_service.default_version} + + +def test_get_context_of_git_repository_with_commit( + init_service, + git_repository_with_commit: tuple[GitRepo, str], + emitter: RecordingEmitter, +): + git_repo, commit_sha = git_repository_with_commit + expected_version = short_commit_sha(commit_sha) + context = init_service._get_context( + name="my-project", + project_dir=git_repo.path, + ) + assert context == {"name": "my-project", "version": expected_version} + emitter.assert_debug(f"Discovered project version: {expected_version!r}") @pytest.mark.parametrize("create_dir", [True, False]) From f924cb263c9c18f3a869bdff56837a9a05051d0f Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Wed, 6 Nov 2024 16:32:57 -0500 Subject: [PATCH 05/15] chore(lint): anottate describe type for mypy Signed-off-by: Dariusz Duda --- craft_application/git/_git_repo.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/craft_application/git/_git_repo.py b/craft_application/git/_git_repo.py index 22674785..662fd63d 100644 --- a/craft_application/git/_git_repo.py +++ b/craft_application/git/_git_repo.py @@ -22,7 +22,6 @@ import time from pathlib import Path from shlex import quote -from typing import cast from craft_parts.utils import os_utils from typing_extensions import Self @@ -401,17 +400,16 @@ def describe( """ logger.debug(f"Trying to describe {committish or 'HEAD'!r}.") try: - return cast( - str, - self._repo.describe( - committish=committish, - abbreviated_size=abbreviated_size, - always_use_long_format=always_use_long_format, - show_commit_oid_as_fallback=show_commit_oid_as_fallback, - ), + described: str = self._repo.describe( + committish=committish, + abbreviated_size=abbreviated_size, + always_use_long_format=always_use_long_format, + show_commit_oid_as_fallback=show_commit_oid_as_fallback, ) except (pygit2.GitError, KeyError) as err: raise GitError("Could not describe given object") from err + else: + return described def _resolve_ref(self, ref: str) -> str: """Get a full reference name for a shorthand ref. From 4efb712938d2546a5e8e7b44fa71b0987adf5816 Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Wed, 6 Nov 2024 16:34:48 -0500 Subject: [PATCH 06/15] chore: remove _const.py leftover Signed-off-by: Dariusz Duda --- craft_application/git/_const.py | 19 ------------------- 1 file changed, 19 deletions(-) delete mode 100644 craft_application/git/_const.py diff --git a/craft_application/git/_const.py b/craft_application/git/_const.py deleted file mode 100644 index 1c45290c..00000000 --- a/craft_application/git/_const.py +++ /dev/null @@ -1,19 +0,0 @@ -# Copyright 2024 Canonical Ltd. -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Lesser General Public License version 3 as -# published by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public License -# along with this program. If not, see . - -"""Git repository consts.""" - -from typing import Final - -COMMIT_SHORT_SHA_LEN: Final[int] = 7 From e3e3a5393d69645f107e249eab086688e1c50e95 Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Wed, 6 Nov 2024 16:41:15 -0500 Subject: [PATCH 07/15] test: check all params combinations Signed-off-by: Dariusz Duda --- tests/unit/git/test_git.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/unit/git/test_git.py b/tests/unit/git/test_git.py index 71384bef..6c94179b 100644 --- a/tests/unit/git/test_git.py +++ b/tests/unit/git/test_git.py @@ -19,6 +19,7 @@ import subprocess from dataclasses import dataclass from pathlib import Path +from typing import cast from unittest.mock import ANY import pygit2 @@ -1007,8 +1008,20 @@ def test_describing_tags(repository_with_annotated_tag: RepositoryDefinition): assert repo.describe() == repository_with_annotated_tag.tag +@pytest.fixture(params=[True, False, None], ids=lambda p: f"fallback={p!r}") +def show_commit_oid_as_fallback(request: pytest.FixtureRequest) -> bool | None: + return cast(bool | None, request.param) + + +@pytest.fixture(params=[True, False, None], ids=lambda p: f"long={p!r}") +def always_use_long_format(request: pytest.FixtureRequest) -> bool | None: + return cast(bool | None, request.param) + + def test_describing_commits_following_tags( repository_with_annotated_tag: RepositoryDefinition, + show_commit_oid_as_fallback: bool | None, + always_use_long_format: bool | None, ): """Describe should be able to discover commits after tags.""" repo = GitRepo(repository_with_annotated_tag.repository_path) @@ -1018,8 +1031,8 @@ def test_describing_commits_following_tags( new_commit = repo.commit("commit after tag") short_new_commit = short_commit_sha(new_commit) describe_result = repo.describe( - show_commit_oid_as_fallback=True, - always_use_long_format=True, + show_commit_oid_as_fallback=show_commit_oid_as_fallback, + always_use_long_format=always_use_long_format, ) assert describe_result == f"{tag}-1-g{short_new_commit}" assert parse_describe(describe_result) == f"{tag}.post1+git{short_new_commit}" From 3e83e783ee57f84b561b64ae14619c7fac78f778 Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Wed, 6 Nov 2024 16:42:13 -0500 Subject: [PATCH 08/15] chore(lint): organize imports in test_init Signed-off-by: Dariusz Duda --- tests/unit/services/test_init.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/services/test_init.py b/tests/unit/services/test_init.py index 896b0fda..77c5122b 100644 --- a/tests/unit/services/test_init.py +++ b/tests/unit/services/test_init.py @@ -23,9 +23,9 @@ import jinja2 import pytest import pytest_check -from craft_cli.pytest_plugin import RecordingEmitter from craft_application import errors, services from craft_application.git import GitRepo, short_commit_sha +from craft_cli.pytest_plugin import RecordingEmitter @pytest.fixture From 372ecfc1578d675cfc8016f3bf23180d97dfc8c9 Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Wed, 6 Nov 2024 17:43:43 -0500 Subject: [PATCH 09/15] test: increate git coverage Signed-off-by: Dariusz Duda --- tests/unit/git/test_git.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/git/test_git.py b/tests/unit/git/test_git.py index 6c94179b..2e33b57a 100644 --- a/tests/unit/git/test_git.py +++ b/tests/unit/git/test_git.py @@ -205,6 +205,7 @@ def test_is_repo_error(empty_working_directory, mocker): ("4.1.1-0-gad012482d", "4.1.1"), ("4.1.1-16-g2d8943dbc", "4.1.1.post16+git2d8943dbc"), ("0ae7c04", "0ae7c04"), + ("unknown-format", "unknown-format"), ], ) def test_parsing_describe(describe: str, expected: str) -> None: From 497b552214338f93a075bef198d208c09eefcb47 Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Thu, 7 Nov 2024 15:41:49 -0500 Subject: [PATCH 10/15] feat: handle tags with hyphens Signed-off-by: Dariusz Duda --- craft_application/git/_git_repo.py | 10 +++++++--- tests/unit/git/test_git.py | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/craft_application/git/_git_repo.py b/craft_application/git/_git_repo.py index 662fd63d..c87dd870 100644 --- a/craft_application/git/_git_repo.py +++ b/craft_application/git/_git_repo.py @@ -97,15 +97,19 @@ def parse_describe(describe_str: str) -> str: Examples (git describe -> parse_describe): 4.1.1-0-gad012482d -> 4.1.1 4.1.1-16-g2d8943dbc -> 4.1.1.post16+git2d8943dbc + curl-8_11_0-0-gb1ef0e1 -> curl-8_11_0 For shallow clones or repositories missing tags: 0ae7c04 -> 0ae7c04 """ if "-" not in describe_str: return describe_str - splitted_describe = describe_str.split("-") - number_of_elements = 3 - if len(splitted_describe) != number_of_elements: + number_of_expected_elements = 3 + splitted_describe = describe_str.rsplit( + "-", + maxsplit=number_of_expected_elements - 1, + ) + if len(splitted_describe) != number_of_expected_elements: logger.warning("Cannot determine version basing on describe result.") return describe_str diff --git a/tests/unit/git/test_git.py b/tests/unit/git/test_git.py index 2e33b57a..6a628d8c 100644 --- a/tests/unit/git/test_git.py +++ b/tests/unit/git/test_git.py @@ -204,6 +204,8 @@ def test_is_repo_error(empty_working_directory, mocker): ("cdaea14", "cdaea14"), ("4.1.1-0-gad012482d", "4.1.1"), ("4.1.1-16-g2d8943dbc", "4.1.1.post16+git2d8943dbc"), + ("curl-8_11_0-6-g0cdde0f", "curl-8_11_0.post6+git0cdde0f"), + ("curl-8_11_0-0-gb1ef0e1", "curl-8_11_0"), ("0ae7c04", "0ae7c04"), ("unknown-format", "unknown-format"), ], From d9874fba4f705515f070a7405a7e9141a3b9e070 Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Fri, 8 Nov 2024 08:53:12 -0500 Subject: [PATCH 11/15] test: add test for initialise project flow Signed-off-by: Dariusz Duda --- tests/unit/services/test_init.py | 59 ++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/tests/unit/services/test_init.py b/tests/unit/services/test_init.py index 77c5122b..82b09eff 100644 --- a/tests/unit/services/test_init.py +++ b/tests/unit/services/test_init.py @@ -23,6 +23,7 @@ import jinja2 import pytest import pytest_check +import pytest_mock from craft_application import errors, services from craft_application.git import GitRepo, short_commit_sha from craft_cli.pytest_plugin import RecordingEmitter @@ -71,6 +72,20 @@ def git_repository_with_commit(tmp_path: pathlib.Path) -> tuple[GitRepo, str]: return git_repo, commit_sha +@pytest.fixture +def project_dir(tmp_path: pathlib.Path) -> pathlib.Path: + project_dir = tmp_path / "my-project" + project_dir.mkdir() + return project_dir + + +@pytest.fixture +def templates_dir(tmp_path: pathlib.Path) -> pathlib.Path: + template_dir = tmp_path / "templates" + template_dir.mkdir() + return template_dir + + def test_get_context_of_empty_git_repository( init_service, empty_git_repository: GitRepo ): @@ -222,7 +237,7 @@ def test_render_project_with_templates(filename, init_service, tmp_path): environment=environment, project_dir=project_dir, template_dir=template_dir, - context={"name": "my-project"}, + context={"name": "my-project", "version": init_service.default_version}, ) assert (project_dir / filename[:-3]).read_text() == "my-project" @@ -243,7 +258,7 @@ def test_render_project_non_templates(filename, init_service, tmp_path): environment=environment, project_dir=project_dir, template_dir=template_dir, - context={"name": "my-project"}, + context={"name": "my-project", "version": init_service.default_version}, ) assert (project_dir / filename).read_text() == "test content" @@ -267,10 +282,48 @@ def test_render_project_executable(init_service, tmp_path): environment=environment, project_dir=project_dir, template_dir=template_dir, - context={"name": "my-project"}, + context={"name": "my-project", "version": init_service.default_version}, ) pytest_check.is_true(os.access(project_dir / "file-1.sh", os.X_OK)) pytest_check.is_true(os.access(project_dir / "file-2.sh", os.X_OK)) pytest_check.is_false(os.access(project_dir / "file-3.txt", os.X_OK)) pytest_check.is_false(os.access(project_dir / "file-4.txt", os.X_OK)) + + +def test_initialise_project( + init_service: services.InitService, + project_dir: pathlib.Path, + templates_dir: pathlib.Path, + mocker: pytest_mock.MockerFixture, +) -> None: + project_name = "test-project" + fake_env = {"templates": templates_dir} + fake_context = {"name": project_name, "version": init_service.default_version} + get_templates_mock = mocker.patch.object( + init_service, "_get_templates_environment", return_value=fake_env + ) + create_project_dir_mock = mocker.patch.object( + init_service, + "_create_project_dir", + ) + get_context_mock = mocker.patch.object( + init_service, + "_get_context", + return_value=fake_context, + ) + render_project_mock = mocker.patch.object( + init_service, + "_render_project", + ) + init_service.initialise_project( + project_dir=project_dir, + project_name=project_name, + template_dir=templates_dir, + ) + get_templates_mock.assert_called_once_with(templates_dir) + create_project_dir_mock.assert_called_once_with(project_dir=project_dir) + get_context_mock.assert_called_once_with(name=project_name, project_dir=project_dir) + render_project_mock.assert_called_once_with( + fake_env, project_dir, templates_dir, fake_context + ) From f7cbe0ad4bde885b10695e727e097938e9909169 Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Fri, 8 Nov 2024 09:51:52 -0500 Subject: [PATCH 12/15] chore: change default version to 0.1 Signed-off-by: Dariusz Duda --- craft_application/services/init.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/craft_application/services/init.py b/craft_application/services/init.py index e4e08f38..a183491f 100644 --- a/craft_application/services/init.py +++ b/craft_application/services/init.py @@ -96,7 +96,7 @@ def check_for_existing_files( @property def default_version(self) -> str: """Return default version that should be used for the InitService context.""" - return "0.0" + return "0.1" def _copy_template_file( self, From ebc9ef0eee6944cc74c62f9e9aee26f5120ebd6f Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Fri, 8 Nov 2024 12:13:52 -0500 Subject: [PATCH 13/15] test: extract helper fixtures to the conftest.py Signed-off-by: Dariusz Duda --- tests/conftest.py | 77 +++++++++++++++++++++++++++++++++++++- tests/unit/git/test_git.py | 74 +----------------------------------- 2 files changed, 77 insertions(+), 74 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index ac105b72..6816532c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -19,6 +19,8 @@ import os import pathlib import shutil +import subprocess +from dataclasses import dataclass from importlib import metadata from typing import TYPE_CHECKING, Any @@ -27,7 +29,7 @@ import jinja2 import pydantic import pytest -from craft_application import application, launchpad, models, services, util +from craft_application import application, git, launchpad, models, services, util from craft_cli import EmitterMode, emit from craft_providers import bases from jinja2 import FileSystemLoader @@ -366,3 +368,76 @@ def new_dir(tmp_path): yield tmp_path os.chdir(cwd) + + +@pytest.fixture +def empty_working_directory( + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, +) -> pathlib.Path: + repo_dir = pathlib.Path(tmp_path, "test-repo") + repo_dir.mkdir() + monkeypatch.chdir(repo_dir) + return repo_dir + + +@pytest.fixture +def empty_repository(empty_working_directory: pathlib.Path) -> pathlib.Path: + subprocess.run(["git", "init"], check=True) + return empty_working_directory + + +@dataclass +class RepositoryDefinition: + repository_path: pathlib.Path + commit: str + tag: str | None = None + + @property + def short_commit(self) -> str: + """Return abbreviated commit.""" + return git.short_commit_sha(self.commit) + + +@pytest.fixture +def repository_with_commit(empty_repository: pathlib.Path) -> RepositoryDefinition: + repo = git.GitRepo(empty_repository) + (empty_repository / "Some file").touch() + repo.add_all() + commit_sha = repo.commit("1") + return RepositoryDefinition( + repository_path=empty_repository, + commit=commit_sha, + ) + + +@pytest.fixture +def repository_with_annotated_tag( + repository_with_commit: RepositoryDefinition, +) -> RepositoryDefinition: + test_tag = "v3.2.1" + subprocess.run( + ["git", "config", "--local", "user.name", "Testcraft", test_tag], check=True + ) + subprocess.run( + ["git", "config", "--local", "user.email", "testcraft@canonical.com", test_tag], + check=True, + ) + subprocess.run(["git", "tag", "-a", "-m", "testcraft tag", test_tag], check=True) + repository_with_commit.tag = test_tag + return repository_with_commit + + +@pytest.fixture +def repository_with_unannotated_tag( + repository_with_commit: RepositoryDefinition, +) -> RepositoryDefinition: + subprocess.run(["git", "config", "--local", "user.name", "Testcraft"], check=True) + subprocess.run( + ["git", "config", "--local", "user.email", "testcraft@canonical.com"], + check=True, + ) + test_tag = "non-annotated" + subprocess.run(["git", "tag", test_tag], check=True) + repository_with_commit.tag = test_tag + return repository_with_commit diff --git a/tests/unit/git/test_git.py b/tests/unit/git/test_git.py index 6a628d8c..cec290c7 100644 --- a/tests/unit/git/test_git.py +++ b/tests/unit/git/test_git.py @@ -17,7 +17,6 @@ import re import subprocess -from dataclasses import dataclass from pathlib import Path from typing import cast from unittest.mock import ANY @@ -39,78 +38,7 @@ check_git_repo_for_remote_build, ) - -@pytest.fixture -def empty_working_directory( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, -) -> Path: - repo_dir = Path(tmp_path, "test-repo") - repo_dir.mkdir() - monkeypatch.chdir(repo_dir) - return repo_dir - - -@pytest.fixture -def empty_repository(empty_working_directory: Path) -> Path: - subprocess.run(["git", "init"], check=True) - return empty_working_directory - - -@dataclass -class RepositoryDefinition: - repository_path: Path - commit: str - tag: str | None = None - - @property - def short_commit(self) -> str: - """Return abbreviated commit.""" - return short_commit_sha(self.commit) - - -@pytest.fixture -def repository_with_commit(empty_repository: Path) -> RepositoryDefinition: - repo = GitRepo(empty_repository) - (empty_repository / "Some file").touch() - repo.add_all() - commit_sha = repo.commit("1") - return RepositoryDefinition( - repository_path=empty_repository, - commit=commit_sha, - ) - - -@pytest.fixture -def repository_with_annotated_tag( - repository_with_commit: RepositoryDefinition, -) -> RepositoryDefinition: - test_tag = "v3.2.1" - subprocess.run( - ["git", "config", "--local", "user.name", "Testcraft", test_tag], check=True - ) - subprocess.run( - ["git", "config", "--local", "user.email", "testcraft@canonical.com", test_tag], - check=True, - ) - subprocess.run(["git", "tag", "-a", "-m", "testcraft tag", test_tag], check=True) - repository_with_commit.tag = test_tag - return repository_with_commit - - -@pytest.fixture -def repository_with_unannotated_tag( - repository_with_commit: RepositoryDefinition, -) -> RepositoryDefinition: - subprocess.run(["git", "config", "--local", "user.name", "Testcraft"], check=True) - subprocess.run( - ["git", "config", "--local", "user.email", "testcraft@canonical.com"], - check=True, - ) - test_tag = "non-annotated" - subprocess.run(["git", "tag", test_tag], check=True) - repository_with_commit.tag = test_tag - return repository_with_commit +from tests.conftest import RepositoryDefinition def test_is_repo(empty_working_directory): From 00c23865a39a7ba46dd30afb8cd7f11936e533b1 Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Fri, 8 Nov 2024 12:15:00 -0500 Subject: [PATCH 14/15] test: add integration tests with versioned template Signed-off-by: Dariusz Duda --- tests/integration/services/test_init.py | 108 +++++++++++++++++++++--- 1 file changed, 96 insertions(+), 12 deletions(-) diff --git a/tests/integration/services/test_init.py b/tests/integration/services/test_init.py index c911cd44..6ed4d72b 100644 --- a/tests/integration/services/test_init.py +++ b/tests/integration/services/test_init.py @@ -26,6 +26,8 @@ from craft_application.models.project import Project from craft_application.services import InitService +from tests.conftest import RepositoryDefinition + # init operates in the current working directory pytestmark = pytest.mark.usefixtures("new_dir") @@ -50,16 +52,8 @@ def project_yaml_filename() -> str: return "testcraft.yaml" -@pytest.fixture -def template_dir_with_testcraft_yaml_j2( - fake_empty_template_dir: pathlib.Path, - project_yaml_filename: str, -) -> pathlib.Path: - """Creates the same testcraft.yaml file in the top-level and nested directories. - - Normally a project would only have one testcraft.yaml file, but two are created for testing. - """ - template_text = textwrap.dedent( +def get_testcraft_yaml(*, version: str = "git") -> str: + return textwrap.dedent( """ # This file configures testcraft. @@ -68,7 +62,7 @@ def template_dir_with_testcraft_yaml_j2( # (Required) # The source package version - version: git + version: <> # (Required) # Version of the build base OS @@ -97,8 +91,22 @@ def template_dir_with_testcraft_yaml_j2( source: . platforms: amd64: - """ + """.replace( + "<>", version + ) ) + + +@pytest.fixture +def template_dir_with_testcraft_yaml_j2( + fake_empty_template_dir: pathlib.Path, + project_yaml_filename: str, +) -> pathlib.Path: + """Creates the same testcraft.yaml file in the top-level and nested directories. + + Normally a project would only have one testcraft.yaml file, but two are created for testing. + """ + template_text = get_testcraft_yaml() top_level_template = fake_empty_template_dir / f"{project_yaml_filename}.j2" top_level_template.write_text(template_text) nested_template = fake_empty_template_dir / "nested" / f"{project_yaml_filename}.j2" @@ -108,6 +116,19 @@ def template_dir_with_testcraft_yaml_j2( return fake_empty_template_dir +@pytest.fixture +def template_dir_with_versioned_testcraft_yaml_j2( + fake_empty_template_dir: pathlib.Path, + project_yaml_filename: str, +) -> pathlib.Path: + """Creates the testcraft.yaml with {{ version }} marker.""" + template_text = get_testcraft_yaml(version="{{ version }}") + top_level_template = fake_empty_template_dir / f"{project_yaml_filename}.j2" + top_level_template.write_text(template_text) + + return fake_empty_template_dir + + @pytest.fixture def template_dir_with_multiple_non_ninja_files( fake_empty_template_dir: pathlib.Path, @@ -474,3 +495,66 @@ def test_check_for_existing_files_error( project_dir=fake_empty_project_dir, template_dir=template_dir_with_testcraft_yaml_j2, ) + + +def test_init_service_with_version_without_git_repository( + init_service: InitService, + empty_working_directory: pathlib.Path, + template_dir_with_versioned_testcraft_yaml_j2: pathlib.Path, + project_yaml_filename: str, + check, +) -> None: + project_path = empty_working_directory + init_service.initialise_project( + project_dir=project_path, + project_name=project_path.name, + template_dir=template_dir_with_versioned_testcraft_yaml_j2, + ) + project_yaml_path = project_path / project_yaml_filename + + with check: + assert project_yaml_path.exists(), "Project should be initialised with template" + project = Project.from_yaml_file(project_yaml_path) + assert project.version == init_service.default_version + + +def test_init_service_with_version_based_on_commit( + init_service: InitService, + repository_with_commit: RepositoryDefinition, + template_dir_with_versioned_testcraft_yaml_j2: pathlib.Path, + project_yaml_filename: str, + check, +) -> None: + project_path = repository_with_commit.repository_path + init_service.initialise_project( + project_dir=project_path, + project_name=project_path.name, + template_dir=template_dir_with_versioned_testcraft_yaml_j2, + ) + project_yaml_path = project_path / project_yaml_filename + + with check: + assert project_yaml_path.exists(), "Project should be initialised with template" + project = Project.from_yaml_file(project_yaml_path) + assert project.version == repository_with_commit.short_commit + + +def test_init_service_with_version_based_on_tag( + init_service: InitService, + repository_with_annotated_tag: RepositoryDefinition, + template_dir_with_versioned_testcraft_yaml_j2: pathlib.Path, + project_yaml_filename: str, + check, +) -> None: + project_path = repository_with_annotated_tag.repository_path + init_service.initialise_project( + project_dir=project_path, + project_name=project_path.name, + template_dir=template_dir_with_versioned_testcraft_yaml_j2, + ) + project_yaml_path = project_path / project_yaml_filename + + with check: + assert project_yaml_path.exists(), "Project should be initialised with template" + project = Project.from_yaml_file(project_yaml_path) + assert project.version == repository_with_annotated_tag.tag From bc92b9718b72aa855e548de665603f80fe0898a9 Mon Sep 17 00:00:00 2001 From: Dariusz Duda Date: Fri, 8 Nov 2024 12:18:21 -0500 Subject: [PATCH 15/15] chore(docs): add changelog entry Signed-off-by: Dariusz Duda --- docs/reference/changelog.rst | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/reference/changelog.rst b/docs/reference/changelog.rst index dc121577..a123a86a 100644 --- a/docs/reference/changelog.rst +++ b/docs/reference/changelog.rst @@ -4,6 +4,23 @@ Changelog ********* +4.5.0 (2024-XX-XX) +------------------- + +Application +=========== + +Commands +======== + +Services +======== + +- Add version to the template generation context of ``InitService``. + +.. + For a complete list of commits, check out the `4.5.0`_ release on GitHub. + 4.4.0 (2024-Oct-XX) -------------------