From 9e5d0aaaeca70ae76384d52fc93a4f404b642858 Mon Sep 17 00:00:00 2001 From: Lucas Molas Date: Wed, 21 Sep 2022 13:08:36 -0300 Subject: [PATCH] feat(cmds/add): --to-files option automates files cp (#8927) * feat(cmds/add): --to-files option as files cp * tests(to-files): ensure error handling is covered this adds bunch of tests that guard UX around importing multiple files into MFS, and the logic around trailing slash to indicate if the MFS destination if a directory. If the destination has a trailing slash, we ensure that the directory exists and is a dir and not a file. this allows us to support adding multipl files into MFS dir: ipfs add file1.txt file2.txt --to-files /some/mfs/dir/ * docs: to-files helptext Co-authored-by: Antonio Navarro Perez Co-authored-by: Marcin Rataj --- core/commands/add.go | 88 +++++++++++++++++++++++- core/commands/files.go | 33 +++++++-- test/sharness/t0040-add-and-cat.sh | 107 +++++++++++++++++++++++++++++ 3 files changed, 219 insertions(+), 9 deletions(-) diff --git a/core/commands/add.go b/core/commands/add.go index fb8ee8a31..9f686e14b 100644 --- a/core/commands/add.go +++ b/core/commands/add.go @@ -13,6 +13,8 @@ import ( "github.com/cheggaaa/pb" cmds "github.com/ipfs/go-ipfs-cmds" files "github.com/ipfs/go-ipfs-files" + ipld "github.com/ipfs/go-ipld-format" + mfs "github.com/ipfs/go-mfs" coreiface "github.com/ipfs/interface-go-ipfs-core" "github.com/ipfs/interface-go-ipfs-core/options" mh "github.com/multiformats/go-multihash" @@ -45,6 +47,7 @@ const ( hashOptionName = "hash" inlineOptionName = "inline" inlineLimitOptionName = "inline-limit" + toFilesOptionName = "to-files" ) const adderOutChanSize = 8 @@ -79,6 +82,20 @@ You can now refer to the added file in a gateway, like so: /ipfs/QmaG4FuMqEBnQNn3C8XJ5bpW8kLs7zq2ZXgHptJHbKDDVx/example.jpg +Files imported with 'ipfs add' are protected from GC (implicit '--pin=true'), +but it is up to you to remember the returned CID to get the data back later. + +Passing '--to-files' creates a reference in Files API (MFS), making it easier +to find it in the future: + + > ipfs files mkdir -p /myfs/dir + > ipfs add example.jpg --to-files /myfs/dir/ + > ipfs files ls /myfs/dir/ + example.jpg + +See 'ipfs files --help' to learn more about using MFS +for keeping track of added files and directories. + The chunker option, '-s', specifies the chunking strategy that dictates how to break files into blocks. Blocks with same content can be deduplicated. Different chunking strategies will produce different @@ -138,7 +155,6 @@ See 'dag export' and 'dag import' for more information. cmds.BoolOption(onlyHashOptionName, "n", "Only chunk and hash - do not write to disk."), cmds.BoolOption(wrapOptionName, "w", "Wrap files with a directory object."), cmds.StringOption(chunkerOptionName, "s", "Chunking algorithm, size-[bytes], rabin-[min]-[avg]-[max] or buzhash").WithDefault("size-262144"), - cmds.BoolOption(pinOptionName, "Pin this object when adding.").WithDefault(true), cmds.BoolOption(rawLeavesOptionName, "Use raw blocks for leaf nodes."), cmds.BoolOption(noCopyOptionName, "Add the file using filestore. Implies raw-leaves. (experimental)"), cmds.BoolOption(fstoreCacheOptionName, "Check the filestore for pre-existing blocks. (experimental)"), @@ -146,6 +162,8 @@ See 'dag export' and 'dag import' for more information. cmds.StringOption(hashOptionName, "Hash function to use. Implies CIDv1 if not sha2-256. (experimental)").WithDefault("sha2-256"), cmds.BoolOption(inlineOptionName, "Inline small blocks into CIDs. (experimental)"), cmds.IntOption(inlineLimitOptionName, "Maximum block size to inline. (experimental)").WithDefault(32), + cmds.BoolOption(pinOptionName, "Pin locally to protect added files from garbage collection.").WithDefault(true), + cmds.StringOption(toFilesOptionName, "Add reference to Files API (MFS) at the provided path."), }, PreRun: func(req *cmds.Request, env cmds.Environment) error { quiet, _ := req.Options[quietOptionName].(bool) @@ -186,10 +204,11 @@ See 'dag export' and 'dag import' for more information. hashFunStr, _ := req.Options[hashOptionName].(string) inline, _ := req.Options[inlineOptionName].(bool) inlineLimit, _ := req.Options[inlineLimitOptionName].(int) + toFilesStr, toFilesSet := req.Options[toFilesOptionName].(string) hashFunCode, ok := mh.Names[strings.ToLower(hashFunStr)] if !ok { - return fmt.Errorf("unrecognized hash function: %s", strings.ToLower(hashFunStr)) + return fmt.Errorf("unrecognized hash function: %q", strings.ToLower(hashFunStr)) } enc, err := cmdenv.GetCidEncoder(req) @@ -235,7 +254,12 @@ See 'dag export' and 'dag import' for more information. opts = append(opts, nil) // events option placeholder + ipfsNode, err := cmdenv.GetNode(env) + if err != nil { + return err + } var added int + var fileAddedToMFS bool addit := toadd.Entries() for addit.Next() { _, dir := addit.Node().(files.Directory) @@ -246,7 +270,65 @@ See 'dag export' and 'dag import' for more information. go func() { var err error defer close(events) - _, err = api.Unixfs().Add(req.Context, addit.Node(), opts...) + pathAdded, err := api.Unixfs().Add(req.Context, addit.Node(), opts...) + if err != nil { + errCh <- err + return + } + + // creating MFS pointers when optional --to-files is set + if toFilesSet { + if toFilesStr == "" { + toFilesStr = "/" + } + toFilesDst, err := checkPath(toFilesStr) + if err != nil { + errCh <- fmt.Errorf("%s: %w", toFilesOptionName, err) + return + } + dstAsDir := toFilesDst[len(toFilesDst)-1] == '/' + + if dstAsDir { + mfsNode, err := mfs.Lookup(ipfsNode.FilesRoot, toFilesDst) + // confirm dst exists + if err != nil { + errCh <- fmt.Errorf("%s: MFS destination directory %q does not exist: %w", toFilesOptionName, toFilesDst, err) + return + } + // confirm dst is a dir + if mfsNode.Type() != mfs.TDir { + errCh <- fmt.Errorf("%s: MFS destination %q is not a directory", toFilesOptionName, toFilesDst) + return + } + // if MFS destination is a dir, append filename to the dir path + toFilesDst += path.Base(addit.Name()) + } + + // error if we try to overwrite a preexisting file destination + if fileAddedToMFS && !dstAsDir { + errCh <- fmt.Errorf("%s: MFS destination is a file: only one entry can be copied to %q", toFilesOptionName, toFilesDst) + return + } + + _, err = mfs.Lookup(ipfsNode.FilesRoot, path.Dir(toFilesDst)) + if err != nil { + errCh <- fmt.Errorf("%s: MFS destination parent %q %q does not exist: %w", toFilesOptionName, toFilesDst, path.Dir(toFilesDst), err) + return + } + + var nodeAdded ipld.Node + nodeAdded, err = api.Dag().Get(req.Context, pathAdded.Cid()) + if err != nil { + errCh <- err + return + } + err = mfs.PutNode(ipfsNode.FilesRoot, toFilesDst, nodeAdded) + if err != nil { + errCh <- fmt.Errorf("%s: cannot put node in path %q: %w", toFilesOptionName, toFilesDst, err) + return + } + fileAddedToMFS = true + } errCh <- err }() diff --git a/core/commands/files.go b/core/commands/files.go index 95432ef61..e48abba28 100644 --- a/core/commands/files.go +++ b/core/commands/files.go @@ -581,7 +581,7 @@ const ( var filesReadCmd = &cmds.Command{ Helptext: cmds.HelpText{ - Tagline: "Read a file in a given MFS.", + Tagline: "Read a file from MFS.", ShortDescription: ` Read a specified number of bytes from a file at a given offset. By default, it will read the entire file similar to the Unix cat. @@ -724,11 +724,16 @@ const ( var filesWriteCmd = &cmds.Command{ Helptext: cmds.HelpText{ - Tagline: "Write to a mutable file in a given filesystem.", + Tagline: "Append to (modify) a file in MFS.", ShortDescription: ` -Write data to a file in a given filesystem. This command allows you to specify -a beginning offset to write to. The entire length of the input will be -written. +A low-level MFS command that allows you to append data to a file. If you want +to add a file without modifying an existing one, use 'ipfs add --to-files' +instead. +`, + LongDescription: ` +A low-level MFS command that allows you to append data at the end of a file, or +specify a beginning offset within a file to write to. The entire length of the +input will be written. If the '--create' option is specified, the file will be created if it does not exist. Nonexistent intermediate directories will not be created unless the @@ -755,6 +760,22 @@ WARNING: Usage of the '--flush=false' option does not guarantee data durability until the tree has been flushed. This can be accomplished by running 'ipfs files stat' on the file or any of its ancestors. + +WARNING: + +The CID produced by 'files write' will be different from 'ipfs add' because +'ipfs file write' creates a trickle-dag optimized for append-only operations +See '--trickle' in 'ipfs add --help' for more information. + +If you want to add a file without modifying an existing one, +use 'ipfs add' with '--to-files': + + > ipfs files mkdir -p /myfs/dir + > ipfs add example.jpg --to-files /myfs/dir/ + > ipfs files ls /myfs/dir/ + example.jpg + +See '--to-files' in 'ipfs add --help' for more information. `, }, Arguments: []cmds.Argument{ @@ -1019,7 +1040,7 @@ func updatePath(rt *mfs.Root, pth string, builder cid.Builder) error { var filesRmCmd = &cmds.Command{ Helptext: cmds.HelpText{ - Tagline: "Remove a file.", + Tagline: "Remove a file from MFS.", ShortDescription: ` Remove files or directories. diff --git a/test/sharness/t0040-add-and-cat.sh b/test/sharness/t0040-add-and-cat.sh index 833d6043e..142ab8ec1 100755 --- a/test/sharness/t0040-add-and-cat.sh +++ b/test/sharness/t0040-add-and-cat.sh @@ -362,6 +362,113 @@ test_add_cat_file() { rm mountdir/same-file/hello.txt && rmdir mountdir/same-file ' + + ## --to-files with single source + + test_expect_success "ipfs add --to-files /mfspath succeeds" ' + mkdir -p mountdir && echo "Hello MFS!" > mountdir/mfs.txt && + ipfs add mountdir/mfs.txt --to-files /ipfs-add-to-files >actual + ' + + test_expect_success "ipfs add --to-files output looks good" ' + HASH_MFS="QmVT8bL3sGBA2TwvX8JPhrv5CYZL8LLLfW7mxkUjPZsgBr" && + echo "added $HASH_MFS mfs.txt" >expected && + test_cmp expected actual + ' + + test_expect_success "ipfs files read succeeds" ' + ipfs files read /ipfs-add-to-files >actual && + ipfs files rm /ipfs-add-to-files + ' + + test_expect_success "ipfs cat output looks good" ' + echo "Hello MFS!" >expected && + test_cmp expected actual + ' + + test_expect_success "ipfs add --to-files requires argument" ' + test_expect_code 1 ipfs add mountdir/mfs.txt --to-files >actual 2>&1 && + test_should_contain "Error: missing argument for option \"to-files\"" actual + ' + + test_expect_success "ipfs add --to-files / (MFS root) works" ' + echo "Hello MFS!" >expected && + ipfs add mountdir/mfs.txt --to-files / && + ipfs files read /mfs.txt >actual && + test_cmp expected actual && + ipfs files rm /mfs.txt && + rm mountdir/mfs.txt + ' + + ## --to-files with multiple sources + + test_expect_success "ipfs add file1 file2 --to-files /mfspath0 (without trailing slash) fails" ' + mkdir -p test && + echo "file1" > test/mfs1.txt && + echo "file2" > test/mfs2.txt && + test_expect_code 1 ipfs add test/mfs1.txt test/mfs2.txt --to-files /mfspath0 >actual 2>&1 && + test_should_contain "MFS destination is a file: only one entry can be copied to \"/mfspath0\"" actual && + ipfs files rm -r --force /mfspath0 + ' + + test_expect_success "ipfs add file1 file2 --to-files /mfsfile1 (without trailing slash + with preexisting file) fails" ' + echo test | ipfs files write --create /mfsfile1 && + test_expect_code 1 ipfs add test/mfs1.txt test/mfs2.txt --to-files /mfsfile1 >actual 2>&1 && + test_should_contain "Error: to-files: cannot put node in path \"/mfsfile1\"" actual && + ipfs files rm -r --force /mfsfile1 + ' + + test_expect_success "ipfs add file1 file2 --to-files /mfsdir1 (without trailing slash + with preexisting dir) fails" ' + ipfs files mkdir -p /mfsdir1 && + test_expect_code 1 ipfs add test/mfs1.txt test/mfs2.txt --to-files /mfsdir1 >actual 2>&1 && + test_should_contain "Error: to-files: cannot put node in path \"/mfsdir1\"" actual && + ipfs files rm -r --force /mfsdir1 + ' + + test_expect_success "ipfs add file1 file2 --to-files /mfsdir2/ (with trailing slash) succeeds" ' + ipfs files mkdir -p /mfsdir2 && + test_expect_code 0 ipfs add --cid-version 1 test/mfs1.txt test/mfs2.txt --to-files /mfsdir2/ > actual 2>&1 && + test_should_contain "added bafkreihm3rktn5z33luic3youqdsn326toaq3ekesmdvsa53sbrd3f5r3a mfs1.txt" actual && + test_should_contain "added bafkreidh5zkhr2vnwa2luwmuj24xo6l3jhfgvkgtk5cyp43oxs7owzpxby mfs2.txt" actual && + test_should_not_contain "Error" actual && + ipfs files ls /mfsdir2/ > lsout && + test_should_contain "mfs1.txt" lsout && + test_should_contain "mfs2.txt" lsout && + ipfs files rm -r --force /mfsdir2 + ' + + test_expect_success "ipfs add file1 file2 --to-files /mfsfile2/ (with trailing slash + with preexisting file) fails" ' + echo test | ipfs files write --create /mfsfile2 && + test_expect_code 1 ipfs add test/mfs1.txt test/mfs2.txt --to-files /mfsfile2/ >actual 2>&1 && + test_should_contain "Error: to-files: MFS destination \"/mfsfile2/\" is not a directory" actual && + ipfs files rm -r --force /mfsfile2 + ' + + ## --to-files with recursive dir + + # test MFS destination without trailing slash + test_expect_success "ipfs add with --to-files /mfs/subdir3 fails because /mfs/subdir3 exists" ' + ipfs files mkdir -p /mfs/subdir3 && + test_expect_code 1 ipfs add -r test --to-files /mfs/subdir3 >actual 2>&1 && + test_should_contain "cannot put node in path \"/mfs/subdir3\": directory already has entry by that name" actual && + ipfs files rm -r --force /mfs + ' + + # test recursive import of a dir into MFS subdirectory + test_expect_success "ipfs add -r dir --to-files /mfs/subdir4/ succeeds (because of trailing slash)" ' + ipfs files mkdir -p /mfs/subdir4 && + ipfs add --cid-version 1 -r test --to-files /mfs/subdir4/ >actual 2>&1 && + test_should_contain "added bafkreihm3rktn5z33luic3youqdsn326toaq3ekesmdvsa53sbrd3f5r3a test/mfs1.txt" actual && + test_should_contain "added bafkreidh5zkhr2vnwa2luwmuj24xo6l3jhfgvkgtk5cyp43oxs7owzpxby test/mfs2.txt" actual && + test_should_contain "added bafybeic7xwqwovt4g4bax6d3udp6222i63vj2rblpbim7uy2uw4a5gahha test" actual && + test_should_not_contain "Error" actual + ipfs files ls /mfs/subdir4/ > lsout && + test_should_contain "test" lsout && + test_should_not_contain "mfs1.txt" lsout && + test_should_not_contain "mfs2.txt" lsout && + ipfs files rm -r --force /mfs + ' + } test_add_cat_5MB() {