Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

project: introduce build-snaps #1518

Merged
merged 21 commits into from
Sep 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions integration_tests/snaps/build-snap-grammar-fail/snapcraft.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
name: build-snap-grammar-fail
version: '0.1'
summary: Test build snap grammar failures
description: Make sure the build snap grammar handles `else fail` as expected
grade: devel
confinement: strict

parts:
my-part:
plugin: nil
build-snaps:
- on other-arch:
- foo
- else fail
15 changes: 15 additions & 0 deletions integration_tests/snaps/build-snap-grammar-on-else/snapcraft.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
name: build-snap-grammar-on-else
version: '0.1'
summary: Test build snap grammar on statement else
description: Verify that the on statement moves to else on other architectures
grade: devel
confinement: strict

parts:
my-part:
plugin: nil
build-snaps:
- on other-arch:
- foo
- else:
- hello
13 changes: 13 additions & 0 deletions integration_tests/snaps/build-snap-grammar-on/snapcraft.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name: build-snap-grammar-on
version: '0.1'
summary: Test build snap grammar on statement
description: Verify that the on statement skips other architecture branches
grade: devel
confinement: strict

parts:
my-part:
plugin: nil
build-snaps:
- on other-arch:
- hello
15 changes: 15 additions & 0 deletions integration_tests/snaps/build-snap-grammar-try-else/snapcraft.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
name: build-snap-grammar-try-else
version: '0.1'
summary: Test build snap grammar try statement else
description: Verify that the try statement moves to the else if invalid
grade: devel
confinement: strict

parts:
my-part:
plugin: nil
build-snaps:
- try:
- invalid-snap
- else:
- hello
13 changes: 13 additions & 0 deletions integration_tests/snaps/build-snap-grammar-try-skip/snapcraft.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name: build-snap-grammar-try
version: '0.1'
summary: Test build snap grammar try statement
description: Verify that the try statement results in an optional build snap
grade: devel
confinement: strict

parts:
my-part:
plugin: nil
build-snaps:
- try:
- invalid-snap
13 changes: 13 additions & 0 deletions integration_tests/snaps/build-snap-grammar-try/snapcraft.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name: build-snap-grammar-try
version: '0.1'
summary: Test build snap grammar try statement
description: Verify that the try statement results in an optional build snap
grade: devel
confinement: strict

parts:
my-part:
plugin: nil
build-snaps:
- try:
- hello
12 changes: 12 additions & 0 deletions integration_tests/snaps/build-snap-grammar/snapcraft.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
name: build-snap-grammar
version: '0.1'
summary: Test the build snap grammar
description: Simple, standalone build-snap
grade: devel
confinement: strict

parts:
my-part:
plugin: nil
build-snaps:
- hello
105 changes: 105 additions & 0 deletions integration_tests/test_build_snap_grammar.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
#
# Copyright (C) 2017 Canonical Ltd
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU 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 General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
import fileinput
import subprocess

import testscenarios
from testtools.matchers import Contains, Equals

import integration_tests
import snapcraft


def _construct_scenarios():
main_scenarios = {
'build-snap-grammar': True,
'build-snap-grammar-try': True,
'build-snap-grammar-try-skip': False,
'build-snap-grammar-try-else': True,
'build-snap-grammar-on': False,
'build-snap-grammar-on-else': True,
}

# Just some combinations
channel_scenarios = [
'', '/stable', '/latest/stable']

all_scenarios = []
for project, expected_install in main_scenarios.items():
for channel in channel_scenarios:
d = dict(project=project, hello_installed=expected_install,
channel=channel)
scenario = ('{}{}'.format(project, channel), d)
all_scenarios.append(scenario)

return all_scenarios


class BuildSnapGrammarTestCase(testscenarios.WithScenarios,
integration_tests.TestCase):

scenarios = _construct_scenarios()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I would use multiply here, but if this makes you happy, ok :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I saw multiply, this feels much more readable IMO, the other options was as @sparkiegeek mentioned, subTest but iirc, testscenarios had issues with that, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


def setUp(self):
super().setUp()
if self._hello_is_installed():
self.fail(
'This integration test cannot run if you already have the '
"'hello' snap installed. Please uninstall it "
"by running 'sudo snap remove hello'.")

