Skip to content

Commit

Permalink
Adder: Fix multi-file add so it works as expected.
Browse files Browse the repository at this point in the history
If neither "-w" or "-r" is specified don't use an mfs-root.  Add and
pin each file separately.

Closes #2811

License: MIT
Signed-off-by: Kevin Atkinson <[email protected]>
  • Loading branch information
kevina committed Sep 30, 2016
1 parent 551f540 commit 8355c74
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 40 deletions.
4 changes: 3 additions & 1 deletion core/commands/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ You can now refer to the added file in a gateway, like so:
silent, _, _ := req.Option(silentOptionName).Bool()
chunker, _, _ := req.Option(chunkerOptionName).String()
dopin, _, _ := req.Option(pinOptionName).Bool()
recursive, _, _ := req.Option(cmds.RecLong).Bool()

if hash {
nilnode, err := core.NewNode(n.Context(), &core.BuildCfg{
Expand All @@ -160,7 +161,8 @@ You can now refer to the added file in a gateway, like so:
outChan := make(chan interface{}, 8)
res.SetOutput((<-chan interface{})(outChan))

fileAdder, err := coreunix.NewAdder(req.Context(), n.Pinning, n.Blockstore, dserv)
useRoot := wrap || recursive
fileAdder, err := coreunix.NewAdder(req.Context(), n.Pinning, n.Blockstore, dserv, useRoot)
if err != nil {
res.SetError(err, cmds.ErrNormal)
return
Expand Down
82 changes: 56 additions & 26 deletions core/coreunix/add.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package coreunix

import (
"errors"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -64,14 +65,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, unixfs.EmptyDirNode(), 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,
Expand All @@ -82,8 +77,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, unixfs.EmptyDirNode(), nil)
if err != nil {
return nil, err
}
adder.mr = mr
}

return adder, nil
}

// Internal structure for holding the switches passed to the `add` call
Expand Down Expand Up @@ -130,6 +134,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
Expand All @@ -153,6 +161,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
Expand All @@ -179,6 +191,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
Expand Down Expand Up @@ -250,7 +269,7 @@ func (adder *Adder) outputDirs(path string, fsn 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
}
Expand Down Expand Up @@ -278,7 +297,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
}
Expand All @@ -302,7 +321,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
}
Expand All @@ -324,23 +343,30 @@ func AddWrapped(n *core.IpfsNode, r io.Reader, filename string) (string, *dag.No
return gopath.Join(c.String(), filename), dagnode, nil
}

func (adder *Adder) addNode(node *dag.Node, path string) error {
// patch it into the root
if path == "" {
path = node.Cid().String()
}
func (adder *Adder) pinOrAddNode(node *dag.Node, path string) error {
if adder.Pin && adder.mr == nil {

adder.pinning.PinWithMode(node.Cid(), pin.Recursive)

} else if adder.mr != nil {

// patch it into the root
if path == "" {
path = node.Cid().String()
}

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)
}
Expand Down Expand Up @@ -384,7 +410,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
Expand All @@ -401,10 +427,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)
Expand Down
2 changes: 1 addition & 1 deletion core/coreunix/add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,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)
}
Expand Down
2 changes: 1 addition & 1 deletion test/sharness/t0040-add-and-cat.sh
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,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
'

Expand Down
4 changes: 2 additions & 2 deletions test/sharness/t0046-multifile-add.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ test_expect_success "add files all at once" '
ipfs add -q fileA fileB fileC > hashes
'

test_expect_failure "unpin one of the files" '
test_expect_success "unpin one of the files" '
ipfs pin rm `head -1 hashes` > pin-out
'

test_expect_failure "unpin output looks good" '
test_expect_success "unpin output looks good" '
echo "unpinned `head -1 hashes`" > pin-expect
test_cmp pin-expect pin-out
'
Expand Down
11 changes: 2 additions & 9 deletions test/sharness/t0080-repo.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
'

test_expect_success "'ipfs refs local' no longer shows file" '
Expand All @@ -114,8 +108,7 @@ test_expect_success "'ipfs refs local' no longer shows file" '
grep "QmYCvbfNbCwFR45HiNP45rwJgvatpiW38D961L5qAhUM5Y" actual8 &&
grep "$EMPTY_DIR" actual8 &&
grep "$HASH_WELCOME_DOCS" actual8 &&
test_must_fail grep "$HASH" actual8 &&
test_must_fail grep "$PATCH_ROOT" actual8
test_must_fail grep "$HASH" actual8
'

test_expect_success "adding multiblock random file succeeds" '
Expand Down

0 comments on commit 8355c74

Please sign in to comment.