Merge remote-tracking branch 'fw42/more_cosmetic_changes'

archiver.go

@@ -24,10 +24,12 @@ const (
 	maxConcurrency        = 10
 	maxConcurrencyPreload = 20
 
-	// chunkerBufSize is used in pool.go
 	chunkerBufSize = 512 * chunker.KiB
 )
 
+var archiverAbortOnAllErrors = func(str string, fi os.FileInfo, err error) error { return err }
+var archiverAllowAllFiles = func(string, os.FileInfo) bool { return true }
+
 type Archiver struct {
 	s *server.Server
 
@@ -37,24 +39,20 @@ type Archiver struct {
 	Filter func(item string, fi os.FileInfo) bool
 }
 
-func NewArchiver(s *server.Server) (*Archiver, error) {
-	var err error
+func NewArchiver(s *server.Server) *Archiver {
 	arch := &Archiver{
 		s:         s,
 		blobToken: make(chan struct{}, maxConcurrentBlobs),
 	}
 
-	// fill blob token
 	for i := 0; i < maxConcurrentBlobs; i++ {
 		arch.blobToken <- struct{}{}
 	}
 
-	// abort on all errors
-	arch.Error = func(string, os.FileInfo, error) error { return err }
-	// allow all files
-	arch.Filter = func(string, os.FileInfo) bool { return true }
+	arch.Error = archiverAbortOnAllErrors
+	arch.Filter = archiverAllowAllFiles
 
-	return arch, nil
+	return arch
 }
 
 func (arch *Archiver) Save(t pack.BlobType, id backend.ID, length uint, rd io.Reader) error {
@@ -78,22 +76,18 @@ func (arch *Archiver) Save(t pack.BlobType, id backend.ID, length uint, rd io.Re
 }
 
 func (arch *Archiver) SaveTreeJSON(item interface{}) (backend.ID, error) {
-	// convert to json
 	data, err := json.Marshal(item)
-	// append newline
-	data = append(data, '\n')
 	if err != nil {
 		return nil, err
 	}
+	data = append(data, '\n')
 
 	// check if tree has been saved before
 	id := backend.Hash(data)
-
 	if arch.s.Index().Has(id) {
 		return id, nil
 	}
 
-	// otherwise save the data
 	return arch.s.SaveJSON(pack.Tree, item)
 }
 
@@ -106,7 +100,7 @@ func (arch *Archiver) SaveFile(p *Progress, node *Node) error {
 		return err
 	}
 
-	// check file again
+	// check file again, since it could have disappeared by now
 	fi, err := file.Stat()
 	if err != nil {
 		return err
@@ -116,14 +110,12 @@ func (arch *Archiver) SaveFile(p *Progress, node *Node) error {
 		e2 := arch.Error(node.path, fi, errors.New("file was updated, using new version"))
 
 		if e2 == nil {
-			// create new node
 			n, err := NodeFromFileInfo(node.path, fi)
 			if err != nil {
 				debug.Log("Archiver.SaveFile", "NodeFromFileInfo returned error for %v: %v", node.path, err)
 				return err
 			}
 
-			// copy node
 			*node = *n
 		}
 	}

archiver_test.go

@@ -121,10 +121,10 @@ func archiveDirectory(b testing.TB) {
 	key := SetupKey(b, server, "geheim")
 	server.SetKey(key)
 
-	arch, err := restic.NewArchiver(server)
-	OK(b, err)
+	arch := restic.NewArchiver(server)
 
 	_, id, err := arch.Snapshot(nil, []string{*benchArchiveDirectory}, nil)
+	OK(b, err)
 
 	b.Logf("snapshot archived as %v", id)
 }
@@ -238,8 +238,7 @@ func BenchmarkLoadTree(t *testing.B) {
 	s.SetKey(key)
 
 	// archive a few files
-	arch, err := restic.NewArchiver(s)
-	OK(t, err)
+	arch := restic.NewArchiver(s)
 	sn, _, err := arch.Snapshot(nil, []string{*benchArchiveDirectory}, nil)
 	OK(t, err)
 	t.Logf("archived snapshot %v", sn.ID())

cache.go

@@ -17,33 +17,26 @@ type Cache struct {
 	base string
 }
 
-func NewCache(be backend.Identifier) (c *Cache, err error) {
-	// try to get explicit cache dir from environment
-	dir := os.Getenv("RESTIC_CACHE")
-
-	// otherwise try OS specific default
-	if dir == "" {
-		dir, err = GetCacheDir()
-		if err != nil {
-			return nil, err
-		}
+func NewCache(be backend.Identifier) (*Cache, error) {
+	cacheDir, err := getCacheDir()
+	if err != nil {
+		return nil, err
 	}
 
-	basedir := filepath.Join(dir, be.ID())
+	basedir := filepath.Join(cacheDir, be.ID())
 	debug.Log("Cache.New", "opened cache at %v", basedir)
 
 	return &Cache{base: basedir}, nil
 }
 
 func (c *Cache) Has(t backend.Type, subtype string, id backend.ID) (bool, error) {
-	// try to open file
 	filename, err := c.filename(t, subtype, id)
 	if err != nil {
 		return false, err
 	}
-
 	fd, err := os.Open(filename)
 	defer fd.Close()
+
 	if err != nil {
 		if os.IsNotExist(err) {
 			debug.Log("Cache.Has", "test for file %v: not cached", filename)
@@ -81,7 +74,6 @@ func (c *Cache) Store(t backend.Type, subtype string, id backend.ID) (io.WriteCl
 }
 
 func (c *Cache) Load(t backend.Type, subtype string, id backend.ID) (io.ReadCloser, error) {
-	// try to open file
 	filename, err := c.filename(t, subtype, id)
 	if err != nil {
 		return nil, err
@@ -90,14 +82,14 @@ func (c *Cache) Load(t backend.Type, subtype string, id backend.ID) (io.ReadClos
 	return os.Open(filename)
 }
 
-func (c *Cache) Purge(t backend.Type, subtype string, id backend.ID) error {
+func (c *Cache) purge(t backend.Type, subtype string, id backend.ID) error {
 	filename, err := c.filename(t, subtype, id)
 	if err != nil {
 		return err
 	}
 
 	err = os.Remove(filename)
-	debug.Log("Cache.Purge", "Remove file %v: %v", filename, err)
+	debug.Log("Cache.purge", "Remove file %v: %v", filename, err)
 
 	if err != nil && os.IsNotExist(err) {
 		return nil
@@ -118,7 +110,7 @@ func (c *Cache) Clear(s *server.Server) error {
 		if ok, err := s.Test(backend.Snapshot, entry.ID.String()); !ok || err != nil {
 			debug.Log("Cache.Clear", "snapshot %v doesn't exist any more, removing %v", entry.ID, entry)
 
-			err = c.Purge(backend.Snapshot, entry.Subtype, entry.ID)
+			err = c.purge(backend.Snapshot, entry.Subtype, entry.ID)
 			if err != nil {
 				return err
 			}
@@ -188,7 +180,6 @@ func (c *Cache) List(t backend.Type) ([]CacheEntry, error) {
 	return entries, nil
 }
 
-// Construct file name for given Type.
 func (c *Cache) filename(t backend.Type, subtype string, id backend.ID) (string, error) {
 	filename := id.String()
 	if subtype != "" {
@@ -203,11 +194,17 @@ func (c *Cache) filename(t backend.Type, subtype string, id backend.ID) (string,
 	return "", fmt.Errorf("cache not supported for type %v", t)
 }
 
-// high-level functions
+func getCacheDir() (string, error) {
+	if dir := os.Getenv("RESTIC_CACHE"); dir != "" {
+		return dir, nil
+	}
 
-// GetCacheDir returns the cache directory according to XDG basedir spec, see
+	return getXDGCacheDir()
+}
+
+// getXDGCacheDir returns the cache directory according to XDG basedir spec, see
 // http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
-func GetCacheDir() (string, error) {
+func getXDGCacheDir() (string, error) {
 	xdgcache := os.Getenv("XDG_CACHE_HOME")
 	home := os.Getenv("HOME")
 
@@ -243,5 +240,3 @@ func GetCacheDir() (string, error) {
 
 	return cachedir, nil
 }
-
-// TODO: implement RefreshIndex()

cache_test.go

@@ -16,8 +16,7 @@ func TestCache(t *testing.T) {
 	_, err := restic.NewCache(server)
 	OK(t, err)
 
-	arch, err := restic.NewArchiver(server)
-	OK(t, err)
+	arch := restic.NewArchiver(server)
 
 	// archive some files, this should automatically cache all blobs from the snapshot
 	_, _, err = arch.Snapshot(nil, []string{*benchArchiveDirectory}, nil)

cmd/restic/cmd_backup.go

@@ -252,10 +252,7 @@ func (cmd CmdBackup) Execute(args []string) error {
 	// 	return true
 	// }
 
-	arch, err := restic.NewArchiver(s)
-	if err != nil {
-		fmt.Fprintf(os.Stderr, "err: %v\n", err)
-	}
+	arch := restic.NewArchiver(s)
 
 	arch.Error = func(dir string, fi os.FileInfo, err error) error {
 		// TODO: make ignoring errors configurable

progress.go

@@ -34,7 +34,7 @@ type Stat struct {
 
 type ProgressFunc func(s Stat, runtime time.Duration, ticker bool)
 
-// NewProgress returns a new progress reporter. When Start() called, the
+// NewProgress returns a new progress reporter. When Start() is called, the
 // function OnStart is executed once. Afterwards the function OnUpdate is
 // called when new data arrives or at least every d interval. The function
 // OnDone is called when Done() is called. Both functions are called
@@ -43,7 +43,7 @@ func NewProgress(d time.Duration) *Progress {
 	return &Progress{d: d}
 }
 
-// Start runs resets and runs the progress reporter.
+// Start resets and runs the progress reporter.
 func (p *Progress) Start() {
 	if p == nil || p.running {
 		return
@@ -63,6 +63,21 @@ func (p *Progress) Start() {
 	go p.reporter()
 }
 
+// Reset resets all statistic counters to zero.
+func (p *Progress) Reset() {
+	if p == nil {
+		return
+	}
+
+	if !p.running {
+		panic("resetting a non-running Progress")
+	}
+
+	p.curM.Lock()
+	p.cur = Stat{}
+	p.curM.Unlock()
+}
+
 // Report adds the statistics from s to the current state and tries to report
 // the accumulated statistics via the feedback channel.
 func (p *Progress) Report(s Stat) {
@@ -79,12 +94,17 @@ func (p *Progress) Report(s Stat) {
 	cur := p.cur
 	p.curM.Unlock()
 
-	// update progress
-	if p.OnUpdate != nil {
-		p.fnM.Lock()
-		p.OnUpdate(cur, time.Since(p.start), false)
-		p.fnM.Unlock()
+	p.updateProgress(cur, false)
+}
+
+func (p *Progress) updateProgress(cur Stat, ticker bool) {
+	if p.OnUpdate == nil {
+		return
 	}
+
+	p.fnM.Lock()
+	p.OnUpdate(cur, time.Since(p.start), ticker)
+	p.fnM.Unlock()
 }
 
 func (p *Progress) reporter() {
@@ -98,12 +118,7 @@ func (p *Progress) reporter() {
 			p.curM.Lock()
 			cur := p.cur
 			p.curM.Unlock()
-
-			if p.OnUpdate != nil {
-				p.fnM.Lock()
-				p.OnUpdate(cur, time.Since(p.start), true)
-				p.fnM.Unlock()
-			}
+			p.updateProgress(cur, true)
 		case <-p.cancel:
 			p.c.Stop()
 			return
@@ -111,40 +126,23 @@ func (p *Progress) reporter() {
 	}
 }
 
-// Reset resets all statistic counters to zero.
-func (p *Progress) Reset() {
-	if p == nil {
-		return
-	}
-
-	if !p.running {
-		panic("resetting a non-running Progress")
-	}
-
-	p.curM.Lock()
-	p.cur = Stat{}
-	p.curM.Unlock()
-}
-
 // Done closes the progress report.
 func (p *Progress) Done() {
 	if p == nil || !p.running {
 		return
 	}
 
-	if p.running {
-		p.running = false
-		p.o.Do(func() {
-			close(p.cancel)
-		})
+	p.running = false
+	p.o.Do(func() {
+		close(p.cancel)
+	})
 
-		cur := p.cur
+	cur := p.cur
 
-		if p.OnDone != nil {
-			p.fnM.Lock()
-			p.OnDone(cur, time.Since(p.start), false)
-			p.fnM.Unlock()
-		}
+	if p.OnDone != nil {
+		p.fnM.Lock()
+		p.OnDone(cur, time.Since(p.start), false)
+		p.fnM.Unlock()
 	}
 }
 

restorer.go

@@ -19,57 +19,31 @@ type Restorer struct {
 	Filter func(item string, dstpath string, node *Node) bool
 }
 
-// NewRestorer creates a restorer preloaded with the content from the snapshot snid.
-func NewRestorer(s *server.Server, snid backend.ID) (*Restorer, error) {
-	r := &Restorer{s: s}
+var restorerAbortOnAllErrors = func(str string, node *Node, err error) error { return err }
+
+// NewRestorer creates a restorer preloaded with the content from the snapshot id.
+func NewRestorer(s *server.Server, id backend.ID) (*Restorer, error) {
+	r := &Restorer{s: s, Error: restorerAbortOnAllErrors}
 
 	var err error
 
-	r.sn, err = LoadSnapshot(s, snid)
+	r.sn, err = LoadSnapshot(s, id)
 	if err != nil {
 		return nil, arrar.Annotate(err, "load snapshot for restorer")
 	}
 
-	// abort on all errors
-	r.Error = func(string, *Node, error) error { return err }
-
 	return r, nil
 }
 
-func (res *Restorer) to(dst string, dir string, treeID backend.ID) error {
+func (res *Restorer) restoreTo(dst string, dir string, treeID backend.ID) error {
 	tree, err := LoadTree(res.s, treeID)
 	if err != nil {
 		return res.Error(dir, nil, arrar.Annotate(err, "LoadTree"))
 	}
 
 	for _, node := range tree.Nodes {
-		dstpath := filepath.Join(dst, dir, node.Name)
-
-		if res.Filter == nil ||
-			res.Filter(filepath.Join(dir, node.Name), dstpath, node) {
-			err := node.CreateAt(dstpath, res.s)
-
-			// Did it fail because of ENOENT?
-			if arrar.Check(err, func(err error) bool {
-				if pe, ok := err.(*os.PathError); ok {
-					errn, ok := pe.Err.(syscall.Errno)
-					return ok && errn == syscall.ENOENT
-				}
-				return false
-			}) {
-				// Create parent directories and retry
-				err = os.MkdirAll(filepath.Dir(dstpath), 0700)
-				if err == nil || err == os.ErrExist {
-					err = node.CreateAt(dstpath, res.s)
-				}
-			}
-
-			if err != nil {
-				err = res.Error(dstpath, node, arrar.Annotate(err, "create node"))
-				if err != nil {
-					return err
-				}
-			}
+		if err := res.restoreNodeTo(node, dir, dst); err != nil {
+			return err
 		}
 
 		if node.Type == "dir" {
@@ -78,7 +52,7 @@ func (res *Restorer) to(dst string, dir string, treeID backend.ID) error {
 			}
 
 			subp := filepath.Join(dir, node.Name)
-			err = res.to(dst, subp, node.Subtree)
+			err = res.restoreTo(dst, subp, node.Subtree)
 			if err != nil {
 				err = res.Error(subp, node, arrar.Annotate(err, "restore subtree"))
 				if err != nil {
@@ -91,10 +65,44 @@ func (res *Restorer) to(dst string, dir string, treeID backend.ID) error {
 	return nil
 }
 
+func (res *Restorer) restoreNodeTo(node *Node, dir string, dst string) error {
+	dstPath := filepath.Join(dst, dir, node.Name)
+
+	if res.Filter != nil && res.Filter(filepath.Join(dir, node.Name), dstPath, node) {
+		return nil
+	}
+
+	err := node.CreateAt(dstPath, res.s)
+
+	// Did it fail because of ENOENT?
+	if arrar.Check(err, func(err error) bool {
+		if pe, ok := err.(*os.PathError); ok {
+			errn, ok := pe.Err.(syscall.Errno)
+			return ok && errn == syscall.ENOENT
+		}
+		return false
+	}) {
+		// Create parent directories and retry
+		err = os.MkdirAll(filepath.Dir(dstPath), 0700)
+		if err == nil || err == os.ErrExist {
+			err = node.CreateAt(dstPath, res.s)
+		}
+	}
+
+	if err != nil {
+		err = res.Error(dstPath, node, arrar.Annotate(err, "create node"))
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // RestoreTo creates the directories and files in the snapshot below dir.
 // Before an item is created, res.Filter is called.
 func (res *Restorer) RestoreTo(dir string) error {
-	return res.to(dir, "", res.sn.Tree)
+	return res.restoreTo(dir, "", res.sn.Tree)
 }
 
 func (res *Restorer) Snapshot() *Snapshot {

snapshot_test.go

@@ -2,29 +2,17 @@ package restic_test
 
 import (
 	"testing"
-	"time"
 
 	"github.com/restic/restic"
-	"github.com/restic/restic/server"
 	. "github.com/restic/restic/test"
 )
 
-func testSnapshot(t *testing.T, s *server.Server) {
-	var err error
-	sn, err := restic.NewSnapshot([]string{"/home/foobar"})
-	OK(t, err)
-	// sn.Tree, err = restic.Blob{ID: backend.ParseID("c3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714caef0c4f2")}
-	// ok(t, err)
-	sn.Time, err = time.Parse(time.RFC3339Nano, "2014-08-03T17:49:05.378595539+02:00")
-	OK(t, err)
-
-	// _, err = sn.Save(be)
-	// ok(t, err)
-}
-
-func TestSnapshot(t *testing.T) {
+func TestNewSnapshot(t *testing.T) {
 	s := SetupBackend(t)
 	defer TeardownBackend(t, s)
 
-	testSnapshot(t, s)
+	paths := []string{"/home/foobar"}
+
+	_, err := restic.NewSnapshot(paths)
+	OK(t, err)
 }

test/backend.go

@@ -50,8 +50,7 @@ func SetupKey(t testing.TB, s *server.Server, password string) *server.Key {
 }
 
 func SnapshotDir(t testing.TB, server *server.Server, path string, parent backend.ID) *restic.Snapshot {
-	arch, err := restic.NewArchiver(server)
-	OK(t, err)
+	arch := restic.NewArchiver(server)
 	sn, _, err := arch.Snapshot(nil, []string{path}, parent)
 	OK(t, err)
 	return sn

tree.go

@@ -60,9 +60,8 @@ func (t Tree) Equals(other *Tree) bool {
 }
 
 func (t *Tree) Insert(node *Node) error {
-	pos, _, err := t.find(node.Name)
+	pos, _, err := t.binarySearch(node.Name)
 	if err == nil {
-		// already present
 		return ErrNodeAlreadyInTree
 	}
 
@@ -74,7 +73,7 @@ func (t *Tree) Insert(node *Node) error {
 	return nil
 }
 
-func (t Tree) find(name string) (int, *Node, error) {
+func (t Tree) binarySearch(name string) (int, *Node, error) {
 	pos := sort.Search(len(t.Nodes), func(i int) bool {
 		return t.Nodes[i].Name >= name
 	})
@@ -87,6 +86,6 @@ func (t Tree) find(name string) (int, *Node, error) {
 }
 
 func (t Tree) Find(name string) (*Node, error) {
-	_, node, err := t.find(name)
+	_, node, err := t.binarySearch(name)
 	return node, err
 }

tree_test.go

@@ -21,8 +21,7 @@ var testFiles = []struct {
 	{"bar/bla/blubb", []byte("This is just a test!\n")},
 }
 
-// prepareDir creates a temporary directory and returns it.
-func prepareDir(t *testing.T) string {
+func createTempDir(t *testing.T) string {
 	tempdir, err := ioutil.TempDir(*TestTempDir, "restic-test-")
 	OK(t, err)
 
@@ -48,7 +47,7 @@ func prepareDir(t *testing.T) string {
 }
 
 func TestTree(t *testing.T) {
-	dir := prepareDir(t)
+	dir := createTempDir(t)
 	defer func() {
 		if *TestCleanup {
 			OK(t, os.RemoveAll(dir))
@@ -89,7 +88,7 @@ func TestNodeComparison(t *testing.T) {
 	n2 := *node
 	Assert(t, node.Equals(n2), "nodes aren't equal")
 
-	n2.Size -= 1
+	n2.Size--
 	Assert(t, !node.Equals(n2), "nodes are equal")
 }
 

walk.go

@@ -18,7 +18,7 @@ type WalkTreeJob struct {
 
 func walkTree(s *server.Server, path string, treeID backend.ID, done chan struct{}, jobCh chan<- WalkTreeJob) {
 	debug.Log("walkTree", "start on %q (%v)", path, treeID.Str())
-	// load tree
+
 	t, err := LoadTree(s, treeID)
 	if err != nil {
 		jobCh <- WalkTreeJob{Path: path, Error: err}
@@ -30,15 +30,15 @@ func walkTree(s *server.Server, path string, treeID backend.ID, done chan struct
 		if node.Type == "dir" {
 			walkTree(s, p, node.Subtree, done, jobCh)
 		} else {
-			jobCh <- WalkTreeJob{Path: p, Node: node, Error: err}
+			jobCh <- WalkTreeJob{Path: p, Node: node}
 		}
 	}
 
-	jobCh <- WalkTreeJob{Path: filepath.Join(path), Tree: t}
+	jobCh <- WalkTreeJob{Path: path, Tree: t}
 	debug.Log("walkTree", "done for %q (%v)", path, treeID.Str())
 }
 
-// WalkTree walks the tree specified by ID recursively and sends a job for each
+// WalkTree walks the tree specified by id recursively and sends a job for each
 // file and directory it finds. When the channel done is closed, processing
 // stops.
 func WalkTree(server *server.Server, id backend.ID, done chan struct{}, jobCh chan<- WalkTreeJob) {

walk_test.go

@@ -22,19 +22,13 @@ func TestWalkTree(t *testing.T) {
 	server.SetKey(key)
 
 	// archive a few files
-	arch, err := restic.NewArchiver(server)
-	OK(t, err)
+	arch := restic.NewArchiver(server)
 	sn, _, err := arch.Snapshot(nil, dirs, nil)
 	OK(t, err)
 
 	// flush server, write all packs
 	OK(t, server.Flush())
 
-	// start benchmark
-	// t.ResetTimer()
-
-	// for i := 0; i < t.N; i++ {
-
 	done := make(chan struct{})
 
 	// start tree walker
@@ -89,5 +83,4 @@ func TestWalkTree(t *testing.T) {
 		Assert(t, fsEntries == treeEntries,
 			"wrong number of entries: %v != %v", fsEntries, treeEntries)
 	}
-	// }
 }