Merge lp:~niemeyer/pyjuju/go-formula-bundle into lp:pyjuju/go

[Gustavo Niemeyer]

Go port must support formula bundles

In addition to formula directories, the Go port
must support formula bundles.

[Kapil Thangavelu (hazmat)]

[0] I don't see any verification of exec bits on scripts which we need to execute hooks from extracted formulas. On unixie platforms, in the python impl, we read off extra bits stored in the zip info entry, which seems to be common for unix zip tools, albeit outside of the zip standard.

[1] ReadBundleBytes reads the whole file into memory, which for naive usage of the amazon api is needed, but that seems like something that would be better exposed via a file path to the zip file and incorporate directly into the provider storage usage, rather than as an api on the bundle. What's the reasoning for exposing this api directly on the bundle?

[2] ReadAt seems to be untested, the comment on why its awesome escapes me. The construction also seems odd, although thats probably my lack of go knowledge, its a public method returning a private type?

[3] Due to the changes in formula-dir (filepath.Rel) i'm unable to run the tests for this.

review: Needs Fixing

[Gustavo Niemeyer (niemeyer)]

[0]

Very good point. Will check it out.

[1]

I don't know what you mean by "exposing the API directly in the bundle". This is a function,
that reads a bundle in memory. I can't see a reason to kill that possibility, even more
considering we can easily decide based on size whether we want to handle something in
memory or not. The documentation also warns about it.

[2]

It's awesome because you have a method on top of a bunch of raw bytes.

It's definitely tested, because ReadBundleBytes depends on it.

[3]

Sorry about that! :-(

[Kapil Thangavelu (hazmat)]

ah.. for some reason i thought [1] was being used to read something off disk into memory, [2] cool. assuming a fix/test for [0] LGTM

review: Approve

[Gustavo Niemeyer (niemeyer)]

I'm going to merge this without [0] handled so that I can move forward with the renaming, but I've filed a bug to track the problem and will fix it shortly:

https://bugs.launchpad.net/juju/+bug/858267

=== modified file 'formula/Makefile'
--- formula/Makefile	2011-09-13 19:55:40 +0000
+++ formula/Makefile	2011-09-13 19:55:40 +0000
@@ -5,6 +5,7 @@
 TARG=launchpad.net/ensemble/go/formula
 
 GOFILES=\
+	bundle.go\
 	config.go\
 	dir.go\
 	formula.go\

=== added file 'formula/bundle.go'
--- formula/bundle.go	1970-01-01 00:00:00 +0000
+++ formula/bundle.go	2011-09-13 19:55:40 +0000
@@ -0,0 +1,161 @@
+package formula
+
+import (
+	"archive/zip"
+	"io"
+	"os"
+	"path/filepath"
+)
+
+// ReadBundle returns a Bundle for the formula in path.
+func ReadBundle(path string) (bundle *Bundle, err os.Error) {
+	f, err := os.Open(path)
+	if err != nil {
+		return
+	}
+	defer f.Close()
+	fi, err := f.Stat()
+	if err != nil {
+		return
+	}
+	b, err := readBundle(f, fi.Size)
+	if err != nil {
+		return
+	}
+	b.Path = path
+	return b, nil
+}
+
+// ReadBundleBytes returns a Bundle read from the given data.
+// Make sure the bundle fits in memory before using this.
+func ReadBundleBytes(data []byte) (bundle *Bundle, err os.Error) {
+	return readBundle(readAtBytes(data), int64(len(data)))
+}
+
+func readBundle(r io.ReaderAt, size int64) (bundle *Bundle, err os.Error) {
+	b := &Bundle{r: r, size: size}
+	zipr, err := zip.NewReader(r, size)
+	if err != nil {
+		return
+	}
+	reader, err := zipOpen(zipr, "metadata.yaml")
+	if err != nil {
+		return
+	}
+	b.meta, err = ReadMeta(reader)
+	reader.Close()
+	if err != nil {
+		return
+	}
+	reader, err = zipOpen(zipr, "config.yaml")
+	if err != nil {
+		return
+	}
+	b.config, err = ReadConfig(reader)
+	reader.Close()
+	if err != nil {
+		return
+	}
+	return b, nil
+}
+
+func zipOpen(zipr *zip.Reader, path string) (rc io.ReadCloser, err os.Error) {
+	for _, fh := range zipr.File {
+		if fh.Name == path {
+			return fh.Open()
+		}
+	}
+	return nil, errorf("bundle file not found: %s", path)
+}
+
+// The Bundle type encapsulates access to data and operations
+// on a formula bundle.
+type Bundle struct {
+	Path   string // May be empty if Bundle wasn't read from a file
+	meta   *Meta
+	config *Config
+	r      io.ReaderAt
+	size   int64
+}
+
+// Trick to ensure *Bundle implements the Formula interface.
+var _ Formula = (*Bundle)(nil)
+
+// Meta returns the Meta representing the metadata.yaml file from bundle.
+func (b *Bundle) Meta() *Meta {
+	return b.meta
+}
+
+// Config returns the Config representing the config.yaml file
+// for the formula bundle.
+func (b *Bundle) Config() *Config {
+	return b.config
+}
+
+// ExpandTo expands the formula bundle into dir, creating it if necessary.
+// If any errors occur during the expansion procedure, the process will
+// continue. Only the last error found is returned.
+func (b *Bundle) ExpandTo(dir string) (err os.Error) {
+	// If we have a Path, reopen the file. Otherwise, try to use
+	// the original ReaderAt.
+	r := b.r
+	size := b.size
+	if b.Path != "" {
+		f, err := os.Open(b.Path)
+		if err != nil {
+			return err
+		}
+		defer f.Close()
+		fi, err := f.Stat()
+		if err != nil {
+			return err
+		}
+		r = f
+		size = fi.Size
+	}
+
+	zipr, err := zip.NewReader(r, size)
+	if err != nil {
+		return err
+	}
+
+	// From here on we won't stop with an error.
+	var lasterr os.Error
+
+	for _, header := range zipr.File {
+		zf, err := header.Open()
+		if err != nil {
+			lasterr = err
+			continue
+		}
+		path := filepath.Join(dir, filepath.Clean(header.Name))
+		base, _ := filepath.Split(path)
+		err = os.MkdirAll(base, 0755)
+		if err != nil {
+			zf.Close()
+			lasterr = err
+			continue
+		}
+		f, err := os.Create(path)
+		if err != nil {
+			zf.Close()
+			lasterr = err
+			continue
+		}
+		_, err = io.Copy(f, zf)
+		if err != nil {
+			lasterr = err
+		}
+		f.Close()
+		zf.Close()
+	}
+
+	return lasterr
+}
+
+// FWIW, being able to do this is awesome.
+type readAtBytes []byte
+
+func (b readAtBytes) ReadAt(out []byte, off int64) (n int, err os.Error) {
+	return copy(out, b[off:]), nil
+}

=== added file 'formula/bundle_test.go'
--- formula/bundle_test.go	1970-01-01 00:00:00 +0000
+++ formula/bundle_test.go	2011-09-13 19:55:40 +0000
@@ -0,0 +1,66 @@
+package formula_test
+
+import (
+	//"archive/zip"
+	"io/ioutil"
+	. "launchpad.net/gocheck"
+	"launchpad.net/ensemble/go/formula"
+	"os"
+	"path/filepath"
+)
+
+type BundleSuite struct {
+	S
+	bundlePath string
+}
+
+var _ = Suite(&BundleSuite{})
+
+func (s *BundleSuite) SetUpSuite(c *C) {
+	s.bundlePath = bundleDir(c, repoDir("dummy"))
+}
+
+func (s BundleSuite) TestReadBundle(c *C) {
+	bundle, err := formula.ReadBundle(s.bundlePath)
+	c.Assert(err, IsNil)
+	checkDummy(c, bundle, s.bundlePath)
+}
+
+func (s BundleSuite) TestReadBundleBytes(c *C) {
+	data, err := ioutil.ReadFile(s.bundlePath)
+	c.Assert(err, IsNil)
+
+	bundle, err := formula.ReadBundleBytes(data)
+	c.Assert(err, IsNil)
+	checkDummy(c, bundle, "")
+}
+
+func (s BundleSuite) TestExpandTo(c *C) {
+	bundle, err := formula.ReadBundle(s.bundlePath)
+	path := filepath.Join(c.MkDir(), "formula")
+
+	err = bundle.ExpandTo(path)
+	c.Assert(err, IsNil)
+
+	dir, err := formula.ReadDir(path)
+	c.Assert(err, IsNil)
+	checkDummy(c, dir, path)
+
+}
+
+func bundleDir(c *C, dirpath string) (path string) {
+	dir, err := formula.ReadDir(dirpath)
+	c.Assert(err, IsNil)
+
+	path = filepath.Join(c.MkDir(), "bundle.charm")
+
+	file, err := os.Create(path)
+	c.Assert(err, IsNil)
+
+	err = dir.BundleTo(file)
+	c.Assert(err, IsNil)
+	file.Close()
+
+	return path
+}
+

=== modified file 'formula/dir.go'
--- formula/dir.go	2011-09-13 19:55:40 +0000
+++ formula/dir.go	2011-09-13 19:55:40 +0000
@@ -40,7 +40,7 @@
 	config *Config
 }
 
