git demo snap update - Bug 1595229

[stub42]

Addresses https://bugs.launchpad.net/snapcraft/+bug/1595229

Adds the 'home' plug to the snap and documentation on how to use it.

[snappy-m-o]

Can one of the admins verify this patch?

[snappy-m-o]

Can one of the admins verify this patch?

[elopio]

thank you :)

[elopio]

*Only access TO the home ...
I think we should explain the type of access. From snapd's docs: Can access non-hidden files in user's $HOME to read/write/lock.

[stub42]

Fixed the grammar. I didn't explain about hidden files as I don't know what snappy considers hidden files - dotfiles seem to work just fine.

[elopio]

there's an error here, but I'm not sure what you were trying to say.
Maybe: Initializing repositories in your...

[stub42]

Fixed

[elopio]

I think it would be a nice touch to use $HOME instead of ~. Don't you think?

[stub42]

Ok. Changed.

[elopio]

I would replace stub with

[stub42]

Changed to ubuntu, per default user name on our images.

[elopio]

What about:
git will have access to files in the user's home directory when you run ...

[stub42]

I like emphasising access is contained by default. Requiring users to manually connect the home interface is a security measure and not a meaningless chore. I'm extended the text a bit to make things clearer.

[elopio]

I like this. I would like to get @didrocks to look at it, because maybe in his plans for the tour and the website there's an explanation of the home interface. In that case we could get rid of this demo, and move everything you wrote to the tour. If it's not in his short-term plans, lets land this one.

And this is missing a test. Take a look at the ones in snaps_tests/demos_tests and see how we can run the connect command, and then verify that the snap is working. Let me know if you need a hand with this.

[snappy-m-o]

Can one of the admins verify this patch?

[snappy-m-o]

Can one of the admins verify this patch?

[didrocks]

Agreed with Leo, this demo is becoming redondant once the tour/documentation will talk more about plugs. However, this isn't going to be released short term, so +1 on shipping something like this.

[sergiusens]

retest this please
ok to test

[snappy-m-o]

Can one of the admins verify this patch?

[snappy-m-o]

Can one of the admins verify this patch?

[elopio]

Thanks for adding the test!
But I was thinking that you could do something like:
self.snappy_testbed.run_command(['mkdir', '-p', '$HOME/tmp/foo'])
self.snappy_testbed.run_command(['/snap/bin/git', 'init', '$HOME/tmp/foo'])

This might need some more work on the run helpers. I'm around to help.

Also, now that this is more extensively tested, would you mind removing the scenario in snaps_tests/demos/tests.py?

[stub42]

Confirming that the 'home' interface does what it claims does not need to be part of this test, so I'd rather leave it like it is than pollute $HOME with temp directories.

Or is this sandboxed somehow and I don't have to worry about cleaning up $HOME safely and reliably?

I have removed the redundant test in snaps_tests/demos/tests.py

[snappy-m-o]

Can one of the admins verify this patch?

[snappy-m-o]

Can one of the admins verify this patch?

[elopio]

The home is not sandboxed. You would have to rm anything you put in there to leave it clean.
We could make it a fixture, to be called by all tests if it becomes ugly.
I am ok with this test checking that the home interface works too. I see this suite as testing the integration with ubuntu and snapd, so checking that if we declare an interface in the snapcraft.yaml, then the snap won't work until we connect that interface is a nice test for me. Of course, there are downsides to this, feel free to disagree :) Also, one can argue that a test called git shouldn't be where we place an interface test, that's why I'm so happy about the tour work, as it documents and tests the features progressively and independently.

I will leave you my 👍 here, as this is clearly way better than before. And I'm waiting to hear your opinion on my comments. We can push many things for later, reporting bugs for them.

[snappy-m-o]

Can one of the admins verify this patch?

[snappy-m-o]

Can one of the admins verify this patch?

[stub42]

The test seems to be legitimately failing, and I haven't got them running locally yet to debug.

[elopio]

Can you please resolve the conflict so we can take a look at the failing test?

[snappy-m-o]

Can one of the admins verify this patch?

[snappy-m-o]

Can one of the admins verify this patch?

[kyrofa]

There seems to be little activity here. I'll go ahead and close this for now-- feel free to reopen once it's ready to progress.