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

lifecycle: automatically clean dirty steps #2145

Merged
merged 1 commit into from
May 29, 2018
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 29 additions & 0 deletions snapcraft/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import base64
import configparser
import enum
import io
import logging
import os
Expand All @@ -33,6 +34,13 @@
logger = logging.getLogger(__name__)


@enum.unique
class OutdatedStepAction(enum.Enum):
# Would like to use enum.auto(), but it's only available in >= 3.6
ERROR = 1
CLEAN = 2


class CLIConfig:
"""Hold general options affecting the CLI.

Expand Down Expand Up @@ -128,6 +136,27 @@ def get_sentry_send_always(self) -> bool:
# Anything but "true" for string_value is considered False.
return string_value == 'true'

def set_outdated_step_action(self, action: OutdatedStepAction) -> None:
"""Setter to define action to take if outdated step is encountered.

:param OutdatedStepAction value: The action to take
"""
self._set_option(
'Lifecycle', 'outdated_step_action', action.name.lower())

def get_outdated_step_action(self) -> OutdatedStepAction:
"""Getter to define action to take if outdated step is encountered.

:returns: The action to take
:rtype: OutdatedStepAction.
"""
action = self._get_option('Lifecycle', 'outdated_step_action')
if action:
return OutdatedStepAction[action.upper()]
else:
# Error by default
return OutdatedStepAction.ERROR
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like we want to make the default the new behavior and maybe warn (a couple of times) about how to set the old behavior back.

Copy link
Contributor Author

@kyrofa kyrofa May 29, 2018

Choose a reason for hiding this comment

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

I guess I was thinking our team could flip that switch for a release or so to make sure it doesn't bite before flipping it for everyone, but yeah, if you want to flip it now and warn that's fine.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, if we flip ourselves in a near future, that is fine as well, please create a bug task to flip the switch.



class Config(object):
"""Hold configuration options in sections.
Expand Down
45 changes: 28 additions & 17 deletions snapcraft/internal/lifecycle/_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import yaml

import snapcraft
from snapcraft import config
from snapcraft.internal import (
common,
errors,
Expand Down Expand Up @@ -158,14 +159,16 @@ def _init_run_states(self):

for part in self.config.all_parts:
steps_run[part.name] = set()
for step in common.COMMAND_ORDER:
dirty_report = part.get_dirty_report(step)
if dirty_report:
self._handle_dirty(part, step, dirty_report)
elif not (part.should_step_run(step)):
steps_run[part.name].add(step)
part.notify_part_progress('Skipping {}'.format(step),
'(already ran)')
with config.CLIConfig() as cli_config:
for step in common.COMMAND_ORDER:
dirty_report = part.get_dirty_report(step)
if dirty_report:
self._handle_dirty(
part, step, dirty_report, cli_config)
elif not (part.should_step_run(step)):
steps_run[part.name].add(step)
part.notify_part_progress('Skipping {}'.format(step),
'(already ran)')

return steps_run

Expand Down Expand Up @@ -244,27 +247,35 @@ def _create_meta(self, step, part_names):
self.config.original_snapcraft_yaml,
self.config.validator.schema)

def _handle_dirty(self, part, step, dirty_report):
def _handle_dirty(self, part, step, dirty_report, cli_config):
dirty_action = cli_config.get_outdated_step_action()
if step not in constants.STEPS_TO_AUTOMATICALLY_CLEAN_IF_DIRTY:
raise errors.StepOutdatedError(
step=step, part=part.name,
dirty_properties=dirty_report.dirty_properties,
dirty_project_options=dirty_report.dirty_project_options)
if dirty_action == config.OutdatedStepAction.ERROR:
raise errors.StepOutdatedError(
step=step, part=part.name,
dirty_properties=dirty_report.dirty_properties,
dirty_project_options=dirty_report.dirty_project_options)

staged_state = self.config.get_project_state('stage')
primed_state = self.config.get_project_state('prime')

# We need to clean this step, but if it involves cleaning the stage
# step and it has dependents that have been built, we need to ask for
# them to first be cleaned (at least back to the build step).
# step and it has dependents that have been built, the dependents need
# to first be cleaned (at least back to the build step). Do it
# automatically if configured to do so.
index = common.COMMAND_ORDER.index(step)
dependents = self.parts_config.get_dependents(part.name)
if (index <= common.COMMAND_ORDER.index('stage') and
not part.is_clean('stage') and dependents):
for dependent in self.config.all_parts:
if (dependent.name in dependents and
not dependent.is_clean('build')):
raise errors.StepOutdatedError(step=step, part=part.name,
dependents=dependents)
if dirty_action == config.OutdatedStepAction.ERROR:
raise errors.StepOutdatedError(
step=step, part=part.name, dependents=dependents)
else:
dependent.clean(
staged_state, primed_state, 'build',
'(out of date)')

