Add new info.Info interface and add minimal package yaml parser #271

Merged
merged 8 commits into from Jan 6, 2016

Conversation

Projects
None yet
3 participants
Collaborator

mvo5 commented Dec 22, 2015

This branch is motivated by the need to have the "type" of a snap available in ubuntu-device-flash so that I can infer if its a kernel or an OS snap. When those are detected u-d-f needs to setup the boot variables accordingly.

It also moves pkg.File:Open into its own package to avoid circular dependencies. If you prefer that in a separate branch I can do that.

I think this change has some potential and we could use it as a start to get packageYaml out of the main "snappy" package.

mvo5 added some commits Dec 22, 2015

Add new info.Info interface and add package yaml parser
This will allow us to use this interface instead of doing
the parsing all in the "snappy" package.
Collaborator

mvo5 commented Dec 22, 2015

retest this please

Member

chipaca commented Dec 22, 2015

I like what you're doing here. I don't like the name opener, because once you've imported it's not clear that all it opens are packages :-) but I don't have a better suggestion.

mvo5 added some commits Dec 22, 2015

Collaborator

mvo5 commented Dec 22, 2015

Thanks @chipaca ! I renamed it to snapfile.Open now and moved the pkg.File in there too (but I can easily revert that last commit if you prefer it as pkg.File).

Collaborator

mvo5 commented Dec 22, 2015

Fwiw, the integration tests shold be ok again once bugfix/oem-compat is merged into master.

Collaborator

mvo5 commented Jan 4, 2016

Once this has landed we can land a branch for ubuntu-device-flash in 16.04 that can create all-snap images (and only all-snap images).

Collaborator

mvo5 commented Jan 4, 2016

And once we have u-d-f in 16.04 with all-snap support the integration-tests can be updated to all-snap images too.

pkg/info/info_package_yaml.go
+ Type pkg.Type
+}
+
+// SnapInfoPackageYaml implements the meta/snap.yaml data
@chipaca

chipaca Jan 5, 2016

Member

snap.yaml already?

@mvo5

mvo5 Jan 5, 2016

Collaborator

Not yet, but soon :) the whole layout is anticipating snap.yaml

Member

chipaca commented Jan 5, 2016

👍

Contributor

niemeyer commented Jan 6, 2016

For the record, we discussed this online and @mvo5 is working on some changes right now.

mvo5 added some commits Jan 6, 2016

Collaborator

mvo5 commented Jan 6, 2016

The branch is updated now based on the feedback from @niemeyer (thanks!). Its a bit long unfortunately mostly due to the renames. Feedback (very) welcome.

+}
+
+// Backend implements a specific snap format
+type Backend struct {
@niemeyer

niemeyer Jan 6, 2016

Contributor

Looks like this should be private for now. I'd also take the chance we have a good term for it after "file format" (e.g. "snapFormat" and "RegisterFormat", or something similar of your liking) and reserve the generic "backend" wildcard term for something else.

+)
+
+// NewFromPackageYaml creates a new info based on the given packageYaml
+func NewFromPackageYaml(yamlData []byte) (*Info, error) {
@niemeyer

niemeyer Jan 6, 2016

Contributor

This should probably be snap.InfoFromPackageYaml now that it sits under the snap package (it's not a "new snap").

Contributor

niemeyer commented Jan 6, 2016

Just a couple of trivials now.. really like the outcome, though! LGTM

Contributor

niemeyer commented Jan 6, 2016

This changed enough that would be good for @chipaca to have another look.

@@ -48,6 +49,12 @@ var (
ErrMemberNotFound = errors.New("member not found")
)
+func init() {
+ snap.RegisterBackend([]byte("!<arch>\ndebian"), func(path string) (snap.File, error) {
+ return Open(path)
@chipaca

chipaca Jan 6, 2016

Member

why isn't this

    snap.RegisterBackend([]byte("!<arch>\ndebian"), Open)

?

@@ -1,7 +1,7 @@
// -*- Mode: Go; indent-tabs-mode: t -*-
/*
- * Copyright (C) 2014-2015 Canonical Ltd
+ * Copyyright (C) 2014-2015 Canonical Ltd
@chipaca

chipaca Jan 6, 2016

Member

ccooppyyrriigghhtt
also maybe +2016.

Member

chipaca commented Jan 6, 2016

👍

chipaca added a commit that referenced this pull request Jan 6, 2016

Merge pull request #271 from mvo5/refactor/pkg-info
Add new info.Info interface and add minimal package yaml parser

@chipaca chipaca merged commit 43cb8a9 into snapcore:master Jan 6, 2016

3 checks passed

Integration tests Success 54 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.03%) to 67.568%
Details

@mvo5 mvo5 referenced this pull request Jan 7, 2016

Merged

Refactor/snap info fixes2 #295

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