Snapcraft

Merge lp://qastaging/~mvo/snapcraft/copy-plugin into lp://qastaging/~snappy-dev/snapcraft/core

  1. copy-plugin
  2. Merge into core
Proposed by Michael Vogt
Status: Merged
Merged at revision: 105
Proposed branch: lp://qastaging/~mvo/snapcraft/copy-plugin
Merge into: lp://qastaging/~snappy-dev/snapcraft/core
Prerequisite: lp://qastaging/~elopio/snapcraft/test_tmp_cwd
Diff against target: 206 lines (+133/-3)
9 files modified
examples/py3-project/meta/package.yaml (+1/-1)
examples/py3-project/snapcraft.yaml (+4/-0)
integration-tests/data/simple-copy/snapcraft.yaml (+5/-0)
integration-tests/data/simple-copy/src (+1/-0)
integration-tests/units/jobs.pxu (+9/-0)
plugins/copy.yaml (+4/-0)
snapcraft/plugins/copy.py (+37/-0)
snapcraft/tests/test_copy_plugin.py (+69/-0)
snapcraft/tests/test_plugin.py (+3/-2)
To merge this branch: bzr merge lp://qastaging/~mvo/snapcraft/copy-plugin
Reviewer Review Type Date Requested Status
Michael Terry (community) Needs Fixing
Review via email: mp+265553@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2015-07-14.

Description of the change

This branch adds a copy plugin. It will copy artifacts into the snap dir. It makes the python3 example a fully working sha3sum that fetches the sha3 lib from github.

To post a comment you must log in.
Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

Feels weird for this to be an add-on to the python3 branch, but I understand how we got here. :)

Yay for tests! :)

Some questions inline.

Revision history for this message
Michael Terry (mterry) : Posted in a previous version of this proposal
Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

Merge conflict now, plus some comments.

Revision history for this message
Michael Vogt (mvo) wrote : Posted in a previous version of this proposal

Thanks. I fixed the merge conflicts and addressed the comments.

Revision history for this message
Michael Vogt (mvo) wrote : Posted in a previous version of this proposal

I'm happy to tell that I did a successful end-to-end test with the py3-example:

$ cd examples/py3-project
$ ../../bin/snapcraft all
created spongeshaker_0_all.snap

which I copied into my kvm snappy and there it works flawlessly:

(amd64)ubuntu@localhost:~/apps/spongeshaker.sideload/0$ spongeshaker.sha3sum $(pwd)/foo
6b4e03423667dbb73b6e15454f0eb1abd4597f9a1b078e3f5b5a6bc7

(within the limits of the confinement of course).

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

Inline comments, looking nice :)

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

The tempdir stuff got upstaged a little by https://code.launchpad.net/~elopio/snapcraft/test_tmp_cwd/+merge/265352

Maybe rebase?

Revision history for this message
Ted Gould (ted) wrote : Posted in a previous version of this proposal

Not to stop this MR, but when I pulled in both this and the focused-assemble branch I had to change the simple copy test to:

id: snapcraft/normal/simple-copy
plugin: shell
estimated_duration: 0.5
command:
    set -ex
    cp -rT $PLAINBOX_PROVIDER_DATA/simple-copy .
    snapcraft stage
    test "$(cat ./stage/dst)" = "I got copied"

Who ever gets merged first, the other branch will have to do that.

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

mvo, you merged trunk but didn't address the issues I mentioned on the last revision (typo and a logic error). Just didn't want those to get lost, since LP hides them after the branch gets updated. :-/

review: Needs Fixing
Revision history for this message
Michael Vogt (mvo) wrote : Posted in a previous version of this proposal

Hey Mike, thanks a bunch, for some reason I missed the earlier inline comments. I addressed them now and merged https://code.launchpad.net/~elopio/snapcraft/test_tmp_cwd/+merge/265352 for the tempdir handling.

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

You have to rebase / update the pre-requisate against test_tmp_cwd.

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

(but looks good besides)

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

Oh, you did! I missed that ;) Ignore me. Will go look at https://code.launchpad.net/~mvo/snapcraft/copy-plugin/+merge/265553

Revision history for this message
Michael Terry (mterry) wrote :

Looks good, thanks mvo!

review: Approve
Revision history for this message
Michael Terry (mterry) wrote :

mvo, I was about to manually merge, but got these failures when running ./runtests.sh:

snapcraft/tests/test_copy_plugin.py:34:26: E222 multiple spaces after operator
snapcraft/tests/test_copy_plugin.py:38:9: E265 block comment should start with '# '
snapcraft/tests/test_copy_plugin.py:39:9: E265 block comment should start with '# '
snapcraft/tests/test_copy_plugin.py:34:26: E222 multiple spaces after operator
snapcraft/tests/test_copy_plugin.py:38:9: E265 block comment should start with '# '
snapcraft/tests/test_copy_plugin.py:39:9: E265 block comment should start with '# '

Revision history for this message
Michael Terry (mterry) :
review: Needs Fixing
Revision history for this message
Michael Terry (mterry) wrote :

(I'll just fix this up as I manually merge anyway.)

Revision history for this message
Michael Vogt (mvo) wrote :

Hi Mike, thanks a lot for the review! I fixed the pep8 issues now in r99

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches