Create Plainbox Provider plugin #364

jocave · 2016-03-03T10:13:54Z

lp: #1552369

It would be useful if this plugin could be released on to the 1.x series as well.

snappy-m-o · 2016-03-03T10:13:56Z

Can one of the admins verify this patch?

snappy-m-o · 2016-03-03T10:13:57Z

Can one of the admins verify this patch?

zyga · 2016-03-04T11:18:15Z

snapcraft/plugins/plainbox_provider.py

+deb http://${security}.ubuntu.com/${suffix} ${release}-security main restricted
+deb http://${security}.ubuntu.com/${suffix} ${release}-security universe
+deb http://${security}.ubuntu.com/${suffix} ${release}-security multiverse
+deb http://ppa.launchpad.net/checkbox-dev/ppa/ubuntu ${release} main


I'd use stable PPA here really or make this customizable with an explicit knob.

jocave · 2016-03-04T17:27:52Z

I've updated the code to make it possible to disable the Checkbox Development PPA. Also, switched to using an explicit namespace (kept the provider name "Simple" though").

come-maiz · 2016-03-16T15:30:01Z

OK to test

come-maiz · 2016-03-18T17:51:28Z

retest this please

coveralls · 2016-03-21T11:43:01Z

Coverage remained the same at 95.213% when pulling 1b738a2 on jocave:plainbox-provider-plugin into 3cc0049 on ubuntu-core:master.

coveralls · 2016-03-21T12:46:32Z

Coverage remained the same at 95.213% when pulling 2f45197 on jocave:plainbox-provider-plugin into 3cc0049 on ubuntu-core:master.

coveralls · 2016-03-21T14:31:22Z

Coverage remained the same at 95.213% when pulling db13ab5 on jocave:plainbox-provider-plugin into 3cc0049 on ubuntu-core:master.

jocave · 2016-03-21T16:27:46Z

retest this please

come-maiz · 2016-03-21T17:45:42Z

retest this please

come-maiz · 2016-03-21T17:46:59Z

add to whitelist

come-maiz · 2016-03-21T19:20:26Z

retest this please

coveralls · 2016-03-23T11:19:28Z

Coverage decreased (-0.1%) to 95.325% when pulling 27bacbd on jocave:plainbox-provider-plugin into 889fe87 on ubuntu-core:master.

come-maiz · 2016-03-23T15:09:47Z

snapcraft/plugins/plainbox_provider.py

+        super().__init__(name, options)
+        self.build_packages.extend(['python3-plainbox', 'intltool'])
+        if self.options.checkbox_dev_ppa:
+            self._PLUGIN_STAGE_SOURCES = self._DEV_PPA_PLUGIN_STAGE_SOURCES


this _PLUGIN_STAGE_SOURCES is not a constant, so it shouldn't be all capital letters.
_plugin_stage_sources.

sorry, not from your branch. After you merge I'll propose something to change it.

For you, it seems safer to overwrite the PLUGIN_STAGE_SOURCES @Property, instead of dealing with the internal attribute.

Yes, don't take advantage of internal implementation details which are subject to change 😉

come-maiz · 2016-03-23T15:18:12Z

👍 from me.
Can you please remove the icon? It's not needed, and I think it's better to keep the test snaps with only the required bits.

sergiusens · 2016-03-23T15:23:20Z

integration_tests/snaps/simple-plainbox-provider/snapcraft.yaml

+description: |
+    Create a snap of a very simple plainbox that could
+    then be used by a checkbox application
+icon: icon.png


remove this entry please

sergiusens · 2016-03-23T15:23:40Z

git rm integration_tests/snaps/simple-plainbox-provider/icon.png

jocave · 2016-03-23T15:23:55Z

Pushed commit with icon file removed from simple-plainbox-provider integration test.

sergiusens · 2016-03-23T15:25:18Z

git mv integration_tests/test_plainbox_provider_plugin.py integration_tests/test_plugin_plainbox_provider.py

coveralls · 2016-03-23T15:26:40Z

Coverage decreased (-0.1%) to 95.325% when pulling 885ef55 on jocave:plainbox-provider-plugin into 889fe87 on ubuntu-core:master.

sergiusens · 2016-03-23T15:27:45Z

snapcraft/plugins/plainbox_provider.py

+        schema['properties']['checkbox-dev-ppa'] = {
+            'type': 'boolean',
+            'default': 'true',
+        }


since you aren't overriding properties it seems you are making use of the common source options, can you add that piece of information to the doc string?

sergiusens · 2016-03-23T15:32:53Z

Thanks for this, I have a bunch of open questions in there though.

Also, does this rely on python provided by the os snap? It is fine from my PoV, just want to make sure.

sergiusens · 2016-03-23T15:42:08Z

Given the previous comment I would like to see an example and an example_test (which would run on a snappy system)

come-maiz · 2016-03-23T15:58:52Z

The example test would be so nice, indeed.
@jocave take a look at the examples_tests dir. Give me a shout if you need a hand with that one.

jocave · 2016-03-23T16:01:11Z

Thank you for the reviews, I will put an example together and try and address the questions ASAP.

coveralls · 2016-03-24T14:22:46Z

Coverage decreased (-0.1%) to 95.739% when pulling 318332b on jocave:plainbox-provider-plugin into 3c2081a on ubuntu-core:master.

coveralls · 2016-03-24T14:31:07Z

Coverage decreased (-0.1%) to 95.739% when pulling 0b894a7 on jocave:plainbox-provider-plugin into 3c2081a on ubuntu-core:master.

coveralls · 2016-03-24T17:48:13Z

Coverage decreased (-0.1%) to 95.744% when pulling 19140e7 on jocave:plainbox-provider-plugin into 56894e8 on ubuntu-core:master.

coveralls · 2016-03-24T18:16:41Z

Coverage decreased (-0.1%) to 95.744% when pulling 6b1bde0 on jocave:plainbox-provider-plugin into 56894e8 on ubuntu-core:master.

coveralls · 2016-03-24T20:17:49Z

Coverage decreased (-0.1%) to 95.744% when pulling 5b07527 on jocave:plainbox-provider-plugin into 56894e8 on ubuntu-core:master.

coveralls · 2016-03-29T09:00:42Z

Coverage decreased (-0.1%) to 95.753% when pulling aad99ce on jocave:plainbox-provider-plugin into b18e4e9 on ubuntu-core:master.

lp: #1552369 Integration tests provided that check the plugin can be used in several common Checkbox use cases.

jocave · 2016-05-09T09:14:38Z

I have added integration tests to cover a set of what I think are expected use cases for this plugin. One of these fails with a python import error for module 'plainbox'. This is provided by the package python3-plainbox which is explicitly added to the build-package list by the plugin.

I would like to know how to guarantee that 'build-package' works as advertised.

sergiusens · 2016-05-27T18:52:12Z

@jocave build-packages will just land on the host and be used to build the system, there is additional magic to crawl everything ELF and bring in any .so it needs not provided in the snap/part area.

stage-packages however, which is what you use in your examples are a bit tricky, specially when it comes to python as you will need to define a PYTHONPATH for this.

Given this still needs a few twists and turns I am going to close it for now. Feel free to reopen when ready.

jocave force-pushed the plainbox-provider-plugin branch 2 times, most recently from 9c04b9e to cf09fe5 Compare March 3, 2016 16:03

zyga reviewed Mar 4, 2016
View reviewed changes

jocave force-pushed the plainbox-provider-plugin branch from cf09fe5 to 5db0952 Compare March 4, 2016 17:06

jocave force-pushed the plainbox-provider-plugin branch from 5db0952 to 9c19059 Compare March 16, 2016 15:55

jocave force-pushed the plainbox-provider-plugin branch from 9c19059 to 1b738a2 Compare March 21, 2016 11:38

jocave force-pushed the plainbox-provider-plugin branch from 1b738a2 to 2f45197 Compare March 21, 2016 12:42

jocave force-pushed the plainbox-provider-plugin branch from 2f45197 to db13ab5 Compare March 21, 2016 14:27

jocave force-pushed the plainbox-provider-plugin branch from db13ab5 to 27bacbd Compare March 23, 2016 11:16

come-maiz reviewed Mar 23, 2016
View reviewed changes

jocave force-pushed the plainbox-provider-plugin branch from 27bacbd to 885ef55 Compare March 23, 2016 15:23

sergiusens reviewed Mar 23, 2016
View reviewed changes

jocave force-pushed the plainbox-provider-plugin branch 2 times, most recently from e76d48a to 318332b Compare March 24, 2016 14:18

jocave force-pushed the plainbox-provider-plugin branch from 318332b to 0b894a7 Compare March 24, 2016 14:27

jocave force-pushed the plainbox-provider-plugin branch from 0b894a7 to 19140e7 Compare March 24, 2016 17:43

jocave force-pushed the plainbox-provider-plugin branch from 19140e7 to 6b1bde0 Compare March 24, 2016 18:10

jocave force-pushed the plainbox-provider-plugin branch from 6b1bde0 to 5b07527 Compare March 24, 2016 20:14

jocave force-pushed the plainbox-provider-plugin branch from 5b07527 to aad99ce Compare March 29, 2016 08:57

jocave force-pushed the plainbox-provider-plugin branch from aad99ce to 6d62c48 Compare May 6, 2016 13:28

Create Plainbox Provider plugin

af05e90

lp: #1552369 Integration tests provided that check the plugin can be used in several common Checkbox use cases.

jocave force-pushed the plainbox-provider-plugin branch from 6d62c48 to af05e90 Compare May 6, 2016 16:05

sergiusens closed this May 27, 2016

Create Plainbox Provider plugin #364

Create Plainbox Provider plugin #364

Conversation

jocave commented Mar 3, 2016

snappy-m-o commented Mar 3, 2016

snappy-m-o commented Mar 3, 2016

zyga Mar 4, 2016

Choose a reason for hiding this comment

jocave commented Mar 4, 2016

come-maiz commented Mar 16, 2016

come-maiz commented Mar 18, 2016

coveralls commented Mar 21, 2016

coveralls commented Mar 21, 2016

coveralls commented Mar 21, 2016

jocave commented Mar 21, 2016

come-maiz commented Mar 21, 2016

come-maiz commented Mar 21, 2016

come-maiz commented Mar 21, 2016

coveralls commented Mar 23, 2016

come-maiz Mar 23, 2016

Choose a reason for hiding this comment

come-maiz Mar 23, 2016

Choose a reason for hiding this comment

sergiusens Mar 23, 2016

Choose a reason for hiding this comment

come-maiz commented Mar 23, 2016

sergiusens Mar 23, 2016

Choose a reason for hiding this comment

sergiusens commented Mar 23, 2016

jocave commented Mar 23, 2016

sergiusens commented Mar 23, 2016

coveralls commented Mar 23, 2016

sergiusens Mar 23, 2016

Choose a reason for hiding this comment

sergiusens commented Mar 23, 2016

sergiusens commented Mar 23, 2016

come-maiz commented Mar 23, 2016

jocave commented Mar 23, 2016

coveralls commented Mar 24, 2016

coveralls commented Mar 24, 2016

coveralls commented Mar 24, 2016

coveralls commented Mar 24, 2016

coveralls commented Mar 24, 2016

coveralls commented Mar 29, 2016

jocave commented May 9, 2016

sergiusens commented May 27, 2016