-// Trick to ensure Dir implements the Formula interface.
+// Trick to ensure *Dir implements the Formula interface.
 var _ Formula = (*Dir)(nil)
 
 // Meta returns the Meta representing the metadata.yaml file

=== modified file 'formula/dir_test.go'
--- formula/dir_test.go	2011-09-13 19:55:40 +0000
+++ formula/dir_test.go	2011-09-13 19:55:40 +0000
@@ -16,9 +16,7 @@
 	path := repoDir("dummy")
 	dir, err := formula.ReadDir(path)
 	c.Assert(err, IsNil)
-	c.Assert(dir.Path, Equals, path)
-	c.Assert(dir.Meta().Name, Equals, "dummy")
-	c.Assert(dir.Config().Options["title"].Default, Equals, "My Title")
+	checkDummy(c, dir, path)
 }
 
 func (s *S) TestBundleTo(c *C) {

=== modified file 'formula/formula_test.go'
--- formula/formula_test.go	2011-09-13 19:55:40 +0000
+++ formula/formula_test.go	2011-09-13 19:55:40 +0000
@@ -8,6 +8,8 @@
 	. "launchpad.net/gocheck"
 	"launchpad.net/ensemble/go/formula"
 	"launchpad.net/goyaml"
+	"os"
+	"path/filepath"
 )
 
 func Test(t *testing.T) {
@@ -38,6 +40,19 @@
 	c.Assert(err, Matches, `Missing formula revision: "local:foo-x"`)
 }
 
+func checkDummy(c *C, f formula.Formula, path string) {
+	c.Assert(f.Meta().Name, Equals, "dummy")
+	c.Assert(f.Config().Options["title"].Default, Equals, "My Title")
+	switch f := f.(type) {
+	case *formula.Bundle:
+		c.Assert(f.Path, Equals, path)
+	case *formula.Dir:
+		c.Assert(f.Path, Equals, path)
+		_, err := os.Stat(filepath.Join(path, "src", "hello.c"))
+		c.Assert(err, IsNil)
+	}
+}
+
 type YamlHacker map[interface{}]interface{}
 
 func ReadYaml(r io.Reader) YamlHacker {