part.clean(staged_state, primed_state, step, '(out of date)')
21 changes: 21 additions & 0 deletions tests/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,27 @@ def test_set_and_get_sentry_send_always(self):
new_cli_config.load()
self.assertThat(new_cli_config.get_sentry_send_always(), Equals(True))

def test_set_and_get_outdated_step_action_with_contextmanager(self):
with config.CLIConfig() as cli_config:
cli_config.set_outdated_step_action(
config.OutdatedStepAction.CLEAN)

with config.CLIConfig() as cli_config:
self.assertThat(
cli_config.get_outdated_step_action(),
Equals(config.OutdatedStepAction.CLEAN))

def test_set_and_get_outdated_step_action(self):
cli_config = config.CLIConfig()
cli_config.set_outdated_step_action(config.OutdatedStepAction.CLEAN)
cli_config.save()

new_cli_config = config.CLIConfig()
new_cli_config.load()
self.assertThat(
new_cli_config.get_outdated_step_action(),
Equals(config.OutdatedStepAction.CLEAN))

def test_set_when_read_only(self):
cli_config = config.CLIConfig(read_only=True)

Expand Down
72 changes: 71 additions & 1 deletion tests/unit/test_lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
)

import snapcraft
from snapcraft import storeapi
from snapcraft import config, storeapi
from snapcraft.file_utils import calculate_sha3_384
from snapcraft.internal import errors, pluginhandler, lifecycle
from snapcraft.internal.lifecycle._runner import _replace_in_part
Expand Down Expand Up @@ -476,6 +476,76 @@ def _fake_dirty_report(self, step):
"The 'stage' step for 'part1' needs to be run again, but "
"'part2' depends on it.\n"))

def test_dirty_steps_can_be_automatically_cleaned(self):
# Set the option to automatically clean dirty steps
with config.CLIConfig() as cli_config:
cli_config.set_outdated_step_action(
config.OutdatedStepAction.CLEAN)

self.make_snapcraft_yaml(
textwrap.dedent("""\
parts:
part1:
plugin: nil
part2:
plugin: nil
after: [part1]
"""))

# Stage dependency
lifecycle.execute('stage', self.project_options, part_names=['part1'])
# Build dependent
lifecycle.execute('build', self.project_options, part_names=['part2'])

# Reset logging since we only care about the following
self.fake_logger = fixtures.FakeLogger(level=logging.INFO)
self.useFixture(self.fake_logger)

def _fake_dirty_report(self, step):
if step == 'stage':
return pluginhandler.DirtyReport({'foo'}, {'bar'})
return None

# Should raise a RuntimeError about the fact that stage is dirty but
# it has dependents that need it.
with mock.patch.object(pluginhandler.PluginHandler, 'get_dirty_report',
_fake_dirty_report):
try:
lifecycle.execute(
'stage', self.project_options, part_names=['part1'])
except errors.StepOutdatedError:
self.fail('Expected the step to automatically be cleaned')

output = self.fake_logger.output.split('\n')
part1_output = [line.strip() for line in output if 'part1' in line]
part2_output = [line.strip() for line in output if 'part2' in line]

self.assertThat(
part1_output,
Equals([
'Skipping pull part1 (already ran)',
'Skipping build part1 (already ran)',
'Skipping cleaning priming area for part1 (out of date) '
'(already clean)',
'Cleaning staging area for part1 (out of date)',
'Staging part1',
]))

self.assertThat(
part2_output,
Equals([
'Skipping cleaning priming area for part2 (out of date) '
'(already clean)',
'Skipping cleaning staging area for part2 (out of date) '
'(already clean)',
'Cleaning build for part2 (out of date)',
'Skipping pull part2 (already ran)',
'Skipping cleaning priming area for part2 (out of date) '
'(already clean)',
'Skipping cleaning staging area for part2 (out of date) '
'(already clean)',
]))

def test_dirty_stage_part_with_unbuilt_dependent(self):
self.make_snapcraft_yaml(
textwrap.dedent("""\
Expand Down