/snapcraft

Support cleaning strip step. #382

Closed
wants to merge 1 commit into
from

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@kyrofa @coveralls @sergiusens
@kyrofa
Member

kyrofa commented Mar 18, 2016

This PR makes progress on LP: #1537786 toward implementing per-step clean rules by adding one for the strip step. The others are currently placeholders and will be implemented in future PRs.
@coveralls

coveralls commented Mar 18, 2016

Coverage Status

Coverage remained the same at 95.288% when pulling cd01294 on kyrofa:feature/1537786/clean_strip_step into 3cc0049 on ubuntu-core:master.
snapcraft/pluginhandler.py
@@ -128,12 +128,11 @@ def is_dirty(self, step):
def should_step_run(self, step, force=False):
return force or self.is_dirty(step)
- def mark_done(self, step):
+ def mark_done(self, step, state={}):
@sergiusens

sergiusens Mar 19, 2016

Collaborator

state=None and then inside check

if not state:
    state = {}

https://pythonconquerstheuniverse.wordpress.com/2012/02/15/mutable-default-arguments/

@kyrofa

kyrofa Mar 21, 2016

Member

Ah, my python knowledge just increased! Thanks for the link 😃 .

snapcraft/pluginhandler.py
+ with open(self._step_state_file('strip'), 'r') as f:
+ state = yaml.load(f.read())
+
+ _clean_migrated_files(state['snap-files'], state['snap-directories'],
@sergiusens

sergiusens Mar 19, 2016

Collaborator

before passing this in, you need to load all parts and check if they migrated the same file to remove it from the wipe list

@kyrofa

kyrofa Mar 21, 2016

Member

Oh right, when writing this I assumed that would imply they were dependent parts, but I forgot that multiple independent parts can supply the same file as long as the contents don't change. Good catch, thank you!

@sergiusens
Collaborator

sergiusens commented Mar 19, 2016

Looks good, just comment about dict and filesets
@kyrofa
Support cleaning strip step. 
This adds a --step argument to the clean command, and implements
per-step clean rules. All per-step clean rules are placeholders
except for the strip clean step, which is done.

LP: #1537786

Signed-off-by: Kyle Fazzari <kyle@canonical.com>
This commit was signed with a verified signature.
@kyrofa kyrofa Kyle Fazzari
GPG key ID: A7D23FB0E778DEDF Learn about signing commits

Some checks were not successful

1 errored, 1 failing, and 2 successful checks
continuous-integration/travis-ci/pr — The Travis CI build could not complete due to an error
Details
@snappy-m-o
Examples tests — 14 tests run, 0 skipped, 4 failed.
Details
@snappy-m-o
autopkgtest — Success No test results found.
Details
coverage/coveralls — Coverage remained the same at 95.29%
Details
4445fd5
@coveralls

coveralls commented Mar 21, 2016

Coverage Status

Coverage remained the same at 95.29% when pulling 4445fd5 on kyrofa:feature/1537786/clean_strip_step into 3cc0049 on ubuntu-core:master.
@kyrofa
Member

kyrofa commented Mar 21, 2016

This PR needs to land to an intermediate branch. Closing.

@kyrofa kyrofa closed this Mar 21, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment