Add new SnapFile to disentangle SnapPart

[mvo5]

Before SnapPart was a mix of local installed snaps and snap files
that can be installed. This branch separates them in a cleaner way.

This should allow us to split the big Part interface into something
like an installed, downloadable and installable interface (that
will need more work obviously).

[mvo5]

retest this please

[mvo5]

Fwiw, the intergration test should be ok again once "bugfix/oem-compat" is merged into mater.

[chipaca]

it returns whether it is active.

[chipaca]

i'm pretty sure it also returns if it isn't active.

[chipaca]

👍

[mvo5]

retest this please

[elopio]

The integration tests fail because it is printing the "Signature check failed" message twice:

... value string = "" +
... "Installing integration-tests/data/snaps/basic-framework/basic-framework_1.0_all.snap\n" +
... "2016/01/05 14:13:27.675681 verify.go:85: Signature check failed, but installing anyway as requested\n" +
... "2016/01/05 14:13:27.677901 verify.go:85: Signature check failed, but installing anyway as requested\n" +
... "Name Date Version Developer \n" +
... "ubuntu-core 2016-01-05 306 ubuntu \n" +
... "basic-framework 2016-01-05 IJUNHFXEOfdL \n"
... regex string = "" +
... "(?ms)Installing integration-tests/data/snaps/basic-framework/basic-framework_1.0_all.snap\n" +
... ".Signature check failed, but installing anyway as requested\n" +
... "Name +Date +Version +Developer \n" +
... ".^basic-framework +.* +.* +sideload \n" +
... "."

Is that something you caused on your branch?

BTW, I love that you are tagging your branch names with refactor/bug 👏

[mvo5]

@elopio Thanks! Nice that the integration tests caught this issue. Verify() was indeed called twice, I fixed that now.

[niemeyer]

Copying the online conversation here, for context and general awareness.

Gustavo Niemeyer, [07.01.16 13:52]
@mvo5 Cool, so the question is similar to yesterday's.. just trying to figure what the grand vision is

Gustavo Niemeyer, [07.01.16 13:52]
@mvo5 That means we'll end up with snap.File an SnapFile

Gustavo Niemeyer, [07.01.16 13:52]
@mvo5 Which is likely not the end goal

Gustavo Niemeyer, [07.01.16 13:52]
@mvo5 So what are we heading towards?

Michael Vogt, [07.01.16 13:55]
@niemeyer hm, so I think snap.File is ok, not what we have right now of course but an interface to interact with the snap files that can be used to inspect a snap file

Michael Vogt, [07.01.16 13:55]
@niemeyer inspect and install it seems is what we need for those

Gustavo Niemeyer, [07.01.16 13:56]
nod

Michael Vogt, [07.01.16 13:57]
@niemeyer so the vision is that the interface becomes much smaller (Info() and maybe Install()) and maybe something for the assertions like generateHash or somesuch

Michael Vogt, [07.01.16 13:57]
@niemeyer does that make sense? or do you have some ideas what we else we could do?

Gustavo Niemeyer, [07.01.16 13:58]
@mvo5 That makes sense.. and what about SnapFile?

Michael Vogt, [07.01.16 14:01]
@niemeyer oh, sorry, I was a moment confused. I think SnapFile is just an intermediate step to untangle. maybe we can even skip this step but it seems like the mishmash we have right now that mixes local snaps and snap files into the single Snap is worse

Michael Vogt, [07.01.16 14:01]
@niemeyer the next step should be to kill the Part interface and replace it with smaller bits

Michael Vogt, [07.01.16 14:02]
@niemeyer a remote snap would just have a download method, a file just hte install method, an installed one just remove() etc

Michael Vogt, [07.01.16 14:02]
@niemeyer thats a bit of a refactor task because the parts interface is used all around

Gustavo Niemeyer, [07.01.16 14:02]
@mvo5 Right, understood, and the overall direction feels good.. just trying to understand what the plan is really

Gustavo Niemeyer, [07.01.16 14:02]
@mvo5 These should probably not even be methods on these types

Michael Vogt, [07.01.16 14:02]
@niemeyer @chipaca suggested to just rewrite it all from scratch, its tempting but I'm a bit hesitant about rewrites

Gustavo Niemeyer, [07.01.16 14:03]
@mvo5 I mean, "Install" will definitely involve a series of generic steps that any given snap format must conform to

Gustavo Niemeyer, [07.01.16 14:03]
@mvo5 So I think "Install" is more a function of the system than that of the format

Michael Vogt, [07.01.16 14:03]
/me nods

Gustavo Niemeyer, [07.01.16 14:04]
@mvo5 We're still lacking one abstraction which isn't entirely clear to me how we're going to introduce it, but it's clear that we need it

Gustavo Niemeyer, [07.01.16 14:04]
@mvo5 We need a representation of "the snappy system" which isn't just the global state

Gustavo Niemeyer, [07.01.16 14:04]
@mvo5 That ties in with the conversation with @chipaca, around a transactional system

Gustavo Niemeyer, [07.01.16 14:05]
@mvo5 That seems to be like the right (surrounding) area for the install, remove, update, etc.. functionality

Michael Vogt, [07.01.16 14:05]
@niemeyer indeed, that makes sense

Gustavo Niemeyer, [07.01.16 14:06]
@mvo5 So the types that represent a snap file, or an installed snap, should hold only the part of the functionality that they themselves are responsible for.. when we're talking about an installation, that's a much wider operation, that requires collaboration of the system itself.. doesn't feel right to ask each of the types to know how to do that

Michael Vogt, [07.01.16 14:07]
@niemeyer agreed

Gustavo Niemeyer, [07.01.16 14:07]
Okay.. that's great.. so, back to the branch, what's the next step for SnapFile?

Michael Vogt, [07.01.16 14:10]
@niemeyer I need to get back to the branch and look at the details. once refactor I ponder is to kill packageYaml and use the new snap.Info instead. as for SnapFile the next step could be to look into slashing the Part interface. We can put the branch on hold until I worked on the next steps if you want. this way the general direction becomes cleaner. plus I could incoperate your feedback into those branch(es), i.e. move out "Install(), Remove" into this new SnappySystemOverlord

Michael Vogt, [07.01.16 14:10]
@niemeyer does that sound reasonable? or any alternative suggesitons :) ?

John Lenton, [07.01.16 14:11]
i wouldn't say i suggested rewriting it from scratch; i think i mentioned that we're changing so much, it might turn out to be less work (but there's a lot more risk)

Michael Vogt, [07.01.16 14:12]
ups, sorry for misrepresenting that then

Gustavo Niemeyer, [07.01.16 14:12]
@mvo5 The initial work you did there sounds fine, and will probably make these follow up steps more comfortable, so I'm also happy to see this going in. It would be great to see these additional steps taking place soon, though, rather than leaving that intermediate state for too long. The only thing I'd like to trouble you with, if that's okay, is to have a short note above the SnapFile type that describes what these next steps are

Michael Vogt, [07.01.16 14:14]
@niemeyer thanks, indeed, that is sensible. I will get to this as soon as I finished the work I am doing right now with the integration tests for all-snaps

Gustavo Niemeyer, [07.01.16 14:19]
@mvo5 In that description, it would be nice to cover the current state of affairs. E.g. what is a SnapPart, a SnapFile, a snap.File, and a squashfs.Snap (did I miss any?)

Gustavo Niemeyer, [07.01.16 14:20]
@mvo5 Explaining those and the relationship between them will likely make more clear the mess we need to clean up

[niemeyer]

So, per the conversation, can we please have a comment here explaining what each of these are, and the relationship between them (did I miss any?):

• SnapFile
• SnapPart
• snap.File
• squashfs.Snap

and then what the next few steps to disentangle them are.

As discussed, we shouldn't leave this intermediate messy state around for too long, or it'll continue to spread its legs wider. The comment will ensure everybody is aligned and prevent further damage while this doesn't take place.

[chipaca]

so... what's the state of this?

[mvo5]

@chipaca This is still something we should merge IMO, its part of the road towards simplifying the system.

[chipaca]

@niemeyer I think this one needs your explicit thumbs-up

[niemeyer]

<explicit 👍>

[mvo5]

retest this please