Skip to content

Commit

Permalink
fix(remote-build): minor edge cases found with snapcraft (#299)
Browse files Browse the repository at this point in the history
  • Loading branch information
lengau authored Apr 4, 2024
1 parent 50602c0 commit 1508796
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 13 deletions.
5 changes: 4 additions & 1 deletion craft_application/remote/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,10 @@ def push_url( # pylint: disable=too-many-branches
# temporarily call git directly due to libgit2 bug that unable to push
# large repos using https. See https://github.com/libgit2/libgit2/issues/6385
# and https://github.com/snapcore/snapcraft/issues/4478
cmd: list[str] = ["git", "push", remote_url, refspec, "--progress"]
# Force push in case this repository already exists. The repository is always
# going to exist solely for remote builds, so the only potential issue here is a
# race condition with multiple remote builds on the same machine.
cmd: list[str] = ["git", "push", "--force", remote_url, refspec, "--progress"]
if push_tags:
cmd.append("--tags")

Expand Down
34 changes: 22 additions & 12 deletions craft_application/services/remotebuild.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,9 @@ def _ensure_repository(
work_tree = WorkTree(self._app.name, self._name, project_dir)
work_tree.init_repo()
try:
lp_repository = self.lp.new_repository(self._name, self._lp_project.name)
lp_repository = self.lp.new_repository(
self._name, project=self._lp_project.name
)
except launchpadlib.errors.HTTPError:
lp_repository = self.lp.get_repository(
name=self._name, project=self._lp_project.name
Expand Down Expand Up @@ -289,21 +291,29 @@ def _ensure_recipe(
repository: launchpad.models.GitRepository,
**kwargs: Any, # noqa: ANN401
) -> launchpad.models.Recipe:
"""Create a new recipe."""
# There are 3 different ways this can fail in expected ways, all of them HTTPErrors:
#
# 1. Recipe doesn't exist (or you don't have access to see it)
# 2. You don't have access to delete the recipe
# 3. There is an ongoing build with the recipep
#
# In case 1, we don't care and are about to make the new recipe anyway.
# In the other cases, the errors from `new()` end up being more useful.
with contextlib.suppress(launchpadlib.errors.HTTPError):
"""Get a recipe or create it if it doesn't exist."""
with contextlib.suppress(ValueError): # Recipe doesn't exist
recipe = self._get_recipe()
recipe.delete()

return self._new_recipe(name, repository, **kwargs)

def _new_recipe(
self,
name: str,
repository: launchpad.models.GitRepository,
**kwargs: Any, # noqa: ANN401
) -> launchpad.models.Recipe:
"""Create a new recipe for the given repository."""
repository.lp_refresh() # Prevents a race condition on new repositories.
git_ref = parse.urlparse(str(repository.git_https_url)).path + "/+ref/main"
return self.RecipeClass.new(
self.lp, name, self.lp.username, git_ref=repository.git_https_url, **kwargs
self.lp,
name,
self.lp.username,
git_ref=git_ref,
project=self._lp_project.name,
**kwargs,
)

def _get_recipe(self) -> launchpad.models.Recipe:
Expand Down
17 changes: 17 additions & 0 deletions tests/unit/services/test_remotebuild.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,23 @@ def test_ensure_repository_already_exists(
assert expiry > datetime.datetime.now(tz=tz)


def test_create_new_recipe(remote_build_service, mock_lp_project):
"""Test that _new_recipe works correctly."""
remote_build_service._lp_project = mock_lp_project
remote_build_service.RecipeClass = mock.Mock()
repo = mock.Mock(git_https_url="https://localhost/~me/some-project/+git/my-repo")

remote_build_service._new_recipe("test-recipe", repo)

remote_build_service.RecipeClass.new.assert_called_once_with(
remote_build_service.lp,
"test-recipe",
"craft_test_user",
git_ref="/~me/some-project/+git/my-repo/+ref/main",
project="craft_test_user-craft-remote-build",
)


def test_not_setup(remote_build_service):
with pytest.raises(RuntimeError):
all(remote_build_service.monitor_builds())
Expand Down

0 comments on commit 1508796

Please sign in to comment.