def tearDown(self):
super().tearDown()

# Remove hello. This is safe since the test fails if hello was already
# installed.
try:
subprocess.check_output(
['sudo', 'snap', 'remove', 'hello'],
stderr=subprocess.STDOUT)
except subprocess.CalledProcessError as e:
self.fail("unable to remove 'hello': {}".format(e.output))

def _hello_is_installed(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_is_hello_installed should be the name of this function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I replicated what was in test_build_package_grammar. Let's change this in a different PR.

return snapcraft.repo.snaps.SnapPackage.is_snap_installed('hello')

def _add_channel_information_to_hello(self):
replacement = '- hello{}'.format(self.channel)
with fileinput.input('snapcraft.yaml', inplace=True) as input_file:
for line in input_file:
print(line.replace('- hello', replacement), end='')

def test_grammar(self):
self.copy_project_to_cwd(self.project)
self._add_channel_information_to_hello()

self.run_snapcraft('pull')

self.assertThat(self._hello_is_installed(),
Equals(self.hello_installed))


class BuildSnapGrammarErrorsTestCase(integration_tests.TestCase):

def test_on_other_arch_else_fail(self):
"""Test that 'on' fails with an error if it hits an 'else fail'."""

exception = self.assertRaises(
subprocess.CalledProcessError, self.run_snapcraft,
['pull'], 'build-snap-grammar-fail')

self.assertThat(exception.output, Contains(
"Unable to satisfy 'on other-arch', failure forced"))
3 changes: 3 additions & 0 deletions schema/snapcraft.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,9 @@ properties:
stage-packages:
$ref: "#/definitions/grammar-array"
default: [] # For some reason this doesn't work if in the ref
build-snaps:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

¡viva! this is the end of compiling go1.8 on every build.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go 1.9 now, already...

$ref: "#/definitions/grammar-array"
default: [] # For some reason this doesn't work if in the ref
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hum, I haven't seen this before. It's ok to leave a comment, but it needs more information.
What doesn't work, specifically? I guess this is a question for @kyrofa.

Copy link
Contributor

@kyrofa kyrofa Aug 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I initially wrote the grammar back in the day I wasn't sure what was happening, and decided it was a jsonschema quirk. However, now that you ask I looked into it with a fresh mind and I think I know what's actually happening: a jsonschema ref isn't expanded in any fancy way by the YAML (which is why I'd like to use a recursive YAML anchor for this, but jsonschema causes things to explode that way since it wants to walk the whole thing). This means that we can't actually access the chunk of YAML representing that bit of the schema very easily in code. Like we do here to expand defaults. So this is kind of an ugly hack to allow specifying the defaults to continue actually resulting in defaults. Thankfully that's the only field for which I imagine we'll need this hack.

build-packages:
$ref: "#/definitions/grammar-array"
default: [] # For some reason this doesn't work if in the ref
Expand Down
3 changes: 3 additions & 0 deletions snapcraft/_baseplugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,16 @@ def stage_packages(self, value):

def __init__(self, name, options, project=None):
self.name = name
self.build_snaps = []
self.build_packages = []
self._stage_packages = []

with contextlib.suppress(AttributeError):
self._stage_packages = options.stage_packages.copy()
with contextlib.suppress(AttributeError):
self.build_packages = options.build_packages.copy()
with contextlib.suppress(AttributeError):
self.build_snaps = options.build_snaps.copy()

self.project = project
self.options = options
Expand Down
3 changes: 3 additions & 0 deletions snapcraft/internal/lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ def execute(step, project_options, part_names=None):
if installed_packages is None:
raise ValueError(
'The repo backend is not returning the list of installed packages')

repo.snaps.install_snaps(config.build_snaps)

os.makedirs(_SNAPCRAFT_INTERNAL_DIR, exist_ok=True)
with open(os.path.join(_SNAPCRAFT_INTERNAL_DIR, 'state'), 'w') as f:
f.write(yaml.dump(states.GlobalState(installed_packages)))
Expand Down
2 changes: 2 additions & 0 deletions snapcraft/internal/pluginhandler/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,12 @@ def mark_pull_done(self):

# Add the annotated list of build packages
part_build_packages = self._part_properties.get('build-packages', [])
part_build_snaps = self._part_properties.get('build-snaps', [])

