Fix multi-file add so it works as expected.

If neither "-w" or "-r" is specified don't use an mfs-root. Add and pin each file separately. Towards #2811 License: MIT Signed-off-by: Kevin Atkinson <[email protected]>
ipfs · Jun 6, 2016 · 293e848 · 293e848
1 parent fd077d3
commit 293e848
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 37 deletions.
diff --git a/core/commands/add.go b/core/commands/add.go
@@ -137,6 +137,7 @@ You can now refer to the added file in a gateway, like so:
 		dopin, _, _ := req.Option(pinOptionName).Bool()
 		nocopy, _, _ := req.Option(nocopyOptionName).Bool()
 		link, _, _ := req.Option(linkOptionName).Bool()
+		recursive, _, _ := req.Option(cmds.RecLong).Bool()
 
 		if hash {
 			nilnode, err := core.NewNode(n.Context(), &core.BuildCfg{
@@ -155,6 +156,7 @@ You can now refer to the added file in a gateway, like so:
 		res.SetOutput((<-chan interface{})(outChan))
 
 		var fileAdder *coreunix.Adder
+		useRoot := wrap || recursive
 		if nocopy || link {
 			fs, ok := n.Repo.SubDatastore(fsrepo.RepoFilestore).(*filestore.Datastore)
 			if !ok {
@@ -165,9 +167,9 @@ You can now refer to the added file in a gateway, like so:
 			blockService := bserv.New(blockstore, n.Exchange)
 			dagService := dag.NewDAGService(blockService)
 			dagService.NodeToBlock = filestore_support.NodeToBlock{}
-			fileAdder, err = coreunix.NewAdder(req.Context(), n.Pinning, blockstore, dagService)
+			fileAdder, err = coreunix.NewAdder(req.Context(), n.Pinning, blockstore, dagService, useRoot)
 		} else {
-			fileAdder, err = coreunix.NewAdder(req.Context(), n.Pinning, n.Blockstore, n.DAG)
+			fileAdder, err = coreunix.NewAdder(req.Context(), n.Pinning, n.Blockstore, n.DAG, useRoot)
 		}
 		if err != nil {
 			res.SetError(err, cmds.ErrNormal)

diff --git a/core/coreunix/add.go b/core/coreunix/add.go
@@ -2,6 +2,7 @@ package coreunix
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -67,14 +68,8 @@ type AddedObject struct {
 	Bytes int64  `json:",omitempty"`
 }
 
-func NewAdder(ctx context.Context, p pin.Pinner, bs bstore.GCBlockstore, ds dag.DAGService) (*Adder, error) {
-	mr, err := mfs.NewRoot(ctx, ds, newDirNode(), nil)
-	if err != nil {
-		return nil, err
-	}
-
-	return &Adder{
-		mr:         mr,
+func NewAdder(ctx context.Context, p pin.Pinner, bs bstore.GCBlockstore, ds dag.DAGService, useRoot bool) (*Adder, error) {
+	adder := &Adder{
 		ctx:        ctx,
 		pinning:    p,
 		blockstore: bs,
@@ -85,8 +80,17 @@ func NewAdder(ctx context.Context, p pin.Pinner, bs bstore.GCBlockstore, ds dag.
 		Trickle:    false,
 		Wrap:       false,
 		Chunker:    "",
-	}, nil
+	}
+
+	if useRoot {
+		mr, err := mfs.NewRoot(ctx, ds, newDirNode(), nil)
+		if err != nil {
+			return nil, err
+		}
+		adder.mr = mr
+	}
 
+	return adder, nil
 }
 
 // Internal structure for holding the switches passed to the `add` call
@@ -128,6 +132,10 @@ func (adder Adder) add(reader io.Reader) (*dag.Node, error) {
 }
 
 func (adder *Adder) RootNode() (*dag.Node, error) {
+	if adder.mr == nil {
+		return nil, nil
+	}
+
 	// for memoizing
 	if adder.root != nil {
 		return adder.root, nil
@@ -151,6 +159,10 @@ func (adder *Adder) RootNode() (*dag.Node, error) {
 }
 
 func (adder *Adder) PinRoot() error {
+	if adder.mr == nil {
+		return nil
+	}
+
 	root, err := adder.RootNode()
 	if err != nil {
 		return err
@@ -177,6 +189,13 @@ func (adder *Adder) PinRoot() error {
 }
 
 func (adder *Adder) Finalize() (*dag.Node, error) {
+	if adder.mr == nil && adder.Pin {
+		err := adder.pinning.Flush()
+		return nil, err
+	} else if adder.mr == nil {
+		return nil, nil
+	}
+
 	root := adder.mr.GetValue()
 
 	// cant just call adder.RootNode() here as we need the name for printing
@@ -248,7 +267,7 @@ func (adder *Adder) outputDirs(path string, fs mfs.FSNode) error {
 func Add(n *core.IpfsNode, r io.Reader) (string, error) {
 	defer n.Blockstore.PinLock().Unlock()
 
-	fileAdder, err := NewAdder(n.Context(), n.Pinning, n.Blockstore, n.DAG)
+	fileAdder, err := NewAdder(n.Context(), n.Pinning, n.Blockstore, n.DAG, true)
 	if err != nil {
 		return "", err
 	}
@@ -280,7 +299,7 @@ func AddR(n *core.IpfsNode, root string) (key string, err error) {
 	}
 	defer f.Close()
 
-	fileAdder, err := NewAdder(n.Context(), n.Pinning, n.Blockstore, n.DAG)
+	fileAdder, err := NewAdder(n.Context(), n.Pinning, n.Blockstore, n.DAG, true)
 	if err != nil {
 		return "", err
 	}
@@ -309,7 +328,7 @@ func AddR(n *core.IpfsNode, root string) (key string, err error) {
 // the directory, and and error if any.
 func AddWrapped(n *core.IpfsNode, r io.Reader, filename string) (string, *dag.Node, error) {
 	file := files.NewReaderFile(filename, filename, ioutil.NopCloser(r), nil)
-	fileAdder, err := NewAdder(n.Context(), n.Pinning, n.Blockstore, n.DAG)
+	fileAdder, err := NewAdder(n.Context(), n.Pinning, n.Blockstore, n.DAG, true)
 	if err != nil {
 		return "", nil, err
 	}
@@ -335,28 +354,40 @@ func AddWrapped(n *core.IpfsNode, r io.Reader, filename string) (string, *dag.No
 	return gopath.Join(k.String(), filename), dagnode, nil
 }
 
-func (adder *Adder) addNode(node *dag.Node, path string) error {
-	// patch it into the root
-	if path == "" {
+func (adder *Adder) pinOrAddNode(node *dag.Node, path string) error {
+	if adder.Pin && adder.mr == nil {
+
 		key, err := node.Key()
 		if err != nil {
 			return err
 		}
 
-		path = key.B58String()
-	}
+		adder.pinning.PinWithMode(key, pin.Recursive)
+
+	} else if adder.mr != nil {
+
+		// patch it into the root
+		if path == "" {
+			key, err := node.Key()
+			if err != nil {
+				return err
+			}
+
+			path = key.B58String()
+		}
+
+		dir := gopath.Dir(path)
+		if dir != "." {
+			if err := mfs.Mkdir(adder.mr, dir, true, false); err != nil {
+				return err
+			}
+		}
 
-	dir := gopath.Dir(path)
-	if dir != "." {
-		if err := mfs.Mkdir(adder.mr, dir, true, false); err != nil {
+		if err := mfs.PutNode(adder.mr, path, node); err != nil {
 			return err
 		}
-	}
 
-	if err := mfs.PutNode(adder.mr, path, node); err != nil {
-		return err
 	}
-
 	if !adder.Silent {
 		return outputDagnode(adder.Out, path, node)
 	}
@@ -396,7 +427,7 @@ func (adder *Adder) addFile(file files.File) error {
 			return err
 		}
 
-		return adder.addNode(dagnode, s.FileName())
+		return adder.pinOrAddNode(dagnode, s.FileName())
 	}
 
 	// case for regular file
@@ -418,10 +449,14 @@ func (adder *Adder) addFile(file files.File) error {
 	}
 
 	// patch it into the root
-	return adder.addNode(dagnode, file.FileName())
+	return adder.pinOrAddNode(dagnode, file.FileName())
 }
 
 func (adder *Adder) addDir(dir files.File) error {
+	if adder.mr == nil {
+		return errors.New("Cananot add directories without mfs root")
+	}
+
 	log.Infof("adding directory: %s", dir.FileName())
 
 	err := mfs.Mkdir(adder.mr, dir.FileName(), true, false)

diff --git a/core/coreunix/add_test.go b/core/coreunix/add_test.go
@@ -54,7 +54,7 @@ func TestAddGCLive(t *testing.T) {
 
 	errs := make(chan error)
 	out := make(chan interface{})
-	adder, err := NewAdder(context.Background(), node.Pinning, node.Blockstore, node.DAG)
+	adder, err := NewAdder(context.Background(), node.Pinning, node.Blockstore, node.DAG, true)
 	if err != nil {
 		t.Fatal(err)
 	}

diff --git a/test/sharness/t0040-add-and-cat.sh b/test/sharness/t0040-add-and-cat.sh
@@ -275,7 +275,7 @@ test_expect_success "'ipfs add' with stdin input succeeds" '
 
 test_expect_success "'ipfs add' output looks good" '
 	HASH="QmZDhWpi8NvKrekaYYhxKCdNVGWsFFe1CREnAjP1QbPaB3" &&
-	echo "added $HASH $HASH" >expected &&
+	echo "added $HASH " >expected &&
 	test_cmp expected actual
 '
 

diff --git a/test/sharness/t0080-repo.sh b/test/sharness/t0080-repo.sh
@@ -29,11 +29,6 @@ test_expect_success "'ipfs repo gc' succeeds" '
 	ipfs repo gc >gc_out_actual
 '
 
-test_expect_success "'ipfs repo gc' looks good (patch root)" '
-	PATCH_ROOT=QmQXirSbubiySKnqaFyfs5YzziXRB5JEVQVjU6xsd7innr &&
-	grep "removed $PATCH_ROOT" gc_out_actual
-'
-
 test_expect_success "'ipfs repo gc' doesnt remove file" '
 	ipfs cat "$HASH" >out &&
 	test_cmp out afile
@@ -104,8 +99,7 @@ test_expect_success "remove direct pin" '
 
 test_expect_success "'ipfs repo gc' removes file" '
 	ipfs repo gc >actual7 &&
-	grep "removed $HASH" actual7 &&
-	grep "removed $PATCH_ROOT" actual7
+	grep "removed $HASH" actual7
 '
 
 # TODO: there seems to be a serious bug with leveldb not returning a key.