From 31add0fd3e6bc942a17acaf3f92b8a0d0ec12539 Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Fri, 30 Sep 2016 11:06:15 -0400 Subject: [PATCH 1/4] Adder: Test that each file given on the command line is pinned individually Add (currently failing and marked as such) test that each file specified on the command line is pinned individually when using "ipfs add". License: MIT Signed-off-by: Kevin Atkinson --- test/sharness/t0046-multifile-add.sh | 33 ++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100755 test/sharness/t0046-multifile-add.sh diff --git a/test/sharness/t0046-multifile-add.sh b/test/sharness/t0046-multifile-add.sh new file mode 100755 index 00000000000..082fbb6c442 --- /dev/null +++ b/test/sharness/t0046-multifile-add.sh @@ -0,0 +1,33 @@ +#!/bin/sh +# +# Copyright (c) 2014 Christian Couder +# MIT Licensed; see the LICENSE file in this repository. +# + +test_description="Test add and cat commands" + +. lib/test-lib.sh + +test_init_ipfs + +test_expect_success "create some files" ' + echo A > fileA && + echo B > fileB && + echo C > fileC +' + +test_expect_success "add files all at once" ' + ipfs add -q fileA fileB fileC > hashes +' + +test_expect_failure "unpin one of the files" ' + ipfs pin rm `head -1 hashes` > pin-out +' + +test_expect_failure "unpin output looks good" ' + echo "unpinned `head -1 hashes`" > pin-expect + test_cmp pin-expect pin-out +' + + +test_done From 8e6138574984688ebb938879d42de4e6ae9b4bd7 Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Mon, 3 Oct 2016 00:02:16 -0400 Subject: [PATCH 2/4] Adder: Test adding two files with the same name but in a different dir. License: MIT Signed-off-by: Kevin Atkinson --- test/sharness/t0046-multifile-add.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/sharness/t0046-multifile-add.sh b/test/sharness/t0046-multifile-add.sh index 082fbb6c442..3a364fe48d9 100755 --- a/test/sharness/t0046-multifile-add.sh +++ b/test/sharness/t0046-multifile-add.sh @@ -29,5 +29,15 @@ test_expect_failure "unpin output looks good" ' test_cmp pin-expect pin-out ' +test_expect_success "create files with same name but in different directories" ' + mkdir dirA && + mkdir dirB && + echo AA > dirA/fileA && + echo BA > dirB/fileA +' + +test_expect_failure "add files with same name but in different directories" ' + ipfs add -q dirA/fileA dirB/fileA > hashes +' test_done From 543f6115076f275072c4de15173df8502eceeb22 Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Mon, 3 Oct 2016 00:09:56 -0400 Subject: [PATCH 3/4] Adder: 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. Closes #2811 License: MIT Signed-off-by: Kevin Atkinson --- core/commands/add.go | 4 +- core/coreunix/add.go | 82 +++++++++++++++++++--------- core/coreunix/add_test.go | 2 +- test/sharness/t0040-add-and-cat.sh | 2 +- test/sharness/t0046-multifile-add.sh | 16 +++++- test/sharness/t0080-repo.sh | 11 +--- 6 files changed, 76 insertions(+), 41 deletions(-) diff --git a/core/commands/add.go b/core/commands/add.go index a5bf084d333..5b29609709d 100644 --- a/core/commands/add.go +++ b/core/commands/add.go @@ -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{ @@ -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 diff --git a/core/coreunix/add.go b/core/coreunix/add.go index f0b21e71540..534e9605b13 100644 --- a/core/coreunix/add.go +++ b/core/coreunix/add.go @@ -1,6 +1,7 @@ package coreunix import ( + "errors" "fmt" "io" "io/ioutil" @@ -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, @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 } @@ -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 } @@ -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 } @@ -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) } @@ -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 @@ -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("cannot 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 index 485d2f689f5..901efcb9297 100644 --- a/core/coreunix/add_test.go +++ b/core/coreunix/add_test.go @@ -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) } diff --git a/test/sharness/t0040-add-and-cat.sh b/test/sharness/t0040-add-and-cat.sh index adc930ec301..e0b7acaff92 100755 --- a/test/sharness/t0040-add-and-cat.sh +++ b/test/sharness/t0040-add-and-cat.sh @@ -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 ' diff --git a/test/sharness/t0046-multifile-add.sh b/test/sharness/t0046-multifile-add.sh index 3a364fe48d9..309116da079 100755 --- a/test/sharness/t0046-multifile-add.sh +++ b/test/sharness/t0046-multifile-add.sh @@ -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 ' @@ -36,8 +36,18 @@ test_expect_success "create files with same name but in different directories" ' echo BA > dirB/fileA ' -test_expect_failure "add files with same name but in different directories" ' +test_expect_success "add files with same name but in different directories" ' ipfs add -q dirA/fileA dirB/fileA > hashes ' +cat < cat-expected +AA +BA +EOF + +test_expect_success "check that both files are added" ' + cat hashes | xargs ipfs cat | LC_ALL=C sort > cat-actual + test_cmp cat-expected cat-actual +' + test_done diff --git a/test/sharness/t0080-repo.sh b/test/sharness/t0080-repo.sh index e4895173625..2af3506fb39 100755 --- 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 ' test_expect_success "'ipfs refs local' no longer shows file" ' @@ -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" ' From bd40cfc8a9824485aa926576ca5a7c32df0dcb05 Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Tue, 4 Oct 2016 21:02:57 -0400 Subject: [PATCH 4/4] Adder: Disallow adding multiple directories unless "-w" is used. Adding multiple directories without "-w" will not produce the expected result, so for now it is best to disallow it. This also added a NumFiles() method to SliceFile. License: MIT Signed-off-by: Kevin Atkinson --- commands/files/slicefile.go | 4 ++++ core/commands/add.go | 10 ++++++++++ test/sharness/t0046-multifile-add.sh | 9 +++++++++ 3 files changed, 23 insertions(+) diff --git a/commands/files/slicefile.go b/commands/files/slicefile.go index 8d18dcaa372..3282202f39e 100644 --- a/commands/files/slicefile.go +++ b/commands/files/slicefile.go @@ -23,6 +23,10 @@ func (f *SliceFile) IsDirectory() bool { return true } +func (f *SliceFile) NumFiles() int { + return len(f.files) +} + func (f *SliceFile) NextFile() (File, error) { if f.n >= len(f.files) { return nil, io.EOF diff --git a/core/commands/add.go b/core/commands/add.go index 5b29609709d..0a96f1465f9 100644 --- a/core/commands/add.go +++ b/core/commands/add.go @@ -80,6 +80,16 @@ You can now refer to the added file in a gateway, like so: cmds.BoolOption(pinOptionName, "Pin this object when adding.").Default(true), }, PreRun: func(req cmds.Request) error { + wrap, _, _ := req.Option(wrapOptionName).Bool() + recursive, _, _ := req.Option(cmds.RecLong).Bool() + sliceFile, ok := req.Files().(*files.SliceFile) + if !ok { + return fmt.Errorf("type assertion failed: req.Files().(*files.SliceFile)") + } + if !wrap && recursive && sliceFile.NumFiles() > 1 { + return fmt.Errorf("adding multiple directories without '-w' unsupported") + } + if quiet, _, _ := req.Option(quietOptionName).Bool(); quiet { return nil } diff --git a/test/sharness/t0046-multifile-add.sh b/test/sharness/t0046-multifile-add.sh index 309116da079..6dafd0a9076 100755 --- a/test/sharness/t0046-multifile-add.sh +++ b/test/sharness/t0046-multifile-add.sh @@ -50,4 +50,13 @@ test_expect_success "check that both files are added" ' test_cmp cat-expected cat-actual ' +test_expect_success "adding multiple directories fails cleanly" ' + test_must_fail ipfs add -q -r dirA dirB +' + +test_expect_success "adding multiple directories with -w is okay" ' + ipfs add -q -r -w dirA dirB > hashes && + ipfs ls `tail -1 hashes` > ls-res +' + test_done