Merge pull request #39 from saracen/buffer-size-zero

A buffer size of zero uses no buffer, rather than default size

Author: saracen

archiver.go

@@ -65,6 +65,7 @@ func NewArchiver(w io.Writer, chroot string, opts ...ArchiverOption) (*Archiver,
 	a.options.method = zip.Deflate
 	a.options.concurrency = runtime.NumCPU()
 	a.options.stageDir = chroot
+	a.options.bufferSize = -1
 	for _, o := range opts {
 		err := o(&a.options)
 		if err != nil {

archiver_options.go

@@ -6,7 +6,6 @@ import (
 
 var (
 	ErrMinConcurrency = errors.New("concurrency must be at least 1")
-	ErrMinBufferSize  = errors.New("buffer size option cannot be less than -1")
 )
 
 // ArchiverOption is an option used when creating an archiver.
@@ -45,13 +44,10 @@ func WithArchiverConcurrency(n int) ArchiverOption {
 // temporary file is written (to the stage directory) to hold the additional
 // data. The default is 2 mebibytes, so if concurrency is 16, 32 mebibytes of
 // memory will be allocated.
-//
-// If set to -1, no buffer will be used and all compressed file content will be
-// written to temporary files before being written back to the zip file.
 func WithArchiverBufferSize(n int) ArchiverOption {
 	return func(o *archiverOptions) error {
-		if n < -1 {
-			return ErrMinBufferSize
+		if n < 0 {
+			n = 0
 		}
 		o.bufferSize = n
 		return nil

archiver_test.go

@@ -110,7 +110,7 @@ func TestArchive(t *testing.T) {
 
 	tests := map[string][]ArchiverOption{
 		"default options":    nil,
-		"no buffer":          {WithArchiverBufferSize(-1)},
+		"no buffer":          {WithArchiverBufferSize(0)},
 		"with store":         {WithArchiverMethod(zip.Store)},
 		"with concurrency 2": {WithArchiverConcurrency(2)},
 	}
@@ -316,17 +316,20 @@ func TestArchiveWithConcurrency(t *testing.T) {
 
 func TestArchiveWithBufferSize(t *testing.T) {
 	testFiles := map[string]testFile{
-		"foo.go": {mode: 0666},
-		"bar.go": {mode: 0666},
+		"foobar.go":      {mode: 0666},
+		"compressible":   {mode: 0666, contents: "11111111111111111111111111111111111111111111111111"},
+		"uncompressible": {mode: 0666, contents: "A3#bez&OqCusPr)d&D]Vot9Eo0z^5O*VZm3:sO3HptL.H-4cOv"},
+		"empty_dir":      {mode: os.ModeDir | 0777},
+		"large_file":     {mode: 0666, contents: strings.Repeat("abcdefzmkdldjsdfkjsdfsdfiqwpsdfa", 65536)},
 	}
 
 	tests := []struct {
 		buffersize int
-		pass       bool
+		zero       bool
 	}{
 		{-100, false},
 		{-2, false},
-		{-1, true},
+		{-1, false},
 		{0, true},
 		{32 * 1024, true},
 		{64 * 1024, true},
@@ -343,18 +346,18 @@ func TestArchiveWithBufferSize(t *testing.T) {
 			defer f.Close()
 
 			a, err := NewArchiver(f, dir, WithArchiverBufferSize(test.buffersize))
-			if !test.pass {
-				require.Error(t, err)
-				return
-			}
-
 			require.NoError(t, err)
 			require.NoError(t, a.Archive(context.Background(), files))
 			require.NoError(t, a.Close())
 
-			bytes, entries := a.Written()
-			require.EqualValues(t, 0, bytes)
-			require.EqualValues(t, 3, entries)
+			if !test.zero {
+				require.Equal(t, 0, a.options.bufferSize)
+			} else {
+				require.Equal(t, test.buffersize, a.options.bufferSize)
+			}
+
+			_, entries := a.Written()
+			require.EqualValues(t, 6, entries)
 
 			testExtract(t, f.Name(), testFiles)
 		}()

internal/filepool/filepool.go

@@ -57,10 +57,7 @@ func New(dir string, poolSize int, bufferSize int) (*FilePool, error) {
 	fp.files = make([]*File, poolSize)
 	fp.limiter = make(chan int, poolSize)
 
-	switch bufferSize {
-	case -1:
-		bufferSize = 0
-	case 0:
+	if bufferSize < 0 {
 		bufferSize = defaultBufferSize
 	}
 

internal/filepool/filepool_test.go

@@ -31,7 +31,7 @@ func TestFilePoolSizes(t *testing.T) {
 			require.NoError(t, err)
 			defer os.RemoveAll(dir)
 
-			fp, err := New(dir, tc.size, -1)
+			fp, err := New(dir, tc.size, 0)
 			require.Equal(t, tc.err, err)
 			if tc.err != nil {
 				return
@@ -63,7 +63,7 @@ func TestFilePoolReset(t *testing.T) {
 	require.NoError(t, err)
 	defer os.RemoveAll(dir)
 
-	fp, err := New(dir, 16, -1)
+	fp, err := New(dir, 16, 0)
 	require.NoError(t, err)
 	for i := range fp.files {
 		file := fp.Get()
@@ -102,7 +102,7 @@ func TestFilePoolCloseError(t *testing.T) {
 	require.NoError(t, err)
 	defer os.RemoveAll(dir)
 
-	fp, err := New(dir, 16, -1)
+	fp, err := New(dir, 16, 0)
 	require.NoError(t, err)
 
 	for _, file := range fp.files {
@@ -135,7 +135,7 @@ func TestFilePoolNoErrorOnAlreadyDeleted(t *testing.T) {
 	dir, err := ioutil.TempDir("", "fastzip-filepool")
 	require.NoError(t, err)
 
-	fp, err := New(dir, 16, -1)
+	fp, err := New(dir, 16, 0)
 	require.NoError(t, err)
 
 	for range fp.files {