self.mark_done('pull', states.PullState(
pull_properties, part_properties=self._part_properties,
project=self._project_options, stage_packages=self.stage_packages,
build_snaps=part_build_snaps,
build_packages=part_build_packages,
source_details=self.source_handler.source_details
))
Expand Down
12 changes: 7 additions & 5 deletions snapcraft/internal/project_loader/_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ def __init__(self, project_options=None):
if project_options is None:
project_options = snapcraft.ProjectOptions()

self.build_snaps = set()
self.build_tools = []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have I said how much I hate this build_tools? :D
I'm just saying it again to put myself pressure to fix it.

self._project_options = project_options

Expand All @@ -115,11 +116,12 @@ def __init__(self, project_options=None):
self.build_tools = grammar_processor.get_build_packages()
self.build_tools |= set(project_options.additional_build_packages)

self.parts = PartsConfig(self.data,
self._project_options,
self._validator,
self.build_tools,
self.snapcraft_yaml_path)
self.parts = PartsConfig(parts=self.data,
project_options=self._project_options,
validator=self._validator,
build_snaps=self.build_snaps,
build_tools=self.build_tools,
snapcraft_yaml=self.snapcraft_yaml_path)

if 'architectures' not in self.data:
self.data['architectures'] = [self._project_options.deb_arch]
Expand Down
6 changes: 4 additions & 2 deletions snapcraft/internal/project_loader/_parts_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,14 @@

class PartsConfig:

def __init__(self, parts, project_options, validator, build_tools,
snapcraft_yaml):
def __init__(self, *, parts, project_options, validator,
build_snaps, build_tools, snapcraft_yaml):
self._snap_name = parts['name']
self._confinement = parts['confinement']
self._parts_data = parts.get('parts', {})
self._project_options = project_options
self._validator = validator
self.build_snaps = build_snaps
self.build_tools = build_tools
self._snapcraft_yaml = snapcraft_yaml

Expand Down Expand Up @@ -192,6 +193,7 @@ def load_part(self, part_name, plugin_name, part_properties):
stage_packages_repo=stage_packages_repo,
grammar_processor=grammar_processor)

self.build_snaps |= grammar_processor.get_build_snaps()
self.build_tools |= grammar_processor.get_build_packages()

if part.source_handler and part.source_handler.command:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.

from snapcraft.internal.project_loader import grammar
from snapcraft.internal import repo


class PartGrammarProcessor:
Expand Down Expand Up @@ -57,12 +58,23 @@ def __init__(self, *, plugin, properties, project_options, repo):
self._project_options = project_options
self._repo = repo

self._build_snap_grammar = getattr(plugin, 'build_snaps', [])
self.__build_snaps = set()

self._build_package_grammar = getattr(plugin, 'build_packages', [])
self.__build_packages = set()

self._stage_package_grammar = getattr(plugin, 'stage_packages', [])
self.__stage_packages = set()

def get_build_snaps(self):
if not self.__build_snaps:
self.__build_snaps = grammar.process_grammar(
self._build_snap_grammar, self._project_options,
repo.snaps.SnapPackage.is_valid_snap)

return self.__build_snaps

def get_build_packages(self):
if not self.__build_packages:
self.__build_packages = grammar.process_grammar(
Expand Down
1 change: 1 addition & 0 deletions snapcraft/internal/repo/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

from snapcraft.internal.errors import MissingCommandError
from . import errors # noqa
from . import snaps # noqa
from . import _platform
from ._base import BaseRepo # noqa
from ._base import fix_pkg_config # noqa
Expand Down
18 changes: 18 additions & 0 deletions snapcraft/internal/repo/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,21 @@ class UnpackError(RepoError):

def __init__(self, package):
super().__init__(package=package)


class SnapInstallError(RepoError):

fmt = ('Error while installing snap {snap_name!r} from channel '
'{snap_channel!r}')

def __init__(self, *, snap_name, snap_channel):
super().__init__(snap_name=snap_name, snap_channel=snap_channel)


class SnapRefreshError(RepoError):

fmt = ('Error while refreshing snap {snap_name!r} to channel '
'{snap_channel!r}')

def __init__(self, *, snap_name, snap_channel):
super().__init__(snap_name=snap_name, snap_channel=snap_channel)
Loading